Re: [PATCH 6/9] fetch: ask server to advertise HEAD for config-less fetch

From: Jeff King <peff@peff.net>
Date: Mar 13, 2025, 05:46 UTC
Message-ID: <20250313054644.GF94015@coredump.intra.peff.net>
In-Reply-To: <Z9H//JHtYTGqHI3n@nand.local>

On Wed, Mar 12, 2025 at 05:43:24PM -0400, Taylor Blau wrote:

Show 13 quoted lines

> > Note that we'll add "HEAD" to the list of prefixes, and later code for
> > updating "refs/remotes/<remote>/HEAD" may likewise do so. In theory this
> > could cause duplicates in the list, but in practice these can't both
> > trigger. We hit our new case only if there are no refspecs, and the
> > "<remote>/HEAD" feature is enabled only when we are fetching from a
> > remote with configured refspecs. We could be defensive with a flag, but
> > it didn't seem worth it to me (the absolute worse case is a useless
> > redundant ref-prefix line sent to the server).
> 
> Yeah, I think that we already do this in some instances that you and I
> talked about off-list, but I can't remember exactly what I did to
> provoke it. In either case, the server responds correctly, so I don't
> think it's so urgent to deal with ATM.

Yeah, I thought we did, too, but after looking at the code, I doubt it. I think we were always playing with exact-oid refspecs, so we'd always hit the "if (rs->nr)" side of that conditional.

In order to hit the case touched by this patch, you can't have refspecs, but still need to somehow get a true answer out of uses_remote_tracking().

Show 7 quoted lines

> > +test_expect_success 'set up parent for prefix tests' '
> > +	git init prefix-parent &&
> > +	git -C prefix-parent commit --allow-empty -m foo &&
> 
> Any reason to use a bona-fide "commit" here instead of "test_commit"?
> 
> Not a big deal either way, of course, I'm just curious.

Nope, I mostly just reach for "git commit" without thinking because that's what I naturally do while debugging or exploring.

But since you asked...;)

I do find test_commit a bit bloated in general. It takes several commands versus one, leaves cruft files in the working tree (that you need to care about not using again, lest your commit fail with "no changes"), and by default makes tags that sometimes cause confusion about fetching, reachability, and so on.

The one thing it does do that git-commit doesn't is increment test_tick. That sometimes is important (if you care about traversal ordering), but usually doesn't.

So I dunno. Maybe I am a bad person for not using test_commit by default and we should have a style suggestion there.

-Peff