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

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

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.

Public