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

Re: [RFC/WIP PATCH 00/11] Protocol version 2, again!

From
Duy Nguyen <pclouds@gmail.com>
Date
Jun 2, 2015, 10:10 UTC
Message-ID
<CACsJy8CGW-Kaq6hkox03w5bQS2=tVNMzShKUBgC6wKX9dkMQGw@mail.gmail.com>
In-Reply-To
<CAGZ79kZxFnkneixquUijd2yfKBh6+XnYiYzCh8L9Mkourh64Fw@mail.gmail.com>
On Tue, Jun 2, 2015 at 12:49 AM, Stefan Beller <sbeller@google.com> wrote:
Show 12 quoted lines
> However the client side with builtin/fetch, builtin/fetch-pack, fetch-pack
> is a bit of a mystery to me, as I cannot fully grasp the difference between
>  * connect.{h,c}
>  * remote.{h.c}
>  * transport.{h.c}
> there. All of it seems to be doing network related stuff, but I have trouble
> getting the big picture. I am assuming all of these 3 are rather a low level,
> used like a library, though there must be even more hierarchy in there,
> connect is most low level judging from the header file and used by
> the other two.
> transport.h seems to provide the most toplevel library stuff as it includes
> remote.h in its header?

I think transport.c is there to support non-native protocols (and later on, smart http). So yeah it's basically the API for fetches and pushes. git-log over those files may reveal their purposes, especially the few first versions of them.

Show 31 quoted lines
> The problem I am currently trying to tackle, is passing the options through all
> the layers early on. so in a few places we have code like
>
>     switch (version) {
>     case 2: /* first talk about capabilities, then get the heads */
>         get_remote_capabilities(data->fd[0], NULL, 0);
>         select_capabilities();
>         request_capabilities(data->fd[1]);
>         /* fall through */
>     case 1:
>         get_remote_heads(data->fd[0], NULL, 0, &refs,
>                  for_push ? REF_NORMAL : 0,
>                  &data->extra_have,
>                  &data->shallow);
>         break;
>     default:
>         die("BUG: Transport version %d not supported", version);
>         break;
>     }
>
> and the issue I am concerned about is the select_capabilities as well as
> the request_capabilities function here. The select_capabilities functionality
> is currently residing in the high level parts of the code as it both depends on
> the advertised server capabilities and on the user input (--use-frotz or config
> options), so the capability selection is done in fetchpack.c
>
> So there are 2 routes to go: Either we leave the select_capabilities in the
> upper layers (near the actual high level command, fetch, fetchpack) or we put
> it into the transport layer and just passing in a struct what the user desires.
> And when the users desire doesn't meet the server capabilities we die deep down
> in the transport layer.

I read the latest re-roll and I think the placement makes sense. You can't put protocol specific at transport level because "pack protocol" is just one of the supported protocols. There is smart-http (which shares a bunch of code, but from transport perspective is a separate protocol), and then user-defined protocols that know nothing about this v2.

-- 
Duy
Previous: Stefan BellerNext: Jeff King
Message 59 of 61 in “Protocol version 2, again!”
  1. 00/11 Protocol version 2, again!Stefan Beller, May 26, 2015
  2. 01/11 upload-pack: make client capability parsing code a separate functionStefan Beller, May 26, 2015
  3. 02/11 upload-pack: only accept capabilities on the first "want" lineStefan Beller, May 26, 2015
  4. Junio C HamanoMay 26, 2015
  5. Stefan BellerMay 26, 2015
  6. 03/11 upload-pack: move capabilities out of send_refStefan Beller, May 26, 2015
  7. 04/11 upload-pack-2: Implement the version 2 of upload-packStefan Beller, May 26, 2015
  8. Eric SunshineMay 27, 2015
  9. Jeff KingMay 27, 2015
  10. Eric SunshineMay 27, 2015
  11. Jeff KingMay 27, 2015
  12. Stefan BellerMay 27, 2015
  13. Jeff KingMay 27, 2015
  14. Stefan BellerMay 27, 2015
  15. Jeff KingMay 27, 2015
  16. 05/11 transport: add infrastructure to support a protocol version numberStefan Beller, May 26, 2015
  17. Jeff KingMay 27, 2015
  18. Stefan BellerMay 27, 2015
  19. Jeff KingMay 27, 2015
  20. Junio C HamanoMay 27, 2015
  21. 06/11 remote.h: add get_remote_capabilities, request_capabilitiesStefan Beller, May 26, 2015
  22. Eric SunshineMay 27, 2015
  23. Jeff KingMay 27, 2015
  24. Eric SunshineMay 27, 2015
  25. Jeff KingMay 27, 2015
  26. Jeff KingMay 27, 2015
  27. Stefan BellerMay 29, 2015
  28. Jeff KingMay 29, 2015
  29. 07/11 fetch-pack: use the configured transport protocolStefan Beller, May 26, 2015
  30. Junio C HamanoMay 26, 2015
  31. Stefan BellerMay 26, 2015
  32. Jeff KingMay 27, 2015
  33. 08/11 transport: connect_setup appends protocol version numberStefan Beller, May 26, 2015
  34. Junio C HamanoMay 26, 2015
  35. Stefan BellerMay 26, 2015
  36. Junio C HamanoMay 27, 2015
  37. Jeff KingMay 27, 2015
  38. Eric SunshineMay 27, 2015
  39. Jeff KingMay 27, 2015
  40. 09/11 transport: get_refs_via_connect exchanges capabilities before refs.Stefan Beller, May 26, 2015
  41. Eric SunshineMay 27, 2015
  42. Jeff KingMay 27, 2015
  43. 10/11 t5544: add a test case for the new protocolStefan Beller, May 26, 2015
  44. Eric SunshineMay 27, 2015
  45. Jeff KingMay 27, 2015
  46. 11/11 Document protocol version 2Stefan Beller, May 26, 2015
  47. Junio C HamanoMay 29, 2015
  48. Stefan BellerMay 29, 2015
  49. Junio C HamanoMay 29, 2015
  50. Jeff KingMay 29, 2015
  51. Stefan BellerJun 1, 2015
  52. Stefan BellerJun 1, 2015
  53. Jeff KingJun 4, 2015
  54. Junio C HamanoJun 4, 2015
  55. Junio C HamanoJun 2, 2015
  56. Jeff KingMay 27, 2015
  57. Jeff KingMay 27, 2015
  58. Stefan BellerJun 1, 2015
  59. Duy NguyenJun 2, 2015
  60. Jeff KingJun 4, 2015
  61. Stefan BellerJun 4, 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.