[PATCH 0/3] Wider exposure for index-v4

[PATCH 0/3] Wider exposure for index-v4

[Thomas Gummerer]

Hi,

since index-v5 didn't seem to generate enough interest to be merged, I
have a few patches that give users users easier access to index-v4.
Until now users have to go into the source code and compile git
themselves to use index-v4 by default, or use git-update-index to
change the index file to the new version.

With this patches it's possible to set the default index file format
either in gitconfig or in an environment variable.  It also simplifies
testing index-v4 by adding a Makefile knob to use it for running the
test suite.  For safety, existing repositories are not changed when
the environment or the config variables are set.

I'm not sure about the precedence in patch 3, right now the environment
variable has precedence, but it should be easy to give the config
option precedence over that.

Thomas Gummerer (3):
introduce GIT_INDEX_VERSION environment variable
test-lib: allow setting the index format version
read-cache: add index.version config variable

Documentation/config.txt              |  4 +++
Documentation/git.txt                 |  5 ++++
Makefile                              |  7 +++++
read-cache.c                          | 36 +++++++++++++++++++++++-
t/t1600-index.sh                      | 52 +++++++++++++++++++++++++++++++++++
t/t2104-update-index-skip-worktree.sh |  2 ++
t/test-lib-functions.sh               |  5 ++++
t/test-lib.sh                         |  3 ++
8 files changed, 113 insertions(+), 1 deletion(-)
create mode 100755 t/t1600-index.sh

--
1.8.3.2

[PATCH 1/3] introduce GIT_INDEX_VERSION environment variable

[Thomas Gummerer]

Respect a GIT_INDEX_VERSION environment variable, when a new index is
initialized.  Setting the environment variable will not cause existing
index files to be converted to another format, but will only affect
newly written index files.  This can be used to initialize repositories
with index-v4.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git.txt |  5 +++++
 read-cache.c          | 18 +++++++++++++++++-
 t/t1600-index.sh      | 24 ++++++++++++++++++++++++
 3 files changed, 46 insertions(+), 1 deletion(-)
 create mode 100755 t/t1600-index.sh

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.
+
 'GIT_OBJECT_DIRECTORY'::
 	If the object storage directory is specified via this
 	environment variable then the sha1 directories are created
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 (version < INDEX_FORMAT_LB || version > INDEX_FORMAT_UB) {
+			warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
+				  "Using version %i"), INDEX_FORMAT_DEFAULT);
+			version = INDEX_FORMAT_DEFAULT;
+		}
+		return version;
+	}
+}
+
 /*
  * dev/ino/uid/gid/size are also just tracked to the low 32 bits
  * Again - this is just a (very strong in practice) heuristic that
@@ -1799,7 +1815,7 @@ int write_index(struct index_state *istate, int newfd)
 	}
 
 	if (!istate->version)
-		istate->version = INDEX_FORMAT_DEFAULT;
+		istate->version = get_index_format_default();
 
 	/* demote version 3 to version 2 when the latter suffices */
 	if (istate->version == 3 || istate->version == 2)
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
+	)
+'
+
+test_done
-- 
1.8.5.2.300.ge613be6.dirty

[PATCH 2/3] test-lib: allow setting the index format version

[Thomas Gummerer]

Allow adding a TEST_GIT_INDEX_VERSION variable to config.mak to set the
index version with which the test suite should be run.

If it isn't set, the default version given in the source code is
used (currently version 3).

To avoid breakages with index versions other than [23], also set the
index version under which t2104 is run to 3.  This test only tests
functionality specific to version 2 and 3 of the index file and would
fail if the test suite is run with any other version.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Makefile                              | 7 +++++++
 t/t2104-update-index-skip-worktree.sh | 2 ++
 t/test-lib-functions.sh               | 5 +++++
 t/test-lib.sh                         | 3 +++
 4 files changed, 17 insertions(+)

diff --git a/Makefile b/Makefile
index 287e6f8..c98d28f 100644
--- a/Makefile
+++ b/Makefile
@@ -342,6 +342,10 @@ all::
 # Define DEFAULT_HELP_FORMAT to "man", "info" or "html"
 # (defaults to "man") if you want to have a different default when
 # "git help" is called without a parameter specifying the format.
