Re: [PATCH v3 00/10] repack: fix geometric repacking with gitalternates

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]

On Thu, Apr 13, 2023 at 07:03:48PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
[snip]
> With a few fix-ups to the test script I sent to the thread for the
> review of [09/10] squashed in, this topic however seems to cause the
> linux-TEST-vars job fail when queued at the tip of 'seen' (bbfd3bf)
> [*1*].  When 'seen' is rewound by one merge to exclude this topic,
> the CI runs OK (c35f3cd)[*2*].  I dug only far enough to identify
> which topic among 40+ ones was causing the CI breakage.  Perhaps the
> code is operating correctly but the expectation in the test needs
> updating?  I dunno.

The issue is that linux-TEST-vars sets a bunch of environment variables
that change the way git-repack(1) operates. Important in this context
are GIT_TEST_MULTI_PACK_INDEX and GIT_TEST_MULTI_PACK_INDEX_GENERATE_BITMAP
as they change the assumptions that we're doing in our test. What is
interesting is that these cause us to write a multi-pack-index and in
fact a bitmap, but the warning that we're testing for is not printed.
And that's because of the following code snippet:

```
if (write_bitmaps < 0) {
        if (!write_midx &&
            (!(pack_everything & ALL_INTO_ONE) || !is_bare_repository()))
                write_bitmaps = 0;
} else if (write_bitmaps &&
           git_env_bool(GIT_TEST_MULTI_PACK_INDEX, 0) &&
           git_env_bool(GIT_TEST_MULTI_PACK_INDEX_WRITE_BITMAP, 0)) {
        write_bitmaps = 0;
}
```

So if either of those environment variables is set we'll disable
generation of bitmaps in git-repack(1) in favor of enabling them in
git-multi-pack-index(1). And consequentially we don't see the expected
error message.

Anyway. I think what we should do here is to simply override the
mechanism with GIT_TEST_MULTI_PACK_INDEX=0. Other tests in this file do
the same already, and it does make sense as we explicitly want to test
the non-multi-pack-index scenario. And given that we already added a
test for the with-multi-pack-index scenario in the same commit we don't
really loose any test coverage there.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]