# t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need time in /usr/bin 17 messages from 2012-10-15 to 2012-10-16. Participants: Joachim Schmitz, Michael J Gruber, Junio C Hamano, Andreas Schwab, Stefano Lattarini, Johannes Sixt. Thread: https://gitlist.dev/t/31823 ## Joachim Schmitz, 2012-10-15 10:56 Subject: t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need time in /usr/bin Message-ID: URL: https://gitlist.dev/e/k5gq60%24q8c%241%40ger.gmane.org ``` Hi folks t3302-notes-index-expensive.sh and t3419-rebase-patch-id.sh need "time " to be in "/usr/bin", however on my system it is in "/bin". Can't this be checked for? Bye, Jojo ``` ## Michael J Gruber, 2012-10-16 11:39 Subject: [PATCH 0/4] Allow different time commands Message-ID: URL: https://gitlist.dev/e/cover.1350387132.git.git%40drmicha.warpmail.net In-Reply-To: ``` This mini series enables the use of the lazy prereq framework for tests which need a (non-builtin) time command. 1/4 is a bugfix 2/4 allows lazy prereq test scripts to set variables for later use 3/4 implements the prereq 4/4 uses it This should solve the problem reported by Joachim Schmitz. 3/4 could be done in various different ways, see the comments there. Michael J Gruber (4): t3419-rebase-patch-id: heed USR_BIN_TIME prereq test-lib: allow variable export from lazy prereq tests test-lib: provide lazy TIME_COMMAND prereq t3302,t3419: use the TIME_COMMAND prereq t/t3302-notes-index-expensive.sh | 5 ++--- t/t3419-rebase-patch-id.sh | 7 +++---- t/test-lib-functions.sh | 6 ++++-- t/test-lib.sh | 7 +++++++ 4 files changed, 16 insertions(+), 9 deletions(-) -- 1.8.0.rc2.304.g9f3ac5c ``` ## Michael J Gruber, 2012-10-16 11:39 Subject: [PATCH 1/4] t3419-rebase-patch-id: heed USR_BIN_TIME prereq Message-ID: URL: https://gitlist.dev/e/b04898886f0c7ed9d943aef0ce6e047f10c97d76.1350387132.git.git%40drmicha.warpmail.net In-Reply-To: ``` t3419 sets the t3419-rebase-patch-id.sh prereq based on the availability of /usr/bin/time but calls the binary unconditionally (in debug mode). Make it run the timing only when the prereq is matched. Signed-off-by: Michael J Gruber --- t/t3419-rebase-patch-id.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh index e70ac10..52af547 100755 --- a/t/t3419-rebase-patch-id.sh +++ b/t/t3419-rebase-patch-id.sh @@ -69,7 +69,7 @@ do_tests() git cherry-pick master >/dev/null 2>&1 " - test_debug " + test_have_prereq USR_BIN_TIME && test_debug " run git diff master^\! " @@ -77,7 +77,7 @@ do_tests() echo 'file binary' >.gitattributes " - test_debug " + test_have_prereq USR_BIN_TIME && test_debug " run git format-patch --stdout master && run git format-patch --stdout --ignore-if-in-upstream master " -- 1.8.0.rc2.304.g9f3ac5c ``` ## Michael J Gruber, 2012-10-16 11:39 Subject: [PATCH 2/4] test-lib: allow variable export from lazy prereq tests Message-ID: URL: https://gitlist.dev/e/ca07c5a1cf836826fc0ee53daf3e7f3dfe111ece.1350387132.git.git%40drmicha.warpmail.net In-Reply-To: ``` Currently, lazy prereq tests are run in a subshell which communicates only the exit code to the outer world. Run it as a subcommand so that variables can be exported to the test environment. Signed-off-by: Michael J Gruber --- I don't think this has any adverse side effects, but I'm begging for another set of eyeballs to have a look. (Test suite passes, of course.) t/test-lib-functions.sh | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 8889ba5..e587902 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -246,13 +246,15 @@ test_lazy_prereq () { test_run_lazy_prereq_ () { script=' mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" && -( +{ cd "$TRASH_DIRECTORY/prereq-test-dir" &&'"$2"' -)' +}' say >&3 "checking prerequisite: $1" say >&3 "$script" + orig_pwd="$(pwd)" test_eval_ "$script" eval_ret=$? + cd "$orig_pwd" rm -rf "$TRASH_DIRECTORY/prereq-test-dir" if test "$eval_ret" = 0; then say >&3 "prerequisite $1 ok" -- 1.8.0.rc2.304.g9f3ac5c ``` ## Michael J Gruber, 2012-10-16 11:39 Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <0140f3f2f2f4bded6868f4082bef0c36e9a8560e.1350387132.git.git@drmicha.warpmail.net> URL: https://gitlist.dev/e/0140f3f2f2f4bded6868f4082bef0c36e9a8560e.1350387132.git.git%40drmicha.warpmail.net In-Reply-To: ``` Some test want to use the time command (not the shell builtin) and test for its availability at /usr/bin/time. Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and /bin/time. If any is found, set TEST_COMMAND_PATH to the first match. Signed-off-by: Michael J Gruber --- Rather than iterating over 2 or more hardcoded paths, one could use "test -P time" or allow a make variable TIME_COMMAND_PATH whose executability is checked by the prereq. I really don't know what's best. t/test-lib.sh | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index 489bc80..7977c15 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC ' esac ' +test_lazy_prereq TIME_COMMAND ' + for command in /usr/bin/time /bin/time + do + test -x "$command" && break + done && TIME_COMMAND_PATH="$command" +' + # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test -w / || test_set_prereq SANITY -- 1.8.0.rc2.304.g9f3ac5c ``` ## Michael J Gruber, 2012-10-16 11:39 Subject: [PATCH 4/4] t3302,t3419: use the TIME_COMMAND prereq Message-ID: <71d3a0f9ab2bfd73fb2abfb4a0bc9fa7be6cd70b.1350387132.git.git@drmicha.warpmail.net> URL: https://gitlist.dev/e/71d3a0f9ab2bfd73fb2abfb4a0bc9fa7be6cd70b.1350387132.git.git%40drmicha.warpmail.net In-Reply-To: ``` Use the TIME_COMMAND prereq in both tests so that time from several paths can be used. Signed-off-by: Michael J Gruber --- t/t3302-notes-index-expensive.sh | 5 ++--- t/t3419-rebase-patch-id.sh | 7 +++---- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/t/t3302-notes-index-expensive.sh b/t/t3302-notes-index-expensive.sh index e35d781..3d9f37e 100755 --- a/t/t3302-notes-index-expensive.sh +++ b/t/t3302-notes-index-expensive.sh @@ -9,7 +9,6 @@ test_description='Test commit notes index (expensive!)' test_set_prereq NOT_EXPENSIVE test -n "$GIT_NOTES_TIMING_TESTS" && test_set_prereq EXPENSIVE -test -x /usr/bin/time && test_set_prereq USR_BIN_TIME create_repo () { number_of_commits=$1 @@ -96,7 +95,7 @@ time_notes () { for mode in no-notes notes do echo $mode - /usr/bin/time "$SHELL_PATH" ../time_notes $mode $1 + "$TIME_COMMAND_PATH" "$SHELL_PATH" ../time_notes $mode $1 done } @@ -113,7 +112,7 @@ do_tests () { test_expect_success $pr 'notes work' "test_notes $count" - test_expect_success USR_BIN_TIME,$pr 'notes timing with /usr/bin/time' "time_notes 100" + test_expect_success TIME_COMMAND,$pr 'notes timing with a time command' "time_notes 100" test_expect_success $pr 'teardown / cd ..' 'cd ..' } diff --git a/t/t3419-rebase-patch-id.sh b/t/t3419-rebase-patch-id.sh index 52af547..c7d90a6 100755 --- a/t/t3419-rebase-patch-id.sh +++ b/t/t3419-rebase-patch-id.sh @@ -6,7 +6,6 @@ test_description='git rebase - test patch id computation' test_set_prereq NOT_EXPENSIVE test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE -test -x /usr/bin/time && test_set_prereq USR_BIN_TIME count() { @@ -35,7 +34,7 @@ scramble() run() { echo \$ "$@" - /usr/bin/time "$@" >/dev/null + "$TIME_COMMAND_PATH" "$@" >/dev/null } test_expect_success 'setup' ' @@ -69,7 +68,7 @@ do_tests() git cherry-pick master >/dev/null 2>&1 " - test_have_prereq USR_BIN_TIME && test_debug " + test_have_prereq TIME_COMMAND && test_debug " run git diff master^\! " @@ -77,7 +76,7 @@ do_tests() echo 'file binary' >.gitattributes " - test_have_prereq USR_BIN_TIME && test_debug " + test_have_prereq TIME_COMMAND && test_debug " run git format-patch --stdout master && run git format-patch --stdout --ignore-if-in-upstream master " -- 1.8.0.rc2.304.g9f3ac5c ``` ## Joachim Schmitz, 2012-10-16 14:13 Subject: RE: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <007001cdaba8$5afc6c00$10f54400$@schmitz-digital.de> URL: https://gitlist.dev/e/007001cdaba8%245afc6c00%2410f54400%24%40schmitz-digital.de In-Reply-To: <0140f3f2f2f4bded6868f4082bef0c36e9a8560e.1350387132.git.git@drmicha.warpmail.net> ``` > From: Michael J Gruber [mailto:git@drmicha.warpmail.net] > Sent: Tuesday, October 16, 2012 1:40 PM > To: git@vger.kernel.org > Cc: Joachim Schmitz; Junio C Hamano > Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq > > Some test want to use the time command (not the shell builtin) and test > for its availability at /usr/bin/time. > > Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and > /bin/time. If any is found, set TEST_COMMAND_PATH to the first match. > > Signed-off-by: Michael J Gruber > --- > Rather than iterating over 2 or more hardcoded paths, one could use > "test -P time" or allow a make variable TIME_COMMAND_PATH whose test -P time won't work for me: test -P: unary operator expected I do have another one in /usr/local/bin, maybe that could get added too? > executability is checked by the prereq. I really don't know what's best. > > t/test-lib.sh | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/t/test-lib.sh b/t/test-lib.sh > index 489bc80..7977c15 100644 > --- a/t/test-lib.sh > +++ b/t/test-lib.sh > @@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC ' > esac > ' > > +test_lazy_prereq TIME_COMMAND ' > + for command in /usr/bin/time /bin/time > + do > + test -x "$command" && break > + done && TIME_COMMAND_PATH="$command" > +' > + > # When the tests are run as root, permission tests will report that > # things are writable when they shouldn't be. > test -w / || test_set_prereq SANITY > -- > 1.8.0.rc2.304.g9f3ac5c ``` ## Michael J Gruber, 2012-10-16 15:06 Subject: Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <507D7809.6040904@drmicha.warpmail.net> URL: https://gitlist.dev/e/507D7809.6040904%40drmicha.warpmail.net In-Reply-To: <007001cdaba8$5afc6c00$10f54400$@schmitz-digital.de> ``` Joachim Schmitz venit, vidit, dixit 16.10.2012 16:13: >> From: Michael J Gruber [mailto:git@drmicha.warpmail.net] >> Sent: Tuesday, October 16, 2012 1:40 PM >> To: git@vger.kernel.org >> Cc: Joachim Schmitz; Junio C Hamano >> Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq >> >> Some test want to use the time command (not the shell builtin) and test >> for its availability at /usr/bin/time. >> >> Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and >> /bin/time. If any is found, set TEST_COMMAND_PATH to the first match. >> >> Signed-off-by: Michael J Gruber >> --- >> Rather than iterating over 2 or more hardcoded paths, one could use >> "test -P time" or allow a make variable TIME_COMMAND_PATH whose > > test -P time won't work for me: > test -P: unary operator expected > > I do have another one in /usr/local/bin, maybe that could get added too? Yikes. If we introduce a make variable TIME_COMMAND_PATH we can even get rid of 2/4 (but have to change Makefile or t/Makefile). >> executability is checked by the prereq. I really don't know what's best. >> >> t/test-lib.sh | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/t/test-lib.sh b/t/test-lib.sh >> index 489bc80..7977c15 100644 >> --- a/t/test-lib.sh >> +++ b/t/test-lib.sh >> @@ -738,6 +738,13 @@ test_lazy_prereq UTF8_NFD_TO_NFC ' >> esac >> ' >> >> +test_lazy_prereq TIME_COMMAND ' >> + for command in /usr/bin/time /bin/time >> + do >> + test -x "$command" && break >> + done && TIME_COMMAND_PATH="$command" >> +' >> + >> # When the tests are run as root, permission tests will report that >> # things are writable when they shouldn't be. >> test -w / || test_set_prereq SANITY >> -- >> 1.8.0.rc2.304.g9f3ac5c > ``` ## Michael J Gruber, 2012-10-16 15:07 Subject: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <28ca29e59c5905ba7232dcdbe4661f37ef90cac5.1350399860.git.git@drmicha.warpmail.net> URL: https://gitlist.dev/e/28ca29e59c5905ba7232dcdbe4661f37ef90cac5.1350399860.git.git%40drmicha.warpmail.net In-Reply-To: <007001cdaba8$5afc6c00$10f54400$@schmitz-digital.de> ``` Some test want to use the time command (not the shell builtin) and test for its availability at /usr/bin/time. Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH, which can be set from config.mak. It defaults to /usr/bin/time. Signed-off-by: Michael J Gruber --- Here's what the make variable version could look like. 1/4 and 4/4 would stay as is, 2/4 could be dropped. t/Makefile | 5 +++-- t/test-lib.sh | 4 ++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/t/Makefile b/t/Makefile index 88e289f..52b4039 100644 --- a/t/Makefile +++ b/t/Makefile @@ -13,6 +13,7 @@ TAR ?= $(TAR) RM ?= rm -f PROVE ?= prove DEFAULT_TEST_TARGET ?= test +TIME_COMMAND_PATH ?= /usr/bin/time # Shell quote; SHELL_PATH_SQ = $(subst ','\'',$(SHELL_PATH)) @@ -27,11 +28,11 @@ test: pre-clean $(TEST_LINT) $(MAKE) aggregate-results-and-cleanup prove: pre-clean $(TEST_LINT) - @echo "*** prove ***"; GIT_CONFIG=.git/config $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) + @echo "*** prove ***"; GIT_CONFIG=.git/config TIME_COMMAND_PATH=$(TIME_COMMAND_PATH) $(PROVE) --exec '$(SHELL_PATH_SQ)' $(GIT_PROVE_OPTS) $(T) :: $(GIT_TEST_OPTS) $(MAKE) clean-except-prove-cache $(T): - @echo "*** $@ ***"; GIT_CONFIG=.git/config '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS) + @echo "*** $@ ***"; GIT_CONFIG=.git/config TIME_COMMAND_PATH=$(TIME_COMMAND_PATH) '$(SHELL_PATH_SQ)' $@ $(GIT_TEST_OPTS) pre-clean: $(RM) -r test-results diff --git a/t/test-lib.sh b/t/test-lib.sh index 489bc80..173eb13 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -738,6 +738,10 @@ test_lazy_prereq UTF8_NFD_TO_NFC ' esac ' +test_lazy_prereq TIME_COMMAND ' + test -x "$TIME_COMMAND_PATH" +' + # When the tests are run as root, permission tests will report that # things are writable when they shouldn't be. test -w / || test_set_prereq SANITY -- 1.8.0.rc2.304.g9f3ac5c ``` ## Joachim Schmitz, 2012-10-16 15:11 Subject: RE: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <007401cdabb0$831d1f10$89575d30$@schmitz-digital.de> URL: https://gitlist.dev/e/007401cdabb0%24831d1f10%2489575d30%24%40schmitz-digital.de In-Reply-To: <507D7809.6040904@drmicha.warpmail.net> ``` > From: Michael J Gruber [mailto:git@drmicha.warpmail.net] > Sent: Tuesday, October 16, 2012 5:07 PM > To: Joachim Schmitz > Cc: git@vger.kernel.org; 'Junio C Hamano' > Subject: Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq > > Joachim Schmitz venit, vidit, dixit 16.10.2012 16:13: > >> From: Michael J Gruber [mailto:git@drmicha.warpmail.net] > >> Sent: Tuesday, October 16, 2012 1:40 PM > >> To: git@vger.kernel.org > >> Cc: Joachim Schmitz; Junio C Hamano > >> Subject: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq > >> > >> Some test want to use the time command (not the shell builtin) and test > >> for its availability at /usr/bin/time. > >> > >> Provide a lazy prereq TIME_COMMAND which tests for /usr/bin/time and > >> /bin/time. If any is found, set TEST_COMMAND_PATH to the first match. > >> > >> Signed-off-by: Michael J Gruber > >> --- > >> Rather than iterating over 2 or more hardcoded paths, one could use > >> "test -P time" or allow a make variable TIME_COMMAND_PATH whose > > > > test -P time won't work for me: > > test -P: unary operator expected > > > > I do have another one in /usr/local/bin, maybe that could get added too? > > Yikes. > > If we introduce a make variable TIME_COMMAND_PATH we can even get rid of > 2/4 (but have to change Makefile or t/Makefile). I don't mind too much. /usr/bin/time and /bin/time should be enough. ``` ## Junio C Hamano, 2012-10-16 16:16 Subject: Re: [PATCH 2/4] test-lib: allow variable export from lazy prereq tests Message-ID: <7vipaatmsb.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vipaatmsb.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Michael J Gruber writes: > I don't think this has any adverse side effects, but I'm begging for > another set of eyeballs to have a look. (Test suite passes, of course.) The lazy prereqs are designed to be used lazily, in any test that he who wrote lazy-prereq did not anticipate. It is run inside a subshell to make it absolutely sure that whatever it does (like use of shell variables, chdir around) will not be able to affect _any_ calling context that is not anticipated by who writes lazy prerequisite. Please don't do this. > > t/test-lib-functions.sh | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh > index 8889ba5..e587902 100644 > --- a/t/test-lib-functions.sh > +++ b/t/test-lib-functions.sh > @@ -246,13 +246,15 @@ test_lazy_prereq () { > test_run_lazy_prereq_ () { > script=' > mkdir -p "$TRASH_DIRECTORY/prereq-test-dir" && > -( > +{ > cd "$TRASH_DIRECTORY/prereq-test-dir" &&'"$2"' > -)' > +}' > say >&3 "checking prerequisite: $1" > say >&3 "$script" > + orig_pwd="$(pwd)" > test_eval_ "$script" > eval_ret=$? > + cd "$orig_pwd" > rm -rf "$TRASH_DIRECTORY/prereq-test-dir" > if test "$eval_ret" = 0; then > say >&3 "prerequisite $1 ok" ``` ## Andreas Schwab, 2012-10-16 16:28 Subject: Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq Message-ID: URL: https://gitlist.dev/e/m2mwzmxtyb.fsf%40igel.home In-Reply-To: <28ca29e59c5905ba7232dcdbe4661f37ef90cac5.1350399860.git.git@drmicha.warpmail.net> ``` Michael J Gruber writes: > Some test want to use the time command (not the shell builtin) and test > for its availability at /usr/bin/time. An alternative way to suppress the builtin meaning is to quote it, like \time. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ``` ## Junio C Hamano, 2012-10-16 16:32 Subject: Re: [PATCH 1/4] t3419-rebase-patch-id: heed USR_BIN_TIME prereq Message-ID: <7vd30itm2k.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vd30itm2k.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Michael J Gruber writes: > t3419 sets the t3419-rebase-patch-id.sh prereq based on the availability > of /usr/bin/time but calls the binary unconditionally (in debug mode). > > Make it run the timing only when the prereq is matched. > > Signed-off-by: Michael J Gruber I do not think we should ship our tests with too many test_debug in the first place (it is something you as a developer who are trying to figure out why your change broke tests can insert into tests). It might be useful to be able to say "sh ./t1234-*.sh -d" and see debug output that somebody else thought that it might be useful, so I wouldn't say we should remove all existing test_debug, but at the same time, if a developer finds existing test_debug does not work for him (either what the debug output gives him is insufficient for his needs, or what the debug command uses is not available on his system), he should be willing to update (and capable of doing so) it to suit his needs. Adding prereq in front of test_debug is simply an act of insanity. For this particular one, I think this should suffice. If the shell does not have "time" built-in, then timeme can be set to even an empty string, but that is left as an exercise to the readers. t/t3419-rebase-patch-id.sh | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git i/t/t3419-rebase-patch-id.sh w/t/t3419-rebase-patch-id.sh index e70ac10..bf2736a 100755 --- i/t/t3419-rebase-patch-id.sh +++ w/t/t3419-rebase-patch-id.sh @@ -6,7 +6,11 @@ test_description='git rebase - test patch id computation' test_set_prereq NOT_EXPENSIVE test -n "$GIT_PATCHID_TIMING_TESTS" && test_set_prereq EXPENSIVE -test -x /usr/bin/time && test_set_prereq USR_BIN_TIME +if test -x /usr/bin/time + timeme=/usr/bin/time +else + timeme=time +fi count() { @@ -35,7 +39,7 @@ scramble() run() { echo \$ "$@" - /usr/bin/time "$@" >/dev/null + $timeme "$@" >/dev/null } test_expect_success 'setup' ' ``` ## Junio C Hamano, 2012-10-16 16:41 Subject: Re: [RFC/PATCH 3/4] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <7v8vb6tlne.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8vb6tlne.fsf%40alter.siamese.dyndns.org In-Reply-To: <0140f3f2f2f4bded6868f4082bef0c36e9a8560e.1350387132.git.git@drmicha.warpmail.net> ``` Michael J Gruber writes: > Some test want to use the time command (not the shell builtin) and test > for its availability at /usr/bin/time. As we have t/perf these days, I suspect that we should aim to remove these uses of /usr/bin/time in the main testsuite instead. The one in 3419 was "run this command, and while at it run it under 'time'" but it is only inside test_debug and I do not think it gives anything more than curiosity values. The one in 3302 is used to compare two runs (one without and one with notes tree), so it has a little more value than just curiosity, but its value inside the main test suite is still highly dubious. It does not have any "under this value the test passes" criteria. When the performance of having to look up notes tree really matters, it shouldn't be done inside the main test suite that is designed to be runnable unattended and the only check the humans do is to see their "ok/fail" output. In short, what you would get out of /usr/bin/time simply is not a good match inside the context of these unit tests. ``` ## Stefano Lattarini, 2012-10-16 18:28 Subject: Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <507DA749.30509@gmail.com> URL: https://gitlist.dev/e/507DA749.30509%40gmail.com In-Reply-To: ``` Hi Andreas. I hope you don't mind my nitpickiness, but ... On 10/16/2012 06:28 PM, Andreas Schwab wrote: > Michael J Gruber writes: > >> Some test want to use the time command (not the shell builtin) and test >> for its availability at /usr/bin/time. > > An alternative way to suppress the builtin meaning is to quote it, like > \time. > ... to be 100% precise, this quoting trick works because 'time' is a shell keyword, rather than a builtin: $ type time time is a shell keyword Regards, Stefano ``` ## Johannes Sixt, 2012-10-16 19:21 Subject: Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <507DB39C.3080005@kdbg.org> URL: https://gitlist.dev/e/507DB39C.3080005%40kdbg.org In-Reply-To: <28ca29e59c5905ba7232dcdbe4661f37ef90cac5.1350399860.git.git@drmicha.warpmail.net> ``` Am 16.10.2012 17:07, schrieb Michael J Gruber: > Some test want to use the time command (not the shell builtin) and test > for its availability at /usr/bin/time. > > Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH, > which can be set from config.mak. It defaults to /usr/bin/time. This avoids the builtin: command time $that_command It works for bash, ksh, zsh, and dash (where the latter doesn't have it as builtin). -- Hannes ``` ## Junio C Hamano, 2012-10-16 19:34 Subject: Re: [RFC/PATCH 3/4v2] test-lib: provide lazy TIME_COMMAND prereq Message-ID: <7v8vb6rz2n.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7v8vb6rz2n.fsf%40alter.siamese.dyndns.org In-Reply-To: <507DB39C.3080005@kdbg.org> ``` Johannes Sixt writes: > Am 16.10.2012 17:07, schrieb Michael J Gruber: >> Some test want to use the time command (not the shell builtin) and test >> for its availability at /usr/bin/time. >> >> Provide a lazy prereq TIME_COMMAND which tests for $TEST_COMMAND_PATH, >> which can be set from config.mak. It defaults to /usr/bin/time. > > This avoids the builtin: > > command time $that_command > > It works for bash, ksh, zsh, and dash (where the latter doesn't have it > as builtin). "command time" works but I think that is because it is not a built-in ;-) Here is what I read in bash(1): command [-pVv] command [arg ...] Run command with args suppressing the normal shell function lookup. Only builtin commands or commands found in the PATH are executed. Taken together with this from "COMMAND EXECUTION": If the command name contains no slashes, the shell attempts to locate it. If there exists a shell function by that name, that function is invoked as described above in FUNCTIONS. If the name does not match a function, the shell searches for it in the list of shell builtins. If a match is found, that builtin is invoked. If the name is neither a shell function nor a builtin, and contains no slashes, bash searches each element of the PATH for a directory containing an executable file by that name. I suspect "command printf 'a b c\n'" would not use $HOME/bin/printf even when I have $HOME/bin early in my $PATH (nor /usr/bin/printf for that matter). ```