Re: [PATCH] Move test-* to t/helper/ subdirectory

3 messages, 2 authors, 2016-06-16 · open the first message on its own page

Re: [PATCH] Move test-* to t/helper/ subdirectory

From: Junio C Hamano <hidden>
Date: 2016-06-16 02:19:07

Duy Nguyen [off-list ref] writes:
I may have rushed to judgement. wrap-for-bin.sh has always been the
dependency for bin-wrappers/*. If we force that file to change, then
bin-wrappers/* will be recreated when switching branches. So how about
this?
I do not think you are "force updating wrap-for-bin" in any way in
the patch, though.  You are building it in such a way that it does
not have to get updated within the revision that contains e6e7530
(assuming that this will be queued directly on top it and merged to
everywhere e6e7530 is contained).

The new case/esac looks somewhat bad (its knowing that where test-*
lives, test-* is the only thing that is special, etc. troubles me at
the same time that case/esac is funnily formated).

Where does "@@PATH@@" come from and who rewrites it?  Is that a
misspelt "@@PROG@@"?
quoted hunk
-- 8< --
Subject: [PATCH] wrap-for-bin.sh: handle t/helper/ paths internally

Commit e6e7530 (test helpers: move test-* to t/helper/ subdirectory -
2016-04-13) moves test-* to t/helper. However because bin-wrappers/*
only depend on wrap-for-bin.sh, when switching between a branch that has
this commit and one that does not, bin-wrappers/* may not be regenerated
and point to the old/outdated test programs. Fix it by force updating
wrap-for-bin.sh so that it will be regenerated anyway.

Signed-off-by: Nguyễn Thái Ngọc Duy <redacted>
---
 Makefile        |  2 +-
 wrap-for-bin.sh | 12 ++++++++++--
 2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/Makefile b/Makefile
index dd178ee..38dcdf7 100644
--- a/Makefile
+++ b/Makefile
@@ -2204,7 +2204,7 @@ bin-wrappers/%: wrap-for-bin.sh
 	@mkdir -p bin-wrappers
 	$(QUIET_GEN)sed -e '1s|#!.*/sh|#!$(SHELL_PATH_SQ)|' \
 	     -e 's|@@BUILD_DIR@@|$(shell pwd)|' \
-	     -e 's|@@PROG@@|$(patsubst test-%,t/helper/test-%,$(@F))|' < $< > $@ && \
+	     -e 's|@@PROG@@|$(@F)|' < $< > $@ && \
 	chmod +x $@
 
 # GNU make supports exporting all variables by "export" without parameters.
diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
index db0ec6a..bb334ed 100644
--- a/wrap-for-bin.sh
+++ b/wrap-for-bin.sh
@@ -18,11 +18,19 @@ GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'"${GITPERLLIB:+:$GITPERLLIB}"
 GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
 PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
+case "@@PATH@@" in
+    test-*)
+	PROG="${GIT_EXEC_PATH}/t/helper/@@PROG@@"
+	;;
+    *)
+	PROG="${GIT_EXEC_PATH}/@@PROG@@"
+	;;
+esac
 
 if test -n "$GIT_TEST_GDB"
 then
 	unset GIT_TEST_GDB
-	exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+	exec gdb --args "$PROG" "$@"
 else
-	exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+	exec "$PROG" "$@"
 fi

Re: [PATCH] Move test-* to t/helper/ subdirectory

From: Duy Nguyen <hidden>
Date: 2016-06-16 02:19:08

On Tue, May 3, 2016 at 12:34 AM, Junio C Hamano [off-list ref] wrote:
Duy Nguyen [off-list ref] writes:
quoted
I may have rushed to judgement. wrap-for-bin.sh has always been the
dependency for bin-wrappers/*. If we force that file to change, then
bin-wrappers/* will be recreated when switching branches. So how about
this?
I do not think you are "force updating wrap-for-bin" in any way in
the patch, though.  You are building it in such a way that it does
not have to get updated within the revision that contains e6e7530
(assuming that this will be queued directly on top it and merged to
everywhere e6e7530 is contained).
Yep.
The new case/esac looks somewhat bad (its knowing that where test-*
lives, test-* is the only thing that is special, etc. troubles me at
the same time that case/esac is funnily formated).
We could just make some random changes in this file. That would have
the same effect.
Where does "@@PATH@@" come from and who rewrites it?  Is that a
misspelt "@@PROG@@"?
Yep. Should have run make distclean before testing :(
-- 
Duy

Re: [PATCH] Move test-* to t/helper/ subdirectory

From: Duy Nguyen <hidden>
Date: 2016-06-16 02:19:15

So among the options we have so far, which way should we go, or leave it as is?

On Tue, May 3, 2016 at 7:15 AM, Duy Nguyen [off-list ref] wrote:
On Tue, May 3, 2016 at 12:34 AM, Junio C Hamano [off-list ref] wrote:
quoted
Duy Nguyen [off-list ref] writes:
quoted
I may have rushed to judgement. wrap-for-bin.sh has always been the
dependency for bin-wrappers/*. If we force that file to change, then
bin-wrappers/* will be recreated when switching branches. So how about
this?
I do not think you are "force updating wrap-for-bin" in any way in
the patch, though.  You are building it in such a way that it does
not have to get updated within the revision that contains e6e7530
(assuming that this will be queued directly on top it and merged to
everywhere e6e7530 is contained).
Yep.
quoted
The new case/esac looks somewhat bad (its knowing that where test-*
lives, test-* is the only thing that is special, etc. troubles me at
the same time that case/esac is funnily formated).
We could just make some random changes in this file. That would have
the same effect.
quoted
Where does "@@PATH@@" come from and who rewrites it?  Is that a
misspelt "@@PROG@@"?
Yep. Should have run make distclean before testing :(
--
Duy
-- 
Duy
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help