git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] Require packfiles to follow the naming convention.

From
Junio C Hamano <junkio@cox.net>
Date
Dec 24, 2005, 10:03 UTC
Message-ID
<7vfyoiu80b.fsf@assigned-by-dhcp.cox.net>
In-Reply-To
<7vd5jmvnkc.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano <junkio@cox.net> writes:
Show 13 quoted lines
> * require packfiles to follow the naming convention more strictly.
>
>   This rejects a packfile whose name does not begin with
>   "pack-" followed by 40-byte hexadecimal digits.  The core
>   never had such limitation (the only thing it required was
>   that the pack and the idx are named with the same basename
>   with .pack and .idx extention), but the way http-fetch
>   handles packs requires this.
> ...
>   This is probably controversial because it would eventually
>   require users to rename their packs created using pre v0.99.9
>   tools.  There is a small utility program that helps the
>   conversion process, so it may not be that painful.

And this is the patch in question. When your packs are the ones produced by pre v0.99.9 tools, you can ask git-convert-packs to suggest you the "right name" to use.

$ git-convert-packs warning: index name '.git/objects/pack/pack-a3b60db8bb6dad970f6dcdc03b0addf3988e8556.idx' does not match contents of the pack '83b60db8bb6dad970f6dcdc03b0addf3988e8556' # Packname does not match contents. mv '.git/objects/pack/pack-a3b60db8bb6dad970f6dcdc03b0addf3988e8556.pack' '.git/objects/pack/pack-83b60db8bb6dad970f6dcdc03b0addf3988e8556.pack' mv '.git/objects/pack/pack-a3b60db8bb6dad970f6dcdc03b0addf3988e8556.idx' '.git/objects/pack/pack-83b60db8bb6dad970f6dcdc03b0addf3988e8556.idx' $ git-convert-packs | sh warning: index name '.git/objects/pack/pack-a3b60db8bb6dad970f6dcdc03b0addf3988e8556.idx' does not match contents of the pack '83b60db8bb6dad970f6dcdc03b0addf3988e8556' $ git-convert-packs $ exit

Comments?

-- >8 -- Pack files names are required to be "pack-[0-9a-f]{40}.pack", with corresponding .idx file. This is not strictly needed at the core level, but the packed file transfer over HTTP relies on this format. With this commit, the core would ignore otherwise a valid packfile in .git/objects/packs that "git-unpack-objects" would expand properly, if it does not follow the naming convention.

The hexadecimal part should be what git-pack-objects computed when the pack was created originally, and it is the SHA1 sum over sorted object names (i.e. 20-byte binary SHA1 checksum each). However, this condition is checked but not enforced currently, because git-pack-objects before v0.99.9 computed a checksum over object names without sorting (i.e. the order git-rev-list fed the object names), just to make pack names unlikely to collide. This made less than ideal use of SHA1 sum to name packs, which we would want to rectify in the future.

Many packfiles in the wild were created with git before v0.99.9, so the check would trigger and will give a warning message like this:

warning: index name '.git/objects/pack/pack-3b60db8....idx' does not match contents of the pack '83b60db...'

You can rename the existing pack that has this old name (3b60db8...) to new name (83b60db...) to squelch this warning message. For that, a new command git-convert-packs is provided to produce suitable "mv" command you can paste into your shell.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
 .gitignore             |    1 +
 Makefile               |    2 +
 convert-packs.c        |   49 ++++++++++++++++++++++++++++++
 sha1_file.c            |   41 ++++++++++++++++++++++++-
 t/t5300-pack-object.sh |   79 ++++++++++++++++++++++++++----------------------
 5 files changed, 133 insertions(+), 39 deletions(-)
 create mode 100644 convert-packs.c
9c9f23b012e4030f433c7d4260a3fae93c302a0e
diff --git a/.gitignore b/.gitignore
index 6bd508e..d713f62 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,6 +18,7 @@ git-clone-pack
 git-commit
 git-commit-tree
 git-convert-objects
+git-convert-packs
 git-count-objects
 git-cvsexportcommit
 git-cvsimport
