On Mon Jan 27, 2025 at 00:35, Bence Ferdinandy <bence@xxxxxxxxxxxxxx> wrote: > > On Tue Jan 21, 2025 at 18:26, Danila Manturov <danila.manturov@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx> wrote: >>> >>> >>> On Mon Jan 13, 2025 at 15:14, Danila Manturov <danila.manturov@xxxxxxxxxxxxx> 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
