On Wed, Jan 21, 2009 at 02:10:17AM +0100, Johannes Schindelin wrote:
> - symbolic links are inspected for correct targets now, and if they
> point somewhere else than expected, they are removed (this can
> error out if the file could not be removed) and recreated.
Now you _do_ have a race on this, and triggering it will cause you to
run a random version of git from your PATH, not using valgrind (instead
of running the version from the repo using valgrind). Something like:
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
> +--valgrind::
> + Execute all Git binaries with valgrind and stop on errors (the
> + exit code will be 126).
It doesn't necessarily stop: it just causes the command to fail, which
causes the test to fail. Which _will_ stop if you have "-i".
Also, you might want to mention that valgrind errors go to stderr, so
using "-v" is helpful.
> + # override all git executables in PATH and TEST_DIRECTORY/..
> + GIT_VALGRIND=$TEST_DIRECTORY/valgrind/bin
I think you should leave GIT_VALGRIND pointing to the main valgrind
directory. That way it is more convenient for people using
GIT_VALGRIND_OPTIONS to make use of GIT_VALGRIND without having to ".."
everything (for example, they may want to pick and choose suppressions
to load for their platform).
> + case "$base" in
> + *.sh|*.perl)
> + symlink_target=../unprocessed-script
> + esac
AFAIK, this triggers an error if I try to call "git-foo.perl" directly.
What does this have to do with valgrind? Why does this error checking
happen when I run --valgrind, but _not_ otherwise?
And yes, I know the answer is "because it's easy to do here, since
--valgrind is munging the PATH anyway". But my point is that that is an
_implementation_ detail, and the external behavior to a user is
nonsensical.
The fact that there are other uses for munging the PATH than valgrind
implies to me that we should _always_ be munging the PATH like this to
catch these sorts of errors. And then "--valgrind" can just change the
way we munge.
> + # create the link, or replace it if it is out of date
> + if test ! -h "$GIT_VALGRIND"/"$base" ||
> + test "$symlink_target" != \
> + "$(readlink "$GIT_VALGRIND"/"$base")"
> + 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.
-Peff
--
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
