On Wed, Aug 25, 2021 at 03:36:15AM -0400, Jeff King wrote: > On Tue, Aug 24, 2021 at 10:10:12PM -0400, Taylor Blau wrote: > > > > It's not clear to me that we have a resolution on whether calling "cd .. > > > && git multi-pack-index write --object-dir repo.git" is supposed to > > > work. > > > > My recommendation would be to do the following things, all in a reroll > > of this series: > > > > - Fix the bug by which we would delete a .rev or .bitmap file out of a > > different object store than we were working in (when the caller > > passes `--object-dir`). > > > > - Disallow running `git multi-pack-index` outside of a Git repository. > > > > - Restrict `--object-dir` to only work with alternates of the > > repository in the current working directory. > > > > To me, that seems like both the least-surprising behavior, and what > > would lend itself to the easiest implementation. I would probably argue > > that the existing behavior (where `--object-dir` would work against > > arbitrary repositories) is a bug, and shouldn't continue to be > > supported. > > All of those seem reasonable to me, and are what I would suggest if we > were starting from scratch. My only hesitation is whether people are > using the weird behavior of --object-dir in the wild (e.g., are bup > folks relying on it). > > Johannes, is this something you're using _now_, and it works, or > something you hoped to use in the future? I did some research[1] on what parts of `--object-dir` have worked (and not worked) in the past, and came to the conclusion that although this behavior is surprising, we do bear the responsibility of continuing to maintain it. And in that sense, I agree with your "only call close_object_store() if the MIDX we are using came from the object store, or otherwise call close_midx() if it didn't", so that's what I did in the tb/multi-pack-bitmaps branch of my fork[2]. I think that this is the most reasonable path forward, since it resolves Johannes' concerns while also not breaking any existing functionality in the meantime as we add new features on top. It has the added benefit of closing some holes that were open in the past, so I think that it's worth doing. Before I drop 27 patches onto the inboxes of list subscribers, would you mind taking a look at [1] (and the rest of the patches in [2]) to make sure that you're OK with the approach too? Thanks, Taylor [1]: https://github.com/ttaylorr/git/commit/a24290489c2b30f3caed7e33fe8f85226a12778f [2]: https://github.com/ttaylorr/git/compare/tb/multi-pack-bitmaps
