Re: [PATCH v5 00/27] multi-pack reachability bitmaps

Taylor Blau [off-list ref] writes:

Here is another version of the multi-pack reachability bitmaps series. It is
virtually unchanged since last time.

The changes that did occur is that I integrated Johannes' patch from [1] to fix
cleaning up MIDX .rev and .bitmap files when using `--object-dir`. That inspired
a lengthy discussion [2] about `--object-dir`, alternates, object-format and
running the MIDX builtin outside of a Git repository.

This series resolves that discussion by leaving everything as-is, and only
changing the following:

  - `git multi-pack-index` will not run when outside of a Git
    repository.

  - The `--object-dir` argument will only recognize object directories
    belonging to an alternate of the current repository.

  - Using `--object-dir` to point to a repository which uses a
    different hash than the repository in the current working directory
    will continue to not work (as was the case before this series).

And because this incorporates [1], we will also not accidentally clean `.rev`
files from the wrong object directory.

I think that this version is ready-to-go, and that we can turn our attention to
squashing some of these cross-alternate buglets, and integrating MIDX bitmaps
with `git repack`.

Thanks.

    +@@ Documentation/git-multi-pack-index.txt: OPTIONS
    + 	Use given directory for the location of Git objects. We check
    + 	`<dir>/packs/multi-pack-index` for the current MIDX file, and
    + 	`<dir>/packs` for the pack-files to index.
    +++
    ++`<dir>` must be an alternate of the current repository.

After replacing the previous round with this round and running "git
diff @{1}" on the branch, I noticed this documentation update, but
did't find any new code that tries to ensure that the requirement is
met.  It's a bit curious omission.

I think it is OK to allow running this command on <dir> and then add
it as a new alternate (iow, the <dir> being an alternate is not a
strict requirement for correct computation and writing of the midx,
even though it may be a requirement for correct use of the resulting
midx), so perhaps that is where the lack of validation comes from?

THanks.

On Wed, Sep 01, 2021 at 11:07:59AM -0700, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

    +@@ Documentation/git-multi-pack-index.txt: OPTIONS
    + 	Use given directory for the location of Git objects. We check
    + 	`<dir>/packs/multi-pack-index` for the current MIDX file, and
    + 	`<dir>/packs` for the pack-files to index.
    +++
    ++`<dir>` must be an alternate of the current repository.

After replacing the previous round with this round and running "git
diff @{1}" on the branch, I noticed this documentation update, but
did't find any new code that tries to ensure that the requirement is
met.  It's a bit curious omission.

I think it is OK to allow running this command on <dir> and then add
it as a new alternate (iow, the <dir> being an alternate is not a
strict requirement for correct computation and writing of the midx,
even though it may be a requirement for correct use of the resulting
midx), so perhaps that is where the lack of validation comes from?

I wasn't sure whether to include it or not, since we technically will
still write a MIDX in that object directory (alternate or not), but we
won't load up an existing MIDX that is already there to reference. So
we'll get the same result, just slower.

I'm comfortable with saying what's written in the documentation, since
even though it happens to work today, we should leave ourselves open to
not supporting directories which aren't alternates.

But I'm equally OK if you would rather drop this hunk from the
documentation when staging.

Thanks,
Taylor