index-format.txt: mention of v4 is missing in some places

PATCH In review Nguyễn Thái Ngọc Duy 3 participants 10 messages Jun 15, 2016 Documentation

lore

Share & tools

lore

Nguyễn Thái Ngọc Duy

author

2016-06-15 22:56 UTC #0 lore raw

PATCH 1/2 index-format.txt: mention of v4 is missing in some places

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 Documentation/technical/index-format.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Documentation/technical/index-format.txt +3 -3

index 27c716b..0810251 100644
@@ -12,7 +12,7 @@ Git index format
        The signature is { 'D', 'I', 'R', 'C' } (stands for "dircache")
 
      4-byte version number:
-       The current supported versions are 2 and 3.
+       The current supported versions are 2, 3 and 4.
 
      32-bit number of index entries.

@@ -93,8 +93,8 @@ Git index format
     12-bit name length if the length is less than 0xFFF; otherwise 0xFFF
     is stored in this field.
 
-  (Version 3) A 16-bit field, only applicable if the "extended flag"
-  above is 1, split into (high to low bits).
+  (Version 3 or later) A 16-bit field, only applicable if the
+  "extended flag" above is 1, split into (high to low bits).
 
     1-bit reserved for future

Signature · 1 line

1.8.1.2.536.gf441e6d

Nguyễn Thái Ngọc Duy

author

2016-06-15 22:56 UTC #1 lore raw

PATCH 2/2 read-cache.c: use INDEX_FORMAT_{LB,UB} in verify_hdr()

9d22778 (read-cache.c: write prefix-compressed names in the index - 2012-04-04) defined these. Interestingly, they were not used by read-cache.c, or anywhere in that patch. They were used in builtin/update-index.c later for checking supported index versions. Use them here too.

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 read-cache.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

read-cache.c +2 -1

index 827ae55..298161f 100644
@@ -1260,7 +1260,8 @@ static int verify_hdr(struct cache_header *hdr, unsigned long size)
 	if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
 		return error("bad signature");
 	hdr_version = ntohl(hdr->hdr_version);
-	if (hdr_version < 2 || 4 < hdr_version)
+	if (hdr_version < INDEX_FORMAT_LB ||
+	    hdr_version > INDEX_FORMAT_UB)
 		return error("bad index version %d", hdr_version);
 	git_SHA1_Init(&c);
 	git_SHA1_Update(&c, hdr, size - 20);

Signature · 1 line

1.8.1.2.536.gf441e6d

Junio C Hamano

2016-06-15 22:56 UTC #2 lore raw

Re: [PATCH 2/2] read-cache.c: use INDEX_FORMAT_{LB,UB} in verify_hdr()

Quoted 5 lines · Nguyễn Thái Ngọc Duy

9d22778 (read-cache.c: write prefix-compressed names in the index - 2012-04-04) defined these. Interestingly, they were not used by read-cache.c, or anywhere in that patch. They were used in builtin/update-index.c later for checking supported index versions. Use them here too.

Thanks.

Quoted 4 lines

read-cache.c +2 -1

-	if (hdr_version < 2 || 4 < hdr_version)
+	if (hdr_version < INDEX_FORMAT_LB ||
+	    hdr_version > INDEX_FORMAT_UB)
 		return error("bad index version %d", hdr_version);

Junio C Hamano

2016-06-15 22:56 UTC #9 lore raw

In reply to Nguyễn Thái Ngọc Duy

Quoted 19 lines · Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
 Documentation/technical/index-format.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Documentation/technical/index-format.txt +1 -1

index 27c716b..0810251 100644
@@ -12,7 +12,7 @@ Git index format
        The signature is { 'D', 'I', 'R', 'C' } (stands for "dircache")
 
      4-byte version number:
-       The current supported versions are 2 and 3.
+       The current supported versions are 2, 3 and 4.
 
      32-bit number of index entries.

Documentation/technical/index-format.txt Git index format · line 99

Quoted 9 lines

Documentation/technical/index-format.txt +2 -2

@@ -93,8 +93,8 @@ Git index format
     12-bit name length if the length is less than 0xFFF; otherwise 0xFFF
     is stored in this field.
 
-  (Version 3) A 16-bit field, only applicable if the "extended flag"
-  above is 1, split into (high to low bits).
+  (Version 3 or later) A 16-bit field, only applicable if the
+  "extended flag" above is 1, split into (high to low bits).
 
     1-bit reserved for future

Depending on how the first later version decides to encode the additional flag information, "or later" may need to be changed to "and 4" when it happens. As we cannot predict the future, I think "or later" is just as good as "add 4" for now.

