* Git 2.48. Changed behavior of the git fetch
@ 2025-01-09 11:49 Danila Manturov
2025-01-12 8:07 ` Bence Ferdinandy
0 siblings, 1 reply; 8+ messages in thread
From: Danila Manturov @ 2025-01-09 11:49 UTC (permalink / raw)
To: git
Hello. I work in TeamCity and we have tests of our git integration
running with the latest master of the git repository. Some tests
started to fail since
https://github.com/git/git/commit/5f212684abb66c9604e745a2296af8c4bb99961c
I noticed that tags are not fetched with shallow clones. I published
the test repository to GitHub and reproduced it with commands, the
result is different for 2.47.1 and 2.48.rc0
git init
git remote add origin git@github•com:manturovDan/repo_for_shallow_fetch.git
git fetch --progress --depth=1 --recurse-submodules=no origin
+fd1eb9776b5fad5cc433586f7933811c6853917d:refs/remotes/origin/main
git tag | cat
RESULT:
tag1 (git version 2.47.1)
<empty> (git version 2.48.0.rc0.38.gff795a5c5e)
the repository log:
* commit fd1eb9776b5fad5cc433586f7933811c6853917d (tag: tag1, main)
| Author: Victory Petrenko <vbedrosova@gmail•com>
| Date: Wed Feb 3 13:05:03 2021 +0100
|
| recent commit
|
* commit 64195c330d99c467a142f682bc23d4de3a68551d
| Author: Victory Petrenko <vbedrosova@gmail•com>
| Date: Wed Feb 3 13:04:44 2021 +0100
|
| change
|
* commit a1d6299597f8d6f6d8316577c46cc8fffd657d5e (tag: tag2)
Author: Victory Petrenko <vbedrosova@gmail•com>
Date: Wed Feb 3 13:04:17 2021 +0100
initial commit
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-09 11:49 Git 2.48. Changed behavior of the git fetch Danila Manturov
@ 2025-01-12 8:07 ` Bence Ferdinandy
[not found] ` <CAM6buW6NbdZ6wLGP6LWePiA7n0At=jxxqtBEUv0fTY6mOdTmyw@mail.gmail.com>
0 siblings, 1 reply; 8+ messages in thread
From: Bence Ferdinandy @ 2025-01-12 8:07 UTC (permalink / raw)
To: Danila Manturov, git
On Thu Jan 09, 2025 at 12:49, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> Hello. I work in TeamCity and we have tests of our git integration
> running with the latest master of the git repository. Some tests
> started to fail since
> https://github.com/git/git/commit/5f212684abb66c9604e745a2296af8c4bb99961c
> I noticed that tags are not fetched with shallow clones. I published
> the test repository to GitHub and reproduced it with commands, the
> result is different for 2.47.1 and 2.48.rc0
>
> git init
> git remote add origin git@github•com:manturovDan/repo_for_shallow_fetch.git
> git fetch --progress --depth=1 --recurse-submodules=no origin
> +fd1eb9776b5fad5cc433586f7933811c6853917d:refs/remotes/origin/main
> git tag | cat
>
> RESULT:
> tag1 (git version 2.47.1)
> <empty> (git version 2.48.0.rc0.38.gff795a5c5e)
>
> the repository log:
> * commit fd1eb9776b5fad5cc433586f7933811c6853917d (tag: tag1, main)
> | Author: Victory Petrenko <vbedrosova@gmail•com>
> | Date: Wed Feb 3 13:05:03 2021 +0100
> |
> | recent commit
> |
> * commit 64195c330d99c467a142f682bc23d4de3a68551d
> | Author: Victory Petrenko <vbedrosova@gmail•com>
> | Date: Wed Feb 3 13:04:44 2021 +0100
> |
> | change
> |
> * commit a1d6299597f8d6f6d8316577c46cc8fffd657d5e (tag: tag2)
> Author: Victory Petrenko <vbedrosova@gmail•com>
> Date: Wed Feb 3 13:04:17 2021 +0100
>
> initial commit
This should already be fixed by
6c915c3f85 (fetch: do not ask for HEAD unnecessarily, 2024-12-06)
[snip]
Incidentally, because the unconditional request to list "HEAD"
affected the number of ref-prefixes requested in the ls-remote
request, this affected how the requests for tags are added to the
same ls-remote request, breaking "git fetch --tags $URL" performed
against a URL that is not configured as a remote.
so using 2.48 should be ok.
Best,
Bence
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
[not found] ` <D706LPHBPUL4.3LN27T1UG1FI2@ferdinandy.com>
@ 2025-01-13 14:14 ` Danila Manturov
2025-01-13 16:02 ` Bence Ferdinandy
0 siblings, 1 reply; 8+ messages in thread
From: Danila Manturov @ 2025-01-13 14:14 UTC (permalink / raw)
To: Bence Ferdinandy, git
According to our CI, the first commit where the bug occurs is
5f212684abb66c9604e745a2296af8c4bb99961c
On Sun, Jan 12, 2025 at 3:58 PM Bence Ferdinandy <bence@ferdinandy•com> wrote:
>
>
> On Sun Jan 12, 2025 at 15:27, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> > Seems in the 'git fetch --progress --depth=1 --recurse-submodules=no
> > origin' the ref-spec is missing
>
> Ah, indeed, with the refspec it doesn't get the tags. I'm not sure what's going on there.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-13 14:14 ` Danila Manturov
@ 2025-01-13 16:02 ` Bence Ferdinandy
2025-01-21 17:26 ` Danila Manturov
0 siblings, 1 reply; 8+ messages in thread
From: Bence Ferdinandy @ 2025-01-13 16:02 UTC (permalink / raw)
To: Danila Manturov, git; +Cc: Junio C Hamano
On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> According to our CI, the first commit where the bug occurs is
> 5f212684abb66c9604e745a2296af8c4bb99961c
That makes sense, what is more interesting is why the fix Junio wrote later
doesn't work in this case ... I didn't have time to dig yet.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-13 16:02 ` Bence Ferdinandy
@ 2025-01-21 17:26 ` Danila Manturov
2025-01-26 23:35 ` Bence Ferdinandy
0 siblings, 1 reply; 8+ messages in thread
From: Danila Manturov @ 2025-01-21 17:26 UTC (permalink / raw)
To: Bence Ferdinandy; +Cc: git
Hello. I have done some experiments. For some reason, it works
correctly with JSch. With native ssh/https it doesn't work
On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy <bence@ferdinandy•com> wrote:
>
>
> On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> > According to our CI, the first commit where the bug occurs is
> > 5f212684abb66c9604e745a2296af8c4bb99961c
>
> That makes sense, what is more interesting is why the fix Junio wrote later
> doesn't work in this case ... I didn't have time to dig yet.
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-21 17:26 ` Danila Manturov
@ 2025-01-26 23:35 ` Bence Ferdinandy
2025-01-27 11:48 ` Danila Manturov
2025-01-27 16:36 ` Bence Ferdinandy
0 siblings, 2 replies; 8+ messages in thread
From: Bence Ferdinandy @ 2025-01-26 23:35 UTC (permalink / raw)
To: Danila Manturov; +Cc: git, Junio C Hamano
On Tue Jan 21, 2025 at 18:26, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> Hello. I have done some experiments. For some reason, it works
> correctly with JSch. With native ssh/https it doesn't work
>
> On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy <bence@ferdinandy•com> wrote:
>>
>>
>> On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@jetbrains•com> wrote:
>> > According to our CI, the first commit where the bug occurs is
>> > 5f212684abb66c9604e745a2296af8c4bb99961c
>>
>> That makes sense, what is more interesting is why the fix Junio wrote later
>> doesn't work in this case ... I didn't have time to dig yet.
>>
>>
I looked up the original thread leading to 6c915c3f85 (fetch: do not ask for
HEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see
https://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).
Originally Josh there suggested just changing the order of adding tags later to
the prefixes should solve the issue. I don't think we ever actually figured out
why the order of the prefixes should matter, and Junio's patch solved that
particular problem by just not asking for HEAD in that case, but it seems that
the current problem can also be solved by swapping the order of tags and HEAD.
This seems like a band-aid again, and I still don't get why the order matters,
but I can turn this into a patch if needed:
diff --git a/builtin/fetch.c b/builtin/fetch.c
index fe2b26c74a..7147f06395 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1768,6 +1768,11 @@ static int do_fetch(struct transport *transport,
}
}
+ if (uses_remote_tracking(transport, rs)) {
+ must_list_refs = 1;
+ strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
+ }
+
if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
must_list_refs = 1;
if (transport_ls_refs_options.ref_prefixes.nr)
@@ -1775,10 +1780,6 @@ static int do_fetch(struct transport *transport,
"refs/tags/");
}
- if (uses_remote_tracking(transport, rs)) {
- must_list_refs = 1;
- strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
- }
if (must_list_refs) {
trace2_region_enter("fetch", "remote_refs", the_repository);
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-26 23:35 ` Bence Ferdinandy
@ 2025-01-27 11:48 ` Danila Manturov
2025-01-27 16:36 ` Bence Ferdinandy
1 sibling, 0 replies; 8+ messages in thread
From: Danila Manturov @ 2025-01-27 11:48 UTC (permalink / raw)
To: Bence Ferdinandy; +Cc: git, Junio C Hamano
Hello. Thank you for the investigation. The patch will help us because
the git command is generated by the code, and it is important to
ensure backward compatibility with previous versions of our product.
Thank you!
On Mon, Jan 27, 2025 at 12:35 AM Bence Ferdinandy <bence@ferdinandy•com> wrote:
>
>
> On Tue Jan 21, 2025 at 18:26, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> > Hello. I have done some experiments. For some reason, it works
> > correctly with JSch. With native ssh/https it doesn't work
> >
> > On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy <bence@ferdinandy•com> wrote:
> >>
> >>
> >> On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@jetbrains•com> wrote:
> >> > According to our CI, the first commit where the bug occurs is
> >> > 5f212684abb66c9604e745a2296af8c4bb99961c
> >>
> >> That makes sense, what is more interesting is why the fix Junio wrote later
> >> doesn't work in this case ... I didn't have time to dig yet.
> >>
> >>
>
> I looked up the original thread leading to 6c915c3f85 (fetch: do not ask for
> HEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see
> https://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).
>
> Originally Josh there suggested just changing the order of adding tags later to
> the prefixes should solve the issue. I don't think we ever actually figured out
> why the order of the prefixes should matter, and Junio's patch solved that
> particular problem by just not asking for HEAD in that case, but it seems that
> the current problem can also be solved by swapping the order of tags and HEAD.
>
> This seems like a band-aid again, and I still don't get why the order matters,
> but I can turn this into a patch if needed:
>
> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index fe2b26c74a..7147f06395 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -1768,6 +1768,11 @@ static int do_fetch(struct transport *transport,
> }
> }
>
> + if (uses_remote_tracking(transport, rs)) {
> + must_list_refs = 1;
> + strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> + }
> +
> if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
> must_list_refs = 1;
> if (transport_ls_refs_options.ref_prefixes.nr)
> @@ -1775,10 +1780,6 @@ static int do_fetch(struct transport *transport,
> "refs/tags/");
> }
>
> - if (uses_remote_tracking(transport, rs)) {
> - must_list_refs = 1;
> - strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
> - }
>
> if (must_list_refs) {
> trace2_region_enter("fetch", "remote_refs", the_repository);
>
>
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Git 2.48. Changed behavior of the git fetch
2025-01-26 23:35 ` Bence Ferdinandy
2025-01-27 11:48 ` Danila Manturov
@ 2025-01-27 16:36 ` Bence Ferdinandy
1 sibling, 0 replies; 8+ messages in thread
From: Bence Ferdinandy @ 2025-01-27 16:36 UTC (permalink / raw)
To: Danila Manturov; +Cc: git, Junio C Hamano, Jonathan Tan
On Mon Jan 27, 2025 at 00:35, Bence Ferdinandy <bence@ferdinandy•com> wrote:
>
> On Tue Jan 21, 2025 at 18:26, Danila Manturov <danila.manturov@jetbrains•com> wrote:
>> Hello. I have done some experiments. For some reason, it works
>> correctly with JSch. With native ssh/https it doesn't work
>>
>> On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy <bence@ferdinandy•com> wrote:
>>>
>>>
>>> On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@jetbrains•com> wrote:
>>> > According to our CI, the first commit where the bug occurs is
>>> > 5f212684abb66c9604e745a2296af8c4bb99961c
>>>
>>> That makes sense, what is more interesting is why the fix Junio wrote later
>>> doesn't work in this case ... I didn't have time to dig yet.
>>>
>>>
>
> I looked up the original thread leading to 6c915c3f85 (fetch: do not ask for
> HEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see
> https://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).
>
> Originally Josh there suggested just changing the order of adding tags later to
> the prefixes should solve the issue. I don't think we ever actually figured out
> why the order of the prefixes should matter, and Junio's patch solved that
> particular problem by just not asking for HEAD in that case, but it seems that
> the current problem can also be solved by swapping the order of tags and HEAD.
I had a little bit of time to investigate.
This is the place of interest in builtin/fetch.h:1771-1781
if (tags == TAGS_SET || tags == TAGS_DEFAULT) {
must_list_refs = 1;
if (transport_ls_refs_options.ref_prefixes.nr)
strvec_push(&transport_ls_refs_options.ref_prefixes,
"refs/tags/");
}
if (uses_remote_tracking(transport, rs)) {
must_list_refs = 1;
strvec_push(&transport_ls_refs_options.ref_prefixes, "HEAD");
}
If `transport_ls_refs_options.ref_prefixes` is empty we fetch tags. If
`transport_ls_refs_options.ref_prefixes` is not empty, we fetch what is in this
`ref_prefixes`. The current code checks if we have anything in ref_prefixes
before adding tags to ref_prefixes and only adds tags if `ref_prefixes` is not
empty. So currently, since we always add HEAD to `ref_prefixes` it is never
empty later down the line, but for this case, it is empty when we get to
checking the TAG conditions. This is why switching the order works, because
then `ref_prefixes` is not empty and tags are explicitly appended.
This checking for non-empty `ref_prefixes` seems to have been added here by Jonathan:
e70a3030e7 (fetch: do not list refs if fetching only hashes, 2018-09-27)
What is not quite clear to me, is that it looks like that the original
intention was to pretty much always fetch tags, yet it was not achieved by
always pushing `refs/tags` into ref_prefixes. Deleting the check for
`ref_prefixes` being empty [1] breaks quite a lot of things, but reversing the
order [2] does not. That feels a bit strange tbh since it feels like the two
should bring about the same state ...
Hopefully someone more knowledgeable knows why things are as they are, but it
seems that reversing the order really is a band-aid here.
1: https://github.com/ferdinandyb/git/commit/6074a9b8c88451e589eade4034282dd9b6c86345
2: https://github.com/ferdinandyb/git/commit/31e3f0a6b829d6c7953bf89d015b98e7edabe6b5
Best,
Bence
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-01-27 16:37 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-09 11:49 Git 2.48. Changed behavior of the git fetch Danila Manturov
2025-01-12 8:07 ` Bence Ferdinandy
[not found] ` <CAM6buW6NbdZ6wLGP6LWePiA7n0At=jxxqtBEUv0fTY6mOdTmyw@mail.gmail.com>
[not found] ` <D705W1554XJ9.30SRYLNGNOX4@ferdinandy.com>
[not found] ` <CAM6buW77CeuKfr3b4SUbYyFaU1OTvRsYBjPBE05YMzJo36bGdw@mail.gmail.com>
[not found] ` <D706LPHBPUL4.3LN27T1UG1FI2@ferdinandy.com>
2025-01-13 14:14 ` Danila Manturov
2025-01-13 16:02 ` Bence Ferdinandy
2025-01-21 17:26 ` Danila Manturov
2025-01-26 23:35 ` Bence Ferdinandy
2025-01-27 11:48 ` Danila Manturov
2025-01-27 16:36 ` Bence Ferdinandy
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox