Thomas Rast <trast@xxxxxxxxxxx> writes:
>>> I'm a bit curious: why isn't it enough to spell that path
>>> $GIT_BUILD_DIR/t/valgrind instead of making it fully configurable?
>>
>> For the same reason that TEST_DIRECTORY is different and unrelated from
>> GIT_BUILD_DIR. It's my understanding that GIT_BUILD_DIR could end up
>> being somewhere compeltely unrelated to where TOP_SRC/t/valgrind is.
>> At least that's why I introduced a new parameter.
>
> I'm just worried that for such a fringe use-case, the maintainer of the
> out-of-tree tests will never notice that he missed to customize *this*
> particular parameter. So I'd rather have it spelled in terms of the
> existing two (?).
I understand your concern. Perhaps it could be mitigated with some
"HOWTO" comments at the top of the script. I'm nervous about basing the
value on other variables because that's what limited the script to such
a narrow scope in the first place.
> Don't we, right now, get stuff as follows:
>
> item path
> --------------------------------------------
> test-lib.sh $TEST_DIRECTORY
Right now, yes, but it breaks for out-of-tree tests. In the out-of-tree
case, TEST_DIRECTORY doesn't contain test-lib.sh. For exmaple, in
t7900-subtree.sh, I do this:
. ../../../t/test-lib.sh
because TEST_DIRECTORY is set to some directory under contrib/subtree.
> git $GIT_BUILD_DIR/bin-wrappers
I think so.
> valgrind.sh $TEST_DIRECTORY/valgrind
That's what it is now and it's wrong for out-of-tree tests.
> git (with --valgrind) $TEST_DIRECTORY/valgrind/bin
Yep. This is ok.
> You are saying this must change to an entirely new path
>
> valgrind.sh $GIT_VALGRIND_TOOLS
> git (with --valgrind) $GIT_VALGRIND_TOOLS/bin
The first, yes. The second, no. We can leave that alone.
> but what's wrong with simply
>
> valgrind.sh $GIT_BUILD_DIR/t/valgrind
> git (with --valgrind) $TEST_DIRECTORY/valgrind/bin
These are two separate issues.
GIT_BUILD_DIR may not be anywhere within the source tree, right? If so,
t/valgrind may not have any relation whatsoever to GIT_BUILD_DIR. Hence
GIT_VALGRIND_TOOLS.
The second part is correct. test-lib.sh sets it up that way (~line 983)
and it works fine for out-of-tree tests.
> In the common case of t/, these just map to what we had before. In the
> out-of-tree case, we'd create valgrind/bin in the test directory for the
> *temporary* stuff, and still look for the wrapping valgrind.sh in the
> git tree.
Putting valgrind/bin in the test directory is fine. There's no change
there. It's this looking for the wrapping valgrind.sh that fails in the
current scheme. We cannot rely on GIT_BUILD_DIR to find it as noted
above.
-Dave
--
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