* [Bug] git fetch --dry-run --filter makes changes to .git/config
@ 2025-09-17 14:44 Kevin Puetz
2025-09-17 21:21 ` brian m. carlson
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Puetz @ 2025-09-17 14:44 UTC (permalink / raw)
To: git@vger.kernel.org
What did you do before the bug happened? (Steps to reproduce your issue)
`git fetch origin --refetch --dry-run {commit} --depth=1 --filter=tree:0 --no-auto-gc`
What did you expect to happen? (Expected behavior)
I expected it to fetch just the one commit object (no blobs, no trees, no history),
as a means of checking whether that commit is actually known to the remote.
What happened instead? (Actual behavior)
the .git/config file was modified, adding
[remote "origin"]
promisor = true
partialclonefilter = tree:0
What's different between what you expected and what actually happened?
I did not expect any changes to the local clone (due to the use of --dry-run)
Anything else you want to add:
Context is https://github.com/conan-io/conan/issues/18949
trying to avoid a full-re-download in the process of checking
whether the HEAD commit hash exists in a remote.
The command was expected to either be a no-op success, or fail
> fatal: remote error: upload-pack: not our ref {commit}"
[System Info]
git version:
git version 2.51.0.windows.1
cpu: x86_64
built from commit: 4d21a77b98af5cf479d8b6f863c2aa94257cd4e1
sizeof-long: 4
sizeof-size_t: 8
shell-path: D:/git-sdk-64-build-installers/usr/bin/sh
feature: fsmonitor--daemon
libcurl: 8.15.0
OpenSSL: OpenSSL 3.2.4 11 Feb 2025
zlib: 1.3.1
SHA-1: SHA1_DC
SHA-256: SHA256_BLK
default-ref-format: files
default-hash: sha1
uname: Windows 10.0 26100
compiler info: gnuc: 15.2
libc info: no libc information available
$SHELL (typically, interactive shell): C:\Program Files\Git\usr\bin\bash.exe
[Enabled Hooks]
Public
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bug] git fetch --dry-run --filter makes changes to .git/config
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
0 siblings, 1 reply; 10+ messages in thread
From: brian m. carlson @ 2025-09-17 21:21 UTC (permalink / raw)
To: Kevin Puetz; +Cc: git@vger.kernel.org
[-- Attachment #1: Type: text/plain, Size: 2050 bytes --]
On 2025-09-17 at 14:44:00, Kevin Puetz wrote:
> What did you do before the bug happened? (Steps to reproduce your issue)
>
> `git fetch origin --refetch --dry-run {commit} --depth=1 --filter=tree:0 --no-auto-gc`
>
> What did you expect to happen? (Expected behavior)
>
> I expected it to fetch just the one commit object (no blobs, no trees, no history),
> as a means of checking whether that commit is actually known to the remote.
>
> What happened instead? (Actual behavior)
>
> the .git/config file was modified, adding
>
> [remote "origin"]
> promisor = true
> partialclonefilter = tree:0
I will note that if this command actually downloads any data, this is
required. That's because your repository is incomplete: you want to
download exactly one commit and without marking the promisor remote, you
will lack the ability to acquire trees or blobs and your repository will
then be corrupt.
> What's different between what you expected and what actually happened?
>
> I did not expect any changes to the local clone (due to the use of --dry-run)
I agree --dry-run should not change your repository. However, I would
also say that it should not contact the server, either, so it may not do
what you want. If that's not what it was intended to do, perhaps
someone can clarify the documentation as to its functionality.
> Anything else you want to add:
>
> Context is https://github.com/conan-io/conan/issues/18949
>
>
> trying to avoid a full-re-download in the process of checking
> whether the HEAD commit hash exists in a remote.
> The command was expected to either be a no-op success, or fail
I don't think that the command you've provided is a good or efficient
way of doing what you want, but I'm also not sure that there's a good or
efficient way to do what you want using command line Git (you might need
to write a small portion of the protocol, for instance).
--
brian m. carlson (they/them)
Toronto, Ontario, CA
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 262 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-17 21:21 ` brian m. carlson
@ 2025-09-17 23:23 ` Kevin Puetz
2025-09-18 19:20 ` Jeff King
0 siblings, 1 reply; 10+ messages in thread
From: Kevin Puetz @ 2025-09-17 23:23 UTC (permalink / raw)
To: brian m. carlson; +Cc: git@vger.kernel.org
Public
On 2025-09-17 at 14:44:00, Kevin Puetz wrote:
> > the .git/config file was modified, adding
> >
> > [remote "origin"]
> > promisor = true
> > partialclonefilter = tree:0
> I will note that if this command actually downloads any data, this is
> required. That's because your repository is incomplete: you want to
> download exactly one commit and without marking the promisor remote, you
> will lack the ability to acquire trees or blobs and your repository will
> then be corrupt.
I agree - had I actually stored the fetch, this would be correct behavior.
It was surprising/wrong *only* because --dry-run threw away the resulting objects
> > I did not expect any changes to the local clone (due to the use of --dry-run)
> I agree --dry-run should not change your repository. However, I would
> also say that it should not contact the server, either
Hmm. I'm not sure I agree there. Usually --dry-run is intended as a way
to find out of the command is going to give any errors, and that's the sense
in which conan is using it. I could see it not actually downloading the pack from
the server, but I'd of expect it to at least negotiate which refs are needed
(and thus fail if the server doesn't have them). Which it currently does.
But I wasn't expecting it to make any lasting changes to the local clone,
So that seemed like a bug I ought to report (though it's admittedly quite a corner-case)
> > Context is https://github.com/conan-io/conan/issues/18949
> > trying to avoid a full-re-download in the process of checking
> > whether the HEAD commit hash exists in a remote.
> > The command was expected to either be a no-op success, or fail
>
> I don't think that the command you've provided is a good or efficient
> way of doing what you want, but I'm also not sure that there's a good or
> efficient way to do what you want using command line Git (you might need
> to write a small portion of the protocol, for instance).
I certainly agree that the `fetch --refetch` conan 2.0 is currently doing is a very inefficient
way to check if a remote has a certain commit or not - which is why I was
experimenting with blobless/treeless to minimize the useless transfer.
Today I also found `git fetch $REMOTE --negotiate-only --negotiation-tip=$COMMIT`.
That supposedly (and seemingly in practice) replies with the list of ancestors,
we have in common, i.e. it will print the same $COMMIT if the server has it,
or some list of ancestor(s) we share if it does not. That seems like a much more
sensible command to check whether the remote has a commit,
so I'd also appreciate any feedback if I'm there's something wrong with using it this way.
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.
--
brian m. carlson (they/them)
Toronto, Ontario, CA
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bug] git fetch --dry-run --filter makes changes to .git/config
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 21:55 ` Kevin Puetz
0 siblings, 2 replies; 10+ messages in thread
From: Jeff King @ 2025-09-18 19:20 UTC (permalink / raw)
To: Kevin Puetz; +Cc: brian m. carlson, git@vger.kernel.org
On Wed, Sep 17, 2025 at 11:23:21PM +0000, Kevin Puetz wrote:
> On 2025-09-17 at 14:44:00, Kevin Puetz wrote:
>
> > > the .git/config file was modified, adding
> > >
> > > [remote "origin"]
> > > promisor = true
> > > partialclonefilter = tree:0
>
> > I will note that if this command actually downloads any data, this is
> > required. That's because your repository is incomplete: you want to
> > download exactly one commit and without marking the promisor remote, you
> > will lack the ability to acquire trees or blobs and your repository will
> > then be corrupt.
>
> I agree - had I actually stored the fetch, this would be correct behavior.
> It was surprising/wrong *only* because --dry-run threw away the resulting objects
Unfortunately it does not throw them away. They're still in your object
database! The dry-run mode of git-fetch is not very dry. It just skips
the final ref update.
It would probably be better if it established a separate tmp-objdir area
(like we do for pushes before we commit to storing them), fetched the
objects into that, and then threw away the result. That technique did
not exist back when "fetch --dry-run" was added, but it probably
wouldn't be too hard to do now.
> I certainly agree that the `fetch --refetch` conan 2.0 is currently doing is a very inefficient
> way to check if a remote has a certain commit or not - which is why I was
> experimenting with blobless/treeless to minimize the useless transfer.
>
> Today I also found `git fetch $REMOTE --negotiate-only --negotiation-tip=$COMMIT`.
> That supposedly (and seemingly in practice) replies with the list of ancestors,
> we have in common, i.e. it will print the same $COMMIT if the server has it,
> or some list of ancestor(s) we share if it does not. That seems like a much more
> sensible command to check whether the remote has a commit,
> so I'd also appreciate any feedback if I'm there's something wrong with using it this way.
>
> 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.
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?).
I can't comment beyond that on the wisdom of using --negotiate-only.
One other option is to use the "object-info" protocol extension which I
think would do exactly what you want. But the server has to enable it
explicitly. And I'm not sure where client support is (I think there
were patches for a "remote-object-info" directive in git-cat-file a few
months ago, but it looks like it is in limbo now?). So probably a
non-starter.
My other instinct would be to try to fetch the single object, which is
what you're trying to do with the --dry-run invocation. I wondered if we
might find some wisdom in how partial clones do one-off fetches to
lazy-load objects we are missing. It looks like they do (this is from
promisor-remote.c's fetch_objects() function):
git -c fetch.negotiationAlgorithm=noop fetch --no-tags \
--no-write-fetch-head --recurse-submodules=no --filter=blob:none
which is not so far off of what you're doing (and actually I wonder if
it even does the right thing when doing a fill-in fetch of a non-blob;
shouldn't we be using the same filter that we originally cloned with?
And possibly --depth=1?).
But I don't think it helps your problem. It still writes the promisor
config (which, in a real partial clone, would already exist anyway).
We could sneak around the odb issue by setting up our own tmp-objdir,
like:
mkdir tmp-objdir
GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/.git/objects \
GIT_OBJECT_DIRECTORY=$PWD/tmp-objdir \
git fetch --dry-run ...
but that would still modify the config. So I think you'd really have to
set up your own fake little repo, like:
mkdir fake-repo
(
cd fake-repo
# this alternate might even be optional; do we even care about
# seeing our own objects here, since we are just trying to grab
# the one from the server? I guess if we have the object in
# question it prevents the server from sending it to us.
GIT_ALTERNATE_OBJECT_DIRECTORIES=$PWD/../.git/objects \
git fetch --dry-run ...
)
rm -rf fake-repo
I dunno. That is pretty ugly, but I couldn't come up with anything
better.
-Peff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bug] git fetch --dry-run --filter makes changes to .git/config
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 21:55 ` Kevin Puetz
1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2025-09-18 20:28 UTC (permalink / raw)
To: Jeff King; +Cc: Kevin Puetz, brian m. carlson, git@vger.kernel.org
Jeff King <peff@peff.net> writes:
> It would probably be better if it established a separate tmp-objdir area
> (like we do for pushes before we commit to storing them), fetched the
> objects into that, and then threw away the result. That technique did
> not exist back when "fetch --dry-run" was added, but it probably
> wouldn't be too hard to do now.
It is true that the quarantine mechanism did not exist. But I am
not sure if a true dryness is actually better. We do verify that
the incoming objects thrown at us by the other side are healthy, so
the only difference is that our object database will have extra
unreachable objects that are known to be healthy between the time
you run a dry fetch and the time you then run a real one. And these
extra objects that are healthy will help your real fetch go faster
if the dry fetch and real fetch are run back to back thanks to "do
we have all the necessary objects already, just that they may not be
connected to the refs yet?" check.
So it may not be hard with the building blocks, but I am not sure if
it is a good idea to do so in the first place.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-18 20:28 ` Junio C Hamano
@ 2025-09-18 20:39 ` Jeff King
2025-09-18 20:46 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: Jeff King @ 2025-09-18 20:39 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kevin Puetz, brian m. carlson, git@vger.kernel.org
On Thu, Sep 18, 2025 at 01:28:28PM -0700, Junio C Hamano wrote:
> Jeff King <peff@peff.net> writes:
>
> > It would probably be better if it established a separate tmp-objdir area
> > (like we do for pushes before we commit to storing them), fetched the
> > objects into that, and then threw away the result. That technique did
> > not exist back when "fetch --dry-run" was added, but it probably
> > wouldn't be too hard to do now.
>
> It is true that the quarantine mechanism did not exist. But I am
> not sure if a true dryness is actually better. We do verify that
> the incoming objects thrown at us by the other side are healthy, so
> the only difference is that our object database will have extra
> unreachable objects that are known to be healthy between the time
> you run a dry fetch and the time you then run a real one. And these
> extra objects that are healthy will help your real fetch go faster
> if the dry fetch and real fetch are run back to back thanks to "do
> we have all the necessary objects already, just that they may not be
> connected to the refs yet?" check.
>
> So it may not be hard with the building blocks, but I am not sure if
> it is a good idea to do so in the first place.
Yeah, I see how the current behavior could be useful. In this particular
case the objects _aren't_ really healthy. They are promisor cruft that
violates all the usual everything_local() rules (but at least we set up
the config to mark them as such). But I take your point that we probably
shouldn't change anything here.
You _can_ do the tmp-objdir thing yourself with GIT_OBJECT_DIRECTORY, as
I showed earlier. It is unfortunate that doing so updates the repo
config, though. That may show a mismatch in how the promisor information
is stored (it is really a property of the packfile in the object
directory, but the config mechanism stores it in the main repo). But it
is probably not worth trying to revisit at this point. It is only when
you start to play weird games like "this repo's object directory is not
$GIT_DIR/objects" that you run into these distinctions.
-Peff
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-18 20:39 ` Jeff King
@ 2025-09-18 20:46 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2025-09-18 20:46 UTC (permalink / raw)
To: Jeff King; +Cc: Kevin Puetz, brian m. carlson, git@vger.kernel.org
Jeff King <peff@peff.net> writes:
> ... It is unfortunate that doing so updates the repo
> config, though. That may show a mismatch in how the promisor information
> is stored (it is really a property of the packfile in the object
> directory, but the config mechanism stores it in the main repo). But it
> is probably not worth trying to revisit at this point. It is only when
> you start to play weird games like "this repo's object directory is not
> $GIT_DIR/objects" that you run into these distinctions.
Hmph, but watching from the sidelines Patrick's topics that revolve
around (re)defining the boundary and relationship among repository,
object store, and on-disk packfiles with a more clearly defined
abstractions like object_source, I am hoping that we somehow can
find a good place to move this information out of the per-repo
configuration to somewhere close to individual packs. I dunno.
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-18 19:20 ` Jeff King
2025-09-18 20:28 ` Junio C Hamano
@ 2025-09-18 21:55 ` Kevin Puetz
2025-09-18 22:21 ` rsbecker
1 sibling, 1 reply; 10+ messages in thread
From: Kevin Puetz @ 2025-09-18 21:55 UTC (permalink / raw)
To: Jeff King; +Cc: brian m. carlson, git@vger.kernel.org
> > 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.
Public
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-18 21:55 ` Kevin Puetz
@ 2025-09-18 22:21 ` rsbecker
2025-09-19 15:31 ` Kevin Puetz
0 siblings, 1 reply; 10+ messages in thread
From: rsbecker @ 2025-09-18 22:21 UTC (permalink / raw)
To: 'Kevin Puetz', 'Jeff King'
Cc: 'brian m. carlson', git
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
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [Bug] git fetch --dry-run --filter makes changes to .git/config
2025-09-18 22:21 ` rsbecker
@ 2025-09-19 15:31 ` Kevin Puetz
0 siblings, 0 replies; 10+ messages in thread
From: Kevin Puetz @ 2025-09-19 15:31 UTC (permalink / raw)
To: rsbecker@nexbridge.com, 'Jeff King'
Cc: 'brian m. carlson', git@vger.kernel.org
> 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.
Definitely in scope, and likely a common scenario for conan. If you have a full clone
with remote-tracking branches and such, then `git branch --remote --contains ...`
is enough to think the remote has this commit. The case where it has to go online is
when something like CI did a `git clone --single-branch` or `git clone ----revision=`
and there's not necessarily any tracking branches.
If the remote has the commit in question, shallow or not, we'd still want
conan.tools.Git.commit_in_remote to return true. And it looks like --negotiate-only
should work fine for that. It looks like, in a shallow clone, the main difference is that
the client only sends "have ..." lines for the commits back to the shallow boundary,
and then the "0000" flush. Which is correct - it *doesn't* have the earlier parents.
The server then responds with ACK or NAK as appropriate to the "have ..." it saw.
Now, that increases the odds that there are no commits in common (if the remote
in question has a more-distant ancestor, below the shallow cutoff, the negotiation
won't figure that out. But that doesn't cause any harm - the negotiation result is NAK
(correct, we don't have anything in common), and if the client does proceed with a
fetch it gets everything (which it does need, since it didn't have that early history).
So it seems fine, other than --negotiate-only perhaps triggering the same
"fatal: expected 'acknowledgments', received 'packfile'" bug we were discussing
for actually-unrelated histories.
Or, if we reverse the roles and make the shallow copy the remote, e.g.
`git fetch shallow --negotiate-only --negotiation-tips=HEAD~1` (pretty weird).
we get a different error "fatal: expected 'acknowledgments', received 'shallow-info'"
But it's still the same bug - the client sends the weird empty fetch
("wait-for-done", no "have", no "want", no "done"), the server falls through into
`case UPLOAD_SEND_PACK` the same way, but now send_shallow_info writes
that before the packile.
Public
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-09-19 15:31 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
2025-09-19 15:31 ` Kevin Puetz
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).