Thread (158 messages) 158 messages, 3 authors, 2025-10-16

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

From: Jeff King <hidden>
Date: 2025-10-10 06:04:04

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help