git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Junio C Hamano <gitster@pobox.com>
Date
Jun 16, 2015, 16:39 UTC
Message-ID
<xmqqvben6214.fsf@gitster.dls.corp.google.com>
In-Reply-To
<CAHcr6HYvVR4uTmtegWHK0h+v_aVs4JVLsSwvjthGY3pb=-Q0yQ@mail.gmail.com>
Augie Fackler <augie@google.com> writes:
Show 29 quoted lines
> 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.

Previous: Augie FacklerNext: Jeff King
Message 15 of 33 in “fetch-pack: optionally save packs to disk”
  1. fetch-pack: optionally save packs to diskAugie Fackler, Jun 11, 2015
  2. Johannes SixtJun 12, 2015
  3. Junio C HamanoJun 12, 2015
  4. Augie FacklerJun 12, 2015
  5. Jeff KingJun 12, 2015
  6. Jeff KingJun 12, 2015
  7. 1/3 pkt-line: simplify starts_with checks in packet tracingJeff King, Jun 12, 2015
  8. Stefan BellerJun 12, 2015
  9. 2/3 pkt-line: tighten sideband PACK check when tracingJeff King, Jun 12, 2015
  10. Stefan BellerJun 12, 2015
  11. Jeff KingJun 12, 2015
  12. Stefan BellerJun 12, 2015
  13. 3/3 pkt-line: support tracing verbatim pack contentsJeff King, Jun 12, 2015
  14. Augie FacklerJun 16, 2015
  15. Junio C HamanoJun 16, 2015
  16. Jeff KingJun 16, 2015
  17. Augie FacklerJun 16, 2015
  18. Jeff KingJun 16, 2015
  19. Jeff KingJun 16, 2015
  20. Augie FacklerJun 16, 2015
  21. Jeff KingJun 16, 2015
  22. Augie FacklerJun 16, 2015
  23. 0/3 add GIT_TRACE_STDINJeff King, Jun 16, 2015
  24. 1/3 trace: implement %p placeholder for filenamesJeff King, Jun 16, 2015
  25. 2/3 trace: add pid to each output lineJeff King, Jun 16, 2015
  26. 3/3 trace: add GIT_TRACE_STDINJeff King, Jun 16, 2015
  27. Jeff KingJun 16, 2015
  28. Jeff KingJun 16, 2015
  29. Duy NguyenJun 17, 2015
  30. Jeff KingJun 17, 2015
  31. Duy NguyenJun 18, 2015
  32. Junio C HamanoJun 26, 2015
  33. Augie FacklerJun 12, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.