+#
+# Define TEST_GIT_INDEX_FORMAT to 2, 3 or 4 to run the test suite
+# with a different indexfile format.  If it isn't set the index file
+# format used is index-v[23].
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -2223,6 +2227,9 @@ endif
 ifdef GIT_PERF_MAKE_OPTS
 	@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@
 endif
+ifdef TEST_GIT_INDEX_VERSION
+	@echo TEST_GIT_INDEX_VERSION=\''$(subst ','\'',$(subst ','\'',$(TEST_GIT_INDEX_VERSION)))'\' >>$@
+endif
 
 ### Detect Python interpreter path changes
 ifndef NO_PYTHON
diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index 1d0879b..29c1fb1 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -7,6 +7,8 @@ test_description='skip-worktree bit test'
 
 . ./test-lib.sh
 
+test_set_index_version 3
+
 cat >expect.full <<EOF
 H 1
 H 2
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index aeae3ca..0bf1e63 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -32,6 +32,11 @@ test_set_editor () {
 	export EDITOR
 }
 
+test_set_index_version () {
+    GIT_INDEX_VERSION="$1"
+    export GIT_INDEX_VERSION
+}
+
 test_decode_color () {
 	awk '
 		function name(n) {
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1cf78d5..e6cf5b0 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -108,6 +108,9 @@ export GIT_AUTHOR_EMAIL GIT_AUTHOR_NAME
 export GIT_COMMITTER_EMAIL GIT_COMMITTER_NAME
 export EDITOR
 
+GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+export GIT_INDEX_VERSION
+
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind
 if expr " $GIT_TEST_OPTS " : ".* --valgrind " >/dev/null ||
-- 
1.8.5.2.300.ge613be6.dirty

[PATCH 3/3] read-cache: add index.version config variable

[Thomas Gummerer]

Add a config variable that allows setting the default index version when
initializing a new index file.  Similar to the GIT_INDEX_VERSION
environment variable this only affects new index files.

Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/config.txt |  4 ++++
 read-cache.c             | 27 ++++++++++++++++++++++-----
 t/t1600-index.sh         | 27 +++++++++++++++++++++++++++
 3 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1655455..033939a 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1591,6 +1591,10 @@ imap::
 	The configuration variables in the 'imap' section are described
 	in linkgit:git-imap-send[1].
 
+index.version::
+	Specify the version with which new index files should be
+	initialized.  This does not affect existing repositories.
+
 init.templatedir::
 	Specify the directory from which templates will be copied.
 	(See the "TEMPLATE DIRECTORY" section of linkgit:git-init[1].)
diff --git a/read-cache.c b/read-cache.c
index 3993e12..ca9b68c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1223,20 +1223,37 @@ static struct cache_entry *refresh_cache_entry(struct cache_entry *ce, int reall
 
 #define INDEX_FORMAT_DEFAULT 3
 
+static int index_format_config(const char *var, const char *value, void *cb)
+{
+	unsigned int *version = cb;
+	if (!strcmp(var, "index.version")) {
+		*version = git_config_int(var, value);
+		return 0;
+	}
+	return 1;
+}
+
+
 static unsigned int get_index_format_default()
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
+	unsigned int version = INDEX_FORMAT_DEFAULT;
 	if (!envversion) {
-		return INDEX_FORMAT_DEFAULT;
-	} else {
-		unsigned int version = strtol(envversion, NULL, 10);
+		git_config(index_format_config, &version);
 		if (version < INDEX_FORMAT_LB || version > INDEX_FORMAT_UB) {
-			warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
+			warning(_("index.version set, but the value is invalid.\n"
 				  "Using version %i"), INDEX_FORMAT_DEFAULT);
-			version = INDEX_FORMAT_DEFAULT;
+			return INDEX_FORMAT_DEFAULT;
 		}
 		return version;
 	}
+	version = strtol(envversion, NULL, 10);
+	if (version < INDEX_FORMAT_LB || version > INDEX_FORMAT_UB) {
+		warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
+			  "Using version %i"), INDEX_FORMAT_DEFAULT);
+		version = INDEX_FORMAT_DEFAULT;
+	}
+	return version;
 }
 
 /*
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 37fd84d..bf34985 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -21,4 +21,31 @@ test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
 	)
 '
 
+test_expect_success 'out of bounds index.version issuses warning' '
+	(
+		unset GIT_INDEX_VERSION &&
+		rm .git/index &&
+		git config --add index.version 1 &&
+		git add a 2>&1 | sed "s/[0-9]//" >actual.err &&
+		sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
+			warning: index.version set, but the value is invalid.
+			Using version Z
+		EOF
+		test_i18ncmp expect.err actual.err
+	)
+'
+
+test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
+	(
+		rm .git/index &&
+		GIT_INDEX_VERSION=4 &&
+		export GIT_INDEX_VERSION &&
+		git config --add index.version 2 &&
+		git add a 2>&1 &&
+		echo 4 >expect &&
+		test-index-version <.git/index >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_done
-- 
1.8.5.2.300.ge613be6.dirty

Re: [PATCH 0/3] Wider exposure for index-v4

[Duy Nguyen]

On Sun, Feb 16, 2014 at 2:23 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Hi,
>
> since index-v5 didn't seem to generate enough interest to be merged, I

I thought there were some comments last time that you were going to
address and resubmit?

> have a few patches that give users users easier access to index-v4.
> Until now users have to go into the source code and compile git
> themselves to use index-v4 by default, or use git-update-index to
> change the index file to the new version.

Not objecting this, but I think something like [1] would give v4 more
exposure. Reading the patch again, I think putting that detection code
in unpack_trees() or git-merge may make more sense because people will
be advised about upgrading to v4 at the next fast-forward.

[1] http://article.gmane.org/gmane.comp.version-control.git/216307

> With this patches it's possible to set the default index file format
> either in gitconfig or in an environment variable.  It also simplifies
> testing index-v4 by adding a Makefile knob to use it for running the
> test suite.  For safety, existing repositories are not changed when
> the environment or the config variables are set.
>
> I'm not sure about the precedence in patch 3, right now the environment
> variable has precedence, but it should be easy to give the config
> option precedence over that.
>
> Thomas Gummerer (3):
> introduce GIT_INDEX_VERSION environment variable
> test-lib: allow setting the index format version
> read-cache: add index.version config variable
>
> Documentation/config.txt              |  4 +++
> Documentation/git.txt                 |  5 ++++
> Makefile                              |  7 +++++
> read-cache.c                          | 36 +++++++++++++++++++++++-
> t/t1600-index.sh                      | 52 +++++++++++++++++++++++++++++++++++
> t/t2104-update-index-skip-worktree.sh |  2 ++
> t/test-lib-functions.sh               |  5 ++++
> t/test-lib.sh                         |  3 ++
> 8 files changed, 113 insertions(+), 1 deletion(-)
> create mode 100755 t/t1600-index.sh
>
> --
> 1.8.3.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html



-- 
Duy

Re: [PATCH 3/3] read-cache: add index.version config variable

[Eric Sunshine]

On Sat, Feb 15, 2014 at 2:23 PM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
> Add a config variable that allows setting the default index version when
> initializing a new index file.  Similar to the GIT_INDEX_VERSION
> environment variable this only affects new index files.
>
> Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
> ---
> index 37fd84d..bf34985 100755
> --- a/t/t1600-index.sh
> +++ b/t/t1600-index.sh
> @@ -21,4 +21,31 @@ test_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
>         )
>  '
>
> +test_expect_success 'out of bounds index.version issuses warning' '

s/issuses/issues/

> +       (
> +               unset GIT_INDEX_VERSION &&
> +               rm .git/index &&
> +               git config --add index.version 1 &&
> +               git add a 2>&1 | sed "s/[0-9]//" >actual.err &&
> +               sed -e "s/ Z$/ /" <<-\EOF >expect.err &&
> +                       warning: index.version set, but the value is invalid.
> +                       Using version Z
> +               EOF
> +               test_i18ncmp expect.err actual.err
> +       )
> +'
> +
> +test_expect_success 'GIT_INDEX_VERSION takes precedence over config' '
> +       (
> +               rm .git/index &&
> +               GIT_INDEX_VERSION=4 &&
> +               export GIT_INDEX_VERSION &&
> +               git config --add index.version 2 &&
> +               git add a 2>&1 &&
> +               echo 4 >expect &&
> +               test-index-version <.git/index >actual &&
> +               test_cmp expect actual
> +       )
> +'
> +
>  test_done
> --
> 1.8.5.2.300.ge613be6.dirty

Re: [PATCH 0/3] Wider exposure for index-v4

[Thomas Gummerer]

Duy Nguyen <pclouds@gmail.com> writes:

> On Sun, Feb 16, 2014 at 2:23 AM, Thomas Gummerer <t.gummerer@gmail.com> wrote:
>> Hi,
>>
>> since index-v5 didn't seem to generate enough interest to be merged, I
>
> I thought there were some comments last time that you were going to
> address and resubmit?

Yes, there were some comments to the last round, which I already fixed
locally, I'd just have to rebase it to make sure it stell applies
cleanly.  No responses from Junio to [1] and [2] gave me the impression
that it's not going to be applied.  I would be happy to rebase and
submit if there is a chance for it getting in.

[1] http://thread.gmane.org/gmane.comp.version-control.git/238414/focus=239065
[2] http://thread.gmane.org/gmane.comp.version-control.git/232488/focus=233504

>> have a few patches that give users users easier access to index-v4.
>> Until now users have to go into the source code and compile git
>> themselves to use index-v4 by default, or use git-update-index to
>> change the index file to the new version.
>
> Not objecting this, but I think something like [1] would give v4 more
> exposure. Reading the patch again, I think putting that detection code
> in unpack_trees() or git-merge may make more sense because people will
> be advised about upgrading to v4 at the next fast-forward.

Thanks, I forgot about this patch.  I still think at least the first two
patches of this series make sense in addition to your patch, allowing
developers to easily run the test suite with index-v4.

> [1] http://article.gmane.org/gmane.comp.version-control.git/216307
>
>> With this patches it's possible to set the default index file format
>> either in gitconfig or in an environment variable.  It also simplifies
>> testing index-v4 by adding a Makefile knob to use it for running the
>> test suite.  For safety, existing repositories are not changed when
>> the environment or the config variables are set.
>>
>> I'm not sure about the precedence in patch 3, right now the environment
>> variable has precedence, but it should be easy to give the config
>> option precedence over that.
>>
>> Thomas Gummerer (3):
>> introduce GIT_INDEX_VERSION environment variable
>> test-lib: allow setting the index format version
>> read-cache: add index.version config variable
>>
>> Documentation/config.txt              |  4 +++
>> Documentation/git.txt                 |  5 ++++
>> Makefile                              |  7 +++++
>> read-cache.c                          | 36 +++++++++++++++++++++++-
>> t/t1600-index.sh                      | 52 +++++++++++++++++++++++++++++++++++
>> t/t2104-update-index-skip-worktree.sh |  2 ++
>> t/test-lib-functions.sh               |  5 ++++
>> t/test-lib.sh                         |  3 ++
>> 8 files changed, 113 insertions(+), 1 deletion(-)
>> create mode 100755 t/t1600-index.sh
>>
>> --
>> 1.8.3.2
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe git" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
>
>
> --
> Duy

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

[Junio C Hamano]

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

> 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.

> 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.

> 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.

Re: [PATCH 2/3] test-lib: allow setting the index format version

[Junio C Hamano]

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

> Allow adding a TEST_GIT_INDEX_VERSION variable to config.mak to set the
> index version with which the test suite should be run.
> ...
> diff --git a/Makefile b/Makefile
> index 287e6f8..c98d28f 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -342,6 +342,10 @@ all::
>  # Define DEFAULT_HELP_FORMAT to "man", "info" or "html"
>  # (defaults to "man") if you want to have a different default when
>  # "git help" is called without a parameter specifying the format.
> +#
> +# Define TEST_GIT_INDEX_FORMAT to 2, 3 or 4 to run the test suite

s/_FORMAT/_VERSION/, I would think.

Will queue on 'pu' with a fix-up on top.

Thanks.

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

[Thomas Gummerer]

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>> 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.

Yes, you're right, to be correct we should say [23] instead of 3 here
maybe?

>> 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.

Thanks, makes sense, will fix it in the re-roll.

>> 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?

I think we shouldn't print anything, since we won't change the file
format.  I'll add another test for that.

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

Thanks, I think both fixups in 'pu' make sense, so I'll include them in
the re-roll.

> Thanks.