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. It has traditionally worked (at least for trivial cases, AFAICT), but I find the behavior surprising and unlike most of the rest of Git, and I'm not at all certain that there aren't subtle bugs lurking (basically anything that wants to do object lookup, like oh say, a bitmap generator). But if we do want to support it, then we have to find a different solution here, don't we? I think the least-painful version of that is probably recording _whether_ we found ctx.m in the_repository's object_store, and switching behavior based on that (e.g., calling close_midx() versus close_object_store() depending). -Peff
