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

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

quoted

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