Re: [PATCH v3 00/35] protocol version 2

2018-02-21 Thread Brandon Williams
On 02/06, Brandon Williams wrote:
> Changes in v3:
>  * There were some comments about how the protocol should be designed
>stateless first.  I've made this change and instead of having to
>supply the `stateless-rpc=true` capability to force stateless
>behavior, the protocol just requires all commands to be stateless.
>  
>  * Added some patches towards the end of the series to force the client
>to not request to use protocol v2 when pushing (even if configured to
>use v2).  This is to ease the roll-out process of a push command in
>protocol v2.  This way when servers gain the ability to accept
>pushing in v2 (and they start responding using v2 when requests are
>sent to the git-receive-pack endpoint) that clients who still don't
>understand how to push using v2 won't request to use v2 and then die
>when they recognize that the server does indeed know how to accept a
>push under v2.
> 
>  * I implemented the `shallow` feature for fetch.  This feature
>encapsulates the existing functionality of all the shallow/deepen
>capabilities in v0.  So now a server can process shallow requests.
> 
>  * Various other small tweaks that I can't remember :)
> 
> After all of that I think the series is in a pretty good state, baring
> any more critical reviewing feedback.
> 
> Thanks!

I'm hoping to get some more in depth review before I do any more
re-rolls, but for those interested I will need to do a re-roll to
eliminate the prelude from the http transport.  This is the prelude
which includes the service line followed by any number of packet lines
culminating in a flush-pkt like so:

  # service=git-upload-pack
  some
  other
  optional
  lines
  

With this eliminated all transports will be exactly the same, the only
difference will be how the protocol is tunneled.

> 
> Brandon Williams (35):
>   pkt-line: introduce packet_read_with_status
>   pkt-line: introduce struct packet_reader
>   pkt-line: add delim packet support
>   upload-pack: convert to a builtin
>   upload-pack: factor out processing lines
>   transport: use get_refs_via_connect to get refs
>   connect: convert get_remote_heads to use struct packet_reader
>   connect: discover protocol version outside of get_remote_heads
>   transport: store protocol version
>   protocol: introduce enum protocol_version value protocol_v2
>   test-pkt-line: introduce a packet-line test helper
>   serve: introduce git-serve
>   ls-refs: introduce ls-refs server command
>   connect: request remote refs using v2
>   transport: convert get_refs_list to take a list of ref patterns
>   transport: convert transport_get_remote_refs to take a list of ref
> patterns
>   ls-remote: pass ref patterns when requesting a remote's refs
>   fetch: pass ref patterns when fetching
>   push: pass ref patterns when pushing
>   upload-pack: introduce fetch server command
>   fetch-pack: perform a fetch using v2
>   upload-pack: support shallow requests
>   fetch-pack: support shallow requests
>   connect: refactor git_connect to only get the protocol version once
>   connect: don't request v2 when pushing
>   transport-helper: remove name parameter
>   transport-helper: refactor process_connect_service
>   transport-helper: introduce stateless-connect
>   pkt-line: add packet_buf_write_len function
>   remote-curl: create copy of the service name
>   remote-curl: store the protocol version the server responded with
>   http: allow providing extra headers for http requests
>   http: don't always add Git-Protocol header
>   remote-curl: implement stateless-connect command
>   remote-curl: don't request v2 when pushing
> 
>  .gitignore  |   1 +
>  Documentation/technical/protocol-v2.txt | 338 +
>  Makefile|   7 +-
>  builtin.h   |   2 +
>  builtin/clone.c |   2 +-
>  builtin/fetch-pack.c|  21 +-
>  builtin/fetch.c |  14 +-
>  builtin/ls-remote.c |   7 +-
>  builtin/receive-pack.c  |   6 +
>  builtin/remote.c|   2 +-
>  builtin/send-pack.c |  20 +-
>  builtin/serve.c |  30 ++
>  builtin/upload-pack.c   |  74 
>  connect.c   | 352 +-
>  connect.h   |   7 +
>  fetch-pack.c| 319 +++-
>  fetch-pack.h|   4 +-
>  git.c   |   2 +
>  http.c  |  25 +-
>  http.h  |   2 +
>  ls-refs.c   |  96 +
>  ls-refs.h   |   9 +
>  pkt-line.c  | 149 +++-
>  pkt-line.h  |  77 
>  protocol.c 

