[PATCH 0/11] dumb-http pack index v1 regression + cleanups

From: Jeff King <peff@peff.net>
Date: Oct 25, 2024, 06:41 UTC
Message-ID: <20241025064148.GA2110169@coredump.intra.peff.net>
In-Reply-To: <20241022051402.GB1247135@coredump.intra.peff.net>

On Tue, Oct 22, 2024 at 01:14:02AM -0400, Jeff King wrote:

Show 7 quoted lines

> I'd also like to see if I can clean things up around parse_pack_index(),
> whose semantics I'm changing here (and which violates all manner of
> assumptions that we usually have about packed_git structs). It's used
> only by the dumb-http code, and I think we want to refactor it a bit so
> that nobody else is tempted to use it.
> 
> I'll try to send something out tonight or tomorrow.

This took me a little longer than I expected, mostly due to triggering an (existing) bug related to midx. So here's the series I came up with.

The first patch fixes the midx bug, which otherwise causes spurious test failures in patch 3.

The second patch beefs up the existing tests for the area we're touching.

Patch 3 is the actual regression fix. I apologize in advance for the length of the commit message. ;) I tried to summarize all of the weird dumb-http implications I discovered.

If we wanted to we could stop there for maint, and treat the rest as a separate series. But since it's cleanup with no intended behavior change, it's pretty low-risk.

Patches 4-10 are mostly about cleaning up crufty interfaces that I ran across while working on the earlier patches.

And then the final one is an oddity where I think the code is doing the wrong thing, but it's not possible to actually trigger the bug in practice.

  [01/11]: midx: avoid duplicate packed_git entries
  [02/11]: t5550: count fetches in "previously-fetched .idx" test
  [03/11]: dumb-http: store downloaded pack idx as tempfile
  [04/11]: packfile: drop has_pack_index()
  [05/11]: packfile: drop sha1_pack_name()
  [06/11]: packfile: drop sha1_pack_index_name()
  [07/11]: packfile: warn people away from parse_packed_git()
  [08/11]: http-walker: use object_id instead of bare hash
  [09/11]: packfile: convert find_sha1_pack() to use object_id
  [10/11]: packfile: use object_id in find_pack_entry_one()
  [11/11]: packfile: use oidread() instead of hashcpy() to fill object_id

 builtin/fast-import.c         |  6 ++---
 builtin/pack-objects.c        |  4 +--
 builtin/pack-redundant.c      |  5 +++-
 connected.c                   |  4 +--
 http-push.c                   |  4 +--
 http-walker.c                 | 25 ++++++++---------
 http.c                        | 43 ++++++++++++++++++++---------
 midx.c                        | 22 ++++++++++++---
 pack-bitmap.c                 |  4 +--
 packfile.c                    | 51 ++++++++++++++---------------------
 packfile.h                    | 40 +++++++++++++--------------
 t/helper/test-find-pack.c     |  2 +-
 t/t5200-update-server-info.sh |  8 ++++++
 t/t5550-http-fetch-dumb.sh    | 28 +++++++++++++++++--
 walker.c                      |  4 +--
 walker.h                      |  4 +--
 16 files changed, 153 insertions(+), 101 deletions(-)