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

From: Jeff King <peff@peff.net>
Date: May 29, 2013, 04:53 UTC
Message-ID: <20130529045356.GA31762@sigill.intra.peff.net>
In-Reply-To: <cover.1368736093.git.trast@inf.ethz.ch>

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

Show 5 quoted lines

> 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