[PATCH 0/9] fetch: further ref-prefix cleanups and optimizations

From: Jeff King <peff@peff.net>
Date: Mar 9, 2025, 03:01 UTC
Message-ID: <20250309030101.GA2334064@coredump.intra.peff.net>
In-Reply-To: <61147be4b9a0ee76f1fe0f3376d7316205da350c.1741389941.git.me@ttaylorr.com>

On Fri, Mar 07, 2025 at 06:27:03PM -0500, Taylor Blau wrote:

Show 9 quoted lines

> Peff and I talked about this today, and neither of us could find any
> reasons not to pursue the approach listed in the footnote of
> 
>   <20250221072558.GA572877@coredump.intra.peff.net>
> 
> , but this is a more conservative approach that should fix the issue and
> apply cleanly on top of 'maint'. It may be worth picking this into 2.49,
> even though we are already quite late into the -rc cycle, this is a
> fairly nasty bug.

Yeah, I favor doing this simple fix first, and then trying the larger (and slightly riskier) change on top.

I started to write up that larger patch, and found a number of interesting things. ;) So here's a 9-patch series, which would apply on top of tb/fetch-follow-tags-fix (but can very much wait to cook in the next development cycle).

  [1/9]: t5702: fix typo in test name
  [2/9]: t5516: prefer "oid" to "sha1" in some test titles
  [3/9]: t5516: drop NEEDSWORK about v2 reachability behavior

    Just some cosmetic fixes in nearby areas.

  [4/9]: t5516: beef up exact-oid ref prefixes test

    Improving test coverage in an area I'm about to touch.

  [5/9]: refspec_ref_prefixes(): clean up refspec_item logic

    Code clean-up. Not necessary (textually or semantically) for the
    other patches.

  [6/9]: fetch: ask server to advertise HEAD for config-less fetch
  [7/9]: fetch: stop protecting additions to ref-prefix list
  [8/9]: fetch: avoid ls-refs only to ask for HEAD symref update

    Here we get to the interesting bits. These get rid of the subtle
    dependency that led to this bug in the first place, and also
    optimize a few cases where we can narrow the ref advertisement.

    The third one is a bit optimization, where we avoid a whole
    round-trip to the server. It's correct as far as the rest of the
    code behaves, but I'm not 100% sure that the new origin/HEAD update
    feature isn't a little buggy. See my comments there.

  [9/9]: fetch: use ref prefix list to skip ls-refs

    And then this is just a final bit of cleanup enabled by the earlier
    patches.

 builtin/fetch.c        | 46 +++++++++++++++++-------------------------
 refspec.c              | 22 ++++++++++++++------
 t/t5516-fetch-push.sh  | 12 ++++++-----
 t/t5702-protocol-v2.sh | 44 +++++++++++++++++++++++++++++++++++++++-
 4 files changed, 85 insertions(+), 39 deletions(-)

-Peff