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

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Aug 1, 2011, 20:48 UTC
Message-ID: <4E371109.7050500@lsrfire.ath.cx>
In-Reply-To: <20110801182516.GC10302@sigill.intra.peff.net>

Am 01.08.2011 20:25, schrieb Jeff King:

Show 34 quoted lines

> On Mon, Aug 01, 2011 at 08:02:22PM +0200, Erik Faye-Lund wrote:
> 
>>> 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.

Hrm, here is another data point, for what it's worth (Windows 7 x64, MinGW):

	$ git archive v1.7.6 | gzip -cn | md5sum
	a0ca1c873a533a5fcd41d248fb325a5b *-

	$ git archive --format=tar.gz v1.7.6 | md5sum
	a0ca1c873a533a5fcd41d248fb325a5b *-

	$ git archive v1.7.6 | gzip -cn >a.tgz && md5sum <a.tgz
	a0ca1c873a533a5fcd41d248fb325a5b *-

	$ ls -l a.tgz
	-rw-r--r--    1 User     Administ  3361584 Aug  1 20:44 a.tgz

	$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
	30886283af1aed05ae6a36fc5aeda077 *-

	$ ls -l a.tgz
	-rw-r--r--    1 User     Administ  3374383 Aug  1 20:45 a.tgz

	$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
	30886283af1aed05ae6a36fc5aeda077 *-

	$ git archive --format=tar.gz v1.7.6 >a.tgz && md5sum <a.tgz
	30886283af1aed05ae6a36fc5aeda077 *-

	$ git archive v1.7.6 | gzip -cn >a.tgz && md5sum <a.tgz
	a0ca1c873a533a5fcd41d248fb325a5b *-

	$ git archive --format=tar.gz v1.7.6 | cat >a.tgz && md5sum <a.tgz
	a0ca1c873a533a5fcd41d248fb325a5b *-

	$ git archive -o a.tgz v1.7.6 && md5sum <a.tgz
	30886283af1aed05ae6a36fc5aeda077 *-

So git archive gives the right results when writing to a pipe, but always the same wrong result when writing directly to a file. With --format=tar there is no difference:

	$ git archive v1.7.6 | md5sum
	fa364f38f9128df019dd8fd509c40a6d *-

	$ git archive v1.7.6 >a.tar && md5sum <a.tar
	fa364f38f9128df019dd8fd509c40a6d *-

René