diff --git a/Makefile b/Makefile
index 3395a9e..2209f97 100644
--- a/Makefile
+++ b/Makefile
@@ -121,7 +121,7 @@ SIMPLE_PROGRAMS = \
 PROGRAMS = \
 	git-apply$X git-cat-file$X \
 	git-checkout-index$X git-clone-pack$X git-commit-tree$X \
-	git-convert-objects$X git-diff-files$X \
+	git-convert-objects$X git-convert-packs$X git-diff-files$X \
 	git-diff-index$X git-diff-stages$X \
 	git-diff-tree$X git-fetch-pack$X git-fsck-objects$X \
 	git-hash-object$X git-index-pack$X git-init-db$X \
diff --git a/convert-packs.c b/convert-packs.c
new file mode 100644
index 0000000..13f5502
--- /dev/null
+++ b/convert-packs.c
@@ -0,0 +1,49 @@
+#include "cache.h"
+
+void verify_pack_name(struct packed_git *p)
+{
+	SHA_CTX ctx;
+	unsigned char sha1[20];
+	int i, nr, len;
+	const char *hex;
+
+	nr = num_packed_objects(p);
+
+	SHA1_Init(&ctx);
+	for (i = 0; i < nr; i++) {
+		unsigned char *ent = (((unsigned char *)p->index_base) +
+				      4*256 + i * 24 + 4);
+		SHA1_Update(&ctx, ent, 20);
+	}
+	SHA1_Final(sha1, &ctx);
+	
+	len = strlen(p->pack_name);
+	hex = sha1_to_hex(sha1);
+	/* "pack-X{40}.pack" */
+	if (len < 50 ||
+	    memcmp(p->pack_name + len - 5 - 40, hex, 40)) {
+		char *d = strrchr(p->pack_name, '/');
+		int dlen;
+		
+		if (d)
+			dlen = (d - p->pack_name) + 1;
+		else
+			dlen = 0;
+		printf("# Packname does not match contents.\n");
+		printf("mv '%s' '%.*spack-%s.pack'\n",
+		       p->pack_name, dlen, p->pack_name, hex);
+		printf("mv '%.*sidx' '%.*spack-%s.idx'\n",
+		       len-4, p->pack_name, dlen, p->pack_name, hex);
+	}
+}
+
+int main(int ac, char **av)
+{
+	struct packed_git *p;
+
+	setup_git_directory();
+	prepare_packed_git();
+	for (p = packed_git; p; p = p->next)
+		verify_pack_name(p);
+	return 0;
+}
diff --git a/sha1_file.c b/sha1_file.c
index 6b7577d..fc24d0e 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -323,6 +323,7 @@ static int check_packed_git_idx(const ch
 {
 	SHA_CTX ctx;
 	unsigned char sha1[20];
+	unsigned char packname[20];
 	void *idx_map;
 	unsigned int *index;
 	unsigned long idx_size;
@@ -337,6 +338,16 @@ static int check_packed_git_idx(const ch
 		close(fd);
 		return -1;
 	}
+
+	/* We require "...../pack-XXXX{40}XXXX.idx" now.
+	 * 5-byte "pack-", 40-byte hex and 4-byte .idx.
+	 */
+	i = strlen(path);
+	if (i < 49 || strcmp(path + i - 4, ".idx") ||
+	    strncmp(path + i - 49, "pack-", 5) ||
+	    get_sha1_hex(path + i - 44, packname))
+		return error("non index file %s ignored", path);
+
 	idx_size = st.st_size;
 	idx_map = mmap(NULL, idx_size, PROT_READ, MAP_PRIVATE, fd, 0);
 	close(fd);
@@ -369,7 +380,7 @@ static int check_packed_git_idx(const ch
 		return error("wrong index file size");
 
 	/*
-	 * File checksum.
+	 * File checksum and index name.
 	 */
 	SHA1_Init(&ctx);
 	SHA1_Update(&ctx, idx_map, idx_size-20);
@@ -378,6 +389,34 @@ static int check_packed_git_idx(const ch
 	if (memcmp(sha1, idx_map + idx_size - 20, 20))
 		return error("index checksum mismatch");
 
+/*
+ * 0: do not check, do not do anything.
+ * 1: check and warn, but do not reject.
+ * 2: check and reject if pack name is obsolete.
+ */
+#define PACK_NAME_ENFORCEMENT_PHASE 1
+
+#if PACK_NAME_ENFORCEMENT_PHASE != 0
+	SHA1_Init(&ctx);
+	for (i = 0; i < nr; i++) {
+		unsigned char *ent = (idx_map + 4*256) + i * 24 + 4;
+		SHA1_Update(&ctx, ent, 20);
+	}
+	SHA1_Final(sha1, &ctx);
+	
+	if (memcmp(sha1, packname, 20)) {
+#if PACK_NAME_ENFORCEMENT_PHASE == 1
+		fprintf(stderr, "warning: index name '%s' does not match "
+			"contents of the pack '%s'\n",
+			path, sha1_to_hex(sha1));
+#else
+		return error("index name '%s' does not match "
+			     "contents of the pack '%s'\n",
+			     path, sha1_to_hex(sha1));
+#endif
+	}
+#endif
+
 	return 0;
 }
 
diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 7dfb1ab..dba8227 100755
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -10,6 +10,8 @@ test_description='git-pack-object
 
 TRASH=`pwd`
 
+mkdir test-1 test-2
+
 test_expect_success \
     'setup' \
     'rm -f .git/index*
@@ -35,7 +37,7 @@ test_expect_success \
 
 test_expect_success \
     'pack without delta' \
-    'packname_1=$(git-pack-objects --window=0 test-1 <obj-list)'
+    'packname_1=$(git-pack-objects --window=0 test-1/pack <obj-list)'
 
 rm -fr .git2
 mkdir .git2
@@ -45,8 +47,8 @@ test_expect_success \
     "GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
      git-init-db &&
-     git-unpack-objects -n <test-1-${packname_1}.pack &&
-     git-unpack-objects <test-1-${packname_1}.pack"
+     git-unpack-objects -n <test-1/pack-${packname_1}.pack &&
+     git-unpack-objects <test-1/pack-${packname_1}.pack"
 
 unset GIT_OBJECT_DIRECTORY
 cd "$TRASH/.git2"
@@ -66,7 +68,8 @@ cd "$TRASH"
 test_expect_success \
     'pack with delta' \
     'pwd &&
-     packname_2=$(git-pack-objects test-2 <obj-list)'
+     packname_2=$(git-pack-objects test-2/pack <obj-list) &&
+     test "$packname_2" = "$packname_1"'
 
 rm -fr .git2
 mkdir .git2
@@ -76,8 +79,8 @@ test_expect_success \
     'GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
      git-init-db &&
-     git-unpack-objects -n <test-2-${packname_2}.pack &&
-     git-unpack-objects <test-2-${packname_2}.pack'
+     git-unpack-objects -n <test-2/pack-${packname_2}.pack &&
+     git-unpack-objects <test-2/pack-${packname_2}.pack'
 
 unset GIT_OBJECT_DIRECTORY
 cd "$TRASH/.git2"
@@ -101,7 +104,8 @@ test_expect_success \
     'GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
      git-init-db &&
-     cp test-1-${packname_1}.pack test-1-${packname_1}.idx .git2/objects/pack && {
+     cp test-1/pack-${packname_1}.pack test-1/pack-${packname_1}.idx \
+     	.git2/objects/pack && {
 	 git-diff-tree --root -p $commit &&
 	 while read object
 	 do
@@ -116,8 +120,9 @@ test_expect_success \
     'use packed deltified objects' \
     'GIT_OBJECT_DIRECTORY=.git2/objects &&
      export GIT_OBJECT_DIRECTORY &&
-     rm -f .git2/objects/pack/test-?.idx &&
-     cp test-2-${packname_2}.pack test-2-${packname_2}.idx .git2/objects/pack && {
+     rm -f .git2/objects/pack/pack-* &&
+     cp test-2/pack-${packname_2}.pack test-2/pack-${packname_2}.idx \
+     	.git2/objects/pack && {
 	 git-diff-tree --root -p $commit &&
 	 while read object
 	 do
@@ -131,41 +136,41 @@ unset GIT_OBJECT_DIRECTORY
 
 test_expect_success \
     'verify pack' \
-    'git-verify-pack test-1-${packname_1}.idx test-2-${packname_2}.idx'
+    'git-verify-pack test-?/pack-${packname_1}.idx'
 
 test_expect_success \
     'corrupt a pack and see if verify catches' \
-    'cp test-1-${packname_1}.idx test-3.idx &&
-     cp test-2-${packname_2}.pack test-3.pack &&
-     if git-verify-pack test-3.idx
+    'cp test-1/pack-${packname_1}.idx . &&
+     cp test-2/pack-${packname_2}.pack . &&
+     if git-verify-pack pack-${packname_1}.idx
      then false
      else :;
      fi &&
 
-     cp test-1-${packname_1}.pack test-3.pack &&
-     dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=2 &&
-     if git-verify-pack test-3.idx
+     cp test-1/pack-${packname_1}.pack . &&
+     dd if=/dev/zero of=pack-${packname_1}.pack count=1 bs=1 conv=notrunc seek=2 &&
+     if git-verify-pack pack-${packname_1}.idx
      then false
      else :;
      fi &&
 
-     cp test-1-${packname_1}.pack test-3.pack &&
-     dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=7 &&
-     if git-verify-pack test-3.idx
+     cp test-1/pack-${packname_1}.pack . &&
+     dd if=/dev/zero of=pack-${packname_1}.pack count=1 bs=1 conv=notrunc seek=7 &&
+     if git-verify-pack pack-${packname_1}.idx
      then false
      else :;
      fi &&
 
-     cp test-1-${packname_1}.pack test-3.pack &&
-     dd if=/dev/zero of=test-3.pack count=1 bs=1 conv=notrunc seek=12 &&
-     if git-verify-pack test-3.idx
+     cp test-1/pack-${packname_1}.pack . &&
+     dd if=/dev/zero of=pack-${packname_1}.pack count=1 bs=1 conv=notrunc seek=12 &&
+     if git-verify-pack pack-${packname_1}.idx
      then false
      else :;
      fi &&
 
-     cp test-1-${packname_1}.pack test-3.pack &&
-     dd if=/dev/zero of=test-3.idx count=1 bs=1 conv=notrunc seek=1200 &&
-     if git-verify-pack test-3.pack
+     cp test-1/pack-${packname_1}.pack . &&
+     dd if=/dev/zero of=pack-${packname_1}.idx count=1 bs=1 conv=notrunc seek=1200 &&
+     if git-verify-pack pack-${packname_1}.idx
      then false
      else :;
      fi &&
@@ -174,19 +179,19 @@ test_expect_success \
 
 test_expect_success \
     'build pack index for an existing pack' \
-    'cp test-1-${packname_1}.pack test-3.pack &&
-     git-index-pack -o tmp.idx test-3.pack &&
-     cmp tmp.idx test-1-${packname_1}.idx &&
-
-     git-index-pack test-3.pack &&
-     cmp test-3.idx test-1-${packname_1}.idx &&
-
-     cp test-2-${packname_2}.pack test-3.pack &&
-     git-index-pack -o tmp.idx test-2-${packname_2}.pack &&
-     cmp tmp.idx test-2-${packname_2}.idx &&
+    'cp test-1/pack-${packname_1}.pack unknown.pack &&
+     git-index-pack -o tmp.idx unknown.pack &&
+     cmp tmp.idx test-1/pack-${packname_1}.idx &&
+
+     git-index-pack unknown.pack &&
+     cmp unknown.idx test-1/pack-${packname_1}.idx &&
+
+     cp test-2/pack-${packname_2}.pack unknown.pack &&
+     git-index-pack -o tmp.idx unknown.pack &&
+     cmp tmp.idx test-2/pack-${packname_2}.idx &&
 
-     git-index-pack test-3.pack &&
-     cmp test-3.idx test-2-${packname_2}.idx &&
+     git-index-pack unknown.pack &&
+     cmp unknown.idx test-2/pack-${packname_2}.idx &&
 
      :'
 

-- 
1.0.GIT
Previous: Junio C Hamano
Message 2 of 2 in “GIT 1.0.4 and what is in master and proposed updates.”
  1. Junio C HamanoDec 24, 2005
  2. Require packfiles to follow the naming convention.Junio C Hamano, Dec 24, 2005

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.