Re: [PATCH v3 00/35] protocol version 2

2018-02-12 Thread Derrick Stolee

On 2/6/2018 8:12 PM, Brandon Williams wrote:

Changes in v3:
  * There were some comments about how the protocol should be designed
stateless first.  I've made this change and instead of having to
supply the `stateless-rpc=true` capability to force stateless
behavior, the protocol just requires all commands to be stateless.
  
  * Added some patches towards the end of the series to force the client

to not request to use protocol v2 when pushing (even if configured to
use v2).  This is to ease the roll-out process of a push command in
protocol v2.  This way when servers gain the ability to accept
pushing in v2 (and they start responding using v2 when requests are
sent to the git-receive-pack endpoint) that clients who still don't
understand how to push using v2 won't request to use v2 and then die
when they recognize that the server does indeed know how to accept a
push under v2.

  * I implemented the `shallow` feature for fetch.  This feature
encapsulates the existing functionality of all the shallow/deepen
capabilities in v0.  So now a server can process shallow requests.

  * Various other small tweaks that I can't remember :)

After all of that I think the series is in a pretty good state, baring
any more critical reviewing feedback.

Thanks!

Brandon Williams (35):
   pkt-line: introduce packet_read_with_status
   pkt-line: introduce struct packet_reader
   pkt-line: add delim packet support
   upload-pack: convert to a builtin
   upload-pack: factor out processing lines
   transport: use get_refs_via_connect to get refs
   connect: convert get_remote_heads to use struct packet_reader
   connect: discover protocol version outside of get_remote_heads
   transport: store protocol version
   protocol: introduce enum protocol_version value protocol_v2
   test-pkt-line: introduce a packet-line test helper
   serve: introduce git-serve
   ls-refs: introduce ls-refs server command
   connect: request remote refs using v2
   transport: convert get_refs_list to take a list of ref patterns
   transport: convert transport_get_remote_refs to take a list of ref
 patterns
   ls-remote: pass ref patterns when requesting a remote's refs
   fetch: pass ref patterns when fetching
   push: pass ref patterns when pushing
   upload-pack: introduce fetch server command
   fetch-pack: perform a fetch using v2
   upload-pack: support shallow requests
   fetch-pack: support shallow requests
   connect: refactor git_connect to only get the protocol version once
   connect: don't request v2 when pushing
   transport-helper: remove name parameter
   transport-helper: refactor process_connect_service
   transport-helper: introduce stateless-connect
   pkt-line: add packet_buf_write_len function
   remote-curl: create copy of the service name
   remote-curl: store the protocol version the server responded with
   http: allow providing extra headers for http requests
   http: don't always add Git-Protocol header
   remote-curl: implement stateless-connect command
   remote-curl: don't request v2 when pushing

  .gitignore  |   1 +
  Documentation/technical/protocol-v2.txt | 338 +
  Makefile|   7 +-
  builtin.h   |   2 +
  builtin/clone.c |   2 +-
  builtin/fetch-pack.c|  21 +-
  builtin/fetch.c |  14 +-
  builtin/ls-remote.c |   7 +-
  builtin/receive-pack.c  |   6 +
  builtin/remote.c|   2 +-
  builtin/send-pack.c |  20 +-
  builtin/serve.c |  30 ++
  builtin/upload-pack.c   |  74 
  connect.c   | 352 +-
  connect.h   |   7 +
  fetch-pack.c| 319 +++-
  fetch-pack.h|   4 +-
  git.c   |   2 +
  http.c  |  25 +-
  http.h  |   2 +
  ls-refs.c   |  96 +
  ls-refs.h   |   9 +
  pkt-line.c  | 149 +++-
  pkt-line.h  |  77 
  protocol.c  |   2 +
  protocol.h  |   1 +
  remote-curl.c   | 257 -
  remote.h|   9 +-
  serve.c | 260 +
  serve.h |  15 +
  t/helper/test-pkt-line.c|  64 
  t/t5701-git-serve.sh| 176 +
  t/t5702-protocol-v2.sh  | 239 
  transport-helper.c  |  84 +++--
  transport-internal.h|   4 +-
  transport.c

