Re: [PATCH jk/pkt-line-cleanup] t5700-clone-reference: send trace to fd 2, not 3, to please Windows git

From: Jeff King <peff@peff.net>
Date: Mar 20, 2013, 17:06 UTC
Message-ID: <20130320170607.GB10752@sigill.intra.peff.net>
In-Reply-To: <51499DE7.2030404@viscovery.net>

On Wed, Mar 20, 2013 at 12:30:47PM +0100, Johannes Sixt wrote:

Show 8 quoted lines

> > I think that is OK, but I'm curious why this is a problem _now_, and not
> > with the code prior to 97a83fa8. The old GIT_DEBUG_SEND_PACK was also
> > just calling write() to descriptor 3.
> 
> Before this change, both affected commands completed and the trace file
> was empty. Notice that in both test cases we only check for the absence of
> certain lines, which is naturally true for an empty file, so that the
> tests pass.

Hmm. The code in t5503 is similar, but before my patch used to actually use "test -s" to make sure that some trace output was written. Did it fail before 97a83fa8 (and does it pass now)?

We should probably be adjusting t5503, too. And possibly adding back in the "test -s" checks (which I removed as redundant, but I guess the double-checking might have caught a platform error in this case).

Also, though I think your use of stderr is probably OK, might it be a little more robust against future output changes to use:

  GIT_TRACE_PACKET=$PWD/$U.D git clone ...

to write directly to the file. The original GIT_DEBUG_SEND_PACK did not support such niceties, but GIT_TRACE gives them to us for free.

> With the updated code, 'git fetch' hung, which is how I noticed the
> problem. As I said, I didn't investigate where and why this happens.

Thinking on it more, almost certainly what is happening is that fd 3 is being used for the actual protocol, and we are jamming random bytes over it. Since we have changed the bytes, the code is reacting in a different way (but the actual reaction doesn't matter; we should stop the cause, not worry about the symptom).

-Peff