{"thread":{"id":"43587","subject":"Re: [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.","startedAt":"2006-10-31T07:56:29Z","lastAt":"2006-10-31T23:51:21Z","messageCount":4,"participants":["Junio C Hamano","Shawn Pearce","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"297823","messageId":"20061031075629.GA7691@spearce.org","threadId":"43587","inReplyTo":null,"subject":"[PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-31T07:56:29Z","receivedAt":"2006-10-31T07:56:29Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Some applications which invoke unpack-objects or index-pack --stdin\nmay want to examine the pack header to determine the number of\nobjects contained in the pack and use that value to determine which\nexecutable to invoke to handle the rest of the pack stream.\n\nHowever if the caller consumes the pack header from the input stream\nthen its no longer available for unpack-objects or index-pack --stdin,\nboth of which need the version and object count to process the stream.\n\nThis change introduces --pack_header=ver,cnt as a command line option\nthat the caller can supply to indicate it has already consumed the\npack header and what version and object count were found in that\nheader.  As this option is only meant for low level applications\nsuch as receive-pack we are not documenting it at this time.\n\nCredit goes to Nicolas Pitre for suggesting this approach on the\ngit mailing list.\n\nSigned-off-by: Shawn O. Pearce <spearce@spearce.org>\n---\n This series of two patches replaces my last receive-pack (3/3)\n patch.\n\n builtin-unpack-objects.c |   31 +++++++++++++++++++++++++------\n index-pack.c             |   30 +++++++++++++++++++++++++-----\n 2 files changed, 50 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c\nindex e70a711..a5cfd0b 100644\n--- a/builtin-unpack-objects.c\n+++ b/builtin-unpack-objects.c\n@@ -326,11 +326,16 @@ static void unpack_one(unsigned nr, unsi\n \t}\n }\n \n-static void unpack_all(void)\n+static void unpack_all(struct pack_header *hdr)\n {\n \tint i;\n-\tstruct pack_header *hdr = fill(sizeof(struct pack_header));\n-\tunsigned nr_objects = ntohl(hdr->hdr_entries);\n+\tunsigned nr_objects;\n+\n+\tif (!hdr) {\n+\t\thdr = fill(sizeof(struct pack_header));\n+\t\tuse(sizeof(struct pack_header));\n+\t}\n+\tnr_objects = ntohl(hdr->hdr_entries);\n \n \tif (ntohl(hdr->hdr_signature) != PACK_SIGNATURE)\n \t\tdie(\"bad pack file\");\n@@ -339,7 +344,6 @@ static void unpack_all(void)\n \tfprintf(stderr, \"Unpacking %d objects\\n\", nr_objects);\n \n \tobj_list = xmalloc(nr_objects * sizeof(*obj_list));\n-\tuse(sizeof(struct pack_header));\n \tfor (i = 0; i < nr_objects; i++)\n \t\tunpack_one(i, nr_objects);\n \tif (delta_list)\n@@ -348,7 +352,8 @@ static void unpack_all(void)\n \n int cmd_unpack_objects(int argc, const char **argv, const char *prefix)\n {\n-\tint i;\n+\tint i, have_hdr = 0;\n+\tstruct pack_header hdr;\n \tunsigned char sha1[20];\n \n \tgit_config(git_default_config);\n@@ -371,6 +376,18 @@ int cmd_unpack_objects(int argc, const c\n \t\t\t\trecover = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strncmp(arg, \"--pack_header=\", 14)) {\n+\t\t\t\tchar *c;\n+\t\t\t\thdr.hdr_signature = htonl(PACK_SIGNATURE);\n+\t\t\t\thdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tif (*c != ',')\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tif (*c)\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thave_hdr = 1;\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tusage(unpack_usage);\n \t\t}\n \n@@ -378,7 +395,9 @@ int cmd_unpack_objects(int argc, const c\n \t\tusage(unpack_usage);\n \t}\n \tSHA1_Init(&ctx);\n-\tunpack_all();\n+\tif (have_hdr)\n+\t\tSHA1_Update(&ctx, &hdr, sizeof(struct pack_header));\n+\tunpack_all(have_hdr ? &hdr : NULL);\n \tSHA1_Update(&ctx, buffer, offset);\n \tSHA1_Final(sha1, &ctx);\n \tif (hashcmp(fill(20), sha1))\ndiff --git a/index-pack.c b/index-pack.c\nindex b37dd78..1911df8 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -161,9 +161,12 @@ static const char *open_pack_file(const\n \treturn pack_name;\n }\n \n-static void parse_pack_header(void)\n+static void parse_pack_header(struct pack_header *hdr)\n {\n-\tstruct pack_header *hdr = fill(sizeof(struct pack_header));\n+\tif (!hdr) {\n+\t\thdr = fill(sizeof(struct pack_header));\n+\t\tuse(sizeof(struct pack_header));\n+\t}\n \n \t/* Header consistency check */\n \tif (hdr->hdr_signature != htonl(PACK_SIGNATURE))\n@@ -172,7 +175,6 @@ static void parse_pack_header(void)\n \t\tdie(\"pack version %d unsupported\", ntohl(hdr->hdr_version));\n \n \tnr_objects = ntohl(hdr->hdr_entries);\n-\tuse(sizeof(struct pack_header));\n }\n \n static void bad_object(unsigned long offset, const char *format,\n@@ -822,11 +824,12 @@ static void final(const char *final_pack\n \n int main(int argc, char **argv)\n {\n-\tint i, fix_thin_pack = 0;\n+\tint i, fix_thin_pack = 0, have_hdr = 0;\n \tconst char *curr_pack, *pack_name = NULL;\n \tconst char *curr_index, *index_name = NULL;\n \tconst char *keep_name = NULL, *keep_msg = NULL;\n \tchar *index_name_buf = NULL, *keep_name_buf = NULL;\n+\tstruct pack_header hdr;\n \tunsigned char sha1[20];\n \n \tfor (i = 1; i < argc; i++) {\n@@ -841,6 +844,16 @@ int main(int argc, char **argv)\n \t\t\t\tkeep_msg = \"\";\n \t\t\t} else if (!strncmp(arg, \"--keep=\", 7)) {\n \t\t\t\tkeep_msg = arg + 7;\n+\t\t\t} else if (!strncmp(arg, \"--pack_header=\", 14)) {\n+\t\t\t\tchar *c;\n+\t\t\t\thdr.hdr_signature = htonl(PACK_SIGNATURE);\n+\t\t\t\thdr.hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tif (*c != ',')\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thdr.hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tif (*c)\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thave_hdr = 1;\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"-o\")) {\n@@ -861,6 +874,8 @@ int main(int argc, char **argv)\n \t\tusage(index_pack_usage);\n \tif (fix_thin_pack && !from_stdin)\n \t\tdie(\"--fix-thin cannot be used without --stdin\");\n+\tif (have_hdr && !from_stdin)\n+\t\tdie(\"--pack_header cannot be used without --stdin\");\n \tif (!index_name && pack_name) {\n \t\tint len = strlen(pack_name);\n \t\tif (!has_extension(pack_name, \".pack\"))\n@@ -883,7 +898,12 @@ int main(int argc, char **argv)\n \t}\n \n \tcurr_pack = open_pack_file(pack_name);\n-\tparse_pack_header();\n+\tif (have_hdr) {\n+\t\tif (output_fd >= 0)\n+\t\t\twrite_or_die(output_fd, &hdr, sizeof(struct pack_header));\n+\t\tSHA1_Update(&input_ctx, &hdr, sizeof(struct pack_header));\n+\t}\n+\tparse_pack_header(have_hdr ? &hdr : NULL);\n \tobjects = xmalloc((nr_objects + 1) * sizeof(struct object_entry));\n \tdeltas = xmalloc(nr_objects * sizeof(struct delta_entry));\n \tif (verbose)\n-- \n1.4.3.3.g7d63\n"},{"id":"296962","messageId":"Pine.LNX.4.64.0610311400180.11384@xanadu.home","threadId":"43587","inReplyTo":"20061031075629.GA7691@spearce.org","subject":"Re: [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-10-31T19:33:09Z","receivedAt":"2006-10-31T19:33:09Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 31 Oct 2006, Shawn Pearce wrote:\n\n> However if the caller consumes the pack header from the input stream\n> then its no longer available for unpack-objects or index-pack --stdin,\n> both of which need the version and object count to process the stream.\n> \n> This change introduces --pack_header=ver,cnt as a command line option\n> that the caller can supply to indicate it has already consumed the\n> pack header and what version and object count were found in that\n> header.  As this option is only meant for low level applications\n> such as receive-pack we are not documenting it at this time.\n\nThis breaks index-pack, and unpack-objects with OBJ_OFS_DELTA, if \n--pack-header is used.  The header is not accounted in the pack's offset \nand therefore every object's offset is wrong.\n\nWhat about this patch instead?  This makes things much simpler IMHO.\n\ndiff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c\nindex 74a90c1..e6d7574 100644\n--- a/builtin-unpack-objects.c\n+++ b/builtin-unpack-objects.c\n@@ -371,6 +371,21 @@ int cmd_unpack_objects(int argc, const c\n \t\t\t\trecover = 1;\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!strncmp(arg, \"--pack_header=\", 14)) {\n+\t\t\t\tstruct pack_header *hdr;\n+\t\t\t\tchar *c;\n+\n+\t\t\t\thdr = (struct pack_header *)buffer;\n+\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n+\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tif (*c != ',')\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tif (*c)\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\tlen = sizeof(*hdr);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tusage(unpack_usage);\n \t\t}\n \ndiff --git a/index-pack.c b/index-pack.c\nindex b37dd78..a3b55f9 100644\n--- a/index-pack.c\n+++ b/index-pack.c\n@@ -841,6 +841,19 @@ int main(int argc, char **argv)\n \t\t\t\tkeep_msg = \"\";\n \t\t\t} else if (!strncmp(arg, \"--keep=\", 7)) {\n \t\t\t\tkeep_msg = arg + 7;\n+\t\t\t} else if (!strncmp(arg, \"--pack_header=\", 14)) {\n+\t\t\t\tstruct pack_header *hdr;\n+\t\t\t\tchar *c;\n+\n+\t\t\t\thdr = (struct pack_header *)input_buffer;\n+\t\t\t\thdr->hdr_signature = htonl(PACK_SIGNATURE);\n+\t\t\t\thdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));\n+\t\t\t\tif (*c != ',')\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\thdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));\n+\t\t\t\tif (*c)\n+\t\t\t\t\tdie(\"bad %s\", arg);\n+\t\t\t\tinput_len = sizeof(*hdr);\n \t\t\t} else if (!strcmp(arg, \"-v\")) {\n \t\t\t\tverbose = 1;\n \t\t\t} else if (!strcmp(arg, \"-o\")) {\n"},{"id":"295950","messageId":"20061031200841.GC23671@spearce.org","threadId":"43587","inReplyTo":"Pine.LNX.4.64.0610311400180.11384@xanadu.home","subject":"Re: [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.","fromName":"Shawn Pearce","fromEmail":"spearce@spearce.org","sentAt":"2006-10-31T20:08:41Z","receivedAt":"2006-10-31T20:08:41Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Nicolas Pitre <nico@cam.org> wrote:\n> On Tue, 31 Oct 2006, Shawn Pearce wrote:\n> \n> > However if the caller consumes the pack header from the input stream\n> > then its no longer available for unpack-objects or index-pack --stdin,\n> > both of which need the version and object count to process the stream.\n> > \n> > This change introduces --pack_header=ver,cnt as a command line option\n> > that the caller can supply to indicate it has already consumed the\n> > pack header and what version and object count were found in that\n> > header.  As this option is only meant for low level applications\n> > such as receive-pack we are not documenting it at this time.\n> \n> This breaks index-pack, and unpack-objects with OBJ_OFS_DELTA, if \n> --pack-header is used.  The header is not accounted in the pack's offset \n> and therefore every object's offset is wrong.\n> \n> What about this patch instead?  This makes things much simpler IMHO.\n\nAgreed.  The idea you are using here came to me in my sleep last\nnight; I didn't have time to look at it until now however.  You just\nbeat me to posting it.  :-)\n\n-- \n"},{"id":"295735","messageId":"7vwt6goxhi.fsf@assigned-by-dhcp.cox.net","threadId":"43587","inReplyTo":"20061031200841.GC23671@spearce.org","subject":"Re: [PATCH 1/2] Allow pack header preprocessing before unpack-objects/index-pack.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-10-31T23:51:21Z","receivedAt":"2006-10-31T23:51:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Shawn Pearce <spearce@spearce.org> writes:\n\n> Nicolas Pitre <nico@cam.org> wrote:\n>> On Tue, 31 Oct 2006, Shawn Pearce wrote:\n>> \n>> > However if the caller consumes the pack header from the input stream\n>> > then its no longer available for unpack-objects or index-pack --stdin,\n>> > both of which need the version and object count to process the stream.\n>> > \n>> > This change introduces --pack_header=ver,cnt as a command line option\n>> > that the caller can supply to indicate it has already consumed the\n>> > pack header and what version and object count were found in that\n>> > header.  As this option is only meant for low level applications\n>> > such as receive-pack we are not documenting it at this time.\n>> \n>> This breaks index-pack, and unpack-objects with OBJ_OFS_DELTA, if \n>> --pack-header is used.  The header is not accounted in the pack's offset \n>> and therefore every object's offset is wrong.\n>> \n>> What about this patch instead?  This makes things much simpler IMHO.\n>\n> Agreed.  The idea you are using here came to me in my sleep last\n> night; I didn't have time to look at it until now however.  You just\n> beat me to posting it.  :-)\n\nWill replace your [1/2] still in \"pu\".\n"}]}