Thanks.

Nguyễn Thái Ngọc Duy

author

2016-06-15 22:56 UTC #3 lore raw

[PATCH 3/2] update-index: list supported idx versions and their features

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
.. and the user should know (briefly) the differences between index versions too.

 Documentation/git-update-index.txt | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Documentation/git-update-index.txt +9 -1

index 77a912d..e5aaba5 100644
@@ -145,7 +145,15 @@ you will need to handle the situation manually.
 
 --index-version <n>::
 	Write the resulting index out in the named on-disk format version.
-	The current default version is 2.
+	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`.
++
+	Version 4 performs a simple pathname compression that could
+	reduce 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 may not support it yet.
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are

Signature · 1 line

1.8.1.2.536.gf441e6d

Jonathan Nieder

2016-06-15 22:56 UTC #4 lore raw

Re: [PATCH 3/2] update-index: list supported idx versions and their features

Quoted 6 lines · Nguyễn Thái Ngọc Duy

Documentation/git-update-index.txt +0 -0

@@ -145,7 +145,15 @@ you will need to handle the situation manually.
 
 --index-version <n>::
 	Write the resulting index out in the named on-disk format version.

Documentation/git-update-index.txt you will need to handle the situation manually. · line 156

Quoted 10 lines

Documentation/git-update-index.txt +9 -1

-	The current default version is 2.
+	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`.
++
+	Version 4 performs a simple pathname compression that could
+	reduce 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 may not support it yet.

Markup nit: the second paragraph needs to be unindented, or asciidoc will treat it as literal text, including line breaks.

Usage nit: s/could/can/

Clarity nit: something like "such as JGit and libgit2" after "Other Git implementations" would make it clearer, at least to my eyes.

Aside from that, this looks like a good change.

Thanks,
Jonathan

Junio C Hamano

2016-06-15 22:56 UTC #5 lore raw

Re: [PATCH 3/2] update-index: list supported idx versions and their features

Thanks, both.

Quoted 9 lines · Jonathan Nieder

Nguyễn Thái Ngọc Duy wrote:
...
Documentation/git-update-index.txt you will need to handle the situation manually. · line 156
Quoted 5 lines
Documentation/git-update-index.txt +5 -0
+	Version 4 performs a simple pathname compression that could
+	reduce 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 may not support it yet.
Usage nit: s/could/can/

I think s/could reduce/reduces/ is even simpler.

Jonathan Nieder

2016-06-15 22:56 UTC #6 lore raw

Re: [PATCH 3/2] update-index: list supported idx versions and their features

Quoted 11 lines · Junio C Hamano

Quoted 9 lines · Jonathan Nieder
Documentation/git-update-index.txt you will need to handle the situation manually. · line 156
Quoted 5 lines · Nguyễn Thái Ngọc Duy
Documentation/git-update-index.txt +5 -0
+	Version 4 performs a simple pathname compression that could
+	reduce 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 may not support it yet.
Usage nit: s/could/can/

Quoted 1 line

I think s/could reduce/reduces/ is even simpler.

Yes, true. Thanks.

Nguyễn Thái Ngọc Duy

author

2016-06-15 22:56 UTC #7 lore raw

[PATCH v2 3/2] update-index: list supported idx versions and their features

In reply to Nguyễn Thái Ngọc Duy

Signed-off-by Nguyễn Thái Ngọc Duy <[email protected]>

---
Oops, bogus indentation in the first 3/2

 Documentation/git-update-index.txt | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

Documentation/git-update-index.txt +8 -1

index 77a912d..dbb75f4 100644
@@ -145,7 +145,14 @@ you will need to handle the situation manually.
 
 --index-version <n>::
 	Write the resulting index out in the named on-disk format version.
-	The current default version is 2.
+	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`.
++
+Version 4 performs a simple pathname compression that could reduce
+index size by 30%-50% on large repositories, which results in faster
+load time. Version 4 is relatively young (first released in in 1.8.0
+in October 2012). Other Git implementations may not support it yet.
 
 -z::
 	Only meaningful with `--stdin` or `--index-info`; paths are

Signature · 1 line

1.8.1.2.536.gf441e6d

Jonathan Nieder

Reviewed-by

2016-06-15 22:56 UTC #8 lore raw

Re: [PATCH v2 3/2] update-index: list supported idx versions and their features

Quoted 1 line · Nguyễn Thái Ngọc Duy

Oops, bogus indentation in the first 3/2

Oops, I missed this. Ok, ignore the comment on indentation on v1. :)

With or without the other changes I suggested,

Reviewed-by Jonathan Nieder <[email protected]>