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

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

On Sat, Mar 31 2018, Nguyễn Thái Ngọc Duy wrote:

I'm testing this and it looks good to me so far, aside from this:

> - use git_env_*() instead of manually handling getenv() values
> [...]
>  	struct packed_git **mapping, *p;
> -	int cnt = 0, nr = 1 << OE_IN_PACK_BITS;
> -
> -	if (getenv("GIT_TEST_FULL_IN_PACK_ARRAY")) {
> -		/*
> -		 * leave in_pack_by_idx NULL to force in_pack[] to be
> -		 * used instead
> -		 */
> -		return;
> -	}
> [...]
>
> +	if (git_env_bool("GIT_TEST_FULL_IN_PACK_ARRAY", 0)) {
> +		/*
> +		 * do not initialize in_pack_by_idx[] to force the
> +		 * slow path in oe_in_pack()
> +		 */
> +	} else {
> +		prepare_in_pack_by_idx(pdata);
> +	}
> [...]
> diff --git a/t/README b/t/README
> index 02bfb3fed5..c01d210c15 100644
> --- a/t/README
> +++ b/t/README
> @@ -291,16 +291,26 @@ expect the rest to function correctly.
>  and know what setup is needed for it.  Or when you want to run
>  everything up to a certain test.
>
> +
> +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.
> +
> +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))".