On 23.06.12 08:18, Junio C Hamano wrote:
> Torsten Bögershausen <tboegi@xxxxxx> writes:
>
>> On 23.06.12 07:22, Junio C Hamano wrote:
>>> Torsten Bögershausen <tboegi@xxxxxx> 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"
Excellent!
Thanks: enjoyed & tested OK both on
457f08c4777b552ad35 (where t4030 was broken when testing here)
and
>commit 9746b046e5651aa7277a0b853819e2d076d403c6
>Date: Fri Jun 22 22:20:27 2012 -0700
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html