Re: [PATCH 0/6] --valgrind improvements

[Jeff King <peff@peff.net>]

On Thu, May 16, 2013 at 10:50:11PM +0200, Thomas Rast wrote:

> One open issue with the last patch that currently eludes me: if I
> combine --valgrind-parallel with any --valgrind=*, there are lots of
> errors as (apparently) the valgrind wrapper setups race against each
> other.  However, without any --valgrind=* (thus defaulting to
> 'memcheck') this doesn't happen.

I noticed two racy error messages. If you do:

  cd t &&
  make clean &&
  ./some-test --valgrind-parallel=8

you will get complaints from mkdir about existing directories, as we use
mkdir as a poor man's O_EXCL to create lockfiles. These error messages
are harmless (we loop and try again), and we should perhaps just squelch
the stderr from mkdir. Although that might make weird situations hard to
diagnose, like another error that prevents creating the lockfile, so
maybe it is better to just live with the extra output (after the
directory is built once, it does not happen at all).

I also notice:

  $ ./t4052-* --valgrind-parallel=8 --valgrind=memcheck
  ...
  ./t4052-stat-output.sh: 572: ./test-lib.sh: cannot open
  /home/peff/compile/git/t/valgrind/bin/git-send-email.perl: No such
  file

Line 572 is checking the "#!" line of the _source_ file. So it shouldn't
be checking t/valgrind/bin in the first place. It looks like it comes
from this loop:

        for path in $PATH
        do
                ls "$path"/git-* 2> /dev/null |
                while read file
                do
                        make_valgrind_symlink "$file"
                done
        done

as t/valgrind/bin seems to be in the $PATH of the parallel
sub-processes. Hmm. It looks like you set up the valgrind dir in the
parent test process, and _then_ call the sub-processes in parallel. We
shouldn't need to do any valgrind setup at all in the subprocesses,
should we? They would just be replicating what the parent already did.

-Peff