threads / patch / 7932

patchCreate pack-write.c for common pack writing code

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

## tl;dr

5 messages between May 1, 2007 and May 2, 2007. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

Dana How· May 1, 2007, 18:26 UTC · lore

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(-)
Show changes to 4 files +45 −39

Makefile, fast-import.c, pack-write.c, pack.h

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
Shawn O. Pearce· May 2, 2007, 16:16 UTC · re: Dana How · lore

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

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.
Nicolas Pitre· May 2, 2007, 16:29 UTC · re: Shawn O. Pearce · lore

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

On Wed, 2 May 2007, Shawn O. Pearce wrote:
Show 11 quoted lines
> 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
Shawn O. Pearce· May 2, 2007, 16:31 UTC · re: Nicolas Pitre · lore

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

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.
Dana How· May 2, 2007, 17:25 UTC · re: Shawn O. Pearce · lore

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

On 5/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:
Show 9 quoted lines
> 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

← back to recent threads