Jeff King <peff@xxxxxxxx> writes: >> 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.
