Re: [PATCH 1/3] introduce GIT_INDEX_VERSION environment variable

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 19, 2014, 00:31 UTC
Message-ID: <xmqqsirg9c4z.fsf@gitster.dls.corp.google.com>
In-Reply-To: <1392492197-7724-2-git-send-email-t.gummerer@gmail.com>

Thomas Gummerer <t.gummerer@gmail.com> writes:

Show 13 quoted lines

> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index aec3726..bc9eeea 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -712,6 +712,11 @@ Git so take care if using Cogito etc.
>  	index file. If not specified, the default of `$GIT_DIR/index`
>  	is used.
>  
> +'GIT_INDEX_VERSION'::
> +	This environment variable allows the specification of an index
> +	version for new repositories.  It won't affect existing index
> +	files.  By default index file version 3 is used.
> +

This is half-correct, isn't it? In-code variable may say version 3 but we demote it to version 2 unless we absolutely need to use the version 3 ugliness.

Show 15 quoted lines

> diff --git a/read-cache.c b/read-cache.c
> index 3f735f3..3993e12 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1223,6 +1223,22 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
>  
>  #define INDEX_FORMAT_DEFAULT 3
>  
> +static unsigned int get_index_format_default()
> +{
> +	char *envversion = getenv("GIT_INDEX_VERSION");
> +	if (!envversion) {
> +		return INDEX_FORMAT_DEFAULT;
> +	} else {
> +		unsigned int version = strtol(envversion, NULL, 10);

If we are using strtol() to parse it carefully, we should make sure it parses to the end by giving a non-NULL second argument and checking where the parsing stopped.

Show 28 quoted lines

> diff --git a/t/t1600-index.sh b/t/t1600-index.sh
> new file mode 100755
> index 0000000..37fd84d
> --- /dev/null
> +++ b/t/t1600-index.sh
> @@ -0,0 +1,24 @@
> +#!/bin/sh
> +
> +test_description='index file specific tests'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +	echo 1 >a
> +'
> +
> +test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
> +	(
> +		GIT_INDEX_VERSION=1 &&
> +		export GIT_INDEX_VERSION &&
> +		git add a 2>&1 | sed "s/[0-9]//" >actual.err &&
> +		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
> +			warning: GIT_INDEX_VERSION set, but the value is invalid.
> +			Using version Z
> +		EOF
> +		test_i18ncmp expect.err actual.err
> +	)
> +'

If we already have an index in version N when this test starts, what should happen?

Will queue on 'pu', with some suggested fixups.

Thanks.