public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks•im>
To: Matthew Dodd <mats.dodd12@gmail•com>
Cc: git@vger•kernel.org, Brandon Williams <bmwill@google•com>,
	Junio C Hamano <gitster@pobox•com>,
	Johannes Schindelin <Johannes.Schindelin@gmx•de>
Subject: Re: [PATCH 1/2] upload-pack: send shallow-info before wanted-refs in protocol v2
Date: Mon, 5 Jan 2026 14:00:44 +0100	[thread overview]
Message-ID: <aVu1_FOWqwuVPH9i@pks.im> (raw)
In-Reply-To: <20251224003504.52660-2-mats.dodd12@gmail.com>

On Wed, Dec 24, 2025 at 01:35:03AM +0100, Matthew Dodd wrote:
> From: Mats-Dodd <mats.dodd12@gmail•com>
> 
> The protocol v2 specification (Documentation/gitprotocol-v2.adoc) defines
> the ordering of optional sections in the fetch response as:
> 
>     [acknowledgments delim-pkt] [shallow-info delim-pkt]
>     [wanted-refs delim-pkt] [packfile-uris delim-pkt]
>     packfile flush-pkt
> 
> However, since the ref-in-want feature was introduced in 516e2b76bdc
> (upload-pack: implement ref-in-want, 2018-06-27), the server sends
> wanted-refs before shallow-info. This violates the specification and
> breaks the client (fetch-pack.c), which expects shallow-info first.
> 
> When a client performs a shallow clone/fetch against a server with
> uploadpack.allowRefInWant=true, the client receives sections in the
> wrong order and fails with:
> 
>     fatal: expected 'packfile', received 'shallow-info'
> 
> Fix by swapping the order of send_shallow_info() and

Nit: is there a word missing here? E.g. "Fix this by..."

> diff --git a/upload-pack.c b/upload-pack.c
> index 1e87ae9559..029ca93e69 100644
> --- a/upload-pack.c
> +++ b/upload-pack.c
> @@ -1830,8 +1830,8 @@ int upload_pack_v2(struct repository *r, struct packet_reader *request)
>  				state = UPLOAD_DONE;
>  			break;
>  		case UPLOAD_SEND_PACK:
> -			send_wanted_ref_info(&data);
>  			send_shallow_info(&data);
> +			send_wanted_ref_info(&data);

Indeed. The accompanying code in "fetch-pack.c" expects information the
other way round:

	if (process_section_header(&reader, "shallow-info", 1))
		receive_shallow_info(args, &reader, shallows, si);

	if (process_section_header(&reader, "wanted-refs", 1))
		receive_wanted_refs(&reader, sought, nr_sought);

The bug seems to exist since the inception of this feature. 516e2b76bd
(upload-pack: implement ref-in-want, 2018-06-27) implements the server
side in the current-broken way, and 733020517a (fetch-pack: implement
ref-in-want, 2018-06-27) implements the client side in the correct way.
So this combination has always been broken, and the fix looks obviously
correct to me indeed.

One nit though: I don't really think it's necessary to split up this
series into two patches. The new test can simply be added to this commit
here.

Thanks!

Patrick

  reply	other threads:[~2026-01-05 13:00 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-24  0:35 [PATCH 0/2] Fix shallow clone with ref-in-want enabled Matthew Dodd
2025-12-24  0:35 ` [PATCH 1/2] upload-pack: send shallow-info before wanted-refs in protocol v2 Matthew Dodd
2026-01-05 13:00   ` Patrick Steinhardt [this message]
2025-12-24  0:35 ` [PATCH 2/2] t5703: add test for shallow fetch with ref-in-want Matthew Dodd

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aVu1_FOWqwuVPH9i@pks.im \
    --to=ps@pks$(echo .)im \
    --cc=Johannes.Schindelin@gmx$(echo .)de \
    --cc=bmwill@google$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=mats.dodd12@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox