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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 1, 2021, 18:07 UTC
Message-ID: <xmqq5yvkqidc.fsf@gitster.g>
In-Reply-To: <cover.1630443072.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> writes:

Show 27 quoted lines

> 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.

Show 6 quoted lines

>     +@@ 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.