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

Re: [RFC 7/7] upload-pack: ack version 2

From
BWBrandon Williams <bmwill@google.com>
Date
Sep 1, 2017, 23:20 UTC
Message-ID
<20170901232012.GA16739@google.com>
In-Reply-To
<CAGyf7-GKAW3Dn_rPKxtidKoXh7a6NmDFYzGrb9tGOszhsMignA@mail.gmail.com>
On 09/01, Bryan Turner wrote:
Show 21 quoted lines
> On Thu, Aug 24, 2017 at 3:53 PM, Brandon Williams <bmwill@google.com> wrote:
> > +
> > +       version = getenv("GIT_PROTOCOL");
> > +       if (!strcmp(version, "2"))
> > +               upload_pack_v2();
> > +
> 
> I think the "if" check here needs some type of NULL check for
> "version" before calling "strcmp". Without that, if the "GIT_PROTOCOL"
> environment variable isn't set, git-upload-pack SEGVs.
> 
> This came up when I was testing the "protocol-v2" branch with
> Bitbucket Server. For performance reasons, we sometimes run ref
> advertisements as a separate process, when serving fetches, to allow
> throttling the ref advertisement separately from actually generating
> and sending a packfile. Since CI servers continuously poll for
> changes, but usually don't find any, we want to be able to serve ref
> advertisements, which have minimal overhead, with much higher
> parallelism than serving packs. To do that, we run "git-upload-pack
> --advertize-refs" directly, and that command has never *required*
> "GIT_PROTOCOL" before this change.

Thanks for pointing this out. Since this was an RFC I wasn't being careful with doing these sorts of checks :). I'm currently working on the non-RFC version of this series and it is getting close to being in a state where I can send it out for more careful review.

-- 
Brandon Williams
Previous: Bryan TurnerNext: Brandon Williams
Message 13 of 31 in “[RFC 0/7] transitioning to protocol v2”
  1. Brandon WilliamsAug 24, 2017
  2. 1/7 pkt-line: add packet_write functionBrandon Williams, Aug 24, 2017
  3. 2/7 pkt-line: add strbuf_packet_readBrandon Williams, Aug 24, 2017
  4. 3/7 protocol: tell server that the client understands v2Brandon Williams, Aug 24, 2017
  5. Junio C HamanoAug 25, 2017
  6. Brandon WilliamsAug 25, 2017
  7. Brandon WilliamsAug 25, 2017
  8. 5/7 http: send Git-Protocol-Version headerBrandon Williams, Aug 24, 2017
  9. Kevin DaudtAug 30, 2017
  10. 6/7 transport: teach client to recognize v2 server responseBrandon Williams, Aug 24, 2017
  11. 7/7 upload-pack: ack version 2Brandon Williams, Aug 24, 2017
  12. Bryan TurnerSep 1, 2017
  13. Brandon WilliamsSep 1, 2017
  14. 4/7 t: fix ssh tests to cope with using '-o SendEnv=GIT_PROTOCOL'Brandon Williams, Aug 24, 2017
  15. Junio C HamanoAug 25, 2017
  16. Stefan BellerAug 25, 2017
  17. Junio C HamanoAug 25, 2017
  18. Jeff KingAug 25, 2017
  19. Jeff KingAug 25, 2017
  20. Jonathan NiederAug 25, 2017
  21. Jeff KingAug 25, 2017
  22. Brandon WilliamsAug 25, 2017
  23. Jeff HostetlerAug 29, 2017
  24. Brandon WilliamsAug 29, 2017
  25. Jeff KingAug 30, 2017
  26. Jeff HostetlerAug 30, 2017
  27. Brandon WilliamsAug 30, 2017
  28. Junio C HamanoAug 25, 2017
  29. Bryan TurnerAug 30, 2017
  30. Brandon WilliamsAug 30, 2017
  31. Bryan TurnerSep 1, 2017

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.