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

Taylor Blau
2025-10-07T20:23:35Z

On Tue, Sep 30, 2025 at 01:21:34AM +0200, Patrick Steinhardt wrote:
> 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.
>
> I would have expected that we also get rid of that static variable
> either in this commit or a follow-up. Is there any reason why you
> skipped over this? If so, it would be great to highlight in the commit
> message.

We can't get rid of "packdir" quite yet, because it's used in other
functions (which get a similar treatment as this one does in later
patches).

We could (and probably should) drop this, packtmp_name, and packtmp as
static variables within builtin/repack.c. We could do it as a fiftieth
patch in this series like so:

--- 8< ---
diff --git a/builtin/repack.c b/builtin/repack.c
index ad60c4290d..565538bb24 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -27,7 +27,6 @@ static int pack_everything;
static int write_bitmaps = -1;
static int use_delta_islands;
static int run_update_server_info = 1;
-static char *packdir, *packtmp_name, *packtmp;
static int midx_must_contain_cruft = 1;

static const char *const git_repack_usage[] = {
@@ -111,6 +110,7 @@ int cmd_repack(int argc,
struct write_pack_opts opts = { 0 };
int i, ret;
int show_progress;
+ char *packdir, *packtmp_name, *packtmp;

/* variables to be filled by option parsing */
struct repack_config_ctx config_ctx;
--- >8 ---

But I'd like to err on the side of post-merge followups here to avoid
dumping 49+ more patches on the list. Of course, if there are
showstoppers or other objections, I can easily reroll the series, but
I'd rather avoid doing so on a series this long.

Thanks,
Taylor