[PATCH v2 0/3] Easier access to index-v4

[PATCH v2 0/3] Easier access to index-v4

[Thomas Gummerer]

Hi,

previous round was at $gmane/242198.

Thanks to Junio, Eric and Duy for comments on the previous round.

Since then I've squashed the fixes suggested by Junio, added a test
showing what should happen if an index file is present and
GIT_INDEX_VERSION is set and fixed the typo found by Eric.

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                          | 38 +++++++++++++++++-
 t/t1600-index.sh                      | 76 +++++++++++++++++++++++++++++++++++
 t/t2104-update-index-skip-worktree.sh |  2 +
 t/test-lib-functions.sh               |  5 +++
 t/test-lib.sh                         |  3 ++
 8 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100755 t/t1600-index.sh

-- 
1.9.0.1.ge0caaa8.dirty

[PATCH v2 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.

Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Thomas Gummerer <t.gummerer@gmail.com>
---
 Documentation/git.txt |  5 +++++
 read-cache.c          | 21 ++++++++++++++++++++-
 t/t1600-index.sh      | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100755 t/t1600-index.sh

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 02bbc08..27a199c 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -720,6 +720,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 [23] 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 33dd676..efc4aae 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1219,6 +1219,25 @@ 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(void)
+{
+	char *envversion = getenv("GIT_INDEX_VERSION");
+	if (!envversion) {
+		return INDEX_FORMAT_DEFAULT;
+	} else {
+		char *endp;
+		unsigned int 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;
+		}
+		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
@@ -1795,7 +1814,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..6195c55
--- /dev/null
+++ b/t/t1600-index.sh
@@ -0,0 +1,49 @@
+#!/bin/sh
+
+test_description='index file specific tests'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+	echo 1 >a
+'
+
+test_expect_success 'bogus GIT_INDEX_VERSION issues warning' '
+	(
+		rm -f .git/index &&
+		GIT_INDEX_VERSION=2bogus &&
+		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_expect_success 'out of bounds GIT_INDEX_VERSION issues warning' '
+	(
+		rm -f .git/index &&
+		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_expect_success 'no warning with bogus GIT_INDEX_VERSION and existing index' '
+	(
+		GIT_INDEX_VERSION=1 &&
+		export GIT_INDEX_VERSION &&
+		git add a 2>actual.err &&
+		>expect.err &&
+		test_i18ncmp expect.err actual.err
+	)
+'
+
+test_done
-- 
1.9.0.1.ge0caaa8.dirty

[PATCH v2 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.

Helped-by: Junio C Hamano <gitster@pobox.com>
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 dddaf4f..5caa3b2 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_VERSION to 2, 3 or 4 to run the test suite
+# with a different indexfile format version.  If it isn't set the index
+# file format used is index-v[23].
 
 GIT-VERSION-FILE: FORCE
 	@$(SHELL_PATH) ./GIT-VERSION-GEN
@@ -2222,6 +2226,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 1531c24..492f81f 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.9.0.1.ge0caaa8.dirty

[PATCH v2 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             | 35 ++++++++++++++++++++++++++---------
 t/t1600-index.sh         | 27 +++++++++++++++++++++++++++
 3 files changed, 57 insertions(+), 9 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 5f4d793..98200aa 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1601,6 +1601,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 efc4aae..6bc9724 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1219,23 +1219,40 @@ 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(void)
 {
 	char *envversion = getenv("GIT_INDEX_VERSION");
-	if (!envversion) {
-		return INDEX_FORMAT_DEFAULT;
-	} else {
-		char *endp;
-		unsigned int version = strtoul(envversion, &endp, 10);
+	char *endp;
+	unsigned int version = INDEX_FORMAT_DEFAULT;
 
-		if (*endp ||
-		    version < INDEX_FORMAT_LB || INDEX_FORMAT_UB < version) {
-			warning(_("GIT_INDEX_VERSION set, but the value is invalid.\n"
+	if (!envversion) {
+		git_config(index_format_config, &version);
+		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 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;
+	}
+	return version;
 }
 
 /*
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 6195c55..079d241 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -46,4 +46,31 @@ test_expect_success 'no warning with bogus GIT_INDEX_VERSION and existing index'
 	)
 '
 
+test_expect_success 'out of bounds index.version issues warning' '
+	(
+		sane_unset GIT_INDEX_VERSION &&
+		rm -f .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 -f .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.9.0.1.ge0caaa8.dirty

Re: [PATCH v2 0/3] Easier access to index-v4

[Junio C Hamano]

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

> previous round was at $gmane/242198.
>
> Since then I've squashed the fixes suggested by Junio, added a test
> showing what should happen if an index file is present and
> GIT_INDEX_VERSION is set and fixed the typo found by Eric.

Looks good; thanks.

Re: [PATCH v2 0/3] Easier access to index-v4

[Junio C Hamano]

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

> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>> previous round was at $gmane/242198.
>>
>> Since then I've squashed the fixes suggested by Junio, added a test
>> showing what should happen if an index file is present and
>> GIT_INDEX_VERSION is set and fixed the typo found by Eric.
>
> Looks good; thanks.

Tests, seem to leak these unnecessary diag (not limited to t0010).

    sh t0010-racy-git.sh 
    warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    ok 1 - Racy GIT trial #0 part A
    ok 2 - Racy GIT trial #0 part B
    warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    ...
    # passed all 10 test(s)
    1..10


The same thing under prove.

    *** prove ***
    t0010-racy-git.sh .. warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    t0010-racy-git.sh .. 2/? warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    t0010-racy-git.sh .. 4/? warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    t0010-racy-git.sh .. 6/? warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    t0010-racy-git.sh .. 8/? warning: GIT_INDEX_VERSION set, but the value is invalid.
    Using version 3
    t0010-racy-git.sh .. ok    
    All tests successful.


I suspect the real culprit is the early part in test-lib.sh that
sets GIT_INDEX_VERSION explicitly from TEST_GIT_INDEX_VERSION when
the latter is not even specified.

Something along this line, perhaps?

 t/test-lib.sh | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/t/test-lib.sh b/t/test-lib.sh
index 492f81f..01a98cb 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -108,8 +108,11 @@ 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
+if test -n "${TEST_GIT_INDEX_VERSION+isset}"
+then
+	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
+	export GIT_INDEX_VERSION
+fi
 
 # Add libc MALLOC and MALLOC_PERTURB test
 # only if we are not executing the test with valgrind

Re: [PATCH v2 0/3] Easier access to index-v4

[Thomas Gummerer]

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

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Thomas Gummerer <t.gummerer@gmail.com> writes:
>>
>>> previous round was at $gmane/242198.
>>>
>>> Since then I've squashed the fixes suggested by Junio, added a test
>>> showing what should happen if an index file is present and
>>> GIT_INDEX_VERSION is set and fixed the typo found by Eric.
>>
>> Looks good; thanks.
>
> Tests, seem to leak these unnecessary diag (not limited to t0010).
>
>     sh t0010-racy-git.sh 
>     warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     ok 1 - Racy GIT trial #0 part A
>     ok 2 - Racy GIT trial #0 part B
>     warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     ...
>     # passed all 10 test(s)
>     1..10
>
>
> The same thing under prove.
>
>     *** prove ***
>     t0010-racy-git.sh .. warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     t0010-racy-git.sh .. 2/? warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     t0010-racy-git.sh .. 4/? warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     t0010-racy-git.sh .. 6/? warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     t0010-racy-git.sh .. 8/? warning: GIT_INDEX_VERSION set, but the value is invalid.
>     Using version 3
>     t0010-racy-git.sh .. ok    
>     All tests successful.
>
>
> I suspect the real culprit is the early part in test-lib.sh that
> sets GIT_INDEX_VERSION explicitly from TEST_GIT_INDEX_VERSION when
> the latter is not even specified.
>
> Something along this line, perhaps?

Sorry about this, I didn't run the test suite without
TEST_GIT_INDEX_VERSION in config.mak which I obviously should have.

Yes, this looks good to me, thanks!

>  t/test-lib.sh | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 492f81f..01a98cb 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -108,8 +108,11 @@ 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
> +if test -n "${TEST_GIT_INDEX_VERSION+isset}"
> +then
> +	GIT_INDEX_VERSION="$TEST_GIT_INDEX_VERSION"
> +	export GIT_INDEX_VERSION
> +fi
>  
>  # Add libc MALLOC and MALLOC_PERTURB test
>  # only if we are not executing the test with valgrind

Re: [PATCH v2 0/3] Easier access to index-v4

[Junio C Hamano]

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

>> Something along this line, perhaps?
>
> Sorry about this, I didn't run the test suite without
> TEST_GIT_INDEX_VERSION in config.mak which I obviously should have.
>
> Yes, this looks good to me, thanks!

OK, will squash it (but using "VAR:+isset" instead of "VAR+isset" to
allow people to set it to empty to disable) into the relevant patch.

Thanks.