Re: [PATCH v3 12/35] serve: introduce git-serve

From: Brandon Williams <bmwill@google.com>
Date: Feb 23, 2018, 21:33 UTC
Message-ID: <20180223213315.GE234838@google.com>
In-Reply-To: <20180221144515.d180a26627a33168b19e0a4f@google.com>

On 02/21, Jonathan Tan wrote:

Show 27 quoted lines

> On Tue,  6 Feb 2018 17:12:49 -0800
> Brandon Williams <bmwill@google.com> wrote:
> 
> >  .gitignore                              |   1 +
> >  Documentation/technical/protocol-v2.txt | 114 +++++++++++++++
> >  Makefile                                |   2 +
> >  builtin.h                               |   1 +
> >  builtin/serve.c                         |  30 ++++
> >  git.c                                   |   1 +
> >  serve.c                                 | 250 ++++++++++++++++++++++++++++++++
> >  serve.h                                 |  15 ++
> >  t/t5701-git-serve.sh                    |  60 ++++++++
> >  9 files changed, 474 insertions(+)
> >  create mode 100644 Documentation/technical/protocol-v2.txt
> >  create mode 100644 builtin/serve.c
> >  create mode 100644 serve.c
> >  create mode 100644 serve.h
> >  create mode 100755 t/t5701-git-serve.sh
> 
> As someone who is implementing the server side of protocol V2 in JGit, I
> now have a bit more insight into this :-)
> 
> First of all, I used to not have a strong opinion on the existence of a
> new endpoint, but now I think that it's better to *not* have git-serve.
> As it is, as far as I can tell, upload-pack also needs to support (and
> does support, as of the end of this patch set) protocol v2 anyway, so it
> might be better to merely upgrade upload-pack.

Having it allows for easier testing and the easy ability to make it a true endpoint when we want to. As of right now, git-serve isn't an endpoint as you can't issue requests there via http-backend or git-daemon.

Show 20 quoted lines

> 
> > +A client then responds to select the command it wants with any particular
> > +capabilities or arguments.  There is then an optional section where the
> > +client can provide any command specific parameters or queries.
> > +
> > +    command-request = command
> > +		      capability-list
> > +		      (command-args)
> 
> If you are stating that this is optional, write "*1command-args". (RFC
> 5234 also supports square brackets, but "*1" is already used in
> pack-protocol.txt and http-protocol.txt.)
> 
> > +		      flush-pkt
> > +    command = PKT-LINE("command=" key LF)
> > +    command-args = delim-pkt
> > +		   *arg
> > +    arg = 1*CHAR
> 
> arg should be wrapped in PKT-LINE, I think, and terminated by an LF.

-- 
Brandon Williams