[PATCH v2 0/3] "update-index --show-index-version"

[PATCH v2 0/3] "update-index --show-index-version"

[Junio C Hamano]

Even though there is "test-tool index-version", there wasn't an
officially supported way for end-users to inspect the index version
of an on-disk index.  Let's have one.

Junio C Hamano (3):
  update-index doc: v4 is OK with JGit and libgit2
  update-index: add --show-index-version
  test-tool: retire "index-version"

 Documentation/git-update-index.txt    | 14 ++++++++++----
 Makefile                              |  1 -
 builtin/update-index.c                | 19 +++++++++++++------
 t/helper/test-index-version.c         | 15 ---------------
 t/helper/test-tool.c                  |  1 -
 t/helper/test-tool.h                  |  1 -
 t/t1600-index.sh                      |  2 +-
 t/t1700-split-index.sh                |  2 +-
 t/t2104-update-index-skip-worktree.sh |  6 +++---
 t/t2107-update-index-basic.sh         | 16 ++++++++++++++++
 10 files changed, 44 insertions(+), 33 deletions(-)
 delete mode 100644 t/helper/test-index-version.c

-- 
2.42.0-rc2-7-gf9972720e9

[PATCH v2 1/3] update-index doc: v4 is OK with JGit and libgit2

[Junio C Hamano]

Being invented in late 2012 no longer makes the index v4 format
"relatively young".

The support for the index version 4 was added to libgit2 with their
5625d86b (index: support index v4, 2016-05-17) and to JGit with
their e9cb0a8e (DirCache: support index V4, 2020-08-10).

Let's update the paragraph that discouraged its use for folks overly
cautious about cross-tool compatibility.

Helped-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-update-index.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index f4bb9c5daf..a367f8d65a 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -166,9 +166,11 @@ you will need to handle the situation manually.
 +
 Version 4 performs a simple pathname compression that reduces index
 size by 30%-50% on large repositories, which results in faster load
-time. Version 4 is relatively young (first released in 1.8.0 in
-October 2012). Other Git implementations such as JGit and libgit2
-may not support it yet.
+time.  Git supports it since version 1.8.0, released in October 2012,
+and support for it was added to libgit2 in 2016 and to JGit in 2020.
+Older editions of this manual page called it "relatively young", but
+it should be considered mature technology these days.
+
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are
-- 
2.42.0-rc2-7-gf9972720e9

[PATCH v2 2/3] update-index: add --show-index-version

[Junio C Hamano]

"git update-index --version N" is used to set the index format
version to a specific version, but there was no way to query the
current version used in the on-disk index file.

Teach the command a new "--show-index-version" option, and also
teach the "--index-version N" option to report what the version was
when run with the "--verbose" option.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-update-index.txt |  6 +++++-
 builtin/update-index.c             | 19 +++++++++++++------
 t/t2107-update-index-basic.sh      | 16 ++++++++++++++++
 3 files changed, 34 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index a367f8d65a..3f204891ce 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -162,7 +162,8 @@ you will need to handle the situation manually.
 	Write the resulting index out in the named on-disk format version.
 	Supported versions are 2, 3 and 4. The current default version is 2
 	or 3, depending on whether extra features are used, such as
-	`git add -N`.
+	`git add -N`.  With `--verbose` option, also reports the
+	version the index file uses before and after this command.
 +
 Version 4 performs a simple pathname compression that reduces index
 size by 30%-50% on large repositories, which results in faster load
@@ -171,6 +172,9 @@ and support for it was added to libgit2 in 2016 and to JGit in 2020.
 Older editions of this manual page called it "relatively young", but
 it should be considered mature technology these days.
 
+--show-index-version::
+	Report the index format version used by the on-disk index file.
+	See `--index-version` above.
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 5fab9ad2ec..e713e2a04c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1089,6 +1089,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			resolve_undo_clear_callback),
 		OPT_INTEGER(0, "index-version", &preferred_index_format,
 			N_("write index in this format")),
