{"thread":{"id":"64161","subject":"[Bug] git fetch --dry-run --filter makes changes to .git/config","startedAt":"2025-09-17T15:05:44Z","lastAt":"2025-09-19T15:31:54Z","messageCount":10,"participants":["Kevin Puetz","brian m. carlson","Jeff King","Junio C Hamano","rsbecker@nexbridge.com"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"526583","messageId":"CY8PR05MB100119985C7C25A72E530556DB517A@CY8PR05MB10011.namprd05.prod.outlook.com","threadId":"64161","inReplyTo":null,"subject":"[Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Kevin Puetz","fromEmail":"puetzkevina@johndeere.com","sentAt":"2025-09-17T14:44:00Z","receivedAt":"2025-09-17T15:05:44Z","isPatch":false,"sender":{"key":"puetzkevina@johndeere.com","avatar":null},"body":"What did you do before the bug happened? (Steps to reproduce your issue)\n\n    `git fetch origin --refetch --dry-run {commit} --depth=1 --filter=tree:0 --no-auto-gc`\n\nWhat did you expect to happen? (Expected behavior)\n\n    I expected it to fetch just the one commit object (no blobs, no trees, no history),\n    as a means of checking whether that commit is actually known to the remote.\n\nWhat happened instead? (Actual behavior)\n\n    the .git/config file was modified, adding\n\n    [remote \"origin\"]\n        promisor = true\n        partialclonefilter = tree:0\n\nWhat's different between what you expected and what actually happened?\n\n    I did not expect any changes to the local clone (due to the use of --dry-run)\n\nAnything else you want to add:\n\n    Context is https://github.com/conan-io/conan/issues/18949\n\n\n    trying to avoid a full-re-download in the process of checking\n    whether the HEAD commit hash exists in a remote.\n    The command was expected to either be a no-op success, or fail\n\n    > fatal: remote error: upload-pack: not our ref {commit}\"\n\n[System Info]\ngit version:\ngit version 2.51.0.windows.1\ncpu: x86_64\nbuilt from commit: 4d21a77b98af5cf479d8b6f863c2aa94257cd4e1\nsizeof-long: 4\nsizeof-size_t: 8\nshell-path: D:/git-sdk-64-build-installers/usr/bin/sh\nfeature: fsmonitor--daemon\nlibcurl: 8.15.0\nOpenSSL: OpenSSL 3.2.4 11 Feb 2025\nzlib: 1.3.1\nSHA-1: SHA1_DC\nSHA-256: SHA256_BLK\ndefault-ref-format: files\ndefault-hash: sha1\nuname: Windows 10.0 26100\ncompiler info: gnuc: 15.2\nlibc info: no libc information available\n$SHELL (typically, interactive shell): C:\\Program Files\\Git\\usr\\bin\\bash.exe\n\n[Enabled Hooks]\n\nPublic\n"},{"id":"526631","messageId":"aMsmbU_Cg0L6kOlm@fruit.crustytoothpaste.net","threadId":"64161","inReplyTo":"CY8PR05MB100119985C7C25A72E530556DB517A@CY8PR05MB10011.namprd05.prod.outlook.com","subject":"Re: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2025-09-17T21:21:49Z","receivedAt":"2025-09-17T21:21:51Z","isPatch":false,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2025-09-17 at 14:44:00, Kevin Puetz wrote:\n> What did you do before the bug happened? (Steps to reproduce your issue)\n> \n>     `git fetch origin --refetch --dry-run {commit} --depth=1 --filter=tree:0 --no-auto-gc`\n> \n> What did you expect to happen? (Expected behavior)\n> \n>     I expected it to fetch just the one commit object (no blobs, no trees, no history),\n>     as a means of checking whether that commit is actually known to the remote.\n> \n> What happened instead? (Actual behavior)\n> \n>     the .git/config file was modified, adding\n> \n>     [remote \"origin\"]\n>         promisor = true\n>         partialclonefilter = tree:0\n\nI will note that if this command actually downloads any data, this is\nrequired.  That's because your repository is incomplete: you want to\ndownload exactly one commit and without marking the promisor remote, you\nwill lack the ability to acquire trees or blobs and your repository will\nthen be corrupt.\n\n> What's different between what you expected and what actually happened?\n> \n>     I did not expect any changes to the local clone (due to the use of --dry-run)\n\nI agree --dry-run should not change your repository.  However, I would\nalso say that it should not contact the server, either, so it may not do\nwhat you want.  If that's not what it was intended to do, perhaps\nsomeone can clarify the documentation as to its functionality.\n\n> Anything else you want to add:\n> \n>     Context is https://github.com/conan-io/conan/issues/18949\n> \n> \n>     trying to avoid a full-re-download in the process of checking\n>     whether the HEAD commit hash exists in a remote.\n>     The command was expected to either be a no-op success, or fail\n\nI don't think that the command you've provided is a good or efficient\nway of doing what you want, but I'm also not sure that there's a good or\nefficient way to do what you want using command line Git (you might need\nto write a small portion of the protocol, for instance).\n-- \nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"526646","messageId":"DS0PR05MB100132C058A3D9D5AA735D905B517A@DS0PR05MB10013.namprd05.prod.outlook.com","threadId":"64161","inReplyTo":"aMsmbU_Cg0L6kOlm@fruit.crustytoothpaste.net","subject":"RE: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Kevin Puetz","fromEmail":"puetzkevina@johndeere.com","sentAt":"2025-09-17T23:23:21Z","receivedAt":"2025-09-17T23:23:30Z","isPatch":false,"sender":{"key":"puetzkevina@johndeere.com","avatar":null},"body":"\n\nPublic\nOn 2025-09-17 at 14:44:00, Kevin Puetz wrote:\n\n> >     the .git/config file was modified, adding\n> >\n> >     [remote \"origin\"]\n> >         promisor = true\n> >         partialclonefilter = tree:0\n\n> I will note that if this command actually downloads any data, this is\n> required.  That's because your repository is incomplete: you want to\n> download exactly one commit and without marking the promisor remote, you\n> will lack the ability to acquire trees or blobs and your repository will\n> then be corrupt.\n\nI agree - had I actually stored the fetch, this would be correct behavior.\nIt was surprising/wrong *only* because --dry-run threw away the resulting objects\n\n> > I did not expect any changes to the local clone (due to the use of --dry-run)\n\n> I agree --dry-run should not change your repository.  However, I would\n> also say that it should not contact the server, either\n\nHmm. I'm not sure I agree there. Usually --dry-run is intended as a way\nto find out of the command is going to give any errors, and that's the sense\nin which conan is using it. I could see it not actually downloading the pack from\nthe server, but I'd of expect it to at least negotiate which refs are needed\n(and thus fail if the server doesn't have them). Which it currently does.\n\nBut I wasn't expecting it to make any lasting changes to the local clone,\nSo that seemed like a bug I ought to report (though it's admittedly quite a corner-case)\n\n> > Context is https://github.com/conan-io/conan/issues/18949\n> > trying to avoid a full-re-download in the process of checking\n> > whether the HEAD commit hash exists in a remote.\n> > The command was expected to either be a no-op success, or fail\n>\n> I don't think that the command you've provided is a good or efficient\n> way of doing what you want, but I'm also not sure that there's a good or\n> efficient way to do what you want using command line Git (you might need\n> to write a small portion of the protocol, for instance).\n\nI certainly agree that the `fetch --refetch` conan 2.0 is currently doing is a very inefficient\nway to check if a remote has a certain commit or not - which is why I was\nexperimenting with blobless/treeless to minimize the useless transfer.\n\nToday I also found `git fetch $REMOTE --negotiate-only --negotiation-tip=$COMMIT`.\nThat supposedly (and seemingly in practice) replies with the list of ancestors,\nwe have in common, i.e. it will print the same $COMMIT if the server has it,\nor some list of ancestor(s) we share if it does not. That seems like a much more\nsensible command to check whether the remote has a commit,\nso I'd also appreciate any feedback if I'm there's something wrong with using it this way.\n\nI did find one odd quirk. When the remote and the requested commit are unrelated\ne.g. I'm taking to the wrong remote, or have done `git checkout --orphan`), I get\n\n$ git fetch https://github.com/git/git --negotiate-only --negotiation-tip=$COMMIT\nfatal: expected 'acknowledgments', received 'packfile'\n\nThis still works for conan's purpose (exiting with an error means it didn't print a matching commit hash),\nbut I expected something more like the \"fatal: remote error: upload-pack: not our ref: ...\"\nerror that you get from git fetch {remote} --refetch $COMMIT. Of course, unexpectly sending a packfile\ncould be a problem with github's server implementation, rather than the git client.\n\n--\nbrian m. carlson (they/them)\nToronto, Ontario, CA\n"},{"id":"526718","messageId":"20250918192045.GA1187769@coredump.intra.peff.net","threadId":"64161","inReplyTo":"DS0PR05MB100132C058A3D9D5AA735D905B517A@DS0PR05MB10013.namprd05.prod.outlook.com","subject":"Re: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-09-18T19:20:45Z","receivedAt":"2025-09-18T19:20:47Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 17, 2025 at 11:23:21PM +0000, Kevin Puetz wrote:\n\n> On 2025-09-17 at 14:44:00, Kevin Puetz wrote:\n> \n> > >     the .git/config file was modified, adding\n> > >\n> > >     [remote \"origin\"]\n> > >         promisor = true\n> > >         partialclonefilter = tree:0\n> \n> > I will note that if this command actually downloads any data, this is\n> > required.  That's because your repository is incomplete: you want to\n> > download exactly one commit and without marking the promisor remote, you\n> > will lack the ability to acquire trees or blobs and your repository will\n> > then be corrupt.\n> \n> I agree - had I actually stored the fetch, this would be correct behavior.\n> It was surprising/wrong *only* because --dry-run threw away the resulting objects\n\nUnfortunately it does not throw them away. They're still in your object\ndatabase! The dry-run mode of git-fetch is not very dry. It just skips\nthe final ref update.\n\nIt would probably be better if it established a separate tmp-objdir area\n(like we do for pushes before we commit to storing them), fetched the\nobjects into that, and then threw away the result. That technique did\nnot exist back when \"fetch --dry-run\" was added, but it probably\nwouldn't be too hard to do now.\n\n> I certainly agree that the `fetch --refetch` conan 2.0 is currently doing is a very inefficient\n> way to check if a remote has a certain commit or not - which is why I was\n> experimenting with blobless/treeless to minimize the useless transfer.\n> \n> Today I also found `git fetch $REMOTE --negotiate-only --negotiation-tip=$COMMIT`.\n> That supposedly (and seemingly in practice) replies with the list of ancestors,\n> we have in common, i.e. it will print the same $COMMIT if the server has it,\n> or some list of ancestor(s) we share if it does not. That seems like a much more\n> sensible command to check whether the remote has a commit,\n> so I'd also appreciate any feedback if I'm there's something wrong with using it this way.\n> \n> I did find one odd quirk. When the remote and the requested commit are unrelated\n> e.g. I'm taking to the wrong remote, or have done `git checkout --orphan`), I get\n> \n> $ git fetch https://github.com/git/git --negotiate-only --negotiation-tip=$COMMIT\n> fatal: expected 'acknowledgments', received 'packfile'\n> \n> This still works for conan's purpose (exiting with an error means it didn't print a matching commit hash),\n> but I expected something more like the \"fatal: remote error: upload-pack: not our ref: ...\"\n> error that you get from git fetch {remote} --refetch $COMMIT. Of course, unexpectly sending a packfile\n> could be a problem with github's server implementation, rather than the git client.\n\nI had never used --negotiate-only before, and did a simple test which\nended up with the exact same problem. I agree it's probably a bug, but\nit's present in git.git itself (my fetch was to a local test repo).\n\nI'm not sure if the bug is in the server or client, though. If I\nunderstand correctly, the server does not know anything about this\n\"negotiate-only\" mode, but is just responding to the client. The client\nis supposed to say \"done\" to tell the server that it is not sending any\nmore \"have\" negotiation, at which point the server sends the packfile.\nBut in this case the packfile comes anyway. Running with GIT_TRACE_PACKET\nin the environment shows the client making an extra request with no\n\"have\" lines at all, which the server then takes as an indication it can\nsend the packfile.\n\nSo I think there is at least one client bug, which is making that extra\nrequest. There might _also_ be a server bug, because it is sending the\npackfile without a \"done\" line, even though it was told the client wants\nthe wait-for-done option (and the client does not even send a \"want\"\nline at all, so what does it think it is sending?).\n\nI can't comment beyond that on the wisdom of using --negotiate-only.\n\nOne other option is to use the \"object-info\" protocol extension which I\nthink would do exactly what you want. But the server has to enable it\nexplicitly.  And I'm not sure where client support is (I think there\nwere patches for a \"remote-object-info\" directive in git-cat-file a few\nmonths ago, but it looks like it is in limbo now?). So probably a\nnon-starter.\n\nMy other instinct would be to try to fetch the single object, which is\nwhat you're trying to do with the --dry-run invocation. I wondered if we\nmight find some wisdom in how partial clones do one-off fetches to\nlazy-load objects we are missing. It looks like they do (this is from\npromisor-remote.c's fetch_objects() function):\n\n  git -c fetch.negotiationAlgorithm=noop fetch --no-tags \\\n    --no-write-fetch-head --recurse-submodules=no --filter=blob:none\n\nwhich is not so far off of what you're doing (and actually I wonder if\nit even does the right thing when doing a fill-in fetch of a non-blob;\nshouldn't we be using the same filter that we originally cloned with?\nAnd possibly --depth=1?).\n\nBut I don't think it helps your problem. It still writes the promisor\nconfig (which, in a real partial clone, would already exist anyway).\n\nWe could sneak around the odb issue by setting up our own tmp-objdir,\nlike:\n\n  mkdir tmp-objdir\n  GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/.git/objects \\\n  GIT_OBJECT_DIRECTORY=$PWD/tmp-objdir \\\n  git fetch --dry-run ...\n\nbut that would still modify the config. So I think you'd really have to\nset up your own fake little repo, like:\n\n  mkdir fake-repo\n  (\n\tcd fake-repo\n\t# this alternate might even be optional; do we even care about\n\t# seeing our own objects here, since we are just trying to grab\n\t# the one from the server? I guess if we have the object in\n\t# question it prevents the server from sending it to us.\n\tGIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/../.git/objects \\\n\tgit fetch --dry-run ...\n  )\n  rm -rf fake-repo\n\nI dunno. That is pretty ugly, but I couldn't come up with anything\nbetter.\n\n-Peff\n"},{"id":"526722","messageId":"xmqqms6r7bf7.fsf@gitster.g","threadId":"64161","inReplyTo":"20250918192045.GA1187769@coredump.intra.peff.net","subject":"Re: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-18T20:28:28Z","receivedAt":"2025-09-18T20:28:31Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It would probably be better if it established a separate tmp-objdir area\n> (like we do for pushes before we commit to storing them), fetched the\n> objects into that, and then threw away the result. That technique did\n> not exist back when \"fetch --dry-run\" was added, but it probably\n> wouldn't be too hard to do now.\n\nIt is true that the quarantine mechanism did not exist.  But I am\nnot sure if a true dryness is actually better.  We do verify that\nthe incoming objects thrown at us by the other side are healthy, so\nthe only difference is that our object database will have extra\nunreachable objects that are known to be healthy between the time\nyou run a dry fetch and the time you then run a real one.  And these\nextra objects that are healthy will help your real fetch go faster\nif the dry fetch and real fetch are run back to back thanks to \"do\nwe have all the necessary objects already, just that they may not be\nconnected to the refs yet?\" check.\n\nSo it may not be hard with the building blocks, but I am not sure if\nit is a good idea to do so in the first place.\n\nThanks.\n"},{"id":"526725","messageId":"20250918203916.GA1199728@coredump.intra.peff.net","threadId":"64161","inReplyTo":"xmqqms6r7bf7.fsf@gitster.g","subject":"Re: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2025-09-18T20:39:16Z","receivedAt":"2025-09-18T20:39:19Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Sep 18, 2025 at 01:28:28PM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > It would probably be better if it established a separate tmp-objdir area\n> > (like we do for pushes before we commit to storing them), fetched the\n> > objects into that, and then threw away the result. That technique did\n> > not exist back when \"fetch --dry-run\" was added, but it probably\n> > wouldn't be too hard to do now.\n> \n> It is true that the quarantine mechanism did not exist.  But I am\n> not sure if a true dryness is actually better.  We do verify that\n> the incoming objects thrown at us by the other side are healthy, so\n> the only difference is that our object database will have extra\n> unreachable objects that are known to be healthy between the time\n> you run a dry fetch and the time you then run a real one.  And these\n> extra objects that are healthy will help your real fetch go faster\n> if the dry fetch and real fetch are run back to back thanks to \"do\n> we have all the necessary objects already, just that they may not be\n> connected to the refs yet?\" check.\n> \n> So it may not be hard with the building blocks, but I am not sure if\n> it is a good idea to do so in the first place.\n\nYeah, I see how the current behavior could be useful. In this particular\ncase the objects _aren't_ really healthy. They are promisor cruft that\nviolates all the usual everything_local() rules (but at least we set up\nthe config to mark them as such). But I take your point that we probably\nshouldn't change anything here.\n\nYou _can_ do the tmp-objdir thing yourself with GIT_OBJECT_DIRECTORY, as\nI showed earlier. It is unfortunate that doing so updates the repo\nconfig, though. That may show a mismatch in how the promisor information\nis stored (it is really a property of the packfile in the object\ndirectory, but the config mechanism stores it in the main repo). But it\nis probably not worth trying to revisit at this point. It is only when\nyou start to play weird games like \"this repo's object directory is not\n$GIT_DIR/objects\" that you run into these distinctions.\n\n-Peff\n"},{"id":"526726","messageId":"xmqqecs37ali.fsf@gitster.g","threadId":"64161","inReplyTo":"20250918203916.GA1199728@coredump.intra.peff.net","subject":"Re: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-09-18T20:46:17Z","receivedAt":"2025-09-18T20:46:20Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> ... It is unfortunate that doing so updates the repo\n> config, though. That may show a mismatch in how the promisor information\n> is stored (it is really a property of the packfile in the object\n> directory, but the config mechanism stores it in the main repo). But it\n> is probably not worth trying to revisit at this point. It is only when\n> you start to play weird games like \"this repo's object directory is not\n> $GIT_DIR/objects\" that you run into these distinctions.\n\nHmph, but watching from the sidelines Patrick's topics that revolve\naround (re)defining the boundary and relationship among repository,\nobject store, and on-disk packfiles with a more clearly defined\nabstractions like object_source, I am hoping that we somehow can\nfind a good place to move this information out of the per-repo\nconfiguration to somewhere close to individual packs.  I dunno.\n\n\n"},{"id":"526730","messageId":"DS0PR05MB10013AC6090CEE7A562B56CBCB516A@DS0PR05MB10013.namprd05.prod.outlook.com","threadId":"64161","inReplyTo":"20250918192045.GA1187769@coredump.intra.peff.net","subject":"RE: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Kevin Puetz","fromEmail":"puetzkevina@johndeere.com","sentAt":"2025-09-18T21:55:02Z","receivedAt":"2025-09-18T21:55:21Z","isPatch":false,"sender":{"key":"puetzkevina@johndeere.com","avatar":null},"body":"> > I did find one odd quirk. When the remote and the requested commit are unrelated\n> > e.g. I'm taking to the wrong remote, or have done `git checkout --orphan`), I get\n> >\n> > $ git fetch https://github.com/git/git\n\n --negotiate-only --negotiation-tip=$COMMIT\n> > fatal: expected 'acknowledgments', received 'packfile'\n> >\n> > This still works for conan's purpose (exiting with an error means it didn't print a matching commit hash),\n> > but I expected something more like the \"fatal: remote error: upload-pack: not our ref: ...\"\n> > error that you get from git fetch {remote} --refetch $COMMIT. Of course, unexpectly sending a packfile\n> > could be a problem with github's server implementation, rather than the git client.\n>\n> I had never used --negotiate-only before, and did a simple test which\n> ended up with the exact same problem. I agree it's probably a bug, but\n> it's present in git.git itself (my fetch was to a local test repo).\n>\n> I'm not sure if the bug is in the server or client, though. If I\n> understand correctly, the server does not know anything about this\n> \"negotiate-only\" mode, but is just responding to the client. The client\n> is supposed to say \"done\" to tell the server that it is not sending any\n> more \"have\" negotiation, at which point the server sends the packfile.\n> But in this case the packfile comes anyway. Running with GIT_TRACE_PACKET\n> in the environment shows the client making an extra request with no\n> \"have\" lines at all, which the server then takes as an indication it can\n> send the packfile.\n\nYep. In the cases where things work as expected, the client sends various `have` lines\nfor ancestors of the negotiation-tips, and then the server sends \"ACK ...\" with the\none it knows.\n\nWhen they are unrelated (none of the have lines are comits known to the remote),\nit replies with NAK (which is sensible enough). But then the client doesn't just error\nout after the NAK it keeps going. I guess because if this really was a fetch,\nrather than negotiate-only, and we have nothing in common with the server,\nwe'd still need to fetch the wanted ref (and receive everything).\nBut it *is* --negotiate-only, so we don't actually want anything either.\nWhich leads to this weird empty request-for-nothing. Seems like the client\nReally should have just exited early after the NAK, given that it doesn't want anything.\n\n> So I think there is at least one client bug, which is making that extra\n> request. There might _also_ be a server bug, because it is sending the\n> packfile without a \"done\" line, even though it was told the client wants\n> the wait-for-done option (and the client does not even send a \"want\"\n> line at all, so what does it think it is sending?).\n\nLooking closer, I think the server bug is at\n    https://github.com/git/git/blob/ca2559c1d630eb4f04cdee2328aaf1c768907a9e/upload-pack.c#L1805-L1813\n\nThere's an early exit to UPLOAD_DONE (without sending any packfile)\nif you didn't want anything, But it's skipped over if you use wait-for-done,\nbecause that's the negotiate-only case, which still wants the acknowledgements.\nBut then (during the weird empty client request) seen_haves isn't true either,\nSo it skips the UPLOAD_SEND_ACKS case too. And it ends up just falling through into\nthe final else { state = UPLOAD_SEND_PACK; }\n\nI think here the client was expecting the server to go into UPLOAD_SEND_ACKS,\nand just send an empty set of acknowledgements, which seems legal.\nhttps://git-scm.com/docs/gitprotocol-v2 says\n\n- If the server has found a suitable cut point and has decided to send a \"ready\" line,\n  then the server can decide to (as an optimization) omit any \"ACK\" lines it would have sent\n\nSo I think the server could/should just reply with no \"ACK\" lines and \"ready\"\n(i.e. you don't want anything, but sure, I can send you an empty pack file).\nBut it's definitely a corner case. And right now it wouldn't work, because\nhttps://github.com/git/git/blob/master/upload-pack.c#L1712-L1713\n\nwould send NAK instead (which also seems legal, depending on how one interprets\n\n- The server will respond with \"NAK\" if none of the object ids sent as have lines were common.\n\nI think the client could deal with either \"ready\" or \"NAK\", it's just the empty pack it's not expecting.\n\nPublic\n"},{"id":"526731","messageId":"022401dc28ea$8be87d40$a3b977c0$@nexbridge.com","threadId":"64161","inReplyTo":"DS0PR05MB10013AC6090CEE7A562B56CBCB516A@DS0PR05MB10013.namprd05.prod.outlook.com","subject":"RE: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2025-09-18T22:21:08Z","receivedAt":"2025-09-18T22:21:31Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On September 18, 2025 5:55 PM, Kevin Puetz wrote:\n>> > I did find one odd quirk. When the remote and the requested commit\n>> > are unrelated e.g. I'm taking to the wrong remote, or have done `git\n>> > checkout --orphan`), I get\n>> >\n>> > $ git fetch https://github.com/git/git\n>\n> --negotiate-only --negotiation-tip=$COMMIT\n>> > fatal: expected 'acknowledgments', received 'packfile'\n>> >\n>> > This still works for conan's purpose (exiting with an error means it\n>> > didn't print a matching commit hash), but I expected something more like the\n>\"fatal: remote error: upload-pack: not our ref: ...\"\n>> > error that you get from git fetch {remote} --refetch $COMMIT. Of\n>> > course, unexpectly sending a packfile could be a problem with github's server\n>implementation, rather than the git client.\n>>\n>> I had never used --negotiate-only before, and did a simple test which\n>> ended up with the exact same problem. I agree it's probably a bug, but\n>> it's present in git.git itself (my fetch was to a local test repo).\n>>\n>> I'm not sure if the bug is in the server or client, though. If I\n>> understand correctly, the server does not know anything about this\n>> \"negotiate-only\" mode, but is just responding to the client. The\n>> client is supposed to say \"done\" to tell the server that it is not\n>> sending any more \"have\" negotiation, at which point the server sends the\n>packfile.\n>> But in this case the packfile comes anyway. Running with\n>> GIT_TRACE_PACKET in the environment shows the client making an extra\n>> request with no \"have\" lines at all, which the server then takes as an\n>> indication it can send the packfile.\n>\n>Yep. In the cases where things work as expected, the client sends various `have`\n>lines for ancestors of the negotiation-tips, and then the server sends \"ACK ...\" with\n>the one it knows.\n>\n>When they are unrelated (none of the have lines are comits known to the remote),\n>it replies with NAK (which is sensible enough). But then the client doesn't just error\n>out after the NAK it keeps going. I guess because if this really was a fetch, rather\n>than negotiate-only, and we have nothing in common with the server, we'd still\n>need to fetch the wanted ref (and receive everything).\n>But it *is* --negotiate-only, so we don't actually want anything either.\n>Which leads to this weird empty request-for-nothing. Seems like the client Really\n>should have just exited early after the NAK, given that it doesn't want anything.\n>\n>> So I think there is at least one client bug, which is making that\n>> extra request. There might _also_ be a server bug, because it is\n>> sending the packfile without a \"done\" line, even though it was told\n>> the client wants the wait-for-done option (and the client does not even send a\n>\"want\"\n>> line at all, so what does it think it is sending?).\n>\n>Looking closer, I think the server bug is at\n>\n>https://github.com/git/git/blob/ca2559c1d630eb4f04cdee2328aaf1c768907a9e/\n>upload-pack.c#L1805-L1813\n>\n>There's an early exit to UPLOAD_DONE (without sending any packfile) if you didn't\n>want anything, But it's skipped over if you use wait-for-done, because that's the\n>negotiate-only case, which still wants the acknowledgements.\n>But then (during the weird empty client request) seen_haves isn't true either, So it\n>skips the UPLOAD_SEND_ACKS case too. And it ends up just falling through into the\n>final else { state = UPLOAD_SEND_PACK; }\n>\n>I think here the client was expecting the server to go into UPLOAD_SEND_ACKS, and\n>just send an empty set of acknowledgements, which seems legal.\n>https://git-scm.com/docs/gitprotocol-v2 says\n>\n>- If the server has found a suitable cut point and has decided to send a \"ready\" line,\n>  then the server can decide to (as an optimization) omit any \"ACK\" lines it would\n>have sent\n>\n>So I think the server could/should just reply with no \"ACK\" lines and \"ready\"\n>(i.e. you don't want anything, but sure, I can send you an empty pack file).\n>But it's definitely a corner case. And right now it wouldn't work, because\n>https://github.com/git/git/blob/master/upload-pack.c#L1712-L1713\n>\n>would send NAK instead (which also seems legal, depending on how one interprets\n>\n>- The server will respond with \"NAK\" if none of the object ids sent as have lines\n>were common.\n>\n>I think the client could deal with either \"ready\" or \"NAK\", it's just the empty pack it's\n>not expecting.\n\nIf I might ask, and I realize this is an edge-condition, but what is the intent when\nthe clone has been done with a --depth=1 or similar? The single commit may not\nbe known locally but may be in the history as far as the remote is concerned. I am\njust curious.\n\n--Randall\n\n"},{"id":"526785","messageId":"DS0PR05MB10013365E4D3BACFF71ACBAACB511A@DS0PR05MB10013.namprd05.prod.outlook.com","threadId":"64161","inReplyTo":"022401dc28ea$8be87d40$a3b977c0$@nexbridge.com","subject":"RE: [Bug] git fetch --dry-run --filter makes changes to .git/config","fromName":"Kevin Puetz","fromEmail":"puetzkevina@johndeere.com","sentAt":"2025-09-19T15:31:36Z","receivedAt":"2025-09-19T15:31:54Z","isPatch":false,"sender":{"key":"puetzkevina@johndeere.com","avatar":null},"body":"> If I might ask, and I realize this is an edge-condition, but what is the intent when\n> the clone has been done with a --depth=1 or similar? The single commit may not\n> be known locally but may be in the history as far as the remote is concerned. I am\n> just curious.\n\nDefinitely in scope, and likely a common scenario for conan. If you have a full clone\nwith remote-tracking branches and such, then `git branch --remote --contains ...`\nis enough to think the remote has this commit. The case where it has to go online is\nwhen something like CI did a `git clone --single-branch` or `git clone ----revision=`\nand there's not necessarily any tracking branches.\n\nIf the remote has the commit in question, shallow or not, we'd still want\nconan.tools.Git.commit_in_remote to return true. And it looks like --negotiate-only\nshould work fine for that. It looks like, in a shallow clone, the main difference is that\nthe client only sends \"have ...\" lines for the commits back to the shallow boundary,\nand then the \"0000\" flush. Which is correct - it *doesn't* have the earlier parents.\nThe server then responds with ACK or NAK as appropriate to the \"have ...\" it saw.\n\nNow, that increases the odds that there are no commits in common (if the remote\nin question has a more-distant ancestor, below the shallow cutoff, the negotiation\nwon't figure that out. But that doesn't cause any harm - the negotiation result is NAK\n(correct, we don't have anything in common), and if the client does proceed with a\nfetch it gets everything (which it does need, since it didn't have that early history).\n\nSo it seems fine, other than --negotiate-only perhaps triggering the same\n\"fatal: expected 'acknowledgments', received 'packfile'\" bug we were discussing\nfor actually-unrelated histories.\n\nOr, if we reverse the roles and make the shallow copy the remote, e.g.\n `git fetch shallow --negotiate-only --negotiation-tips=HEAD~1` (pretty weird).\nwe get a different error \"fatal: expected 'acknowledgments', received 'shallow-info'\"\nBut it's still the same bug - the client sends the weird empty fetch\n(\"wait-for-done\", no \"have\", no \"want\", no \"done\"), the server falls through into\n`case UPLOAD_SEND_PACK` the same way, but now send_shallow_info writes\nthat before the packile.\n\n\nPublic\n"}]}