From: <rsbecker@nexbridge.com>
To: "'Kevin Puetz'" <PuetzKevinA@johndeere.com>,
"'Jeff King'" <peff@peff.net>
Cc: "'brian m. carlson'" <sandals@crustytoothpaste.net>,
<git@vger.kernel.org>
Subject: RE: [Bug] git fetch --dry-run --filter makes changes to .git/config
Date: Thu, 18 Sep 2025 18:21:08 -0400 [thread overview]
Message-ID: <022401dc28ea$8be87d40$a3b977c0$@nexbridge.com> (raw)
In-Reply-To: <DS0PR05MB10013AC6090CEE7A562B56CBCB516A@DS0PR05MB10013.namprd05.prod.outlook.com>
On September 18, 2025 5:55 PM, Kevin Puetz wrote:
>> > I did find one odd quirk. When the remote and the requested commit
>> > are unrelated e.g. I'm taking to the wrong remote, or have done `git
>> > checkout --orphan`), I get
>> >
>> > $ git fetch https://github.com/git/git
>
> --negotiate-only --negotiation-tip=$COMMIT
>> > fatal: expected 'acknowledgments', received 'packfile'
>> >
>> > This still works for conan's purpose (exiting with an error means it
>> > didn't print a matching commit hash), but I expected something more like the
>"fatal: remote error: upload-pack: not our ref: ..."
>> > error that you get from git fetch {remote} --refetch $COMMIT. Of
>> > course, unexpectly sending a packfile could be a problem with github's server
>implementation, rather than the git client.
>>
>> I had never used --negotiate-only before, and did a simple test which
>> ended up with the exact same problem. I agree it's probably a bug, but
>> it's present in git.git itself (my fetch was to a local test repo).
>>
>> I'm not sure if the bug is in the server or client, though. If I
>> understand correctly, the server does not know anything about this
>> "negotiate-only" mode, but is just responding to the client. The
>> client is supposed to say "done" to tell the server that it is not
>> sending any more "have" negotiation, at which point the server sends the
>packfile.
>> But in this case the packfile comes anyway. Running with
>> GIT_TRACE_PACKET in the environment shows the client making an extra
>> request with no "have" lines at all, which the server then takes as an
>> indication it can send the packfile.
>
>Yep. In the cases where things work as expected, the client sends various `have`
>lines for ancestors of the negotiation-tips, and then the server sends "ACK ..." with
>the one it knows.
>
>When they are unrelated (none of the have lines are comits known to the remote),
>it replies with NAK (which is sensible enough). But then the client doesn't just error
>out after the NAK it keeps going. I guess because if this really was a fetch, rather
>than negotiate-only, and we have nothing in common with the server, we'd still
>need to fetch the wanted ref (and receive everything).
>But it *is* --negotiate-only, so we don't actually want anything either.
>Which leads to this weird empty request-for-nothing. Seems like the client Really
>should have just exited early after the NAK, given that it doesn't want anything.
>
>> So I think there is at least one client bug, which is making that
>> extra request. There might _also_ be a server bug, because it is
>> sending the packfile without a "done" line, even though it was told
>> the client wants the wait-for-done option (and the client does not even send a
>"want"
>> line at all, so what does it think it is sending?).
>
>Looking closer, I think the server bug is at
>
>https://github.com/git/git/blob/ca2559c1d630eb4f04cdee2328aaf1c768907a9e/
>upload-pack.c#L1805-L1813
>
>There's an early exit to UPLOAD_DONE (without sending any packfile) if you didn't
>want anything, But it's skipped over if you use wait-for-done, because that's the
>negotiate-only case, which still wants the acknowledgements.
>But then (during the weird empty client request) seen_haves isn't true either, So it
>skips the UPLOAD_SEND_ACKS case too. And it ends up just falling through into the
>final else { state = UPLOAD_SEND_PACK; }
>
>I think here the client was expecting the server to go into UPLOAD_SEND_ACKS, and
>just send an empty set of acknowledgements, which seems legal.
>https://git-scm.com/docs/gitprotocol-v2 says
>
>- If the server has found a suitable cut point and has decided to send a "ready" line,
> then the server can decide to (as an optimization) omit any "ACK" lines it would
>have sent
>
>So I think the server could/should just reply with no "ACK" lines and "ready"
>(i.e. you don't want anything, but sure, I can send you an empty pack file).
>But it's definitely a corner case. And right now it wouldn't work, because
>https://github.com/git/git/blob/master/upload-pack.c#L1712-L1713
>
>would send NAK instead (which also seems legal, depending on how one interprets
>
>- The server will respond with "NAK" if none of the object ids sent as have lines
>were common.
>
>I think the client could deal with either "ready" or "NAK", it's just the empty pack it's
>not expecting.
If I might ask, and I realize this is an edge-condition, but what is the intent when
the clone has been done with a --depth=1 or similar? The single commit may not
be known locally but may be in the history as far as the remote is concerned. I am
just curious.
--Randall
next prev parent reply other threads:[~2025-09-18 22:21 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-17 14:44 [Bug] git fetch --dry-run --filter makes changes to .git/config Kevin Puetz
2025-09-17 21:21 ` brian m. carlson
2025-09-17 23:23 ` Kevin Puetz
2025-09-18 19:20 ` Jeff King
2025-09-18 20:28 ` Junio C Hamano
2025-09-18 20:39 ` Jeff King
2025-09-18 20:46 ` Junio C Hamano
2025-09-18 21:55 ` Kevin Puetz
2025-09-18 22:21 ` rsbecker [this message]
2025-09-19 15:31 ` Kevin Puetz
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='022401dc28ea$8be87d40$a3b977c0$@nexbridge.com' \
--to=rsbecker@nexbridge.com \
--cc=PuetzKevinA@johndeere.com \
--cc=git@vger.kernel.org \
--cc=peff@peff.net \
--cc=sandals@crustytoothpaste.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
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;
as well as URLs for NNTP newsgroup(s).