[PATCH] Create pack-write.c for common pack writing code

[PATCH] Create pack-write.c for common pack writing code

[Dana How]

Include a generalized fixup_header_footer() in this new file.
Needed by git-repack --max-pack-size feature in a later patchset.

Signed-off-by: Dana L. How <danahow@gmail.com>
---
 Makefile      |    4 ++--
 fast-import.c |   39 ++-------------------------------------
 pack-write.c  |   38 ++++++++++++++++++++++++++++++++++++++
 pack.h        |    3 +++
 4 files changed, 45 insertions(+), 39 deletions(-)

diff --git a/Makefile b/Makefile
index 2fea115..e0a1308 100644
--- a/Makefile
+++ b/Makefile
@@ -301,8 +301,8 @@ LIB_OBJS = \
 	interpolate.o \
 	lockfile.o \
 	patch-ids.o \
-	object.o pack-check.o patch-delta.o path.o pkt-line.o sideband.o \
-	reachable.o reflog-walk.o \
+	object.o pack-check.o pack-write.o patch-delta.o path.o pkt-line.o \
+	sideband.o reachable.o reflog-walk.o \
 	quote.o read-cache.o refs.o run-command.o dir.o object-refs.o \
 	server-info.o setup.o sha1_file.o sha1_name.o strbuf.o \
 	tag.o tree.o usage.o config.o environment.o ctype.o copy.o \
diff --git a/fast-import.c b/fast-import.c
index b4cbcd9..276e0e0 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -651,42 +651,6 @@ static void start_packfile(void)
 	all_packs[pack_id] = p;
 }
 
-static void fixup_header_footer(void)
-{
-	static const int buf_sz = 128 * 1024;
-	int pack_fd = pack_data->pack_fd;
-	SHA_CTX c;
-	struct pack_header hdr;
-	char *buf;
-
-	if (lseek(pack_fd, 0, SEEK_SET) != 0)
-		die("Failed seeking to start: %s", strerror(errno));
-	if (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
-		die("Unable to reread header of %s", pack_data->pack_name);
-	if (lseek(pack_fd, 0, SEEK_SET) != 0)
-		die("Failed seeking to start: %s", strerror(errno));
-	hdr.hdr_entries = htonl(object_count);
-	write_or_die(pack_fd, &hdr, sizeof(hdr));
-
-	SHA1_Init(&c);
-	SHA1_Update(&c, &hdr, sizeof(hdr));
-
-	buf = xmalloc(buf_sz);
-	for (;;) {
-		ssize_t n = xread(pack_fd, buf, buf_sz);
-		if (!n)
-			break;
-		if (n < 0)
-			die("Failed to checksum %s", pack_data->pack_name);
-		SHA1_Update(&c, buf, n);
-	}
-	free(buf);
-
-	SHA1_Final(pack_data->sha1, &c);
-	write_or_die(pack_fd, pack_data->sha1, sizeof(pack_data->sha1));
-	close(pack_fd);
-}
-
 static int oecmp (const void *a_, const void *b_)
 {
 	struct object_entry *a = *((struct object_entry**)a_);
@@ -802,7 +766,8 @@ static void end_packfile(void)
 		struct branch *b;
 		struct tag *t;
 
-		fixup_header_footer();
+		fixup_header_footer(pack_data->pack_fd, pack_data->sha1,
+				    pack_data->pack_name, object_count);
 		idx_name = keep_pack(create_index());
 
 		/* Register the packfile with core git's machinary. */
diff --git a/pack-write.c b/pack-write.c
new file mode 100644
index 0000000..e4c1408
--- /dev/null
+++ b/pack-write.c
@@ -0,0 +1,38 @@
+#include "cache.h"
+#include "pack.h"
+
+void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
+			 const char *pack_name, uint32_t object_count)
+{
+	static const int buf_sz = 128 * 1024;
+	SHA_CTX c;
+	struct pack_header hdr;
+	char *buf;
+
+	if (lseek(pack_fd, 0, SEEK_SET) != 0)
+		die("Failed seeking to start: %s", strerror(errno));
+	if (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))
+		die("Unable to reread header of %s", pack_name);
+	if (lseek(pack_fd, 0, SEEK_SET) != 0)
+		die("Failed seeking to start: %s", strerror(errno));
+	hdr.hdr_entries = htonl(object_count);
+	write_or_die(pack_fd, &hdr, sizeof(hdr));
+
+	SHA1_Init(&c);
+	SHA1_Update(&c, &hdr, sizeof(hdr));
+
+	buf = xmalloc(buf_sz);
+	for (;;) {
+		size_t n = xread(pack_fd, buf, buf_sz);
+		if (!n)
+			break;
+		if (n < 0)
+			die("Failed to checksum %s", pack_name);
+		SHA1_Update(&c, buf, n);
+	}
+	free(buf);
+
+	SHA1_Final(pack_file_sha1, &c);
+	write_or_die(pack_fd, pack_file_sha1, 20);
+	close(pack_fd);
+}
diff --git a/pack.h b/pack.h
index d4d412c..dc296cc 100644
--- a/pack.h
+++ b/pack.h
@@ -45,6 +45,9 @@ struct pack_idx_header {
 
 extern int verify_pack(struct packed_git *, int);
 
+void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,
+			 const char *pack_name, uint32_t object_count);
+
 #define PH_ERROR_EOF		(-1)
 #define PH_ERROR_PACK_SIGNATURE	(-2)
 #define PH_ERROR_PROTOCOL	(-3)
