Re: [PATCH 1/2 v2] Add valgrind support in test scripts

[Jeff King <peff@peff.net>]

On Wed, Jan 21, 2009 at 09:49:14PM +0100, Johannes Schindelin wrote:

> >   A: execvp("git-foo")
> >   B: oops, "git-foo" is out of date
> >   B: rm $GIT_VALGRIND/git-foo
> >   A: look for $GIT_VALGRIND/git-foo; not there
> >   A: look for $PATH[1]/git-foo; ok, there it is
> >   B: ln -s ../../git-valgrind $GIT_VALGRIND/git-foo
> 
> Except that A had to check the link first, and it was out-of-date already 
> -- except if you changed a script into a builtin _and_ run make while a 
> valgrinded test is called _and_ you're unlucky.

Hrm, true. I consider running "make" in the middle of tests and
expecting them to work properly to be a bit crazy, so I guess this is
not a problem in practice.

I'll stop bugging you about race conditions for now, then. :)

> > readlink is not portable; it's part of GNU coreutils. Right now valgrind
> > basically only runs on Linux, which I think generally means that
> > readlink will be available (though I have no idea if there are
> > distributions that vary in this). However, there is an experimental
> > valgrind port to FreeBSD and NetBSD, which are unlikely to have
> > readlink.
> 
> As I mentioned earlier: let's bridge this bridge when we face it 
> (probably it involves making a test-readlink).

Actually, I am wrong. There is a stripped-down readlink that has
shipped with FreeBSD (since 4.10) and NetBSD (since 1.6). So while
readlink isn't portable, I think it should generally work on platforms
supported by valgrind.

> Or are you insisting that the patch should be reworked _now_ so that 
> GIT_EXEC_PATH _always_ points somewhere else?

No, I'm not insisting. It was merely a suggestion that the patch be
split into two parts so non-valgrind invocations can benefit from this
type of bug checking (and by this type I mean general PATH issues -- I
think we had some problems in the past with invoking dashed forms of
commands which were supposed to be available only via exec-path).

> I hope not, because then you break Windows.

Only if you use the same symlink technique.

-Peff