Re: [PATCH v4 00/25] multi-pack reachability bitmaps

[Taylor Blau <me@ttaylorr.com>]

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@sipsolutions.net/
>
> and its siblings:
>
>   https://lore.kernel.org/git/20210823094049.44136-1-johannes@sipsolutions.net/
>
>   https://lore.kernel.org/git/20210823171011.80588-1-johannes@sipsolutions.net/
>
> 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