+		OPT_SET_INT(0, "show-index-version", &preferred_index_format,
+			    N_("show index format version"), -1),
 		OPT_BOOL(0, "split-index", &split_index,
 			N_("enable or disable split index")),
 		OPT_BOOL(0, "untracked-cache", &untracked_cache,
@@ -1181,15 +1183,20 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 
 	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
 	if (preferred_index_format) {
-		if (preferred_index_format < INDEX_FORMAT_LB ||
-		    INDEX_FORMAT_UB < preferred_index_format)
+		if (preferred_index_format < 0) {
+			printf(_("%d\n"), the_index.version);
+		} else if (preferred_index_format < INDEX_FORMAT_LB ||
+			   INDEX_FORMAT_UB < preferred_index_format) {
 			die("index-version %d not in range: %d..%d",
 			    preferred_index_format,
 			    INDEX_FORMAT_LB, INDEX_FORMAT_UB);
-
-		if (the_index.version != preferred_index_format)
-			the_index.cache_changed |= SOMETHING_CHANGED;
-		the_index.version = preferred_index_format;
+		} else {
+			if (the_index.version != preferred_index_format)
+				the_index.cache_changed |= SOMETHING_CHANGED;
+			report(_("index-version: was %d, set to %d"),
+			       the_index.version, preferred_index_format);
+			the_index.version = preferred_index_format;
+		}
 	}
 
 	if (read_from_stdin) {
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 89b285fa3a..c78d1e9396 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -111,4 +111,20 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
 	test_cmp expect actual
 '
 
+test_expect_success '--index-version' '
+	git commit --allow-empty -m snap &&
+	git reset --hard &&
+
+	git update-index --index-version 2 >actual &&
+	test_must_be_empty actual &&
+
+	git update-index --show-index-version >actual &&
+	echo 2 >expect &&
+	test_cmp expect actual &&
+
+	git update-index --index-version 4 --verbose >actual &&
+	echo "index-version: was 2, set to 4" >expect &&
+	test_cmp expect actual
+'
+
 test_done
-- 
2.42.0-rc2-7-gf9972720e9

[PATCH v2 3/3] test-tool: retire "index-version"

[Junio C Hamano]

As "git update-index --show-index-version" can do the same thing,
the test-tool lost its reason to exist.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile                              |  1 -
 t/helper/test-index-version.c         | 15 ---------------
 t/helper/test-tool.c                  |  1 -
 t/helper/test-tool.h                  |  1 -
 t/t1600-index.sh                      |  2 +-
 t/t1700-split-index.sh                |  2 +-
 t/t2104-update-index-skip-worktree.sh |  6 +++---
 7 files changed, 5 insertions(+), 23 deletions(-)
 delete mode 100644 t/helper/test-index-version.c

diff --git a/Makefile b/Makefile
index e440728c24..5323c76aa0 100644
--- a/Makefile
+++ b/Makefile
@@ -808,7 +808,6 @@ TEST_BUILTINS_OBJS += test-hash-speed.o
 TEST_BUILTINS_OBJS += test-hash.o
 TEST_BUILTINS_OBJS += test-hashmap.o
 TEST_BUILTINS_OBJS += test-hexdump.o
-TEST_BUILTINS_OBJS += test-index-version.o
 TEST_BUILTINS_OBJS += test-json-writer.o
 TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
 TEST_BUILTINS_OBJS += test-match-trees.o
diff --git a/t/helper/test-index-version.c b/t/helper/test-index-version.c
deleted file mode 100644
index a06c45c1f8..0000000000
--- a/t/helper/test-index-version.c
+++ /dev/null
@@ -1,15 +0,0 @@
-#include "test-tool.h"
-#include "cache.h"
-
-int cmd__index_version(int argc UNUSED, const char **argv UNUSED)
-{
-	struct cache_header hdr;
-	int version;
-
-	memset(&hdr,0,sizeof(hdr));
-	if (read(0, &hdr, sizeof(hdr)) != sizeof(hdr))
-		return 0;
-	version = ntohl(hdr.hdr_version);
-	printf("%d\n", version);
-	return 0;
-}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index abe8a785eb..621ac3dd10 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -38,7 +38,6 @@ static struct test_cmd cmds[] = {
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },
 	{ "hexdump", cmd__hexdump },
-	{ "index-version", cmd__index_version },
 	{ "json-writer", cmd__json_writer },
 	{ "lazy-init-name-hash", cmd__lazy_init_name_hash },
 	{ "match-trees", cmd__match_trees },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index ea2672436c..a641c3a81d 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -32,7 +32,6 @@ int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);
 int cmd__hexdump(int argc, const char **argv);
-int cmd__index_version(int argc, const char **argv);
 int cmd__json_writer(int argc, const char **argv);
 int cmd__lazy_init_name_hash(int argc, const char **argv);
 int cmd__match_trees(int argc, const char **argv);
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 9368d82f7d..62e7fd1596 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -118,7 +118,7 @@ test_index_version () {
 		fi &&
 		git add a &&
 		echo $EXPECTED_OUTPUT_VERSION >expect &&
-		test-tool index-version <.git/index >actual &&
+		git update-index --show-index-version >actual &&
 		test_cmp expect actual
 	)
 }
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b4ab166369..a7b7263b35 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -43,7 +43,7 @@ test_expect_success 'enable split index' '
 	git config splitIndex.maxPercentChange 100 &&
 	git update-index --split-index &&
 	test-tool dump-split-index .git/index >actual &&
-	indexversion=$(test-tool index-version <.git/index) &&
+	indexversion=$(git update-index --show-index-version) &&
 
 	# NEEDSWORK: Stop hard-coding checksums.
 	if test "$indexversion" = "4"
diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index b8686aabd3..0bab134d71 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -39,7 +39,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'index is at version 2' '
-	test "$(test-tool index-version < .git/index)" = 2
+	test "$(git update-index --show-index-version)" = 2
 '
 
 test_expect_success 'update-index --skip-worktree' '
@@ -48,7 +48,7 @@ test_expect_success 'update-index --skip-worktree' '
 '
 
 test_expect_success 'index is at version 3 after having some skip-worktree entries' '
-	test "$(test-tool index-version < .git/index)" = 3
+	test "$(git update-index --show-index-version)" = 3
 '
 
 test_expect_success 'ls-files -t' '
@@ -61,7 +61,7 @@ test_expect_success 'update-index --no-skip-worktree' '
 '
 
 test_expect_success 'index version is back to 2 when there is no skip-worktree entry' '
