Re: [PATCH 3/3] pkt-line: support tracing verbatim pack contents

[Junio C Hamano <gitster@pobox.com>]

Augie Fackler <augie@google.com> writes:

> On Fri, Jun 12, 2015 at 5:28 PM, Jeff King <peff@peff.net> wrote:
>> When debugging the pack protocol, it is sometimes useful to
>> store the verbatim pack that we sent or received on the
>> wire. Looking at the on-disk result is often not helpful for
>> a few reasons:
>>
>>   1. If the operation is a clone, we destroy the repo on
>>      failure, leaving nothing on disk.
>>
>>   2. If the pack is small, we unpack it immediately, and the
>>      full pack never hits the disk.
>>
>>   3. If we feed the pack to "index-pack --fix-thin", the
>>      resulting pack has the extra delta bases added to it.
>>
>> We already have a GIT_TRACE_PACKET mechanism for tracing
>> packets. Let's extend it with GIT_TRACE_PACK to dump the
>> verbatim packfile.
>
> FWIW, this also works for me - I have no preference between my patches
> and Jeff's. I suspect yours are much better given that you have a clue
> about git internals ;).
>
> One bit of feedback is that it might be worth mentioning (though I
> don't feel strongly) that GIT_TRACE_PACK works with or without
> GIT_TRACE_PACKET - that wasn't immediately obvious to me, but it makes
> sense once I read the code.
>
> Thanks!

Thanks, both.  I think this series makes sense.

As to the documentation, I have a feeling that, unless the reader
and/or the user intimately knows that TRACE_PACK is implemented by
hooking into the same mechanism that TRACE_PACKET needs to, s/he
would not even wonder if TRACE_PACKET needs to be enabled when
asking for TRACE_PACK.  Yes, one is a proper substring of the other,
but the similarity between the two stops there.  While I do not
think it would hurt very much to mention that they are independent,
I have a slight suspicion that it might make it more likely to get
user confused.