Matthew Ogilvie <mmogilvi_git@xxxxxxxxxxxx> writes:
> The new test-bin directory contains wrapper scripts for executables that
> will be installed into the standard bindir. It explicitly does not
> contain most dashed-commands. The scripts automatically set environment
> variables to run out of the source tree, not the installed directory.
Even though I haven't tried the series, I like most of the things I see in
this series.
- Patch #1 and #2 are good and are independent from the later patches, as
without them running tests with GIT_TEST_INSTALLED would not work.
By the way, 6720721 (test-lib.sh: Allow running the test suite against
installed git, 2009-03-16) failed to document the feature in t/README.
Could you please fix this while you are at it?
- Running tests before installation in an environment slightly closer to
the final installation (i.e. lacks dashed commands in the $PATH during
the time tests are run) is a good direction to go.
- I like the Makefile changes that uses these new BINDIR_PROGRAMS_NEED_X,
TEST_PROGRAMS_NEED_X. Parameterizing commands listed in the actions
part is good.
- It certainly is _possible_ to add $(pwd)/test-bin to $PATH instead of
the established practice of using GIT_EXEC_PATH for every day/permanent
use without installation, but I doubt we should _encourage_ it for a
few reasons:
. The set-up will force one extra exec due to the wrapper; this is good
for the purpose of running tests, but unnecessary for a set-up for
every day/permanent use by people, compared with the already working
approach. The user needs to change an environment variable _anyway_
(either GIT_EXEC_PATH with the traditional approach, or PATH with
your patch).
. The new component to be added to $PATH shouldn't be named "test-bin/"
if it is meant for every day/permanent use.
. Advertising this forces the Makefile build test-bin/ contents from
"all" target. I think test-bin/ should only depend on "test" (iow,
after "make all && make install" there shouldn't have to be "test-bin"
directory.
I would rather treat it an unintended side-effect that you can add that
directory to the $PATH. It is designed to work in such an environment
(otherwise the tests won't exercise the version of git they are meant
to test), but I do not think it is _meant_ to be _used_ by end users
that way. And an unintended side-effect does not have to be mentioned
in INSTALL (especially with the directory name with "test" in it).
--
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