-	test "$(test-tool index-version < .git/index)" = 2
+	test "$(git update-index --show-index-version)" = 2
 '
 
 test_done
-- 
2.42.0-rc2-7-gf9972720e9

Re: [PATCH v2 1/3] update-index doc: v4 is OK with JGit and libgit2

[Linus Arver]

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

> [...]
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index f4bb9c5daf..a367f8d65a 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -166,9 +166,11 @@ you will need to handle the situation manually.
>  +
>  Version 4 performs a simple pathname compression that reduces index
>  size by 30%-50% on large repositories, which results in faster load
> -time. Version 4 is relatively young (first released in 1.8.0 in
> -October 2012). Other Git implementations such as JGit and libgit2
> -may not support it yet.
> +time.  Git supports it since version 1.8.0, released in October 2012,
> +and support for it was added to libgit2 in 2016 and to JGit in 2020.
> +Older editions of this manual page called it "relatively young", but

Nit: s/editions/versions because typically "edition" means a version of
physically published text (such as books). Also I don't think we use the
word "edition" anywhere else in our codebase.

Re: [PATCH v2 2/3] update-index: add --show-index-version

[Linus Arver]

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

> "git update-index --version N" is used to set the index format

s/--version/--index-version

> version to a specific version, but there was no way to query the
> current version used in the on-disk index file.
>
> Teach the command a new "--show-index-version" option, and also
> teach the "--index-version N" option to report what the version was
> when run with the "--verbose" option.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-update-index.txt |  6 +++++-
>  builtin/update-index.c             | 19 +++++++++++++------
>  t/t2107-update-index-basic.sh      | 16 ++++++++++++++++
>  3 files changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
> index a367f8d65a..3f204891ce 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -162,7 +162,8 @@ you will need to handle the situation manually.
>  	Write the resulting index out in the named on-disk format version.
>  	Supported versions are 2, 3 and 4. The current default version is 2
>  	or 3, depending on whether extra features are used, such as
> -	`git add -N`.
> +	`git add -N`.  With `--verbose` option, also reports the

How about

    `git add -N`.  With `--verbose`, also report the

> +	version the index file uses before and after this command.
>  +
>  Version 4 performs a simple pathname compression that reduces index
>  size by 30%-50% on large repositories, which results in faster load
> @@ -171,6 +172,9 @@ and support for it was added to libgit2 in 2016 and to JGit in 2020.
>  Older editions of this manual page called it "relatively young", but
>  it should be considered mature technology these days.
>
> +--show-index-version::
> +	Report the index format version used by the on-disk index file.
> +	See `--index-version` above.

Looks good.

>  -z::
>  	Only meaningful with `--stdin` or `--index-info`; paths are
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 5fab9ad2ec..e713e2a04c 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -1089,6 +1089,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  			resolve_undo_clear_callback),
>  		OPT_INTEGER(0, "index-version", &preferred_index_format,
>  			N_("write index in this format")),
> +		OPT_SET_INT(0, "show-index-version", &preferred_index_format,
> +			    N_("show index format version"), -1),

How about

    "show on-disk index format version"

for the help text? Also "report" may be preferable instead of "show" for
consistency with the manpage.

Also, I think it would be easier to read if we didn't overload
preferred_index_format here and instead used a new variable (perhaps named
"show_index_format").

And I just realized that we are now using not only "show" and "report"
interchangeably in this patch, but also "format" and "version" ---
it would be nice to clean up as #leftoverbits.

