Re: [PATCH v2 4/4] upload-archive: use start_command instead of fork

From: Jeff King <peff@peff.net>
Date: Aug 1, 2011, 18:25 UTC
Message-ID: <20110801182516.GC10302@sigill.intra.peff.net>
In-Reply-To: <CABPQNSbyp2hEgXvzo3bMHHxrbc9fQLqaABweqfjP7xQzBUdj6A@mail.gmail.com>

On Mon, Aug 01, 2011 at 08:02:22PM +0200, Erik Faye-Lund wrote:

Show 17 quoted lines

> > Weird. What does j.tgz end up looking like? Is it empty, or does it have
> > bogus data in it? Does gzip actually get invoked at all? Try running
> > with GIT_TRACE=1. I don't suppose you guys have something like strace,
> > which might be helpful.
> >
> 
> It does have data, and gzip does get invoked:
> $ gunzip -c j.tgz | wc -c
> 
> gzip: j.tgz: invalid compressed data--format violated
>  131072
> 
> So it seems there are around 130k of valid data before it barfs.
> 
> Hmm, but when I try the same after re-running the test, I get a
> different amount of valid data (491520 bytes this time)... Is this a
> timing-related issue?

Hmm. Non-deterministic output is not good. Could be timing, or we could be touching memory we're not supposed to. I just ran t5000 through valgrind, and nothing turned up. And of course I can't replicate the test failure on Linux. So I assume it really is Windows-specific.

That makes me suspect some difference in how the run-command API works on the two platforms, since that is the code here that will be vastly difference. I notice the start_command code uses dup() on Windows instead of dup2() (I guess you guys don't have dup2?). I wonder if there could be some issue with another descriptor accidentally pointing to the same spot. But that's just a wild guess.

I think at this point, I'd probably start stepping through the archive code with gdb.

-Peff