On Thu, Feb 26, 2015 at 2:15 AM, Duy Nguyen <pclouds@xxxxxxxxx> wrote:
> On Thu, Feb 26, 2015 at 2:31 PM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>> On Wed, Feb 25, 2015 at 10:04 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Duy Nguyen <pclouds@xxxxxxxxx> writes:
>>>
>>>> On Wed, Feb 25, 2015 at 6:37 AM, Stefan Beller <sbeller@xxxxxxxxxx> wrote:
>>>>> I can understand, that we maybe want to just provide one generic
>>>>> "version 2" of the protocol which is an allrounder not doing bad in
>>>>> all of these aspects, but I can see usecases of having the desire to
>>>>> replace the wire protocol by your own implementation. To do so
>>>>> we could try to offer an API which makes implementing a new
>>>>> protocol somewhat easy. The current state of affairs is not providing
>>>>> this flexibility.
>>>>
>>>> I think we are quite flexible after initial ref advertisement.
>>>
>>> Yes, that is exactly where my "I am not convinced" comes from.
>>>
>>
>> We are not. (not really at least). We can tune some parameters or
>> change the behavior slightly,
>> but we cannot fix core assumptions made when creating v2 protocol.
>> This you can see when when talking about v1 as well: we cannot fix any
>> wrongdoings of v1 now by adding another capability.
>
> Step 1 then should be identifying these wrongdoings and assumptions.
So I think one of the key assumption was to not have many refs to advertise,
and advertising the refs is fine under that assumption.
So from my point of view it is hard to change the general
>
> We can really go wild with these capabilities. The only thing that
> can't be changed is perhaps sending the first ref. I don't know
> whether we can accept a dummy first ref... After that point, you can
> turn the protocol upside down because both client and server know what
> it would be.
So the way I currently envision (the transition to and) version 2 of
the protocol:
First connection (using the protocol as of now):
Server: Here are all the refs and capabilities I can offer. The capabilities
include "not-send-refs-first" (aka version2)
Client: Ok, I'll store not-send-refs-first for next time. Now we will
continue with
these options: <...>. For now we continue using the current
protocol and I want
to update the "master" branch.
Server: Ok here is a pack file, and then master advances $SHA1..$SHA1
Client: ok, thanks, bye
For the next connection I have different ideas:
Client thinks v2 is supported, so it talks first: Last time we talked
your capabilities
hashed to "$SHA1", is that still correct?
Server: yes it is
# In the above roundtrip we would have a new key assumption that the
capabilities
# don't change often. Having push-certs enabled, this is invalid of
today. Hover this
# could be implemented with very low bandwidth usage
# The alternative path would be:
# Server: No my new capabilities are: <....>
Client: Ok I want to update all of refs/heads/{master,next,pu}. My
last fetch was
yesterday at noon.
Server: Let me check the ref logs for these refs. Here is a packfile of length
1000 bytes: <binary gibberish>
{master, next} did not update since yesterday noon, pu updates from A..B
Client: ok, thanks, bye
Another approach would be this:
Client thinks v2 is supported, so it talks first: Last time we talked
you sent me
refs advertisement including capabilities which hash to $SHA1.
Server: I see, I have stored that. Now that time has advanced there are a few
differences, here is a diff of the refs advertisement:
* b3a551adf53c224b04c40f05b72a8790807b3138 HEAD\0 <capabilities>
* b3a551adf53c224b04c40f05b72a8790807b3138 refs/heads/master
- 24ca137a384aa1ac5a776eddaf35bb820fc6f6e6 refs/heads/tmp-fix
+ 1da8335ad5d0e46062a929ba6481bbbe35c8eef0 refs/pull/123/head
Note that I do not include changed lines as +one line and - one
line as you know
what the line was by your given $SHA1, so changed lines are marked
with *, while
lines starting with '-' indicate deleted refs and '+' indicate new refs.
Client: I see, I can reconstruct the refs advertisement. Now we can
continue talking
as we always talked using v1 protocol.
>
>> So from my point
>> of view we don't waste resources when having an advertisement of
>> possible protocols instead of a boolean flag indicating v2 is
>> supported. There is really not much overhead in coding nor bytes
>> exchanged on the wire, so why not accept stuff that comes for free
>> (nearly) ?
>
> You realize you're advertising v2 as a new capability, right? Instead
> of defining v2 feature set then advertise v2, people could simply add
> new features directly. I don't see v2 (at least with these patches)
> adds any value.
Yes, we can go wild after the refs advertisement, but that is not the
critical problem as it works ok-ish? The problem I see for now is the huge
refs advertisement even before the capabilities are exchanged. So maybe
I do not want to talk about v2 but about changing the current protocol to first
talk about the capabilities in the first round trip, not sure if we
ever want to attach
data into the first RT as it may explode as soon as that information grows in
the future. The disadvantage would be one added round trip time, though I'd
guess that's ok for most compared to the huge refs advertisement.
>
>> I mean how do we know all the core assumptions made for v2 hold in the
>> future? We don't. That's why I'd propose a plain and easy exchange at
>> first stating the version to talk.
>
> And we already does that, except that we don't state what version (as
> a number) exactly, but what feature that version supports. The focus
> should be the new protocol at daemon.c and maybe remote-curl.c where
> we do know the current protocol is not flexible enough.
Most features are pretty non-invasive and at a few points in the code, you
can add
if (featureX)
doFeatureXRelatedStuff();
For changing the initial talking we'd need to make a more drastic change
to the code, which is why I thought to call it v2.
Thanks for your input,
Stefan
> --
> Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html