Re: [RFC 0/7] transitioning to protocol v2

From: Jeff King <peff@peff.net>
Date: Aug 25, 2017, 17:41 UTC
Message-ID: <20170825174145.x6oa7btkpy5yii54@sigill.intra.peff.net>
In-Reply-To: <20170825173550.GJ13924@aiede.mtv.corp.google.com>

On Fri, Aug 25, 2017 at 10:35:50AM -0700, Jonathan Nieder wrote:

Show 10 quoted lines

> > Sadly, while splitting these things apart makes the protocol
> > conceptually cleaner, I'm not sure if we can consider them separately
> > and avoid adding an extra round-trip to the protocol.
> 
> How about the idea of using this mechanism to implement a protocol
> "v1"?
> 
> The reply would be the same as today, except that it has a "protocol
> v1" pkt-line at the beginning.  So this doesn't change the number of
> round-trips --- it just validates the protocol migration approach.

I'm not that worried about validating the ideas here to shoe-horn a version field. I'm worried about what "step 2" is going to look like, and whether "we shoe-horned a version field" can be extended to "we shoe-horned arbitrary data".

Maybe those are the same thing, and validating the first validates the second. But I don't think it's a foregone conclusion.

Show 7 quoted lines

> > Current git.git servers, I assume?. How much do we want to care about
> > alternate implementations? I would not be surprised if other git://
> > implementations are more picky about cruft after the virtual-host field
> > (though I double-checked GitHub's implementation at least, and it is
> > fine).
> 
> FWIW JGit copes fine with this.

Thanks for checking.

Show 5 quoted lines

> > I don't think libgit2 implements the server side. That leaves probably
> > JGit, Microsoft's VSTS (which I think is custom), and whatever Atlassian
> > and GitLab use.
> 
> I'd be happy if someone tests the patches against those. :)

Me too. I don't have an easy setup for the last 3.

Show 6 quoted lines

> >                        Or alternatively, I guess make this optional to
> > start with, and let early adopters turn it on and complain to their server
> > vendors for a while before flipping the default to on.
> 
> Can we do that by making it a patch / letting it cook for a while in
> 'next'? :)

If people actually ran 'next', that would help. I was hoping that we could get it out to the masses behind a feature flag, and dangle it in front of them with "this will improve fetch performance if you turn it on". But that carrot implies going all the way through the follow-on steps of designing the performance-improving v2 extensions and getting them implemented on the server side.

-Peff