Thread (7 messages) flat view 7 messages, 4 authors, 2025-09-19

RE: [Bug] git fetch --dry-run --filter makes changes to .git/config

From: <hidden>
Date: 2025-09-18 22:21:31

On September 18, 2025 5:55 PM, Kevin Puetz wrote:
quoted
quoted
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
quoted
quoted
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: ..."
quoted
quoted
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.
quoted
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.
quoted
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.
quoted
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"
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help