public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed
From: "Bence Ferdinandy" <bence@ferdinandy•com>
To: "Danila Manturov" <danila.manturov@jetbrains•com>
Cc: <git@vger•kernel.org>, "Junio C Hamano" <gitster@pobox•com>,
	"Jonathan Tan" <jonathantanmy@google•com>
Subject: Re: Git 2.48. Changed behavior of the git fetch
Date: Mon, 27 Jan 2025 17:36:17 +0100	[thread overview]
Message-ID: <D7D031QT4HEX.14TRNKRC6FC7S@ferdinandy.com> (raw)
In-Reply-To: <D7CEDCJ0KKYL.YS0EWVFCN72X@ferdinandy.com>


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

      parent reply	other threads:[~2025-01-27 16:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=D7D031QT4HEX.14TRNKRC6FC7S@ferdinandy.com \
    --to=bence@ferdinandy$(echo .)com \
    --cc=danila.manturov@jetbrains$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitster@pobox$(echo .)com \
    --cc=jonathantanmy@google$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox