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

Jeff King [off-list ref] writes:

quoted

The new test, specifically this snippet:

    git init repo &&
    test_when_finished "rm -fr repo" &&
    (
      cd repo &&
      test_commit base &&
      git repack -d
    ) &&

    nongit git multi-pack-index --object-dir=$(pwd)/repo/.git/objects write

will fail with GIT_TEST_DEFAULT_HASH=sha256, since the MIDX internals
settle on the hash size via `the_hash_algo` which doesn't respect the
hash algorithm used by the target repository.

Yeah, I think this is a good example of the class of things that might
fail: anything that requires the repo config to behave correctly.

I do think the hash format is somewhat unusual here. Most of the changes
to the on-disk files are reflected in the files themselves (e.g., pack
index v2 is chosen by config at _write_ time, but readers can interpret
the file stand-alone).

There may be other config that could influence the writing of the midx,
and we'd skip it in this kind of non-repo setup. An example here is
repack.usedeltabaseoffset, which midx_repack() tries to respect.
Ignoring that doesn't produce a nonsense result, but it doesn't follow
what would happen if run from inside the repo.

The other class of problems I'd expect is where part of the midx
operation needs to look at other parts of the repo. Bitmap generation is
an obvious one there, since we'd want to look at refs to find the
reachable tips. Now obviously that's a new feature we're trying to
introduce here, so it can't be an existing breakage. But it does make me
wonder what other problems might be lurking.

So I dunno. Even if it mostly works now, I'm not sure it's something
that I'm all that happy about supporting going forward. It seems like a
recipe for subtle bugs where the midx code calls into other library code
that assumes that it can look at the repository struct.

I tend to agree that we should first disallow things that are not
what we know we definitely need, and the non-repo setup is something
we would want to punt on to make sure we have a solid support for
the mainstream usecase.

Thanks.