Subject: Re: [PATCH v8 00/15] nd/pack-objects-pack-struct updates

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Sat, 31 Mar 2018 17:43:38 +0200

On Sat, Mar 31 2018, Duy Nguyen wrote:

> On Sat, Mar 31, 2018 at 1:36 PM, Ævar Arnfjörð Bjarmason
> <[email-masked]> wrote:
>>> +GIT_TEST_SPLIT_INDEX forces split-index mode on the whole test suite.
>>> +
>>>  GIT_TEST_FULL_IN_PACK_ARRAY exercises the uncommon pack-objects code
>>>  path where there are more than 1024 packs even if the actual number of
>>>  packs in repository is below this limit.
>>>
>>> -GIT_TEST_OE_SIZE_BITS=<bits> exercises the uncommon pack-objects
>>> -code path where we do not cache objecct size in memory and read it
>>> -from existing packs on demand. This normally only happens when the
>>> -object size is over 2GB. This variable forces the code path on any
>>> -object larger than 2^<bits> bytes.
>>
>> The docs here say set these env variables, but actually
>> GIT_TEST_FULL_IN_PACK_ARRAY is a special snowflake in requiring you to
>> set a bool value.
>>
>> I'd set GIT_TEST_SPLIT_INDEX=YesPlease already in my test setup & just
>> copied that as GIT_TEST_FULL_IN_PACK_ARRAY=YesPlease, but that'll error
>> out since it's expecting bool, not the env variable to be set.
>>
>> I really don't care which we use, but let's use either if(getenv()) or
>> if(git_env_bool()) consistently, and then have the docs either say "if
>> set" or "if set to a boolean value (see git-config(1))".
>
> I'll change GIT_TEST_SPLIT_INDEX to boolean too since I document it
> here anyway. Will wait for a while though to see if anything else
> should be part of v9.

Sounds good, FWIW (since I spied your forced push to your private branch
on Github) I mean something like this on top of what you just pushed:

    diff --git a/t/README b/t/README
    index 65dee935c0..583bede192 100644
    --- a/t/README
    +++ b/t/README
    @@ -298,7 +298,8 @@ Running tests with special setups
     The whole test suite could be run to test some special features
     that cannot be easily covered by a few specific test cases. These
     could be enabled by running the test suite with correct GIT_TEST_
    -environment set.
    +environment variable set to a boolean value, as documented in the
    +"Values" section of git-config(1).

     GIT_TEST_SPLIT_INDEX=<true|false> forces split-index mode on the whole
     test suite.

I.e. the part above where we just say it has to be set should be changed
to indicate it's a boolean as understood by git, since in shell/*nix
idiom saying something has to be set just means ensure getenv() won't
return NULL.