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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Apr 13, 2023 at 07:03:48PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@xxxxxx> 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: signature.asc
Description: PGP signature


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux