git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Allow Overriding GIT_BUILD_DIR
@ 2012-02-05 22:28 David A. Greene
  2012-02-26 22:19 ` Junio C Hamano
  0 siblings, 1 reply; 4+ messages in thread
From: David A. Greene @ 2012-02-05 22:28 UTC (permalink / raw)
  To: git


Let tests override GIT_BUILD_DIR so git will work if tests are not at
the same directory level as standard git tests.  Prior to this change,
GIT_BUILD_DIR is hardwired to be exactly one directory above where the
test lives.  A test within contrib/, for example, can now use
test-lib.sh and set an appropriate value for GIT_BUILD_DIR.

Signed-off-by: David A. Greene <greened@obbligato.org>
---
 t/test-lib.sh |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)


------------------

diff --git a/t/test-lib.sh b/t/test-lib.sh
index a65dfc7..4585138 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -55,6 +55,7 @@ unset $(perl -e '
 		.*_TEST
 		PROVE
 		VALGRIND
+                BUILD_DIR
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
@@ -901,7 +902,14 @@ then
 	# itself.
 	TEST_DIRECTORY=$(pwd)
 fi
-GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+
+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.
+        GIT_BUILD_DIR="$TEST_DIRECTORY"/..
+fi
 
 if test -n "$valgrind"
 then

--------------------

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow Overriding GIT_BUILD_DIR
  2012-02-05 22:28 [PATCH] Allow Overriding GIT_BUILD_DIR David A. Greene
@ 2012-02-26 22:19 ` Junio C Hamano
  2012-03-02  3:51   ` David A. Greene
  0 siblings, 1 reply; 4+ messages in thread
From: Junio C Hamano @ 2012-02-26 22:19 UTC (permalink / raw)
  To: David A. Greene; +Cc: git

"David A. Greene" <greened@obbligato.org> writes:

> Let tests override GIT_BUILD_DIR so git will work if tests are not at
> the same directory level as standard git tests.  Prior to this change,
> GIT_BUILD_DIR is hardwired to be exactly one directory above where the
> test lives.  A test within contrib/, for example, can now use
> test-lib.sh and set an appropriate value for GIT_BUILD_DIR.

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.

There are two more things that worries me a bit.

One is what TEST_DIRECTORY should mean in the new world order.  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).

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

> Signed-off-by: David A. Greene <greened@obbligato.org>
> ---
>  t/test-lib.sh |   10 +++++++++-
>  1 files changed, 9 insertions(+), 1 deletions(-)
>
>
> ------------------
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index a65dfc7..4585138 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -55,6 +55,7 @@ unset $(perl -e '
>  		.*_TEST
>  		PROVE
>  		VALGRIND
> +                BUILD_DIR

A funny    indentation found here.

>  	));

> @@ -901,7 +902,14 @@ then
>  	# itself.
>  	TEST_DIRECTORY=$(pwd)
>  fi
> -GIT_BUILD_DIR="$TEST_DIRECTORY"/..
> +
> +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/.

> +        GIT_BUILD_DIR="$TEST_DIRECTORY"/..
> +fi
>  
>  if test -n "$valgrind"
>  then
>
> --------------------

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow Overriding GIT_BUILD_DIR
  2012-02-26 22:19 ` Junio C Hamano
@ 2012-03-02  3:51   ` David A. Greene
  2012-03-02  9:25     ` Thomas Rast
  0 siblings, 1 reply; 4+ messages in thread
From: David A. Greene @ 2012-03-02  3:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

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

> 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.

> 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.  :)

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

Will fix.

>> +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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Allow Overriding GIT_BUILD_DIR
  2012-03-02  3:51   ` David A. Greene
@ 2012-03-02  9:25     ` Thomas Rast
  0 siblings, 0 replies; 4+ messages in thread
From: Thomas Rast @ 2012-03-02  9:25 UTC (permalink / raw)
  To: David A. Greene; +Cc: Junio C Hamano, git

greened@obbligato.org (David A. Greene) writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> 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 think Junio meant TRASH_DIRECTORY here, too.

> 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.

--root tells the tests where to put its TRASH_DIRECTORY and is
frequently used to place it on a tmpfs (such as /dev/shm) as that gives
the tests a *huge* speed boost.

That is, if I say

  ./t5510-fetch.sh --root=/dev/shm

then the "main test repo" aka TRASH_DIRECTORY is

  /dev/shm/trash directory.t5510-fetch

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2012-03-02  9:25 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-05 22:28 [PATCH] Allow Overriding GIT_BUILD_DIR David A. Greene
2012-02-26 22:19 ` Junio C Hamano
2012-03-02  3:51   ` David A. Greene
2012-03-02  9:25     ` Thomas Rast

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).