Subject: Re: [RFC] push: add documentation on push v2

From: Brandon Williams <[email-masked]>
Date: Wed, 25 Jul 2018 10:46:22 -0700

On 07/25, Duy Nguyen wrote:
> On Tue, Jul 24, 2018 at 9:29 PM Brandon Williams <[email-masked]> wrote:
> >
> > On 07/17, Brandon Williams wrote:
> > > Signed-off-by: Brandon Williams <[email-masked]>
> > > ---
> > >
> > > Since introducing protocol v2 and enabling fetch I've been thinking
> > > about what its inverse 'push' would look like.  After talking with a
> > > number of people I have a longish list of things that could be done to
> > > improve push and I think I've been able to distill the core features we
> > > want in push v2.  Thankfully (due to the capability system) most of the
> > > other features/improvements can be added later with ease.
> > >
> > > What I've got now is a rough design for a more flexible push, more
> > > flexible because it allows for the server to do what it wants with the
> > > refs that are pushed and has the ability to communicate back what was
> > > done to the client.  The main motivation for this is to work around
> > > issues when working with Gerrit and other code-review systems where you
> > > need to have Change-Ids in the commit messages (now the server can just
> > > insert them for you and send back new commits) and you need to push to
> > > magic refs to get around various limitations (now a Gerrit server should
> > > be able to communicate that pushing to 'master' doesn't update master
> > > but instead creates a refs/changes/<id> ref).
> > >
> > > Before actually moving to write any code I'm hoping to get some feedback
> > > on if we think this is an acceptable base design for push (other
> > > features like atomic-push, signed-push, etc can be added as
> > > capabilities), so any comments are appreciated.
> > >
> > >  Documentation/technical/protocol-v2.txt | 76 +++++++++++++++++++++++++
> > >  1 file changed, 76 insertions(+)
> >
> > Pinging this thread again to hopefully reach some more people for
> > commentary.
> 
> Could you send a v2 that covers all the push features in pack version
> 1? I see some are discussed but it's probably good to summarize in
> this document too.

I can mention the ones we want to implement, but I expect that a push v2
would not require that all features in the current push are supported
out of the box.  Some servers may not want to support signed-push, etc.
Also I don't want to have to implement every single feature that exists
before getting something merged.  This way follow on series can be
written to implement those as new features to push v2.

> 
> A few other comments
> 
> If I remember correctly, we always update the remote refs locally
> after a push, assuming that the next 'fetch' will do the same anyway.
> This is not true for special refs (like those from gerrit). Looking
> from this I don't think it can say "yes we have received your pack and
> stored it "somewhere" but there's no visible ref created for it" so
> that we can skip the local remote ref update?

This is one of the pain points for gerrit and one of the reasons why
they have this funky push syntax "push origin HEAD:refs/for/master".
Because its not a remote tracking branch for the current branch, we
don't update (or create) a local branch "for/master" under the
"refs/remotes/origin" namespace (at least that's how i understand it).

So in order to support the server doing more things (rebasing, or
creating new branches) based on what is pushed the status that the
server sends in response needs to be more fluid so that the server can
describe what it did in a way that the client can either: update the
remote tracking branches, or not (and in this case maybe do what you
suggest down below).

> 
> Is it simpler to tell a push client at the end that "yes there's new
> stuff now on the server, do another fetch", sort of like HTTP
> redirect, then the client can switch to fetch protocol to get the new
> stuff that the server has created (e.g. rebase stuff)? I assume we
> could reuse the same connection for both push and fetch if needed.
> This way both fetch and push send packs in just one direction.

I really, really like this suggestion.  Thank you for your input.  This
would actually make the protocol much simpler by keeping push for
pushing packs and fetch for fetching packs.  And since my plan is to
have the status-report for push include all the relevant ref changes
that the server made, if there are any that don't correspond with what
was pushed (either the server rebased a change or created a new ref I
didn't) then we can skip the ref-advertisement and go straight to
fetching those refs.  And yes, since protocol v2 is command based we
could reuse the existing connection and simply send a "fetch" request
after our "push" one.

This does mean that it'll be an extra round trip than what I originally
had in mind, but it should be simpler.

-- 
Brandon Williams