Re: [RFC] test-lib.sh: preprocess to use PERL_PATH

[Junio C Hamano <gitster@pobox.com>]

Torsten Bögershausen <tboegi@web.de> writes:

> On 23.06.12 07:22, Junio C Hamano wrote:
>> Torsten Bögershausen <tboegi@web.de> writes:
>>
>>> All test cases found in t/*.sh must include test-lib instead of test-lib.sh
>> Please don't.  That is too much churning for too little gain, I am afraid.
> Ok, would it be better to rename
>
> t/test-lib.sh -> t/test-lib.sh.sh
>
> and let the Makefile generate t/test-lib.sh?

It isn't as bad as the patch posted, but not very much.

There are number of a lot lower impact options before you
contemplate such a large change, given that there is only one
invocation of bare "perl" before GIT-BUILD-OPTIONS is dot-sourced.

 (1) Perhaps that use does not have any portability issues, and we
     can leave it as-is, with a comment to forbid people from
     turning into "$PERL_PATH" and be done with it?

 (2) Perhaps that use can be rewritten in such a way that it does
     not have to be done with perl in the first place?

 (3) Perhaps what that use of perl does can be delayed until we
     dot-source GIT-BUILD-OPTIONS and have $PERL_PATH defined, in
     which case we can move that use to a later position (and we can
     turn that sole use of perl into "$PERL_PATH")?

 (3) Perhaps what test-lib.sh does before it dot-sources
     GIT-BUILD-OPTIONS does not be affected if we dot-sourced
     GIT-BUILD-OPTIONS a lot earlier (and we can turn that sole use
     of perl into "$PERL_PATH")?


For example (this is not tested at all, nor I did not think it
through), a patch that moves the definition of TEST_DIRECTORY which
GIT_BUILD_DIR depends on higher, so that we can dot-source the file
a lot earlier, may look like this.


 t/test-lib.sh | 43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 9e2b711..f3e7cf9 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -34,6 +34,26 @@ esac
 # Keep the original TERM for say_color
 ORIGINAL_TERM=$TERM
 
+# Test the binaries we have just built.  The tests are kept in
+# t/ subdirectory and are run in 'trash directory' subdirectory.
+if test -z "$TEST_DIRECTORY"
+then
+	# We allow tests to override this, in case they want to run tests
+	# outside of t/, e.g. for running tests on the test library
+	# itself.
+	TEST_DIRECTORY=$(pwd)
+fi
+if test -z "$TEST_OUTPUT_DIRECTORY"
+then
+	# Similarly, override this to store the test-results subdir
+	# elsewhere
+	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
+fi
+GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
+. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
+export PERL_PATH SHELL_PATH
+
 # For repeatability, reset the environment to known value.
 LANG=C
 LC_ALL=C
@@ -46,7 +66,7 @@ EDITOR=:
 # /usr/xpg4/bin/sh and /bin/ksh to bail out.  So keep the unsets
 # deriving from the command substitution clustered with the other
 # ones.
-unset VISUAL EMAIL LANGUAGE COLUMNS $(perl -e '
+unset VISUAL EMAIL LANGUAGE COLUMNS $("$PERL_PATH" -e '
 	my @env = keys %ENV;
 	my $ok = join("|", qw(
 		TRACE
@@ -229,7 +249,7 @@ trap 'die' EXIT
 
 # The user-facing functions are loaded from a separate file so that
 # test_perf subshells can have them too
-. "${TEST_DIRECTORY:-.}"/test-lib-functions.sh
+. "$TEST_DIRECTORY/test-lib-functions.sh"
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
@@ -380,23 +400,6 @@ test_done () {
 	esac
 }
 
-# Test the binaries we have just built.  The tests are kept in
-# t/ subdirectory and are run in 'trash directory' subdirectory.
-if test -z "$TEST_DIRECTORY"
-then
-	# We allow tests to override this, in case they want to run tests
-	# outside of t/, e.g. for running tests on the test library
-	# itself.
-	TEST_DIRECTORY=$(pwd)
-fi
-if test -z "$TEST_OUTPUT_DIRECTORY"
-then
-	# Similarly, override this to store the test-results subdir
-	# elsewhere
-	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
-fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
-
 if test -n "$valgrind"
 then
 	make_symlink () {
@@ -492,8 +495,6 @@ GIT_CONFIG_NOSYSTEM=1
 GIT_ATTR_NOSYSTEM=1
 export PATH GIT_EXEC_PATH GIT_TEMPLATE_DIR GIT_CONFIG_NOSYSTEM GIT_ATTR_NOSYSTEM
 
-. "$GIT_BUILD_DIR"/GIT-BUILD-OPTIONS
-
 if test -z "$GIT_TEST_CMP"
 then
 	if test -n "$GIT_TEST_CMP_USE_COPIED_CONTEXT"