(+cc: Elijah Newren and Derrick Stolee because of [1]) Hi, Jonathan Tan wrote: > Includes protocol documentation and a design document. Thanks for writing this. > Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> > --- > Documentation/technical/pack-protocol.txt | 10 ++-- > Documentation/technical/push-with-base.txt | 61 ++++++++++++++++++++++ > 2 files changed, 68 insertions(+), 3 deletions(-) > create mode 100644 Documentation/technical/push-with-base.txt > > diff --git a/Documentation/technical/pack-protocol.txt b/Documentation/technical/pack-protocol.txt > index e13a2c064d..0485616701 100644 > --- a/Documentation/technical/pack-protocol.txt > +++ b/Documentation/technical/pack-protocol.txt > @@ -59,9 +59,9 @@ Parameters", and are supported by the Git, SSH, and HTTP protocols. > Each Extra Parameter takes the form of `<key>=<value>` or `<key>`. > > Servers that receive any such Extra Parameters MUST ignore all > -unrecognized keys. Currently, the only Extra Parameter recognized is > -"version" with a value of '1' or '2'. See protocol-v2.txt for more > -information on protocol version 2. > +unrecognized keys. Currently, the only Extra Parameters recognized are > +"version" with a value of '1' or '2' and, for push, "base" with an OID. See > +protocol-v2.txt for more information on protocol version 2. It's nice to see an example of extra parameters being useful in protocol v0 as well. :) > > Git Transport > ------------- > @@ -506,6 +506,10 @@ real difference is that the capability listing is different - the only > possible values are 'report-status', 'report-status-v2', 'delete-refs', > 'ofs-delta', 'atomic' and 'push-options'. > > +If a "base=<oid>" Extra Parameter was sent by the client, and the > +server recognizes that object, the server MAY send "<oid> .have" in > +lieu of all the reference obj-ids and names. > + Can this only appear once, or is it permitted to pass multiple oids this way? I'm also curious about what Junio asked about elsewhere in the thread: for cases that benefit from more complex negotiation than the client proposing a particular oid, what comes next in this roadmap? I like that this is an optional feature so we could clean up later by removing support for it; do we expect to? If we do expect to, is there anything we could do to minimize the impact of the feature later (e.g. using a less short-and-sweet key name than `base`, maybe)? > Reference Update Request and Packfile Transfer > ---------------------------------------------- > > diff --git a/Documentation/technical/push-with-base.txt b/Documentation/technical/push-with-base.txt > new file mode 100644 > index 0000000000..d56aa7f900 > --- /dev/null > +++ b/Documentation/technical/push-with-base.txt > @@ -0,0 +1,61 @@ > +Push with base design notes > +=========================== > + > +This feature allows clients, when pushing, to indicate that a > +certain object is an ancestor of all pushed commits and that they > +believe that the server knows of this object. This in turn allows > +servers to send an abbreviated ref advertisement containing only that > +object. > + > +Besides bandwidth savings, this also ensures that the ref > +advertisement contains information relevant to the client. For > +example, at least one project (Gerrit [1]) have included workarounds > +to send ancestors of refs that move often, even though the ref > +advertisement is only meant to contain refs. > + > +[1] https://gerrit.googlesource.com/gerrit/+/refs/heads/master/java/com/google/gerrit/server/git/receive/HackPushNegotiateHook.java > + > + > +Design overview > +--------------- > + > +The "base" being sent is sent as an Extra Parameter, supported in the > +git://, ssh://, and http(s):// protocols. By sending it as an Extra > +Parameter, the server is aware of this parameter before it generates > +the ref advertisement, thus making it able to tailor the ref > +advertisement accordingly. Sending it as an Extra Parameter also makes > +this protocol backwards-compatible, as servers will ignore any Extra > +Parameters they do not understand. (The push will then proceed as if > +neither party had this feature.) > + > +The remote helper protocol has been extended to support the > +"push-base" capability and an option of the same name. When a remote > +helper advertises this capability, it thus indicates that it supports > +this option. Git then will send "option push-base" if the user > +specifies it when invoking "git push". > + > +The remote-curl remote helper bundled with Git has been updated to > +support this capability and option. Since this is an optional remote helper capability, it could be removed later. Good (but the capability and option names would still need to be reserved). > + > + > +Future work > +----------- > + > +In the future, we might want a way to automatically determine the base > +instead of always having the user specify it. However, this does not > +make obsolete any of the current work - once the base is automatically > +determined, we still need this protocol to communicate it to the > +server, and allowing the user to specify the base manually is still > +useful. Makes sense. I think this isn't "might", but "would"; most users that do not know why their push is slow wouldn't know to use this feature. > + > + > +Alternatives > +------------ > + > +- Making a more substantial protocol change like "fetch" protocol v2. > + This would eliminate the need for some of the remote helper updates; > + as part of the protocol change, the protocol could be made to > + support "stateless-connect" and thus no remote helper updates (like > + "push-base") would be needed. For "fetch", the protocol change has > + enabled features like wanted-refs and packfile-uris, but I do not > + have any similar ideas in mind for "push". I think you're saying that we don't need a "push" v2 because v0 already has what a user would want. Git protocol v2 for fetch brought two major changes: - it changed the response for the initial request, allowing abbreviating the ref advertisement at last - it defined a structure for requests and responses, simplifying the addition of later protocol improvements. In particular, because the initial response is a capability advertisement, it allows changing the ref advertisement format more in the future. Both of those changes would be valuable for push. The ref advertisements are large, and matching the structure of commands used by fetchv2 would make debugging easier. There are some specific applications I'm interested in after that (e.g., pushing symrefs), but the fundamental extensibility improvement is larger than any particular application I could think of. That said, I'm not against experimenting with extra parameters before we go there, as a way of getting more information about what a workable negotiation for push looks like. The push ref advertisement serves two roles: it advertises commits, to serve as negotiation, and it tells the client the current values of refs they may want to update, so they can send an appropriate compare-and-swap command for a force-push. If we have a workable replacement negotiation system, then the other role can be replaced with something simpler. For example: - listing ref updates before the pack, to allow the server to bail out early if they would be rejected, and - allowing an explicit force update of a ref instead of requiring the client to compare-and-swap if they don't care about the old value (but taking care to still make the client "fetch first" when appropriate!) Thanks, Jonathan [1] https://lore.kernel.org/git/CABPp-BEswHLhymQ1_07g3qqu=7kFR3eQyAHR0qMgSvi6THy=zQ@xxxxxxxxxxxxxx/