-- 
1.5.2.rc0.789.gd951

Re: [PATCH] Create pack-write.c for common pack writing code

[Shawn O. Pearce]

Dana How <danahow@gmail.com> wrote:
> 
> Include a generalized fixup_header_footer() in this new file.
> Needed by git-repack --max-pack-size feature in a later patchset.

Thanks.  I'm applying this to my fastimport.git tree, but I changed
the name to fixup_pack_header_footer().  I'm also refactoring the
same code out of index-pack, to call your version.

I'll ask Junio to pull your patch, and my index-pack cleanup soon.
As soon as I'm sure everything still passes the tests.  ;-)

-- 
Shawn.

Re: [PATCH] Create pack-write.c for common pack writing code

[Nicolas Pitre]

On Wed, 2 May 2007, Shawn O. Pearce wrote:

> Dana How <danahow@gmail.com> wrote:
> > 
> > Include a generalized fixup_header_footer() in this new file.
> > Needed by git-repack --max-pack-size feature in a later patchset.
> 
> Thanks.  I'm applying this to my fastimport.git tree, but I changed
> the name to fixup_pack_header_footer().  I'm also refactoring the
> same code out of index-pack, to call your version.
> 
> I'll ask Junio to pull your patch, and my index-pack cleanup soon.
> As soon as I'm sure everything still passes the tests.  ;-)

BTW I think the common function should _not_ close the file descriptor 
it is being handed.   It is more flexible to let the caller close the 
file, or possibly do whatever other operations like fchmod().


Nicolas

Re: [PATCH] Create pack-write.c for common pack writing code

[Shawn O. Pearce]

Nicolas Pitre <nico@cam.org> wrote:
> BTW I think the common function should _not_ close the file descriptor 
> it is being handed.   It is more flexible to let the caller close the 
> file, or possibly do whatever other operations like fchmod().

I didn't mention it, but I agree, and fixed it in Dana's patch.

-- 
Shawn.

Re: [PATCH] Create pack-write.c for common pack writing code

[Dana How]

On 5/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Dana How <danahow@gmail.com> wrote:
> > Include a generalized fixup_header_footer() in this new file.
> > Needed by git-repack --max-pack-size feature in a later patchset.
> Thanks.  I'm applying this to my fastimport.git tree, but I changed
> the name to fixup_pack_header_footer().  I'm also refactoring the
> same code out of index-pack, to call your version.
>
> I'll ask Junio to pull your patch, and my index-pack cleanup soon.
> As soon as I'm sure everything still passes the tests.  ;-)
Sounds good.  I didn't refactor index-pack.c since that wasn't
what we discussed; pulling out the close() as you or Nicolas later
write is the only thing needed to include it too.

At this point,  I guess there's not much left to comment on in the
--max-pack-size=N patchset?  Should I update it for your _pack
name change/moving out the close()?  I' feel like I've really
spammed the list with the versions of this patchset (5 so far).

Also, any reaction to the pack.compression/--compression=N patch?
It was the smallest code delta/most significant low-hanging fruit
I could identify for my day-job workflow's performance.

Thanks,
-- 
Dana L. How  danahow@gmail.com  +1 650 804 5991 cell