Subject: [PATCH 12/17] pack-objects: do not add type OBJ_NONE to objects[] in pack v4

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Sat, 21 Sep 2013 20:57:58 +0700

This is a longer explation of what is noted in the patch. When object
names are received from stdin, we lazily put OBJ_NONE as type to
objects[]. check_object() is called for each entry in objects[] later,
when it checks for ref-delta and ofs-delta for straight copy to the
pack later.

In pack v4, we don't store commits and trees as ref-delta and we need
a way to know those are delta objects in pack v2 sources are actually
commits or trees. We detect that with "type" field in struct
object_entry, which is correctly filled when --revs is passed. Without
--revs, the "type" field would be OBJ_NONE and we would need another
sha1_object_info() or similar call to detect the true object type.

Because we need object type anyway in this code path for building up
ident and path dictionaries, fill correct type as well.

Without this, the condition "pack_version < 4 || entry->type !=
OBJ_TREE" in check_object() is always true when --revs is not used,
and we will encode trees as OBJ_REF_DELTA with the base either
OBJ_TREE or OBJ_PV4_TREE. That kills pv4 tree format advantages.

Signed-off-by: Nguyễn Thái Ngọc Duy <[email-masked]>
---
 builtin/pack-objects.c | 17 ++++++++++++++---
 1 file changed, 14 insertions(+), 3 deletions(-)

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 1e0c2e6..01954cb 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1496,6 +1496,7 @@ static void check_object(struct object_entry *entry)
 			 */
 			if (pack_version < 4 || entry->type != OBJ_TREE)
 				entry->type = entry->in_pack_type;
+			assert(entry->type != OBJ_NONE);
 			entry->delta = base_entry;
 			entry->delta_size = entry->size;
 			entry->delta_sibling = base_entry->delta_child;
@@ -2361,7 +2362,6 @@ static void read_object_list_from_stdin(void)
 			die("expected sha1, got garbage:\n %s", line);
 
 		add_preferred_base_object(line+41);
-		add_object_entry(sha1, 0, line+41, 0);
 
 		if (pack_version == 4) {
 			void *data;
@@ -2370,7 +2370,17 @@ static void read_object_list_from_stdin(void)
 			int (*add_dict_entries)(struct dict_table *, void *, unsigned long);
 			struct dict_table *dict;
 
-			switch (sha1_object_info(sha1, &size)) {
+			type = sha1_object_info(sha1, &size);
+			/*
+			 * In v2, we can afford to keep entry->type ==
+			 * OBJ_NONE and check_object() will fill it
+			 * later. In v4, we need to know the type
+			 * right now, don't waste time looking for
+			 * type again later in check_object() when the
+			 * in-pack type is ref-delta.
+			 */
+			add_object_entry(sha1, type, line+41, 0);
+			switch (type) {
 			case OBJ_COMMIT:
 				add_dict_entries = add_commit_dict_entries;
 				dict = v4.commit_ident_table;
@@ -2389,7 +2399,8 @@ static void read_object_list_from_stdin(void)
 				die("can't process %s object %s",
 				    typename(type), sha1_to_hex(sha1));
 			free(data);
-		}
+		} else
+			add_object_entry(sha1, 0, line+41, 0);
 	}
 }
 
-- 
1.8.2.83.gc99314b