public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox•com>
To: "Torsten Bögershausen" <tboegi@web•de>
Cc: git@vger•kernel.org
Subject: Re: [RFC]  test-lib.sh: preprocess to use PERL_PATH
Date: Fri, 22 Jun 2012 23:18:54 -0700	[thread overview]
Message-ID: <7vipeilfe9.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <4FE55372.3080008@web.de> ("Torsten Bögershausen"'s message of "Sat, 23 Jun 2012 07:26:10 +0200")

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"

  reply	other threads:[~2012-06-23  6:19 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-06-23  5:04 [RFC] test-lib.sh: preprocess to use PERL_PATH Torsten Bögershausen
2012-06-23  5:22 ` Junio C Hamano
2012-06-23  5:26   ` Torsten Bögershausen
2012-06-23  6:18     ` Junio C Hamano [this message]
2012-06-23 13:11       ` Torsten Bögershausen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7vipeilfe9.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=tboegi@web$(echo .)de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox