Re: [PATCH] Allow Overriding GIT_BUILD_DIR

From: David A. Greene <greened@obbligato.org>
Date: Mar 2, 2012, 03:51 UTC
Message-ID: <87fwdrk6d4.fsf@smith.obbligato.org>
In-Reply-To: <7vpqd1dyq7.fsf@alter.siamese.dyndns.org>

Junio C Hamano <gitster@pobox.com> writes:

Show 7 quoted lines

> Ok, this is getting closer. We use GIT_BUILD_DIR to find out crucial bits
> of the build environment in order to run tests, like the binaries being
> tested that are in $GIT_BUILD_DIR/bin-wrappers, but we set GIT_BUILD_DIR
> always to one level above where the test script being run is (because they
> are typically t/t1234-name.sh).  By making them able to name GIT_BUILD_DIR
> directly to a place that is different from a level above "$TEST_DIRECTORY",
> a test script can live anywhere.

Right.

> There are two more things that worries me a bit.
>
> One is what TEST_DIRECTORY should mean in the new world order.

That and its interaction with GIT_BUILD_DIR really confused me when I was trying to get the subtree tests to work. I think they could use either a bit more documentation or a bit more separation of purpose. I'm not sure which.

Show 9 quoted lines

> We use it to find where the test-lib.sh and other lib-*.sh helper
> definitions are, also we use it to find large-ish test vectors like
> t3900/ and t4013/.  If an external test script t1234-git-subtree.sh
> wants to use a separate file to keep its own helper definitions, how
> should it name it?  It cannot be relative to TEST_DIRECTORY that is
> typically "t/".  It cannot be relative to "../" as TRASH_DIRECTORY
> where the script runs, as the --root option may move it elsewhere on
> the filesystem (and is the reason TEST_DIRECTORY variable exists in
> the first place).

I must admit I am still confused about what these variables do even after reading this explanation several times. I have a pretty good idea what TRASH_DIRECTORY is.

> And how well does an external test script work with the --root option that
> moves the TEST_DIRECTORY?

I have no idea. I didn't even know about --root and I still don't know what it does. It would be helpful to have some commentary about options.

I am willing to add comments (as a separate patch?) as I figure out what these things do. Perhaps I'll write up a patch and get feedback to spur discussion so I can come to better understanding. :)

Show 7 quoted lines

>> @@ -55,6 +55,7 @@ unset $(perl -e '
>>  		.*_TEST
>>  		PROVE
>>  		VALGRIND
>> +                BUILD_DIR
>
> A funny    indentation found here.

Will fix.

Show 10 quoted lines

>> +if test -z "$GIT_BUILD_DIR"
>> +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.
>
> # For in-tree test scripts, this is one level above the TEST_DIRECTORY
> # (t/), but a test script that lives outside t/ can set this variable to
> # point at the right place so that it can find t/ directory that house
> # test helpers like lib-pager*.sh and test vectors like t4013/.

This is good commentary.  Will add.

                                -Dave