[PATCH v3 00/35] protocol version 2

2018-02-06 Thread Brandon Williams
Changes in v3:
 * There were some comments about how the protocol should be designed
   stateless first.  I've made this change and instead of having to
   supply the `stateless-rpc=true` capability to force stateless
   behavior, the protocol just requires all commands to be stateless.
 
 * Added some patches towards the end of the series to force the client
   to not request to use protocol v2 when pushing (even if configured to
   use v2).  This is to ease the roll-out process of a push command in
   protocol v2.  This way when servers gain the ability to accept
   pushing in v2 (and they start responding using v2 when requests are
   sent to the git-receive-pack endpoint) that clients who still don't
   understand how to push using v2 won't request to use v2 and then die
   when they recognize that the server does indeed know how to accept a
   push under v2.

 * I implemented the `shallow` feature for fetch.  This feature
   encapsulates the existing functionality of all the shallow/deepen
   capabilities in v0.  So now a server can process shallow requests.

 * Various other small tweaks that I can't remember :)

After all of that I think the series is in a pretty good state, baring
any more critical reviewing feedback.

Thanks!

Brandon Williams (35):
  pkt-line: introduce packet_read_with_status
  pkt-line: introduce struct packet_reader
  pkt-line: add delim packet support
  upload-pack: convert to a builtin
  upload-pack: factor out processing lines
  transport: use get_refs_via_connect to get refs
  connect: convert get_remote_heads to use struct packet_reader
  connect: discover protocol version outside of get_remote_heads
  transport: store protocol version
  protocol: introduce enum protocol_version value protocol_v2
  test-pkt-line: introduce a packet-line test helper
  serve: introduce git-serve
  ls-refs: introduce ls-refs server command
  connect: request remote refs using v2
  transport: convert get_refs_list to take a list of ref patterns
  transport: convert transport_get_remote_refs to take a list of ref
patterns
  ls-remote: pass ref patterns when requesting a remote's refs
  fetch: pass ref patterns when fetching
  push: pass ref patterns when pushing
  upload-pack: introduce fetch server command
  fetch-pack: perform a fetch using v2
  upload-pack: support shallow requests
  fetch-pack: support shallow requests
  connect: refactor git_connect to only get the protocol version once
  connect: don't request v2 when pushing
  transport-helper: remove name parameter
  transport-helper: refactor process_connect_service
  transport-helper: introduce stateless-connect
  pkt-line: add packet_buf_write_len function
  remote-curl: create copy of the service name
  remote-curl: store the protocol version the server responded with
  http: allow providing extra headers for http requests
  http: don't always add Git-Protocol header
  remote-curl: implement stateless-connect command
  remote-curl: don't request v2 when pushing

 .gitignore  |   1 +
 Documentation/technical/protocol-v2.txt | 338 +
 Makefile|   7 +-
 builtin.h   |   2 +
 builtin/clone.c |   2 +-
 builtin/fetch-pack.c|  21 +-
 builtin/fetch.c |  14 +-
 builtin/ls-remote.c |   7 +-
 builtin/receive-pack.c  |   6 +
 builtin/remote.c|   2 +-
 builtin/send-pack.c |  20 +-
 builtin/serve.c |  30 ++
 builtin/upload-pack.c   |  74 
 connect.c   | 352 +-
 connect.h   |   7 +
 fetch-pack.c| 319 +++-
 fetch-pack.h|   4 +-
 git.c   |   2 +
 http.c  |  25 +-
 http.h  |   2 +
 ls-refs.c   |  96 +
 ls-refs.h   |   9 +
 pkt-line.c  | 149 +++-
 pkt-line.h  |  77 
 protocol.c  |   2 +
 protocol.h  |   1 +
 remote-curl.c   | 257 -
 remote.h|   9 +-
 serve.c | 260 +
 serve.h |  15 +
 t/helper/test-pkt-line.c|  64 
 t/t5701-git-serve.sh| 176 +
 t/t5702-protocol-v2.sh  | 239 
 transport-helper.c  |  84 +++--
 transport-internal.h|   4 +-
 transport.c | 116 --
 transport.h |   9 +-
 upload-pack.c   | 625