Re: [PATCH] [PATCH] Fix upload_pack_v2 response ordering for shallow fetch when server has uploadpack.allowRefInWant=true

[Junio C Hamano <gitster@pobox.com>]

Royce Gerard Remer <royceremer@gmail.com> writes:

> Apologies, clearly struggling with using git send-email for the first
> time (and thank you for the reply). Here's the missing context from my
> cover:
>
> On my fleet of git severs (running Gitea, although it just shells out
> to the git cli and uses this client verbatim), I enabled this in the
> upload-pack config:
> uploadpack.allowRefInWant=true
>
> Clients performing fetches and clones all worked as normal unless they
> attempted a clone with the --depth parameter, where the client would
> get this error:
> fatal: expected 'packfile', received 'shallow-info'
>
> Looking at Documentation/gitprotocol-v2.adoc, it seems like when this
> feature was added, the ordering was just incorrect server-side. You
> wouldn't notice unless:
> 1) the server enabled the config above (I suspect it's not a heavily
> used feature in the wild)
> 2) the client performed a fetch operation with --depth
>
> That's what the new test cases in t/t5703-upload-pack-ref-in-want.sh
> are, those were written to prove the failure before the fix. I've been
> running this in my dev fleet of servers for a day now, trying various
> combinations of clone, with/without --depth, and fetches with
> --unshallow-since . This is purely server-side to honor the existing
> documented contract when these two features are in use together.
>
>> If a new version of "git upload-pack" suddenly swapped the order of
> them, would it break existing "git fetch" and "git clone"?
>
> I think this is a question about backwards-compatibility? This
> combination of features appears to have never worked, so clients which
> were receiving failures would no longer. If servers were previously
> configured to advertise allowRefInWant, clients could not have shallow
> cloned. If they did not have this feature configured, shallow clones
> would work the same way (the ordering of packets is unchanged).

Yes, all of the above are good material to be distilled into an
excellent commit log message.  The way how the problematic packet
sequence is produced, how the server and the client would behave and
cause reliable breakage on the client, how recent the features
involved in the bug are, and that apparently the combination are
rarely used, which would all explain why this breakage hasn't been
reported and diagnosed so far.