Re: [PATCH 08/10] run test suite without dashed git-commands in PATH

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Jan 26, 2009, 11:06 UTC
Message-ID: <alpine.DEB.1.00.0901261201470.14855@racer>
In-Reply-To: <20090126064004.GA3004@comcast.net>

Hi,

On Sun, 25 Jan 2009, Matthew Ogilvie wrote:

Show 26 quoted lines

> On Sun, Jan 25, 2009 at 02:59:53AM +0100, Johannes Schindelin wrote:
> 
> > On Sat, 24 Jan 2009, Matthew Ogilvie wrote:
> > 
> > >  .gitignore          |    1 +
> > >  Makefile            |   42 +++++++++++++++++++++++++++++++-----------
> > >  t/test-lib.sh       |   14 +++++++++++++-
> > >  test-bin-wrapper.sh |   12 ++++++++++++
> > >  4 files changed, 57 insertions(+), 12 deletions(-)
> > >  create mode 100644 test-bin-wrapper.sh
> > 
> > I am strongly opposed to a patch this big, just for something as 3rd 
> > class as CVS server faking.  We already have a big fallout from all 
> > that bending over for Windows support, and I do not like it at all.
> > 
> > Note: I do not even have to look further than the diffstat to see that 
> > it is wrong.
> > 
> > The point is: if cvsserver wants to pretend that it is in a fake bin 
> > where almost none of the other Git programs are, fine, let's do that 
> > _in the test for cvsserver_.
> 
> Since by default git is installed such that most of the dashed-form 
> commands are not in a user's default PATH, my thought was that it would 
> make sense for the test suite to mimick that environment as much as 
> possible.

This sounds very generic, but you hid it in cvsserver-specific patch series.

So maybe I was wrong to assume that this is cvsserver specific, but then, you made that mistake rather easy to make.

> This could detect regressions in any installed/tested git script that 
> erroneously assumes the dashed form commands are in the PATH, not just 
> git-cvsserver.

The major point is that these scripts _will_ run if you call _them_ using the dash-less form, as GIT_EXEC_PATH will be added to the PATH by the Git wrapper.

>     3. Perhaps just use symlinks or hardlinks instead of a wrapper 
>        script.  This might have some promise, except that links are more 
>        likely to fail on windows, and the wrappers generally give you 
>        more flexibility for testing odd scenarios.

Not likely.  Sure as hell.

Show 8 quoted lines

>     4. The test-bin-wrapper.sh script does not actually need to set 
>        environment variables (GIT_EXEC_DIT and templates) for purposes 
>        of this patch.  But my thought was that in this form you could 
>        run things straight out of the test-bin directory to manually try 
>        out new code without needing to actually install a build or mess 
>        with the environment variables yourself.  It could also be 
>        extended to handle other global wrapper needs relatively easily, 
>        such as valgrind.

Umm.

You missed the valgrind patch series.

Ciao, Dscho