Re: [PATCH v1] read-cache: add GIT_TEST_INDEX_VERSION support

[Thomas Gummerer <t.gummerer@gmail.com>]

On 09/12, Ben Peart wrote:
> Teach get_index_format_default() to support running the test suite
> with specific index versions.  In particular, this enables the test suite
> to be run using index version 4 which is not the default so gets less testing.

I found this commit message slightly misleading.  Running the test
suite with specific index versions is already supported, by defining
TEST_GIT_INDEX_VERSION in 'config.mak'.  What we're doing here is
introduce an additional environment variable that can also be used to
set the index format in tests.

Even setting TEST_GIT_INDEX_VERSION=4 in the environment does run the
test suite with index-v4.  Admittedly the name is a bit strange
compared to our usual GIT_TEST_* environment variable names, and it
should probably be documented better (it's only documented in the
Makefile currently), but I'm not sure we should introduce another
environment variable for this purpose?

> Signed-off-by: Ben Peart <Ben.Peart@microsoft.com>
> ---
> 
> Notes:
>     Base Ref: v2.19.0
>     Web-Diff: https://github.com/benpeart/git/commit/52e733e2ce
>     Checkout: git fetch https://github.com/benpeart/git git-test-index-version-v1 && git checkout 52e733e2ce
> 
>  read-cache.c | 47 +++++++++++++++++++++++++++++++++--------------
>  t/README     |  6 +++++-
>  2 files changed, 38 insertions(+), 15 deletions(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index 7b1354d759..d140ce9989 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1570,26 +1570,45 @@ static unsigned int get_index_format_default(void)
>  	char *envversion = getenv("GIT_INDEX_VERSION");
>  	char *endp;
>  	int value;
> -	unsigned int version = INDEX_FORMAT_DEFAULT;
> +	unsigned int version = -1;
> +
> +	if (envversion) {
> +		version = strtoul(envversion, &endp, 10);
> +		if (*endp ||
> +			version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> +			warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
> +				"Using version %i"), INDEX_FORMAT_DEFAULT);
> +			version = INDEX_FORMAT_DEFAULT;
> +		}
> +	}
>  
> -	if (!envversion) {
> -		if (!git_config_get_int("index.version", &value))
> +	if (version == -1) {
> +		if (!git_config_get_int("index.version", &value)) {
>  			version = value;
> -		if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> -			warning(_("index.version set, but the value is invalid.\n"
> -				  "Using version %i"), INDEX_FORMAT_DEFAULT);
> -			return INDEX_FORMAT_DEFAULT;
> +			if (version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> +				warning(_("index.version set, but the value is invalid.\n"
> +					"Using version %i"), INDEX_FORMAT_DEFAULT);
> +				version = INDEX_FORMAT_DEFAULT;
> +			}
>  		}
> -		return version;
>  	}
>  
> -	version = strtoul(envversion, &endp, 10);
> -	if (*endp ||
> -	    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> -		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
> -			  "Using version %i"), INDEX_FORMAT_DEFAULT);
> -		version = INDEX_FORMAT_DEFAULT;
> +	if (version == -1) {
> +		envversion = getenv("GIT_TEST_INDEX_VERSION");
> +		if (envversion) {
> +			version = strtoul(envversion, &endp, 10);
> +			if (*endp ||
> +				version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
> +				warning(_("GIT_TEST_INDEX_VERSION set, but the value is invalid.\n"
> +					"Using version %i"), INDEX_FORMAT_DEFAULT);
> +				version = INDEX_FORMAT_DEFAULT;
> +			}
> +		}
>  	}
> +
> +	if (version == -1)
> +		version = INDEX_FORMAT_DEFAULT;
> +
>  	return version;
>  }
>  
> diff --git a/t/README b/t/README
> index 9028b47d92..f872638a78 100644
> --- a/t/README
> +++ b/t/README
> @@ -315,10 +315,14 @@ 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
>  <n> bytes.
>  
> -GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncomon pack-objects code
> +GIT_TEST_OE_DELTA_SIZE=<n> exercises the uncommon pack-objects code
>  path where deltas larger than this limit require extra memory
>  allocation for bookkeeping.
>  
> +GIT_TEST_INDEX_VERSION=<n> exercises the index read/write code path
> +for the index version specified.  Can be set to any valid version
> +but the non-default version 4 is probably the most beneficial.
> +
>  Naming Tests
>  ------------
>  
> 
> base-commit: 1d4361b0f344188ab5eec6dcea01f61a3a3a1670
> -- 
> 2.18.0.windows.1
>