{"thread":{"id":"62773","subject":"Git 2.48. Changed behavior of the git fetch","startedAt":"2025-01-09T11:49:13Z","lastAt":"2025-01-27T16:37:09Z","messageCount":8,"participants":["Danila Manturov","Bence Ferdinandy"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"510246","messageId":"CAM6buW5KSDGHD7txroqVa0TN_Ou_eV-LocMy06cPy0ZGDQmY9A@mail.gmail.com","threadId":"62773","inReplyTo":null,"subject":"Git 2.48. Changed behavior of the git fetch","fromName":"Danila Manturov","fromEmail":"danila.manturov@jetbrains.com","sentAt":"2025-01-09T11:49:02Z","receivedAt":"2025-01-09T11:49:13Z","isPatch":false,"sender":{"key":"danila.manturov@jetbrains.com","avatar":null},"body":"Hello. I work in TeamCity and we have tests of our git integration\nrunning with the latest master of the git repository. Some tests\nstarted to fail since\nhttps://github.com/git/git/commit/5f212684abb66c9604e745a2296af8c4bb99961c\nI noticed that tags are not fetched with shallow clones. I published\nthe test repository to GitHub and reproduced it with commands, the\nresult is different for 2.47.1 and 2.48.rc0\n\ngit init\ngit remote add origin git@github.com:manturovDan/repo_for_shallow_fetch.git\ngit fetch --progress --depth=1 --recurse-submodules=no origin\n+fd1eb9776b5fad5cc433586f7933811c6853917d:refs/remotes/origin/main\ngit tag | cat\n\nRESULT:\ntag1 (git version 2.47.1)\n (git version 2.48.0.rc0.38.gff795a5c5e)\n\nthe repository log:\n* commit fd1eb9776b5fad5cc433586f7933811c6853917d (tag: tag1, main)\n| Author: Victory Petrenko \n| Date: Wed Feb 3 13:05:03 2021 +0100\n|\n| recent commit\n|\n* commit 64195c330d99c467a142f682bc23d4de3a68551d\n| Author: Victory Petrenko \n| Date: Wed Feb 3 13:04:44 2021 +0100\n|\n| change\n|\n* commit a1d6299597f8d6f6d8316577c46cc8fffd657d5e (tag: tag2)\n Author: Victory Petrenko \n Date: Wed Feb 3 13:04:17 2021 +0100\n\n initial commit\n"},{"id":"510377","messageId":"D6ZXVILR1D36.3W0QVQCVE1P2J@ferdinandy.com","threadId":"62773","inReplyTo":"CAM6buW5KSDGHD7txroqVa0TN_Ou_eV-LocMy06cPy0ZGDQmY9A@mail.gmail.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Bence Ferdinandy","fromEmail":"bence@ferdinandy.com","sentAt":"2025-01-12T08:07:45Z","receivedAt":"2025-01-12T08:13:51Z","isPatch":false,"sender":{"key":"bence@ferdinandy.com","avatar":"https://avatars.githubusercontent.com/u/6343487?v=4"},"body":"\nOn Thu Jan 09, 2025 at 12:49, Danila Manturov wrote:\n> Hello. I work in TeamCity and we have tests of our git integration\n> running with the latest master of the git repository. Some tests\n> started to fail since\n> https://github.com/git/git/commit/5f212684abb66c9604e745a2296af8c4bb99961c\n> I noticed that tags are not fetched with shallow clones. I published\n> the test repository to GitHub and reproduced it with commands, the\n> result is different for 2.47.1 and 2.48.rc0\n>\n> git init\n> git remote add origin git@github.com:manturovDan/repo_for_shallow_fetch.git\n> git fetch --progress --depth=1 --recurse-submodules=no origin\n> +fd1eb9776b5fad5cc433586f7933811c6853917d:refs/remotes/origin/main\n> git tag | cat\n>\n> RESULT:\n> tag1 (git version 2.47.1)\n> (git version 2.48.0.rc0.38.gff795a5c5e)\n>\n> the repository log:\n> * commit fd1eb9776b5fad5cc433586f7933811c6853917d (tag: tag1, main)\n> | Author: Victory Petrenko \n> | Date: Wed Feb 3 13:05:03 2021 +0100\n> |\n> | recent commit\n> |\n> * commit 64195c330d99c467a142f682bc23d4de3a68551d\n> | Author: Victory Petrenko \n> | Date: Wed Feb 3 13:04:44 2021 +0100\n> |\n> | change\n> |\n> * commit a1d6299597f8d6f6d8316577c46cc8fffd657d5e (tag: tag2)\n> Author: Victory Petrenko \n> Date: Wed Feb 3 13:04:17 2021 +0100\n>\n> initial commit\n\nThis should already be fixed by\n\n6c915c3f85 (fetch: do not ask for HEAD unnecessarily, 2024-12-06)\n\n\t[snip]\n Incidentally, because the unconditional request to list \"HEAD\"\n affected the number of ref-prefixes requested in the ls-remote\n request, this affected how the requests for tags are added to the\n same ls-remote request, breaking \"git fetch --tags $URL\" performed\n against a URL that is not configured as a remote.\n\nso using 2.48 should be ok.\n\nBest,\nBence\n\n"},{"id":"510414","messageId":"CAM6buW4UiCs9pFeH0cxxdhLHCSNO9wLVz9_p4Y0u8LaGWy--ng@mail.gmail.com","threadId":"62773","inReplyTo":"D706LPHBPUL4.3LN27T1UG1FI2@ferdinandy.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Danila Manturov","fromEmail":"danila.manturov@jetbrains.com","sentAt":"2025-01-13T14:14:39Z","receivedAt":"2025-01-13T14:14:51Z","isPatch":false,"sender":{"key":"danila.manturov@jetbrains.com","avatar":null},"body":"According to our CI, the first commit where the bug occurs is\n5f212684abb66c9604e745a2296af8c4bb99961c\n\nOn Sun, Jan 12, 2025 at 3:58 PM Bence Ferdinandy wrote:\n>\n>\n> On Sun Jan 12, 2025 at 15:27, Danila Manturov wrote:\n> > Seems in the 'git fetch --progress --depth=1 --recurse-submodules=no\n> > origin' the ref-spec is missing\n>\n> Ah, indeed, with the refspec it doesn't get the tags. I'm not sure what's going on there.\n"},{"id":"510425","messageId":"D712LKI48ZUD.2UK8FX0YZBEYM@ferdinandy.com","threadId":"62773","inReplyTo":"CAM6buW4UiCs9pFeH0cxxdhLHCSNO9wLVz9_p4Y0u8LaGWy--ng@mail.gmail.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Bence Ferdinandy","fromEmail":"bence@ferdinandy.com","sentAt":"2025-01-13T16:02:31Z","receivedAt":"2025-01-13T16:08:23Z","isPatch":false,"sender":{"key":"bence@ferdinandy.com","avatar":"https://avatars.githubusercontent.com/u/6343487?v=4"},"body":"\nOn Mon Jan 13, 2025 at 15:14, Danila Manturov wrote:\n> According to our CI, the first commit where the bug occurs is\n> 5f212684abb66c9604e745a2296af8c4bb99961c\n\nThat makes sense, what is more interesting is why the fix Junio wrote later\ndoesn't work in this case ... I didn't have time to dig yet.\n\n\n"},{"id":"511006","messageId":"CAM6buW4e4c_3BgPo_GU64Fvi7XGcP7tuxdaap1LypyFCOZvZEw@mail.gmail.com","threadId":"62773","inReplyTo":"D712LKI48ZUD.2UK8FX0YZBEYM@ferdinandy.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Danila Manturov","fromEmail":"danila.manturov@jetbrains.com","sentAt":"2025-01-21T17:26:59Z","receivedAt":"2025-01-21T17:27:11Z","isPatch":false,"sender":{"key":"danila.manturov@jetbrains.com","avatar":null},"body":"Hello. I have done some experiments. For some reason, it works\ncorrectly with JSch. With native ssh/https it doesn't work\n\nOn Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy wrote:\n>\n>\n> On Mon Jan 13, 2025 at 15:14, Danila Manturov wrote:\n> > According to our CI, the first commit where the bug occurs is\n> > 5f212684abb66c9604e745a2296af8c4bb99961c\n>\n> That makes sense, what is more interesting is why the fix Junio wrote later\n> doesn't work in this case ... I didn't have time to dig yet.\n>\n>\n"},{"id":"511216","messageId":"D7CEDCJ0KKYL.YS0EWVFCN72X@ferdinandy.com","threadId":"62773","inReplyTo":"CAM6buW4e4c_3BgPo_GU64Fvi7XGcP7tuxdaap1LypyFCOZvZEw@mail.gmail.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Bence Ferdinandy","fromEmail":"bence@ferdinandy.com","sentAt":"2025-01-26T23:35:20Z","receivedAt":"2025-01-26T23:40:57Z","isPatch":false,"sender":{"key":"bence@ferdinandy.com","avatar":"https://avatars.githubusercontent.com/u/6343487?v=4"},"body":"\nOn Tue Jan 21, 2025 at 18:26, Danila Manturov wrote:\n> Hello. I have done some experiments. For some reason, it works\n> correctly with JSch. With native ssh/https it doesn't work\n>\n> On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy wrote:\n>>\n>>\n>> On Mon Jan 13, 2025 at 15:14, Danila Manturov wrote:\n>> > According to our CI, the first commit where the bug occurs is\n>> > 5f212684abb66c9604e745a2296af8c4bb99961c\n>>\n>> That makes sense, what is more interesting is why the fix Junio wrote later\n>> doesn't work in this case ... I didn't have time to dig yet.\n>>\n>>\n\nI looked up the original thread leading to 6c915c3f85 (fetch: do not ask for\nHEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see\nhttps://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).\n\nOriginally Josh there suggested just changing the order of adding tags later to\nthe prefixes should solve the issue. I don't think we ever actually figured out\nwhy the order of the prefixes should matter, and Junio's patch solved that\nparticular problem by just not asking for HEAD in that case, but it seems that\nthe current problem can also be solved by swapping the order of tags and HEAD.\n\nThis seems like a band-aid again, and I still don't get why the order matters,\nbut I can turn this into a patch if needed:\n\ndiff --git a/builtin/fetch.c b/builtin/fetch.c\nindex fe2b26c74a..7147f06395 100644\n--- a/builtin/fetch.c\n+++ b/builtin/fetch.c\n@@ -1768,6 +1768,11 @@ static int do_fetch(struct transport *transport,\n \t\t}\n \t}\n \n+\tif (uses_remote_tracking(transport, rs)) {\n+\t\tmust_list_refs = 1;\n+\t\tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n+\t}\n+\n \tif (tags == TAGS_SET || tags == TAGS_DEFAULT) {\n \t\tmust_list_refs = 1;\n \t\tif (transport_ls_refs_options.ref_prefixes.nr)\n@@ -1775,10 +1780,6 @@ static int do_fetch(struct transport *transport,\n \t\t\t\t \"refs/tags/\");\n \t}\n \n-\tif (uses_remote_tracking(transport, rs)) {\n-\t\tmust_list_refs = 1;\n-\t\tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n-\t}\n \n \tif (must_list_refs) {\n \t\ttrace2_region_enter(\"fetch\", \"remote_refs\", the_repository);\n\n\n\n\n"},{"id":"511240","messageId":"CAM6buW5TTwtcCvpbkPBJp2=DuQmQSpUjDh+9u8NY7e4+QJxdGA@mail.gmail.com","threadId":"62773","inReplyTo":"D7CEDCJ0KKYL.YS0EWVFCN72X@ferdinandy.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Danila Manturov","fromEmail":"danila.manturov@jetbrains.com","sentAt":"2025-01-27T11:48:37Z","receivedAt":"2025-01-27T11:48:49Z","isPatch":false,"sender":{"key":"danila.manturov@jetbrains.com","avatar":null},"body":"Hello. Thank you for the investigation. The patch will help us because\nthe git command is generated by the code, and it is important to\nensure backward compatibility with previous versions of our product.\nThank you!\n\nOn Mon, Jan 27, 2025 at 12:35 AM Bence Ferdinandy wrote:\n>\n>\n> On Tue Jan 21, 2025 at 18:26, Danila Manturov wrote:\n> > Hello. I have done some experiments. For some reason, it works\n> > correctly with JSch. With native ssh/https it doesn't work\n> >\n> > On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy wrote:\n> >>\n> >>\n> >> On Mon Jan 13, 2025 at 15:14, Danila Manturov wrote:\n> >> > According to our CI, the first commit where the bug occurs is\n> >> > 5f212684abb66c9604e745a2296af8c4bb99961c\n> >>\n> >> That makes sense, what is more interesting is why the fix Junio wrote later\n> >> doesn't work in this case ... I didn't have time to dig yet.\n> >>\n> >>\n>\n> I looked up the original thread leading to 6c915c3f85 (fetch: do not ask for\n> HEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see\n> https://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).\n>\n> Originally Josh there suggested just changing the order of adding tags later to\n> the prefixes should solve the issue. I don't think we ever actually figured out\n> why the order of the prefixes should matter, and Junio's patch solved that\n> particular problem by just not asking for HEAD in that case, but it seems that\n> the current problem can also be solved by swapping the order of tags and HEAD.\n>\n> This seems like a band-aid again, and I still don't get why the order matters,\n> but I can turn this into a patch if needed:\n>\n> diff --git a/builtin/fetch.c b/builtin/fetch.c\n> index fe2b26c74a..7147f06395 100644\n> --- a/builtin/fetch.c\n> +++ b/builtin/fetch.c\n> @@ -1768,6 +1768,11 @@ static int do_fetch(struct transport *transport,\n> }\n> }\n>\n> + if (uses_remote_tracking(transport, rs)) {\n> + must_list_refs = 1;\n> + strvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n> + }\n> +\n> if (tags == TAGS_SET || tags == TAGS_DEFAULT) {\n> must_list_refs = 1;\n> if (transport_ls_refs_options.ref_prefixes.nr)\n> @@ -1775,10 +1780,6 @@ static int do_fetch(struct transport *transport,\n> \"refs/tags/\");\n> }\n>\n> - if (uses_remote_tracking(transport, rs)) {\n> - must_list_refs = 1;\n> - strvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n> - }\n>\n> if (must_list_refs) {\n> trace2_region_enter(\"fetch\", \"remote_refs\", the_repository);\n>\n>\n>\n>\n"},{"id":"511281","messageId":"D7D031QT4HEX.14TRNKRC6FC7S@ferdinandy.com","threadId":"62773","inReplyTo":"D7CEDCJ0KKYL.YS0EWVFCN72X@ferdinandy.com","subject":"Re: Git 2.48. Changed behavior of the git fetch","fromName":"Bence Ferdinandy","fromEmail":"bence@ferdinandy.com","sentAt":"2025-01-27T16:36:17Z","receivedAt":"2025-01-27T16:37:09Z","isPatch":false,"sender":{"key":"bence@ferdinandy.com","avatar":"https://avatars.githubusercontent.com/u/6343487?v=4"},"body":"\nOn Mon Jan 27, 2025 at 00:35, Bence Ferdinandy wrote:\n>\n> On Tue Jan 21, 2025 at 18:26, Danila Manturov wrote:\n>> Hello. I have done some experiments. For some reason, it works\n>> correctly with JSch. With native ssh/https it doesn't work\n>>\n>> On Mon, Jan 13, 2025 at 5:03 PM Bence Ferdinandy wrote:\n>>>\n>>>\n>>> On Mon Jan 13, 2025 at 15:14, Danila Manturov wrote:\n>>> > According to our CI, the first commit where the bug occurs is\n>>> > 5f212684abb66c9604e745a2296af8c4bb99961c\n>>>\n>>> That makes sense, what is more interesting is why the fix Junio wrote later\n>>> doesn't work in this case ... I didn't have time to dig yet.\n>>>\n>>>\n>\n> I looked up the original thread leading to 6c915c3f85 (fetch: do not ask for\n> HEAD unnecessarily, 2024-12-06) by Junio, which fixed a similar issue (see\n> https://lore.kernel.org/git/444kgiknevb3kwtypjjc2glryaav27t5fafgyzqq5257w7o4pf@4fngcyfmvfcp/T/#u).\n>\n> Originally Josh there suggested just changing the order of adding tags later to\n> the prefixes should solve the issue. I don't think we ever actually figured out\n> why the order of the prefixes should matter, and Junio's patch solved that\n> particular problem by just not asking for HEAD in that case, but it seems that\n> the current problem can also be solved by swapping the order of tags and HEAD.\n\nI had a little bit of time to investigate.\n\nThis is the place of interest in builtin/fetch.h:1771-1781 \n\n\tif (tags == TAGS_SET || tags == TAGS_DEFAULT) {\n\t\tmust_list_refs = 1;\n\t\tif (transport_ls_refs_options.ref_prefixes.nr)\n\t\t\tstrvec_push(&transport_ls_refs_options.ref_prefixes,\n\t\t\t\t \"refs/tags/\");\n\t}\n\n\tif (uses_remote_tracking(transport, rs)) {\n\t\tmust_list_refs = 1;\n\t\tstrvec_push(&transport_ls_refs_options.ref_prefixes, \"HEAD\");\n\t}\n\n\nIf `transport_ls_refs_options.ref_prefixes` is empty we fetch tags. If\n`transport_ls_refs_options.ref_prefixes` is not empty, we fetch what is in this\n`ref_prefixes`. The current code checks if we have anything in ref_prefixes\nbefore adding tags to ref_prefixes and only adds tags if `ref_prefixes` is not\nempty. So currently, since we always add HEAD to `ref_prefixes` it is never\nempty later down the line, but for this case, it is empty when we get to\nchecking the TAG conditions. This is why switching the order works, because\nthen `ref_prefixes` is not empty and tags are explicitly appended.\n\nThis checking for non-empty `ref_prefixes` seems to have been added here by Jonathan:\n\ne70a3030e7 (fetch: do not list refs if fetching only hashes, 2018-09-27)\n\nWhat is not quite clear to me, is that it looks like that the original\nintention was to pretty much always fetch tags, yet it was not achieved by\nalways pushing `refs/tags` into ref_prefixes. Deleting the check for\n`ref_prefixes` being empty [1] breaks quite a lot of things, but reversing the\norder [2] does not. That feels a bit strange tbh since it feels like the two\nshould bring about the same state ...\n\nHopefully someone more knowledgeable knows why things are as they are, but it\nseems that reversing the order really is a band-aid here.\n\n1: https://github.com/ferdinandyb/git/commit/6074a9b8c88451e589eade4034282dd9b6c86345\n2: https://github.com/ferdinandyb/git/commit/31e3f0a6b829d6c7953bf89d015b98e7edabe6b5\n\nBest,\nBence\n"}]}