{"thread":{"id":"7932","subject":"[PATCH] Create pack-write.c for common pack writing code","startedAt":"2007-05-01T18:26:30Z","lastAt":"2007-05-02T17:25:07Z","messageCount":5,"participants":["Dana How","Shawn O. Pearce","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"40842","messageId":"46378656.9080109@gmail.com","threadId":"7932","inReplyTo":null,"subject":"[PATCH] Create pack-write.c for common pack writing code","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-01T18:26:30Z","receivedAt":"2007-05-01T18:26:30Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"\nInclude a generalized fixup_header_footer() in this new file.\nNeeded by git-repack --max-pack-size feature in a later patchset.\n\nSigned-off-by: Dana L. How <danahow@gmail.com>\n---\n Makefile      |    4 ++--\n fast-import.c |   39 ++-------------------------------------\n pack-write.c  |   38 ++++++++++++++++++++++++++++++++++++++\n pack.h        |    3 +++\n 4 files changed, 45 insertions(+), 39 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 2fea115..e0a1308 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -301,8 +301,8 @@ LIB_OBJS = \\\n \tinterpolate.o \\\n \tlockfile.o \\\n \tpatch-ids.o \\\n-\tobject.o pack-check.o patch-delta.o path.o pkt-line.o sideband.o \\\n-\treachable.o reflog-walk.o \\\n+\tobject.o pack-check.o pack-write.o patch-delta.o path.o pkt-line.o \\\n+\tsideband.o reachable.o reflog-walk.o \\\n \tquote.o read-cache.o refs.o run-command.o dir.o object-refs.o \\\n \tserver-info.o setup.o sha1_file.o sha1_name.o strbuf.o \\\n \ttag.o tree.o usage.o config.o environment.o ctype.o copy.o \\\ndiff --git a/fast-import.c b/fast-import.c\nindex b4cbcd9..276e0e0 100644\n--- a/fast-import.c\n+++ b/fast-import.c\n@@ -651,42 +651,6 @@ static void start_packfile(void)\n \tall_packs[pack_id] = p;\n }\n \n-static void fixup_header_footer(void)\n-{\n-\tstatic const int buf_sz = 128 * 1024;\n-\tint pack_fd = pack_data->pack_fd;\n-\tSHA_CTX c;\n-\tstruct pack_header hdr;\n-\tchar *buf;\n-\n-\tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n-\t\tdie(\"Failed seeking to start: %s\", strerror(errno));\n-\tif (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))\n-\t\tdie(\"Unable to reread header of %s\", pack_data->pack_name);\n-\tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n-\t\tdie(\"Failed seeking to start: %s\", strerror(errno));\n-\thdr.hdr_entries = htonl(object_count);\n-\twrite_or_die(pack_fd, &hdr, sizeof(hdr));\n-\n-\tSHA1_Init(&c);\n-\tSHA1_Update(&c, &hdr, sizeof(hdr));\n-\n-\tbuf = xmalloc(buf_sz);\n-\tfor (;;) {\n-\t\tssize_t n = xread(pack_fd, buf, buf_sz);\n-\t\tif (!n)\n-\t\t\tbreak;\n-\t\tif (n < 0)\n-\t\t\tdie(\"Failed to checksum %s\", pack_data->pack_name);\n-\t\tSHA1_Update(&c, buf, n);\n-\t}\n-\tfree(buf);\n-\n-\tSHA1_Final(pack_data->sha1, &c);\n-\twrite_or_die(pack_fd, pack_data->sha1, sizeof(pack_data->sha1));\n-\tclose(pack_fd);\n-}\n-\n static int oecmp (const void *a_, const void *b_)\n {\n \tstruct object_entry *a = *((struct object_entry**)a_);\n@@ -802,7 +766,8 @@ static void end_packfile(void)\n \t\tstruct branch *b;\n \t\tstruct tag *t;\n \n-\t\tfixup_header_footer();\n+\t\tfixup_header_footer(pack_data->pack_fd, pack_data->sha1,\n+\t\t\t\t    pack_data->pack_name, object_count);\n \t\tidx_name = keep_pack(create_index());\n \n \t\t/* Register the packfile with core git's machinary. */\ndiff --git a/pack-write.c b/pack-write.c\nnew file mode 100644\nindex 0000000..e4c1408\n--- /dev/null\n+++ b/pack-write.c\n@@ -0,0 +1,38 @@\n+#include \"cache.h\"\n+#include \"pack.h\"\n+\n+void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,\n+\t\t\t const char *pack_name, uint32_t object_count)\n+{\n+\tstatic const int buf_sz = 128 * 1024;\n+\tSHA_CTX c;\n+\tstruct pack_header hdr;\n+\tchar *buf;\n+\n+\tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n+\t\tdie(\"Failed seeking to start: %s\", strerror(errno));\n+\tif (read_in_full(pack_fd, &hdr, sizeof(hdr)) != sizeof(hdr))\n+\t\tdie(\"Unable to reread header of %s\", pack_name);\n+\tif (lseek(pack_fd, 0, SEEK_SET) != 0)\n+\t\tdie(\"Failed seeking to start: %s\", strerror(errno));\n+\thdr.hdr_entries = htonl(object_count);\n+\twrite_or_die(pack_fd, &hdr, sizeof(hdr));\n+\n+\tSHA1_Init(&c);\n+\tSHA1_Update(&c, &hdr, sizeof(hdr));\n+\n+\tbuf = xmalloc(buf_sz);\n+\tfor (;;) {\n+\t\tsize_t n = xread(pack_fd, buf, buf_sz);\n+\t\tif (!n)\n+\t\t\tbreak;\n+\t\tif (n < 0)\n+\t\t\tdie(\"Failed to checksum %s\", pack_name);\n+\t\tSHA1_Update(&c, buf, n);\n+\t}\n+\tfree(buf);\n+\n+\tSHA1_Final(pack_file_sha1, &c);\n+\twrite_or_die(pack_fd, pack_file_sha1, 20);\n+\tclose(pack_fd);\n+}\ndiff --git a/pack.h b/pack.h\nindex d4d412c..dc296cc 100644\n--- a/pack.h\n+++ b/pack.h\n@@ -45,6 +45,9 @@ struct pack_idx_header {\n \n extern int verify_pack(struct packed_git *, int);\n \n+void fixup_header_footer(int pack_fd, unsigned char *pack_file_sha1,\n+\t\t\t const char *pack_name, uint32_t object_count);\n+\n #define PH_ERROR_EOF\t\t(-1)\n #define PH_ERROR_PACK_SIGNATURE\t(-2)\n #define PH_ERROR_PROTOCOL\t(-3)\n-- \n1.5.2.rc0.789.gd951\n"},{"id":"40906","messageId":"20070502161648.GK5942@spearce.org","threadId":"7932","inReplyTo":"46378656.9080109@gmail.com","subject":"Re: [PATCH] Create pack-write.c for common pack writing code","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-02T16:16:48Z","receivedAt":"2007-05-02T16:16:48Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Dana How <danahow@gmail.com> wrote:\n> \n> Include a generalized fixup_header_footer() in this new file.\n> Needed by git-repack --max-pack-size feature in a later patchset.\n\nThanks.  I'm applying this to my fastimport.git tree, but I changed\nthe name to fixup_pack_header_footer().  I'm also refactoring the\nsame code out of index-pack, to call your version.\n\nI'll ask Junio to pull your patch, and my index-pack cleanup soon.\nAs soon as I'm sure everything still passes the tests.  ;-)\n\n-- \nShawn.\n"},{"id":"40909","messageId":"alpine.LFD.0.98.0705021226010.6574@xanadu.home","threadId":"7932","inReplyTo":"20070502161648.GK5942@spearce.org","subject":"Re: [PATCH] Create pack-write.c for common pack writing code","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-05-02T16:29:40Z","receivedAt":"2007-05-02T16:29:40Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Wed, 2 May 2007, Shawn O. Pearce wrote:\n\n> Dana How <danahow@gmail.com> wrote:\n> > \n> > Include a generalized fixup_header_footer() in this new file.\n> > Needed by git-repack --max-pack-size feature in a later patchset.\n> \n> Thanks.  I'm applying this to my fastimport.git tree, but I changed\n> the name to fixup_pack_header_footer().  I'm also refactoring the\n> same code out of index-pack, to call your version.\n> \n> I'll ask Junio to pull your patch, and my index-pack cleanup soon.\n> As soon as I'm sure everything still passes the tests.  ;-)\n\nBTW I think the common function should _not_ close the file descriptor \nit is being handed.   It is more flexible to let the caller close the \nfile, or possibly do whatever other operations like fchmod().\n\n\nNicolas\n"},{"id":"40910","messageId":"20070502163122.GL5942@spearce.org","threadId":"7932","inReplyTo":"alpine.LFD.0.98.0705021226010.6574@xanadu.home","subject":"Re: [PATCH] Create pack-write.c for common pack writing code","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-02T16:31:22Z","receivedAt":"2007-05-02T16:31:22Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> BTW I think the common function should _not_ close the file descriptor \n> it is being handed.   It is more flexible to let the caller close the \n> file, or possibly do whatever other operations like fchmod().\n\nI didn't mention it, but I agree, and fixed it in Dana's patch.\n\n-- \nShawn.\n"},{"id":"40915","messageId":"56b7f5510705021025j23e2ca3al7a72aeee33ff86aa@mail.gmail.com","threadId":"7932","inReplyTo":"20070502161648.GK5942@spearce.org","subject":"Re: [PATCH] Create pack-write.c for common pack writing code","fromName":"Dana How","fromEmail":"danahow@gmail.com","sentAt":"2007-05-02T17:25:07Z","receivedAt":"2007-05-02T17:25:07Z","isPatch":true,"sender":{"key":"danahow@gmail.com","avatar":null},"body":"On 5/2/07, Shawn O. Pearce <spearce@spearce.org> wrote:\n> Dana How <danahow@gmail.com> wrote:\n> > Include a generalized fixup_header_footer() in this new file.\n> > Needed by git-repack --max-pack-size feature in a later patchset.\n> Thanks.  I'm applying this to my fastimport.git tree, but I changed\n> the name to fixup_pack_header_footer().  I'm also refactoring the\n> same code out of index-pack, to call your version.\n>\n> I'll ask Junio to pull your patch, and my index-pack cleanup soon.\n> As soon as I'm sure everything still passes the tests.  ;-)\nSounds good.  I didn't refactor index-pack.c since that wasn't\nwhat we discussed; pulling out the close() as you or Nicolas later\nwrite is the only thing needed to include it too.\n\nAt this point,  I guess there's not much left to comment on in the\n--max-pack-size=N patchset?  Should I update it for your _pack\nname change/moving out the close()?  I' feel like I've really\nspammed the list with the versions of this patchset (5 so far).\n\nAlso, any reaction to the pack.compression/--compression=N patch?\nIt was the smallest code delta/most significant low-hanging fruit\nI could identify for my day-job workflow's performance.\n\nThanks,\n-- \nDana L. How  danahow@gmail.com  +1 650 804 5991 cell\n"}]}