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

Re: [RFC PATCH v2 4/8] Support remote helpers implementing smart transports

From
ILIlari Liusvaara <ilari.liusvaara@elisanet.fi>
Date
Dec 5, 2009, 13:16 UTC
Message-ID
<20091205131630.GA1684@Knoppix>
In-Reply-To
<20091204183713.GE1483@spearce.org>
On Fri, Dec 04, 2009 at 10:37:13AM -0800, Shawn O. Pearce wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> wrote:
> 
> Why not 'fallback' to remain consistent with this protocol and many
> others in git where we stick to lowercase ASCII?
Changed.
 
> Why is the initial advertisement special?  If the helper always
> flushes both sides, it shouldn't ever deadlock the protocol.  Also,
> note that a helper should be able to implement a tiny delay like
> Nagle's algorithm does in TCP.  It just can't sit on a byte forever.

That's just extremely badly worded. The point is that only place where buffering has even seemed useful is initial adverts. Those can really trigger large numbers of small transfers if unbuffered.

Granted, small delay should work quite well.
I'll reword it.
> This hunk appears to be unrelated.  And actually, if transport has
> no remote, shouldn't the arg here be NULL so the helper gets only
> 1 argument and not 2 arguments?

Actually, it can't be NULL (the code would have already die()'d in that case). Fixed.

> I think exec winds up defaulting to name if --upload-pack was not
> used on the command line, and remote.$name.uploadpack was not set.
> See transport.c where you initialize the git options struct, these
> fields were defaulted in.

Actually, there was bug in initializing git options struct. It was properly initialized if one used foo://, but not with foo::bar://. Tripping it caused NULL exec to be passed if you didn't override. I fixed that bug and now the servpath code has just strcmp.

> My point is, we shouldn't send option servpath to the helper if
> name is equal to servpath, because the helper might not support
> servpath and the option command will issue a warning above for no
> reason at all.

This is exactly what happens (and yes, I tested). No --receive-pack doesn't trip warning, with --receive-pack it does.

-Ilari
Previous: Shawn O. PearceNext: Ilari Liusvaara
Message 14 of 20 in “Remote helpers smart transport extensions”
  1. 0/8 Remote helpers smart transport extensionsIlari Liusvaara, Dec 4, 2009
  2. 0/8 Remote helpers smart transport extensionsIlari Liusvaara, Dec 4, 2009
  3. 0/8 Remote helpers smart transport extensionsIlari Liusvaara, Dec 4, 2009
  4. 1/8 Pass unknown protocols to external protocol handlersIlari Liusvaara, Dec 4, 2009
  5. Sverre RabbelierDec 4, 2009
  6. Shawn O. PearceDec 4, 2009
  7. Ilari LiusvaaraDec 5, 2009
  8. 2/8 Refactor git transport options parsingIlari Liusvaara, Dec 4, 2009
  9. 3/8 Support taking over transportsIlari Liusvaara, Dec 4, 2009
  10. Shawn O. PearceDec 4, 2009
  11. Ilari LiusvaaraDec 5, 2009
  12. 4/8 Support remote helpers implementing smart transportsIlari Liusvaara, Dec 4, 2009
  13. Shawn O. PearceDec 4, 2009
  14. Ilari LiusvaaraDec 5, 2009
  15. 5/8 Support remote archive from external protocol helpersIlari Liusvaara, Dec 4, 2009
  16. 6/8 Remove special casing of http, https and ftpIlari Liusvaara, Dec 4, 2009
  17. Sverre RabbelierDec 4, 2009
  18. 7/8 Add remote helper debug modeIlari Liusvaara, Dec 4, 2009
  19. Sverre RabbelierDec 4, 2009
  20. 8/8 Support mandatory capabilitiesIlari Liusvaara, Dec 4, 2009

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.