Subject: Re: [PATCH 17/49] builtin/repack.c: pass "packdir" when removing packs

From: Jeff King <[email-masked]>
Date: Fri, 10 Oct 2025 02:04:01 -0400

On Sun, Sep 28, 2025 at 06:08:16PM -0400, Taylor Blau wrote:

> builtin/repack.c defines a static "packdir" to instruct pack-objects on
> where to write any new packfiles. This is also the directory scanned
> when removing any packfiles which were made redundant by the latest
> repack.
> 
> Prepare to move the "existing_packs_remove_redundant" function to its
> own compilation unit by passing in this information as a parameter to
> that function.

OK. I don't think there's anything wrong here, but it feels like we
ought to be able to pull this from the existing "struct repository"
context that's in the existing_packs struct.

I had to dig a little to find it, but I think that always comes from:

  packdir = mkpathdup("%s/pack", repo_get_object_directory(repo));

in cmd_repack(). So we'd have to recreate that string ourselves. It
feels like object_database (or I guess odb_source) should have an easy
way to get the pack directory, but it doesn't seem to.

So I dunno. I think it is mostly academic, as you are just moving around
code here. But as an API, the separate "packdir" argument makes me
wonder if I can pass anything I like to these functions. But really, it
should match the packdir used to create the lists in the existing_packs
struct. So it's kind of a brittle API. I'm not sure how much we should
care for this series, as it is mostly about moving code around, and not
providing some kind of flexible repack API.

-Peff