{"thread":{"id":"19731","subject":"[PATCH] Shift object enumeration out of upload-pack","startedAt":"2009-06-08T01:34:28Z","lastAt":"2009-06-10T07:20:19Z","messageCount":5,"participants":["Nick Edelen","Nicolas Pitre","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"115760","messageId":"c77435a80906071834m1600d23dpb4a1b58c7a61361d@mail.gmail.com","threadId":"19731","inReplyTo":null,"subject":"[PATCH] Shift object enumeration out of upload-pack","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-06-08T01:34:28Z","receivedAt":"2009-06-08T01:34:28Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"okie dokie.  is this good?\n\nFrom: Nick Edelen <sirnot@gmail.com>\nSubject: [PATCH] Shift object enumeration out of upload-pack\n\nOffload object enumeration in upload-pack to pack-objects, but fall\nback on internal revision walker for shallow interaction.  Test t5530\nupdated to reflect mechanism change.\n\nSigned-off-by: Nick Edelen <sirnot@gmail.com>\nAcked-by: Nicolas Pitre <nico@cam.org>\n\n---\n t/t5530-upload-pack-error.sh |   14 ++++++++++-\n upload-pack.c                |   49 +++++++++++++++++++++++++++++++++--------\n 2 files changed, 51 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh\nindex f5102b9..22eec24 100755\n--- a/t/t5530-upload-pack-error.sh\n+++ b/t/t5530-upload-pack-error.sh\n@@ -30,11 +30,12 @@ test_expect_success 'fsck fails' '\n \ttest_must_fail git fsck\n '\n\n-test_expect_success 'upload-pack fails due to error in pack-objects' '\n+test_expect_success 'upload-pack fails due to error in pack-objects packing' '\n\n \t! echo \"0032want $(git rev-parse HEAD)\n 00000009done\n 0000\" | git upload-pack . > /dev/null 2> output.err &&\n+\tgrep \"unable to read\" output.err &&\n \tgrep \"pack-objects died\" output.err\n '\n\n@@ -51,11 +52,20 @@ test_expect_success 'fsck fails' '\n test_expect_success 'upload-pack fails due to error in rev-list' '\n\n \t! echo \"0032want $(git rev-parse HEAD)\n-00000009done\n+0034shallow $(git rev-parse HEAD^)00000009done\n 0000\" | git upload-pack . > /dev/null 2> output.err &&\n \tgrep \"waitpid (async) failed\" output.err\n '\n\n+test_expect_success 'upload-pack fails due to error in pack-objects\nenumeration' '\n+\n+\t! echo \"0032want $(git rev-parse HEAD)\n+00000009done\n+0000\" | git upload-pack . > /dev/null 2> output.err &&\n+\tgrep \"bad tree object\" output.err &&\n+\tgrep \"pack-objects died\" output.err\n+'\n+\n test_expect_success 'create empty repository' '\n\n \tmkdir foo &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex edc7861..397cada 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -29,6 +29,7 @@ static unsigned long oldest_have;\n static int multi_ack, nr_our_refs;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress;\n+static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n static unsigned int timeout;\n@@ -107,8 +108,6 @@ static int do_rev_list(int fd, void *create_full_pack)\n \tstruct rev_info revs;\n\n \tpack_pipe = fdopen(fd, \"w\");\n-\tif (create_full_pack)\n-\t\tuse_thin_pack = 0; /* no point doing it */\n \tinit_revisions(&revs, NULL);\n \trevs.tag_objects = 1;\n \trevs.tree_objects = 1;\n@@ -155,13 +154,21 @@ static void create_pack_file(void)\n \tconst char *argv[10];\n \tint arg = 0;\n\n-\trev_list.proc = do_rev_list;\n-\t/* .data is just a boolean: any non-NULL value will do */\n-\trev_list.data = create_full_pack ? &rev_list : NULL;\n-\tif (start_async(&rev_list))\n-\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n+\tif (shallow_nr) {\n+\t\trev_list.proc = do_rev_list;\n+\t\trev_list.data = 0;\n+\t\tif (start_async(&rev_list))\n+\t\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n+\t\targv[arg++] = \"pack-objects\";\n+\t} else {\n+\t\targv[arg++] = \"pack-objects\";\n+\t\targv[arg++] = \"--revs\";\n+\t\tif (create_full_pack)\n+\t\t\targv[arg++] = \"--all\";\n+\t\telse if (use_thin_pack)\n+\t\t\targv[arg++] = \"--thin\";\n+\t}\n\n-\targv[arg++] = \"pack-objects\";\n \targv[arg++] = \"--stdout\";\n \tif (!no_progress)\n \t\targv[arg++] = \"--progress\";\n@@ -172,7 +179,7 @@ static void create_pack_file(void)\n \targv[arg++] = NULL;\n\n \tmemset(&pack_objects, 0, sizeof(pack_objects));\n-\tpack_objects.in = rev_list.out;\t/* start_command closes it */\n+\tpack_objects.in = shallow_nr ? rev_list.out : -1;\n \tpack_objects.out = -1;\n \tpack_objects.err = -1;\n \tpack_objects.git_cmd = 1;\n@@ -181,6 +188,24 @@ static void create_pack_file(void)\n \tif (start_command(&pack_objects))\n \t\tdie(\"git upload-pack: unable to fork git-pack-objects\");\n\n+\t/* pass on revisions we (don't) want */\n+\tif (!shallow_nr) {\n+\t\tFILE *pipe_fd = fdopen(pack_objects.in, \"w\");\n+\t\tif (!create_full_pack) {\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < want_obj.nr; i++)\n+\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(want_obj.objects[i].item->sha1));\n+\t\t\tfprintf(pipe_fd, \"--not\\n\");\n+\t\t\tfor (i = 0; i < have_obj.nr; i++)\n+\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(have_obj.objects[i].item->sha1));\n+\t\t}\n+\n+\t\tfprintf(pipe_fd, \"\\n\");\n+\t\tfflush(pipe_fd);\n+\t\tfclose(pipe_fd);\n+\t}\n+\n+\n \t/* We read from pack_objects.err to capture stderr output for\n \t * progress bar, and pack_objects.out to capture the pack data.\n \t */\n@@ -276,7 +301,7 @@ static void create_pack_file(void)\n \t\terror(\"git upload-pack: git-pack-objects died with error.\");\n \t\tgoto fail;\n \t}\n-\tif (finish_async(&rev_list))\n+\tif (shallow_nr && finish_async(&rev_list))\n \t\tgoto fail;\t/* error was already reported */\n\n \t/* flush the data */\n@@ -451,6 +476,7 @@ static void receive_needs(void)\n \tstatic char line[1000];\n \tint len, depth = 0;\n\n+\tshallow_nr = 0;\n \tif (debug_fd)\n \t\twrite_in_full(debug_fd, \"#S\\n\", 3);\n \tfor (;;) {\n@@ -534,6 +560,7 @@ static void receive_needs(void)\n \t\t\t\tpacket_write(1, \"shallow %s\",\n \t\t\t\t\t\tsha1_to_hex(object->sha1));\n \t\t\t\tregister_shallow(object->sha1);\n+\t\t\t\tshallow_nr++;\n \t\t\t}\n \t\t\tresult = result->next;\n \t\t}\n@@ -567,6 +594,8 @@ static void receive_needs(void)\n \t\t\tfor (i = 0; i < shallows.nr; i++)\n \t\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n \t\t}\n+\n+\tshallow_nr += shallows.nr;\n \tfree(shallows.objects);\n }\n\n-- \ntg: (503f464..) t/revcache/upload-pack-dont-enumerate (depends on: master)\n"},{"id":"115761","messageId":"alpine.LFD.2.00.0906072206130.3906@xanadu.home","threadId":"19731","inReplyTo":"c77435a80906071834m1600d23dpb4a1b58c7a61361d@mail.gmail.com","subject":"Re: [PATCH] Shift object enumeration out of upload-pack","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-06-08T02:12:52Z","receivedAt":"2009-06-08T02:12:52Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Mon, 8 Jun 2009, Nick Edelen wrote:\n\n> okie dokie.  is this good?\n\nYes.\n\nNext time please just put those \"okie dokie\" remarks right below the \n\"---\" line and that would be perfect.  ;-)  The reason is that you \nprobably don't want those remarks to be included in the commit log, and \nthe \"git am\" command automatically strips out everything between the \n\"---\" and the patch without manual editing which is nicer for the \ncommitter.\n\n> \n> From: Nick Edelen <sirnot@gmail.com>\n> Subject: [PATCH] Shift object enumeration out of upload-pack\n> \n> Offload object enumeration in upload-pack to pack-objects, but fall\n> back on internal revision walker for shallow interaction.  Test t5530\n> updated to reflect mechanism change.\n> \n> Signed-off-by: Nick Edelen <sirnot@gmail.com>\n> Acked-by: Nicolas Pitre <nico@cam.org>\n> \n> ---\n>  t/t5530-upload-pack-error.sh |   14 ++++++++++-\n>  upload-pack.c                |   49 +++++++++++++++++++++++++++++++++--------\n>  2 files changed, 51 insertions(+), 12 deletions(-)\n> \n> diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh\n> index f5102b9..22eec24 100755\n> --- a/t/t5530-upload-pack-error.sh\n> +++ b/t/t5530-upload-pack-error.sh\n> @@ -30,11 +30,12 @@ test_expect_success 'fsck fails' '\n>  \ttest_must_fail git fsck\n>  '\n> \n> -test_expect_success 'upload-pack fails due to error in pack-objects' '\n> +test_expect_success 'upload-pack fails due to error in pack-objects packing' '\n> \n>  \t! echo \"0032want $(git rev-parse HEAD)\n>  00000009done\n>  0000\" | git upload-pack . > /dev/null 2> output.err &&\n> +\tgrep \"unable to read\" output.err &&\n>  \tgrep \"pack-objects died\" output.err\n>  '\n> \n> @@ -51,11 +52,20 @@ test_expect_success 'fsck fails' '\n>  test_expect_success 'upload-pack fails due to error in rev-list' '\n> \n>  \t! echo \"0032want $(git rev-parse HEAD)\n> -00000009done\n> +0034shallow $(git rev-parse HEAD^)00000009done\n>  0000\" | git upload-pack . > /dev/null 2> output.err &&\n>  \tgrep \"waitpid (async) failed\" output.err\n>  '\n> \n> +test_expect_success 'upload-pack fails due to error in pack-objects\n> enumeration' '\n> +\n> +\t! echo \"0032want $(git rev-parse HEAD)\n> +00000009done\n> +0000\" | git upload-pack . > /dev/null 2> output.err &&\n> +\tgrep \"bad tree object\" output.err &&\n> +\tgrep \"pack-objects died\" output.err\n> +'\n> +\n>  test_expect_success 'create empty repository' '\n> \n>  \tmkdir foo &&\n> diff --git a/upload-pack.c b/upload-pack.c\n> index edc7861..397cada 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -29,6 +29,7 @@ static unsigned long oldest_have;\n>  static int multi_ack, nr_our_refs;\n>  static int use_thin_pack, use_ofs_delta, use_include_tag;\n>  static int no_progress;\n> +static int shallow_nr;\n>  static struct object_array have_obj;\n>  static struct object_array want_obj;\n>  static unsigned int timeout;\n> @@ -107,8 +108,6 @@ static int do_rev_list(int fd, void *create_full_pack)\n>  \tstruct rev_info revs;\n> \n>  \tpack_pipe = fdopen(fd, \"w\");\n> -\tif (create_full_pack)\n> -\t\tuse_thin_pack = 0; /* no point doing it */\n>  \tinit_revisions(&revs, NULL);\n>  \trevs.tag_objects = 1;\n>  \trevs.tree_objects = 1;\n> @@ -155,13 +154,21 @@ static void create_pack_file(void)\n>  \tconst char *argv[10];\n>  \tint arg = 0;\n> \n> -\trev_list.proc = do_rev_list;\n> -\t/* .data is just a boolean: any non-NULL value will do */\n> -\trev_list.data = create_full_pack ? &rev_list : NULL;\n> -\tif (start_async(&rev_list))\n> -\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n> +\tif (shallow_nr) {\n> +\t\trev_list.proc = do_rev_list;\n> +\t\trev_list.data = 0;\n> +\t\tif (start_async(&rev_list))\n> +\t\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n> +\t\targv[arg++] = \"pack-objects\";\n> +\t} else {\n> +\t\targv[arg++] = \"pack-objects\";\n> +\t\targv[arg++] = \"--revs\";\n> +\t\tif (create_full_pack)\n> +\t\t\targv[arg++] = \"--all\";\n> +\t\telse if (use_thin_pack)\n> +\t\t\targv[arg++] = \"--thin\";\n> +\t}\n> \n> -\targv[arg++] = \"pack-objects\";\n>  \targv[arg++] = \"--stdout\";\n>  \tif (!no_progress)\n>  \t\targv[arg++] = \"--progress\";\n> @@ -172,7 +179,7 @@ static void create_pack_file(void)\n>  \targv[arg++] = NULL;\n> \n>  \tmemset(&pack_objects, 0, sizeof(pack_objects));\n> -\tpack_objects.in = rev_list.out;\t/* start_command closes it */\n> +\tpack_objects.in = shallow_nr ? rev_list.out : -1;\n>  \tpack_objects.out = -1;\n>  \tpack_objects.err = -1;\n>  \tpack_objects.git_cmd = 1;\n> @@ -181,6 +188,24 @@ static void create_pack_file(void)\n>  \tif (start_command(&pack_objects))\n>  \t\tdie(\"git upload-pack: unable to fork git-pack-objects\");\n> \n> +\t/* pass on revisions we (don't) want */\n> +\tif (!shallow_nr) {\n> +\t\tFILE *pipe_fd = fdopen(pack_objects.in, \"w\");\n> +\t\tif (!create_full_pack) {\n> +\t\t\tint i;\n> +\t\t\tfor (i = 0; i < want_obj.nr; i++)\n> +\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(want_obj.objects[i].item->sha1));\n> +\t\t\tfprintf(pipe_fd, \"--not\\n\");\n> +\t\t\tfor (i = 0; i < have_obj.nr; i++)\n> +\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(have_obj.objects[i].item->sha1));\n> +\t\t}\n> +\n> +\t\tfprintf(pipe_fd, \"\\n\");\n> +\t\tfflush(pipe_fd);\n> +\t\tfclose(pipe_fd);\n> +\t}\n> +\n> +\n>  \t/* We read from pack_objects.err to capture stderr output for\n>  \t * progress bar, and pack_objects.out to capture the pack data.\n>  \t */\n> @@ -276,7 +301,7 @@ static void create_pack_file(void)\n>  \t\terror(\"git upload-pack: git-pack-objects died with error.\");\n>  \t\tgoto fail;\n>  \t}\n> -\tif (finish_async(&rev_list))\n> +\tif (shallow_nr && finish_async(&rev_list))\n>  \t\tgoto fail;\t/* error was already reported */\n> \n>  \t/* flush the data */\n> @@ -451,6 +476,7 @@ static void receive_needs(void)\n>  \tstatic char line[1000];\n>  \tint len, depth = 0;\n> \n> +\tshallow_nr = 0;\n>  \tif (debug_fd)\n>  \t\twrite_in_full(debug_fd, \"#S\\n\", 3);\n>  \tfor (;;) {\n> @@ -534,6 +560,7 @@ static void receive_needs(void)\n>  \t\t\t\tpacket_write(1, \"shallow %s\",\n>  \t\t\t\t\t\tsha1_to_hex(object->sha1));\n>  \t\t\t\tregister_shallow(object->sha1);\n> +\t\t\t\tshallow_nr++;\n>  \t\t\t}\n>  \t\t\tresult = result->next;\n>  \t\t}\n> @@ -567,6 +594,8 @@ static void receive_needs(void)\n>  \t\t\tfor (i = 0; i < shallows.nr; i++)\n>  \t\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n>  \t\t}\n> +\n> +\tshallow_nr += shallows.nr;\n>  \tfree(shallows.objects);\n>  }\n> \n> -- \n> tg: (503f464..) t/revcache/upload-pack-dont-enumerate (depends on: master)\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"115763","messageId":"7vvdn78n98.fsf@alter.siamese.dyndns.org","threadId":"19731","inReplyTo":"c77435a80906071834m1600d23dpb4a1b58c7a61361d@mail.gmail.com","subject":"Re: [PATCH] Shift object enumeration out of upload-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-08T02:36:51Z","receivedAt":"2009-06-08T02:36:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Edelen <sirnot@gmail.com> writes:\n\n> okie dokie.  is this good?\n\nNo.  The above line should be after \"---\".\n\n> From: Nick Edelen <sirnot@gmail.com>\n> Subject: [PATCH] Shift object enumeration out of upload-pack\n\nAnd these two are not necessary, as you are not forwarding somebody else's\nmail nor sending from an account that you do not to be recorded as the\nauthor (i.e. your RFC2822 \"From: \" header is the same as this one).\n\n> Offload object enumeration in upload-pack to pack-objects, but fall\n> back on internal revision walker for shallow interaction.  Test t5530\n> updated to reflect mechanism change.\n\nHere, \"to reflect mechanism change\" is not quite a good enough\ndescription.  Your explanation to Nico was much better.\n\n> Signed-off-by: Nick Edelen <sirnot@gmail.com>\n> Acked-by: Nicolas Pitre <nico@cam.org>\n\nBy the way, the proposed commit log message describes what the patch does\n(i.e. \"shifts enumeration from one process to the other\"), but does not\nanswer more important question.  Please say _why_ it is a good idea to do\nso in the message.\n\nThanks.\n"},{"id":"115970","messageId":"c77435a80906091650j7709d006j64749c9692c25b29@mail.gmail.com","threadId":"19731","inReplyTo":"7vvdn78n98.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] Shift object enumeration out of upload-pack","fromName":"Nick Edelen","fromEmail":"sirnot@gmail.com","sentAt":"2009-06-09T23:50:18Z","receivedAt":"2009-06-09T23:50:18Z","isPatch":true,"sender":{"key":"sirnot@gmail.com","avatar":null},"body":"Offload object enumeration in upload-pack to pack-objects, but fall\nback on internal revision walker for shallow interaction.   Aside from\narchitecturally making more sense, this also leaves the door open for\npack-objects to employ a revision cache mechanism.  Test t5530 updated\nin order to explicitly check both enumeration methods.\n\nSigned-off-by: Nick Edelen <sirnot@gmail.com>\nAcked-by: Nicolas Pitre <nico@cam.org>\n\n---\n err, I guess you wanted me to resubmit this?\n\n t/t5530-upload-pack-error.sh |   14 ++++++++++-\n upload-pack.c                |   49 +++++++++++++++++++++++++++++++++--------\n 2 files changed, 51 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh\nindex f5102b9..22eec24 100755\n--- a/t/t5530-upload-pack-error.sh\n+++ b/t/t5530-upload-pack-error.sh\n@@ -30,11 +30,12 @@ test_expect_success 'fsck fails' '\n \ttest_must_fail git fsck\n '\n\n-test_expect_success 'upload-pack fails due to error in pack-objects' '\n+test_expect_success 'upload-pack fails due to error in pack-objects packing' '\n\n \t! echo \"0032want $(git rev-parse HEAD)\n 00000009done\n 0000\" | git upload-pack . > /dev/null 2> output.err &&\n+\tgrep \"unable to read\" output.err &&\n \tgrep \"pack-objects died\" output.err\n '\n\n@@ -51,11 +52,20 @@ test_expect_success 'fsck fails' '\n test_expect_success 'upload-pack fails due to error in rev-list' '\n\n \t! echo \"0032want $(git rev-parse HEAD)\n-00000009done\n+0034shallow $(git rev-parse HEAD^)00000009done\n 0000\" | git upload-pack . > /dev/null 2> output.err &&\n \tgrep \"waitpid (async) failed\" output.err\n '\n\n+test_expect_success 'upload-pack fails due to error in pack-objects\nenumeration' '\n+\n+\t! echo \"0032want $(git rev-parse HEAD)\n+00000009done\n+0000\" | git upload-pack . > /dev/null 2> output.err &&\n+\tgrep \"bad tree object\" output.err &&\n+\tgrep \"pack-objects died\" output.err\n+'\n+\n test_expect_success 'create empty repository' '\n\n \tmkdir foo &&\ndiff --git a/upload-pack.c b/upload-pack.c\nindex edc7861..397cada 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -29,6 +29,7 @@ static unsigned long oldest_have;\n static int multi_ack, nr_our_refs;\n static int use_thin_pack, use_ofs_delta, use_include_tag;\n static int no_progress;\n+static int shallow_nr;\n static struct object_array have_obj;\n static struct object_array want_obj;\n static unsigned int timeout;\n@@ -107,8 +108,6 @@ static int do_rev_list(int fd, void *create_full_pack)\n \tstruct rev_info revs;\n\n \tpack_pipe = fdopen(fd, \"w\");\n-\tif (create_full_pack)\n-\t\tuse_thin_pack = 0; /* no point doing it */\n \tinit_revisions(&revs, NULL);\n \trevs.tag_objects = 1;\n \trevs.tree_objects = 1;\n@@ -155,13 +154,21 @@ static void create_pack_file(void)\n \tconst char *argv[10];\n \tint arg = 0;\n\n-\trev_list.proc = do_rev_list;\n-\t/* .data is just a boolean: any non-NULL value will do */\n-\trev_list.data = create_full_pack ? &rev_list : NULL;\n-\tif (start_async(&rev_list))\n-\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n+\tif (shallow_nr) {\n+\t\trev_list.proc = do_rev_list;\n+\t\trev_list.data = 0;\n+\t\tif (start_async(&rev_list))\n+\t\t\tdie(\"git upload-pack: unable to fork git-rev-list\");\n+\t\targv[arg++] = \"pack-objects\";\n+\t} else {\n+\t\targv[arg++] = \"pack-objects\";\n+\t\targv[arg++] = \"--revs\";\n+\t\tif (create_full_pack)\n+\t\t\targv[arg++] = \"--all\";\n+\t\telse if (use_thin_pack)\n+\t\t\targv[arg++] = \"--thin\";\n+\t}\n\n-\targv[arg++] = \"pack-objects\";\n \targv[arg++] = \"--stdout\";\n \tif (!no_progress)\n \t\targv[arg++] = \"--progress\";\n@@ -172,7 +179,7 @@ static void create_pack_file(void)\n \targv[arg++] = NULL;\n\n \tmemset(&pack_objects, 0, sizeof(pack_objects));\n-\tpack_objects.in = rev_list.out;\t/* start_command closes it */\n+\tpack_objects.in = shallow_nr ? rev_list.out : -1;\n \tpack_objects.out = -1;\n \tpack_objects.err = -1;\n \tpack_objects.git_cmd = 1;\n@@ -181,6 +188,24 @@ static void create_pack_file(void)\n \tif (start_command(&pack_objects))\n \t\tdie(\"git upload-pack: unable to fork git-pack-objects\");\n\n+\t/* pass on revisions we (don't) want */\n+\tif (!shallow_nr) {\n+\t\tFILE *pipe_fd = fdopen(pack_objects.in, \"w\");\n+\t\tif (!create_full_pack) {\n+\t\t\tint i;\n+\t\t\tfor (i = 0; i < want_obj.nr; i++)\n+\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(want_obj.objects[i].item->sha1));\n+\t\t\tfprintf(pipe_fd, \"--not\\n\");\n+\t\t\tfor (i = 0; i < have_obj.nr; i++)\n+\t\t\t\tfprintf(pipe_fd, \"%s\\n\", sha1_to_hex(have_obj.objects[i].item->sha1));\n+\t\t}\n+\n+\t\tfprintf(pipe_fd, \"\\n\");\n+\t\tfflush(pipe_fd);\n+\t\tfclose(pipe_fd);\n+\t}\n+\n+\n \t/* We read from pack_objects.err to capture stderr output for\n \t * progress bar, and pack_objects.out to capture the pack data.\n \t */\n@@ -276,7 +301,7 @@ static void create_pack_file(void)\n \t\terror(\"git upload-pack: git-pack-objects died with error.\");\n \t\tgoto fail;\n \t}\n-\tif (finish_async(&rev_list))\n+\tif (shallow_nr && finish_async(&rev_list))\n \t\tgoto fail;\t/* error was already reported */\n\n \t/* flush the data */\n@@ -451,6 +476,7 @@ static void receive_needs(void)\n \tstatic char line[1000];\n \tint len, depth = 0;\n\n+\tshallow_nr = 0;\n \tif (debug_fd)\n \t\twrite_in_full(debug_fd, \"#S\\n\", 3);\n \tfor (;;) {\n@@ -534,6 +560,7 @@ static void receive_needs(void)\n \t\t\t\tpacket_write(1, \"shallow %s\",\n \t\t\t\t\t\tsha1_to_hex(object->sha1));\n \t\t\t\tregister_shallow(object->sha1);\n+\t\t\t\tshallow_nr++;\n \t\t\t}\n \t\t\tresult = result->next;\n \t\t}\n@@ -567,6 +594,8 @@ static void receive_needs(void)\n \t\t\tfor (i = 0; i < shallows.nr; i++)\n \t\t\t\tregister_shallow(shallows.objects[i].item->sha1);\n \t\t}\n+\n+\tshallow_nr += shallows.nr;\n \tfree(shallows.objects);\n }\n\n-- \ntg: (503f464..) t/revcache/upload-pack-dont-enumerate (depends on: master)\n"},{"id":"115992","messageId":"7vr5xssggc.fsf@alter.siamese.dyndns.org","threadId":"19731","inReplyTo":"c77435a80906091650j7709d006j64749c9692c25b29@mail.gmail.com","subject":"Re: [PATCH] Shift object enumeration out of upload-pack","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-06-10T07:20:19Z","receivedAt":"2009-06-10T07:20:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nick Edelen <sirnot@gmail.com> writes:\n\n> Offload object enumeration in upload-pack to pack-objects, but fall\n> back on internal revision walker for shallow interaction.   Aside from\n> architecturally making more sense, this also leaves the door open for\n> pack-objects to employ a revision cache mechanism.  Test t5530 updated\n> in order to explicitly check both enumeration methods.\n>\n> Signed-off-by: Nick Edelen <sirnot@gmail.com>\n> Acked-by: Nicolas Pitre <nico@cam.org>\n>\n> ---\n>  err, I guess you wanted me to resubmit this?\n\nStrictly speaking, _I_ don't, but _you_ might ;-)\n\nI doubt that the proposed commit log message justifies the claim\n\"architecturally making more sense\" with concrete enough discussion, but\nI'll let it pass.\n\nYour log message got much better this time, by hinting that this is a\npreparatory step to introduce the rev-cache mechanism.  It is good way to\ndefend a patch saying that it is not just a useless code churn, but it is\na necessary step to get us closer to a more useful goal.\n\n> diff --git a/t/t5530-upload-pack-error.sh b/t/t5530-upload-pack-error.sh\n> index f5102b9..22eec24 100755\n> --- a/t/t5530-upload-pack-error.sh\n> +++ b/t/t5530-upload-pack-error.sh\n> ...\n> @@ -51,11 +52,20 @@ test_expect_success 'fsck fails' '\n>  test_expect_success 'upload-pack fails due to error in rev-list' '\n>\n>  \t! echo \"0032want $(git rev-parse HEAD)\n> -00000009done\n> +0034shallow $(git rev-parse HEAD^)00000009done\n>  0000\" | git upload-pack . > /dev/null 2> output.err &&\n>  \tgrep \"waitpid (async) failed\" output.err\n>  '\n>\n> +test_expect_success 'upload-pack fails due to error in pack-objects\n> enumeration' '\n\nYou have a wrapped line here.  Please check the setting of your MUA,\nespecially if you are planning to send more patches to the list in the\nfuture.\n\nNo need to resend; I fixed this up when applying.\n\nThanks.\n"}]}