git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Turner <dturner@twopensource.com>
To: Stefan Beller <sbeller@google.com>
Cc: git@vger.kernel.org
Subject: Re: [WIP PATCH 00/14] Protocol v2 patches
Date: Tue, 24 May 2016 18:46:48 -0400	[thread overview]
Message-ID: <1464130008.24478.134.camel@twopensource.com> (raw)
In-Reply-To: <1461972887-22100-1-git-send-email-sbeller@google.com>

I was looking at this again today, and noticed that it doesn't really
address the HTTP case.

The central problem is that protocol v2 goes like this:
server: I have capabilities w,x,y, and z
client: I want capabilities x and z.

But HTTP goes like this:
client: [request]
server: [response]

I tried to make libcurl do the receive-before-sending thing, but it
doesn't seem to be designed for it (even if you prime things by sending
a "hello" from the client first).  My thought was to hook up
CURLOPT_READFUNCTION and CURLOPT_WRITEFUNCTION, and have the read
function return CURL_READFUNC_PAUSE and then have the write (=client
receiving data ) function unpause the reader (= client sending data)
once it gets the capabilities.  But apparently pausing only works with
chunked encoding, which seems to cause Apache's mod_cgi to fail.

Maybe I'm missing something.  Has anyone else ever made something like
this work?

Of course, I could always use CURLOPT_CONNECT_ONLY to write my own HTTP
client, but that seems pretty unreasonable.

I also looked to see if libcurl had websockets support, since that's
one kind of bidirectional conversation over HTTP, but it doesn't seem
to.

Another choice is to make a separate /capabilities endpoint that gets
hit before /info/refs.  This is a bit bad because:
(a) it's another HTTP request
(b) it adds implicit state to the HTTP conversation.  If multiple git
servers were behind a load balancer, you might end up getting server A
for /capabilities and server B for /info/refs, and those servers might
have different capabilities.  This is not impossible when testing a git
server upgrade on one machine before rolling it out to a whole fleet. 
 Maybe the rule for clients re capabilities is that they can request
whatever capabilities they want, but the server is free to ignore that
request and send whatever data it feels like.  That's not great, but it
should work (I think).

Does anyone else have any thoughts on how this ought to work?

  parent reply	other threads:[~2016-05-24 22:46 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-04-29 23:34 [WIP PATCH 00/14] Protocol v2 patches Stefan Beller
2016-04-29 23:34 ` [PATCH 01/14] upload-pack: make client capability parsing code a separate function Stefan Beller
2016-04-29 23:34 ` [PATCH 02/14] upload-pack.c: Refactor capability advertising Stefan Beller
2016-04-30  1:04   ` David Turner
2016-05-04 20:05   ` Junio C Hamano
2016-04-29 23:34 ` [PATCH 03/14] upload-pack-2: Implement the version 2 of upload-pack Stefan Beller
2016-05-02 17:43   ` David Turner
2016-05-02 17:51     ` Stefan Beller
2016-05-02 18:56       ` David Turner
2016-05-03  0:31         ` Duy Nguyen
2016-05-04 20:11     ` Junio C Hamano
2016-04-29 23:34 ` [PATCH 04/14] connect: rewrite feature parsing to work on string_list Stefan Beller
2016-05-02 18:18   ` David Turner
2016-05-02 18:46     ` Stefan Beller
2016-05-04 20:13   ` Junio C Hamano
2016-05-17 22:23     ` David Turner
2016-04-29 23:34 ` [PATCH 05/14] transport: add infrastructure to support a protocol version number Stefan Beller
2016-04-29 23:34 ` [PATCH 06/14] remote.h: add get_remote_capabilities, request_capabilities Stefan Beller
2016-05-02 18:57   ` David Turner
2016-05-03  5:33     ` Jeff King
2016-05-03 21:21       ` David Turner
2016-05-04 16:44         ` Stefan Beller
2016-04-29 23:34 ` [PATCH 07/14] fetch-pack: move capability selection out of do_fetch_pack Stefan Beller
2016-04-29 23:34 ` [PATCH 08/14] fetch-pack: factor out get_selected_capabilities_list Stefan Beller
2016-05-02 19:09   ` David Turner
2016-04-29 23:34 ` [PATCH 09/14] fetch-pack: Add negotiate_capabilities Stefan Beller
2016-04-29 23:34 ` [PATCH 10/14] do_fetch_pack: select capabilities for transport version 1 only Stefan Beller
2016-04-29 23:34 ` [PATCH 11/14] builtin/fetch-pack: add argument for transport version Stefan Beller
2016-04-29 23:34 ` [PATCH 12/14] Add test for fetch-pack Stefan Beller
2016-05-02 19:45   ` David Turner
2016-04-29 23:34 ` [PATCH 13/14] WIP add test for git pull Stefan Beller
2016-04-29 23:34 ` [PATCH 14/14] WIP test git fetch Stefan Beller
2016-05-02 20:41 ` [WIP PATCH 00/14] Protocol v2 patches David Turner
2016-05-02 20:43   ` Stefan Beller
2016-05-24 22:46 ` David Turner [this message]
2016-05-24 23:03   ` Duy Nguyen
2016-05-25 16:45     ` David Turner
2016-05-25 16:23   ` Junio C Hamano
2016-05-25 19:31     ` David Turner
2016-05-25 21:29   ` Jeff King

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1464130008.24478.134.camel@twopensource.com \
    --to=dturner@twopensource.com \
    --cc=git@vger.kernel.org \
    --cc=sbeller@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).