On Tue, Aug 24, 2021 at 08:28:36PM -0400, Jeff King wrote: > On Tue, Aug 24, 2021 at 12:15:47PM -0400, Taylor Blau wrote: > > > Range-diff against v3: > > [...] > > 9: 40cff5beb5 ! 9: c9fea31fa8 midx: avoid opening multiple MIDXs when writing > > @@ Commit message > > one and should invalidate the object store's memory of any MIDX that > > might have existed beforehand. > > > > + Note that this now forbids passing object directories that don't belong > > + to alternate repositories over `--object-dir`, since before we would > > + have happily opened a MIDX in any directory, but now restrict ourselves > > + to only those reachable by `r->objects->multi_pack_index` (and alternate > > + MIDXs that we can see by walking the `next` pointer). > > + > > + As far as I can tell, supporting arbitrary directories with > > + `--object-dir` was a historical accident, since even the documentation > > + says `<alt>` when referring to the value passed to this option. > > + > > + A future patch could clean this up and provide a warning() when a > > + non-alternate directory was given, since we'll still write a new MIDX > > + there, we just won't reuse any MIDX that might happen to already exist > > + in that directory. > > + > > So this is definitely fixed as we discussed. But since that discussion, > we've had the thread over in: > > https://lore.kernel.org/git/20210820195558.44275-1-johannes@xxxxxxxxxxxxxxxx/ > > and its siblings: > > https://lore.kernel.org/git/20210823094049.44136-1-johannes@xxxxxxxxxxxxxxxx/ > > https://lore.kernel.org/git/20210823171011.80588-1-johannes@xxxxxxxxxxxxxxxx/ > > 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. So my plan would be to do that, which would generate something like the following range-diff. If nobody has any objections, I'd like to send what I currently have in ttaylorr/git on GitHub in the tb/multi-pack-bitmaps branch as a reroll of this series, and then merge that early in the cycle to give it a chance to be tested before we cut 2.34. Thanks, Taylor
