On Thu, Jun 29, 2023 at 05:24:40AM -0400, Taylor Blau wrote: > > I also kind of wonder if this repack code should simply be loading and > > iterating the packed_git list, but that is a much bigger change. > > I have wanted to do this for a while ;-). The minimal patch is less > invasive than I had thought: Yeah, I agree it's not too bad. If we want to go that route, though, I think we should do it on top of Stolee's patch, though (which makes it a pure cleanup once the behaviors are aligned). I'm also not sure if you'd need to do anything tricky with alternate object dirs (it looks like the existing code ignores them entirely, so I guess we'd want to skip entries without pack_local set). > [...] > I think you could probably go further than this, since having to store > the suffix-less pack names in the fname_kept and fname_nonkept lists is > kind of weird. > > It would be nice if we could store pointers to the actual packed_git > structs themselves in place of those lists instead, but I'm not > immediately sure how feasible it would be to do since we re-prepare the > object store between enumerating and then removing these packs. I think that would work, because we do not ever drop packed_git entries from the list (even if the files were deleted between prepare/reprepare). But it also creates a subtle memory ownership dependency/assumption between the two bits of code. It seems clearer to me to just copy the names out to our own lists here (i.e., the patch you showed). -Peff
