On Sun, 8 Apr 2007, Junio C Hamano wrote:

quoted text
> Nicolas Pitre <nico@cam.org> writes:
> 
> > Support for large packs exceeding 31 bits in size won't impose an index
> > size bloat for packs within that range that don't need a 64-bit offset.
> > And because newer objects which are likely to be the most frequently used
> > are located at the beginning of the pack, they won't pay the 64-bit offset
> > lookup at run time either even if the pack is large.
> >
> > Right now an index version 2 is created only when the biggest offset in a
> > pack reaches 31 bits.  It might be a good idea to always use index version
> > 2 eventually to benefit from the CRC32 it contains when reusing pack data
> > while repacking.
> > ...
> > @@ -582,6 +602,18 @@ static void write_index_file(void)
> >  	struct object_entry **list = sorted_by_sha;
> >  	struct object_entry **last = list + nr_result;
> >  	uint32_t array[256];
> > +	uint32_t index_version;
> > +
> > +	/* if last object's offset is >= 2^31 we should use index V2 */
> > +	index_version = (objects[nr_result-1].offset >> 31) ? 2 : 1;
> 
> Although write_pack_file() iterates objects[] array in the
> ascending order of index and calls write_one() for each of them,
> in the presense of "we write base object before delta" rule, is
> it always true that the last object in the objects[] array has
> the largest offset?

Oops, indeed it is not.  It is true in the index-pack but not here.

Please could you amend this patch with the following:

diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
index c7e0a42..8cf2871 100644
--- a/builtin-pack-objects.c
+++ b/builtin-pack-objects.c
@@ -548,11 +548,11 @@ static off_t write_one(struct sha1file *f,
 	return offset + size;
 }
 
-static void write_pack_file(void)
+static off_t write_pack_file(void)
 {
 	uint32_t i;
 	struct sha1file *f;
-	off_t offset;
+	off_t offset, last_obj_offset = 0;
 	struct pack_header hdr;
 	unsigned last_percent = 999;
 	int do_progress = progress;
@@ -575,6 +575,7 @@ static void write_pack_file(void)
 	if (!nr_result)
 		goto done;
 	for (i = 0; i < nr_objects; i++) {
+		last_obj_offset = offset;
 		offset = write_one(f, objects + i, offset);
 		if (do_progress) {
 			unsigned percent = written * 100 / nr_result;
@@ -592,9 +593,11 @@ static void write_pack_file(void)
 	if (written != nr_result)
 		die("wrote %u objects while expecting %u", written, nr_result);
 	sha1close(f, pack_file_sha1, 1);
+
+	return last_obj_offset;
 }
 
-static void write_index_file(void)
+static void write_index_file(off_t last_obj_offset)
 {
 	uint32_t i;
 	struct sha1file *f = sha1create("%s-%s.%s", base_name,
@@ -605,7 +608,7 @@ static void write_index_file(void)
 	uint32_t index_version;
 
 	/* if last object's offset is >= 2^31 we should use index V2 */
-	index_version = (objects[nr_result-1].offset >> 31) ? 2 : 1;
+	index_version = (last_obj_offset >> 31) ? 2 : 1;
 
 	/* index versions 2 and above need a header */
 	if (index_version >= 2) {
@@ -1769,6 +1772,7 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 	if (reuse_cached_pack(object_list_sha1))
 		;
 	else {
+		off_t last_obj_offset;
 		if (nr_result)
 			prepare_pack(window, depth);
 		if (progress == 1 && pack_to_stdout) {
@@ -1778,9 +1782,9 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
 			signal(SIGALRM, SIG_IGN );
 			progress_update = 0;
 		}
-		write_pack_file();
+		last_obj_offset = write_pack_file();
 		if (!pack_to_stdout) {
-			write_index_file();
+			write_index_file(last_obj_offset);
 			puts(sha1_to_hex(object_list_sha1));
 		}
 	}

Nicolas
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html