>  		OPT_BOOL(0, "split-index", &split_index,
>  			N_("enable or disable split index")),
>  		OPT_BOOL(0, "untracked-cache", &untracked_cache,
> @@ -1181,15 +1183,20 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>
>  	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
>  	if (preferred_index_format) {
> -		if (preferred_index_format < INDEX_FORMAT_LB ||
> -		    INDEX_FORMAT_UB < preferred_index_format)
> +		if (preferred_index_format < 0) {
> +			printf(_("%d\n"), the_index.version);
> +		} else if (preferred_index_format < INDEX_FORMAT_LB ||
> +			   INDEX_FORMAT_UB < preferred_index_format) {
>  			die("index-version %d not in range: %d..%d",
>  			    preferred_index_format,
>  			    INDEX_FORMAT_LB, INDEX_FORMAT_UB);
> -
> -		if (the_index.version != preferred_index_format)
> -			the_index.cache_changed |= SOMETHING_CHANGED;
> -		the_index.version = preferred_index_format;
> +		} else {
> +			if (the_index.version != preferred_index_format)
> +				the_index.cache_changed |= SOMETHING_CHANGED;
> +			report(_("index-version: was %d, set to %d"),
> +			       the_index.version, preferred_index_format);
> +			the_index.version = preferred_index_format;
> +		}
>  	}

Looks good. But if you decide to use a new variable like
"show_index_format" as suggested above, you might want to refactor this
out to a separate function. Then you could obviously do away with the
chained if/else and replace them with early returns, which would be
easier to read.

>  	if (read_from_stdin) {
> diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
> index 89b285fa3a..c78d1e9396 100755
> --- a/t/t2107-update-index-basic.sh
> +++ b/t/t2107-update-index-basic.sh
> @@ -111,4 +111,20 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
>  	test_cmp expect actual
>  '
>
> +test_expect_success '--index-version' '
> +	git commit --allow-empty -m snap &&
> +	git reset --hard &&

Not sure why this "git reset --hard" is needed here --- is it to clear
out state from previous test cases? If so, perhaps it's better to run
this as the very first command in this test case?

But this also makes me wonder why "git commit --allow-empty -m snap" is
even necessary (if we already have a git repo)?

> +	git update-index --index-version 2 >actual &&
> +	test_must_be_empty actual &&

Before we do "--index-version 2", it may be desirable to run the
"--show-index-version" flag first to see what we have before modifying
it with "2".

> +	git update-index --show-index-version >actual &&
> +	echo 2 >expect &&
> +	test_cmp expect actual &&
> +
> +	git update-index --index-version 4 --verbose >actual &&
> +	echo "index-version: was 2, set to 4" >expect &&
> +	test_cmp expect actual
> +'

How about adding this check below to check what happens if the newly-set
version is the same as the existing version?

        git update-index --index-version 4 --verbose >actual &&
        echo "index-version: was 4, set to 4" >expect &&
        test_cmp expect actual

And also how about a similar check, but for downgrading from a higher
version number to a lower one?

        git update-index --index-version 2 --verbose >actual &&
        echo "index-version: was 4, set to 2" >expect &&
        test_cmp expect actual

> +
>  test_done
> --
> 2.42.0-rc2-7-gf9972720e9

Re: [PATCH v2 3/3] test-tool: retire "index-version"

[Linus Arver]

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

> As "git update-index --show-index-version" can do the same thing,
> the test-tool lost its reason to exist.

I think you meant

    the equivalent functionality in test-tool ...

and not that the test-tool (in its entirety) lost its reason to exist.

The rest of the patch LGTM.

Re: [PATCH v2 1/3] update-index doc: v4 is OK with JGit and libgit2

[Junio C Hamano]

Linus Arver <linusa@google.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
> ...
>> +Older editions of this manual page called it "relatively young", but
>
> Nit: s/editions/versions because typically "edition" means a version of
> physically published text (such as books).

This text belonging to the manual, that was exactly the reason why I
used that word, but ...

> Also I don't think we use the
> word "edition" anywhere else in our codebase.

... I think "version" is just fine.  Will fix.

Thanks.

Re: [PATCH v2 2/3] update-index: add --show-index-version

[Junio C Hamano]

Linus Arver <linusa@google.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> "git update-index --version N" is used to set the index format
>
> s/--version/--index-version

Good eyes.

>> +	`git add -N`.  With `--verbose` option, also reports the
>
> How about
>
>     `git add -N`.  With `--verbose`, also report the

Yup, that is much better.

>>  		OPT_INTEGER(0, "index-version", &preferred_index_format,
>>  			N_("write index in this format")),
>> +		OPT_SET_INT(0, "show-index-version", &preferred_index_format,
>> +			    N_("show index format version"), -1),
>
> How about
>
>     "show on-disk index format version"

OK.  The entry before that says "write" and it makes it clear enough
that it is talking about the "on-disk" thing, hence "write index" is
sufficient.  The readers, who do not know the "format version"
matters only on-disk and does not matter once the index data gets
slurped into memory, would benefit with "on-disk index" here.  I
will also take your "show" -> "report" suggestion.

>> diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
>> index 89b285fa3a..c78d1e9396 100755
>> --- a/t/t2107-update-index-basic.sh
>> +++ b/t/t2107-update-index-basic.sh
>> @@ -111,4 +111,20 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
>>  	test_cmp expect actual
>>  '
>>
>> +test_expect_success '--index-version' '
>> +	git commit --allow-empty -m snap &&
>> +	git reset --hard &&
>
> Not sure why this "git reset --hard" is needed here --- is it to clear
> out state from previous test cases? If so, perhaps it's better to run
> this as the very first command in this test case?
>
> But this also makes me wonder why "git commit --allow-empty -m snap" is
> even necessary (if we already have a git repo)?

There is no commit yet in this repository at this point, which means
that "reset --hard [HEAD]" cannot be the first command.  There are
newly added paths in the index, some of which are further modified
in the working tree.  Creating a snapshot commit will record what is
in the index, and a hard reset will make the contents of the working
tree match exactly what was in the index, reverting the modification
made to some paths in the working tree.

Having said that, we may further want to tighten these "clean the
slate" steps for later tests (read on).

>> +	git update-index --index-version 2 >actual &&
>> +	test_must_be_empty actual &&
>
> Before we do "--index-version 2", it may be desirable to run the
> "--show-index-version" flag first to see what we have before modifying
> it with "2".

That was what I specifically wanted to avoid.  I am not interested
in testing what the default version of the day is.  The only thing
this invocation cares about is to (try to) use the version 2; it
will be detected as a failure if the next show-index-version
reported something else ...

>> +	git update-index --show-index-version >actual &&
>> +	echo 2 >expect &&
>> +	test_cmp expect actual &&

... which is checked here.

>> +	git update-index --index-version 4 --verbose >actual &&
>> +	echo "index-version: was 2, set to 4" >expect &&
>> +	test_cmp expect actual

And how a transition is reported with --verbose is tested here.

>> +'
>
> How about adding this check below to check what happens if the newly-set
> version is the same as the existing version?
>
>         git update-index --index-version 4 --verbose >actual &&
>         echo "index-version: was 4, set to 4" >expect &&
>         test_cmp expect actual
>
> And also how about a similar check, but for downgrading from a higher
> version number to a lower one?
>
>         git update-index --index-version 2 --verbose >actual &&
>         echo "index-version: was 4, set to 2" >expect &&
>         test_cmp expect actual

I assumed that the flipping between versions without --verbose had
adequate coverage and the tests this patch adds were solely about
reporting with the "--verbose" option.  It seems there is no such
test coverage, so these may want to be added.  Perhaps we should
flip from 2 to 4 _without_ verbose and make sure we are still silent
as well.

Testing downgrading an index with unspecified [*] contents from 3
(or higher) to 2 would be unwise, as such a conversion may or may
not work depending on the contents.  I will tighten the "clean the
slate" steps and add downgrades to the list, as the initial "try to
set to 2" may not even work with unknown contents in the index.

Thanks.


[Footnote] 

* This is in a very late part of the test script and the state
  previous tests left us will change over time.

[PATCH v3 0/3] "update-index --show-index-version"

[Junio C Hamano]

Even though there is "test-tool index-version", there wasn't an
officially supported way for end-users to inspect the index version
of an on-disk index.  Let's have one.

This iteration takes suggestions by Linus Arver; the tests added by
[2/3] have been clarified.

Junio C Hamano (3):
  update-index doc: v4 is OK with JGit and libgit2
  update-index: add --show-index-version
  test-tool: retire "index-version"

 Documentation/git-update-index.txt    | 14 ++++++++----
 Makefile                              |  1 -
 builtin/update-index.c                | 19 ++++++++++------
 t/helper/test-index-version.c         | 15 -------------
 t/helper/test-tool.c                  |  1 -
 t/helper/test-tool.h                  |  1 -
 t/t1600-index.sh                      |  2 +-
 t/t1700-split-index.sh                |  2 +-
 t/t2104-update-index-skip-worktree.sh |  6 +++---
 t/t2107-update-index-basic.sh         | 31 +++++++++++++++++++++++++++
 10 files changed, 59 insertions(+), 33 deletions(-)
 delete mode 100644 t/helper/test-index-version.c

Range-diff against v2:
1:  7bc7f7877d ! 1:  224a72529a update-index doc: v4 is OK with JGit and libgit2
    @@ Commit message
         cautious about cross-tool compatibility.
     
         Helped-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
    +    Helped-by: Linus Arver <linusa@google.com>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Documentation/git-update-index.txt ##
    @@ Documentation/git-update-index.txt: you will need to handle the situation manual
     -may not support it yet.
     +time.  Git supports it since version 1.8.0, released in October 2012,
     +and support for it was added to libgit2 in 2016 and to JGit in 2020.
    -+Older editions of this manual page called it "relatively young", but
    ++Older versions of this manual page called it "relatively young", but
     +it should be considered mature technology these days.
     +
      
2:  33c924f893 ! 2:  bd3fc76b3f update-index: add --show-index-version
    @@ Metadata
      ## Commit message ##
         update-index: add --show-index-version
     
    -    "git update-index --version N" is used to set the index format
    +    "git update-index --index-version N" is used to set the index format
         version to a specific version, but there was no way to query the
         current version used in the on-disk index file.
     
    @@ Commit message
         teach the "--index-version N" option to report what the version was
         when run with the "--verbose" option.
     
    +    Helped-by: Linus Arver <linusa@google.com>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Documentation/git-update-index.txt ##
    @@ Documentation/git-update-index.txt: you will need to handle the situation manual
      	Supported versions are 2, 3 and 4. The current default version is 2
      	or 3, depending on whether extra features are used, such as
     -	`git add -N`.
    -+	`git add -N`.  With `--verbose` option, also reports the
    -+	version the index file uses before and after this command.
    ++	`git add -N`.  With `--verbose`, also report the version the index
    ++	file uses before and after this command.
      +
      Version 4 performs a simple pathname compression that reduces index
      size by 30%-50% on large repositories, which results in faster load
     @@ Documentation/git-update-index.txt: and support for it was added to libgit2 in 2016 and to JGit in 2020.
    - Older editions of this manual page called it "relatively young", but
    + Older versions of this manual page called it "relatively young", but
      it should be considered mature technology these days.
      
     +--show-index-version::
    @@ builtin/update-index.c: int cmd_update_index(int argc, const char **argv, const
      		OPT_INTEGER(0, "index-version", &preferred_index_format,
      			N_("write index in this format")),
     +		OPT_SET_INT(0, "show-index-version", &preferred_index_format,
    -+			    N_("show index format version"), -1),
    ++			    N_("report on-disk index format version"), -1),
      		OPT_BOOL(0, "split-index", &split_index,
      			N_("enable or disable split index")),
      		OPT_BOOL(0, "untracked-cache", &untracked_cache,
    @@ t/t2107-update-index-basic.sh: test_expect_success '--chmod=+x and chmod=-x in t
     +test_expect_success '--index-version' '
     +	git commit --allow-empty -m snap &&
     +	git reset --hard &&
    ++	git rm -f -r --cached . &&
     +
    -+	git update-index --index-version 2 >actual &&
    -+	test_must_be_empty actual &&
    -+
    ++	# The default index version is 2 --- update this test
    ++	# when you change it in the code
     +	git update-index --show-index-version >actual &&
     +	echo 2 >expect &&
     +	test_cmp expect actual &&
     +
    ++	# The next test wants us to be using version 2
    ++	git update-index --index-version 2 &&
    ++
     +	git update-index --index-version 4 --verbose >actual &&
     +	echo "index-version: was 2, set to 4" >expect &&
    -+	test_cmp expect actual
    ++	test_cmp expect actual &&
    ++
    ++	git update-index --index-version 4 --verbose >actual &&
    ++	echo "index-version: was 4, set to 4" >expect &&
    ++	test_cmp expect actual &&
    ++
    ++	git update-index --index-version 2 --verbose >actual &&
    ++	echo "index-version: was 4, set to 2" >expect &&
    ++	test_cmp expect actual &&
    ++
    ++	# non-verbose should be silent
    ++	git update-index --index-version 4 >actual &&
    ++	test_must_be_empty actual
     +'
     +
      test_done
3:  137ec82973 ! 3:  d1144a8240 test-tool: retire "index-version"
    @@ Commit message
         test-tool: retire "index-version"
     
         As "git update-index --show-index-version" can do the same thing,
    -    the test-tool lost its reason to exist.
    +    the 'index-version' subcommand in the test-tool lost its reason to
    +    exist.  Remove it and replace its use with the end-user facing
    +    'git update-index --show-index-version'.
     
    +    Helped-by: Linus Arver <linusa@google.com>
         Signed-off-by: Junio C Hamano <gitster@pobox.com>
     
      ## Makefile ##
-- 
2.42.0-158-g94e83dcf5b

[PATCH v3 1/3] update-index doc: v4 is OK with JGit and libgit2

[Junio C Hamano]

Being invented in late 2012 no longer makes the index v4 format
"relatively young".

The support for the index version 4 was added to libgit2 with their
5625d86b (index: support index v4, 2016-05-17) and to JGit with
their e9cb0a8e (DirCache: support index V4, 2020-08-10).

Let's update the paragraph that discouraged its use for folks overly
cautious about cross-tool compatibility.

Helped-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-update-index.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index f4bb9c5daf..71d6ef9457 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -166,9 +166,11 @@ you will need to handle the situation manually.
 +
 Version 4 performs a simple pathname compression that reduces index
 size by 30%-50% on large repositories, which results in faster load
-time. Version 4 is relatively young (first released in 1.8.0 in
-October 2012). Other Git implementations such as JGit and libgit2
-may not support it yet.
+time.  Git supports it since version 1.8.0, released in October 2012,
+and support for it was added to libgit2 in 2016 and to JGit in 2020.
+Older versions of this manual page called it "relatively young", but
+it should be considered mature technology these days.
+
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are
-- 
2.42.0-158-g94e83dcf5b

[PATCH v3 2/3] update-index: add --show-index-version

[Junio C Hamano]

"git update-index --index-version N" is used to set the index format
version to a specific version, but there was no way to query the
current version used in the on-disk index file.

Teach the command a new "--show-index-version" option, and also
teach the "--index-version N" option to report what the version was
when run with the "--verbose" option.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-update-index.txt |  6 +++++-
 builtin/update-index.c             | 19 ++++++++++++------
 t/t2107-update-index-basic.sh      | 31 ++++++++++++++++++++++++++++++
 3 files changed, 49 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 71d6ef9457..1271486ae9 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -162,7 +162,8 @@ you will need to handle the situation manually.
 	Write the resulting index out in the named on-disk format version.
 	Supported versions are 2, 3 and 4. The current default version is 2
 	or 3, depending on whether extra features are used, such as
-	`git add -N`.
+	`git add -N`.  With `--verbose`, also report the version the index
+	file uses before and after this command.
 +
 Version 4 performs a simple pathname compression that reduces index
 size by 30%-50% on large repositories, which results in faster load
@@ -171,6 +172,9 @@ and support for it was added to libgit2 in 2016 and to JGit in 2020.
 Older versions of this manual page called it "relatively young", but
 it should be considered mature technology these days.
 
+--show-index-version::
+	Report the index format version used by the on-disk index file.
+	See `--index-version` above.
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 5fab9ad2ec..8c3454c1c1 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1089,6 +1089,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			resolve_undo_clear_callback),
 		OPT_INTEGER(0, "index-version", &preferred_index_format,
 			N_("write index in this format")),
+		OPT_SET_INT(0, "show-index-version", &preferred_index_format,
+			    N_("report on-disk index format version"), -1),
 		OPT_BOOL(0, "split-index", &split_index,
 			N_("enable or disable split index")),
 		OPT_BOOL(0, "untracked-cache", &untracked_cache,
@@ -1181,15 +1183,20 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 
 	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
 	if (preferred_index_format) {
-		if (preferred_index_format < INDEX_FORMAT_LB ||
-		    INDEX_FORMAT_UB < preferred_index_format)
+		if (preferred_index_format < 0) {
+			printf(_("%d\n"), the_index.version);
+		} else if (preferred_index_format < INDEX_FORMAT_LB ||
+			   INDEX_FORMAT_UB < preferred_index_format) {
 			die("index-version %d not in range: %d..%d",
 			    preferred_index_format,
 			    INDEX_FORMAT_LB, INDEX_FORMAT_UB);
-
-		if (the_index.version != preferred_index_format)
-			the_index.cache_changed |= SOMETHING_CHANGED;
-		the_index.version = preferred_index_format;
+		} else {
+			if (the_index.version != preferred_index_format)
+				the_index.cache_changed |= SOMETHING_CHANGED;
+			report(_("index-version: was %d, set to %d"),
+			       the_index.version, preferred_index_format);
+			the_index.version = preferred_index_format;
+		}
 	}
 
 	if (read_from_stdin) {
diff --git a/t/t2107-update-index-basic.sh b/t/t2107-update-index-basic.sh
index 89b285fa3a..22f4c92399 100755
--- a/t/t2107-update-index-basic.sh
+++ b/t/t2107-update-index-basic.sh
@@ -111,4 +111,35 @@ test_expect_success '--chmod=+x and chmod=-x in the same argument list' '
 	test_cmp expect actual
 '
 
+test_expect_success '--index-version' '
+	git commit --allow-empty -m snap &&
+	git reset --hard &&
+	git rm -f -r --cached . &&
+
+	# The default index version is 2 --- update this test
+	# when you change it in the code
+	git update-index --show-index-version >actual &&
+	echo 2 >expect &&
+	test_cmp expect actual &&
+
+	# The next test wants us to be using version 2
+	git update-index --index-version 2 &&
+
+	git update-index --index-version 4 --verbose >actual &&
+	echo "index-version: was 2, set to 4" >expect &&
+	test_cmp expect actual &&
+
+	git update-index --index-version 4 --verbose >actual &&
+	echo "index-version: was 4, set to 4" >expect &&
+	test_cmp expect actual &&
+
+	git update-index --index-version 2 --verbose >actual &&
+	echo "index-version: was 4, set to 2" >expect &&
+	test_cmp expect actual &&
+
+	# non-verbose should be silent
+	git update-index --index-version 4 >actual &&
+	test_must_be_empty actual
+'
+
 test_done
-- 
2.42.0-158-g94e83dcf5b

[PATCH v3 3/3] test-tool: retire "index-version"

[Junio C Hamano]

As "git update-index --show-index-version" can do the same thing,
the 'index-version' subcommand in the test-tool lost its reason to
exist.  Remove it and replace its use with the end-user facing
'git update-index --show-index-version'.

Helped-by: Linus Arver <linusa@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Makefile                              |  1 -
 t/helper/test-index-version.c         | 15 ---------------
 t/helper/test-tool.c                  |  1 -
 t/helper/test-tool.h                  |  1 -
 t/t1600-index.sh                      |  2 +-
 t/t1700-split-index.sh                |  2 +-
 t/t2104-update-index-skip-worktree.sh |  6 +++---
 7 files changed, 5 insertions(+), 23 deletions(-)
 delete mode 100644 t/helper/test-index-version.c

diff --git a/Makefile b/Makefile
index e440728c24..5323c76aa0 100644
--- a/Makefile
+++ b/Makefile
@@ -808,7 +808,6 @@ TEST_BUILTINS_OBJS += test-hash-speed.o
 TEST_BUILTINS_OBJS += test-hash.o
 TEST_BUILTINS_OBJS += test-hashmap.o
 TEST_BUILTINS_OBJS += test-hexdump.o
-TEST_BUILTINS_OBJS += test-index-version.o
 TEST_BUILTINS_OBJS += test-json-writer.o
 TEST_BUILTINS_OBJS += test-lazy-init-name-hash.o
 TEST_BUILTINS_OBJS += test-match-trees.o
diff --git a/t/helper/test-index-version.c b/t/helper/test-index-version.c
deleted file mode 100644
index a06c45c1f8..0000000000
--- a/t/helper/test-index-version.c
+++ /dev/null
@@ -1,15 +0,0 @@
-#include "test-tool.h"
-#include "cache.h"
-
-int cmd__index_version(int argc UNUSED, const char **argv UNUSED)
-{
-	struct cache_header hdr;
-	int version;
-
-	memset(&hdr,0,sizeof(hdr));
-	if (read(0, &hdr, sizeof(hdr)) != sizeof(hdr))
-		return 0;
-	version = ntohl(hdr.hdr_version);
-	printf("%d\n", version);
-	return 0;
-}
diff --git a/t/helper/test-tool.c b/t/helper/test-tool.c
index abe8a785eb..621ac3dd10 100644
--- a/t/helper/test-tool.c
+++ b/t/helper/test-tool.c
@@ -38,7 +38,6 @@ static struct test_cmd cmds[] = {
 	{ "hashmap", cmd__hashmap },
 	{ "hash-speed", cmd__hash_speed },
 	{ "hexdump", cmd__hexdump },
-	{ "index-version", cmd__index_version },
 	{ "json-writer", cmd__json_writer },
 	{ "lazy-init-name-hash", cmd__lazy_init_name_hash },
 	{ "match-trees", cmd__match_trees },
diff --git a/t/helper/test-tool.h b/t/helper/test-tool.h
index ea2672436c..a641c3a81d 100644
--- a/t/helper/test-tool.h
+++ b/t/helper/test-tool.h
@@ -32,7 +32,6 @@ int cmd__getcwd(int argc, const char **argv);
 int cmd__hashmap(int argc, const char **argv);
 int cmd__hash_speed(int argc, const char **argv);
 int cmd__hexdump(int argc, const char **argv);
-int cmd__index_version(int argc, const char **argv);
 int cmd__json_writer(int argc, const char **argv);
 int cmd__lazy_init_name_hash(int argc, const char **argv);
 int cmd__match_trees(int argc, const char **argv);
diff --git a/t/t1600-index.sh b/t/t1600-index.sh
index 9368d82f7d..62e7fd1596 100755
--- a/t/t1600-index.sh
+++ b/t/t1600-index.sh
@@ -118,7 +118,7 @@ test_index_version () {
 		fi &&
 		git add a &&
 		echo $EXPECTED_OUTPUT_VERSION >expect &&
-		test-tool index-version <.git/index >actual &&
+		git update-index --show-index-version >actual &&
 		test_cmp expect actual
 	)
 }
diff --git a/t/t1700-split-index.sh b/t/t1700-split-index.sh
index b4ab166369..a7b7263b35 100755
--- a/t/t1700-split-index.sh
+++ b/t/t1700-split-index.sh
@@ -43,7 +43,7 @@ test_expect_success 'enable split index' '
 	git config splitIndex.maxPercentChange 100 &&
 	git update-index --split-index &&
 	test-tool dump-split-index .git/index >actual &&
-	indexversion=$(test-tool index-version <.git/index) &&
+	indexversion=$(git update-index --show-index-version) &&
 
 	# NEEDSWORK: Stop hard-coding checksums.
 	if test "$indexversion" = "4"
diff --git a/t/t2104-update-index-skip-worktree.sh b/t/t2104-update-index-skip-worktree.sh
index b8686aabd3..0bab134d71 100755
--- a/t/t2104-update-index-skip-worktree.sh
+++ b/t/t2104-update-index-skip-worktree.sh
@@ -39,7 +39,7 @@ test_expect_success 'setup' '
 '
 
 test_expect_success 'index is at version 2' '
-	test "$(test-tool index-version < .git/index)" = 2
+	test "$(git update-index --show-index-version)" = 2
 '
 
 test_expect_success 'update-index --skip-worktree' '
@@ -48,7 +48,7 @@ test_expect_success 'update-index --skip-worktree' '
 '
 
 test_expect_success 'index is at version 3 after having some skip-worktree entries' '
-	test "$(test-tool index-version < .git/index)" = 3
+	test "$(git update-index --show-index-version)" = 3
 '
 
 test_expect_success 'ls-files -t' '
@@ -61,7 +61,7 @@ test_expect_success 'update-index --no-skip-worktree' '
 '
 
 test_expect_success 'index version is back to 2 when there is no skip-worktree entry' '
-	test "$(test-tool index-version < .git/index)" = 2
+	test "$(git update-index --show-index-version)" = 2
 '
 
 test_done
-- 
2.42.0-158-g94e83dcf5b

Re: [PATCH v3 0/3] "update-index --show-index-version"

[Linus Arver]

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

>
> This iteration takes suggestions by Linus Arver; the tests added by
> [2/3] have been clarified.
>

This version LGTM, thanks!

Re: [PATCH v3 0/3] "update-index --show-index-version"

[Junio C Hamano]

Linus Arver <linusa@google.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>>
>> This iteration takes suggestions by Linus Arver; the tests added by
>> [2/3] have been clarified.
>>
>
> This version LGTM, thanks!

Thanks.

[PATCH v3 2/3] update-index: add --show-index-version

[Teng Long]

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

> @@ -1181,15 +1183,20 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  
>  	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
>  	if (preferred_index_format) {
> -		if (preferred_index_format < INDEX_FORMAT_LB ||
> -		    INDEX_FORMAT_UB < preferred_index_format)
> +		if (preferred_index_format < 0) {
> +			printf(_("%d\n"), the_index.version);

Maybe the "%d\n" shouldn't be translated? :)

Thanks.

Re: [PATCH v3 2/3] update-index: add --show-index-version

[Junio C Hamano]

Teng Long <dyroneteng@gmail.com> writes:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> @@ -1181,15 +1183,20 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>>  
>>  	getline_fn = nul_term_line ? strbuf_getline_nul : strbuf_getline_lf;
>>  	if (preferred_index_format) {
>> -		if (preferred_index_format < INDEX_FORMAT_LB ||
>> -		    INDEX_FORMAT_UB < preferred_index_format)
>> +		if (preferred_index_format < 0) {
>> +			printf(_("%d\n"), the_index.version);
>
> Maybe the "%d\n" shouldn't be translated? :)

Excellent.