From: rsbecker@nexbridge.com Date: Thu, 18 Sep 2025 22:21:08 GMT Subject: RE: [Bug] git fetch --dry-run --filter makes changes to .git/config Message-ID: <022401dc28ea$8be87d40$a3b977c0$@nexbridge.com> In-Reply-To: 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