{"thread":{"id":"24498","subject":"[PATCH] upload-pack: Remove some unused code","startedAt":"2010-07-24T20:09:11Z","lastAt":"2010-07-26T18:29:44Z","messageCount":3,"participants":["Elijah Newren","Junio C Hamano","Nicolas Pitre"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"146204","messageId":"1280002151-7527-1-git-send-email-newren@gmail.com","threadId":"24498","inReplyTo":null,"subject":"[PATCH] upload-pack: Remove some unused code","fromName":"Elijah Newren","fromEmail":"newren@gmail.com","sentAt":"2010-07-24T20:09:11Z","receivedAt":"2010-07-24T20:09:11Z","isPatch":true,"sender":{"key":"newren@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5455730?v=4"},"body":"Signed-off-by: Elijah Newren <newren@gmail.com>\n---\nI'm not sure if this is too trivial to submit, but I noticed it while\npoking around in upload-pack.\n\nEver since f0cea83f631689331fce73b51f22707e897f7939, the\n'create_full_pack' argument has been unused and is always false.  We\ncan make it clear that this argument is currently unused and remove some\nassociated code from an 'if' block.\n\n upload-pack.c |   29 ++++++++++++-----------------\n 1 files changed, 12 insertions(+), 17 deletions(-)\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex dc464d7..c2e4eae 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -105,7 +105,7 @@ static void show_edge(struct commit *commit)\n \tfprintf(pack_pipe, \"-%s\\n\", sha1_to_hex(commit->object.sha1));\n }\n \n-static int do_rev_list(int in, int out, void *create_full_pack)\n+static int do_rev_list(int in, int out, void *unused)\n {\n \tint i;\n \tstruct rev_info revs;\n@@ -118,23 +118,18 @@ static int do_rev_list(int in, int out, void *create_full_pack)\n \tif (use_thin_pack)\n \t\trevs.edge_hint = 1;\n \n-\tif (create_full_pack) {\n-\t\tconst char *args[] = {\"rev-list\", \"--all\", NULL};\n-\t\tsetup_revisions(2, args, &revs, NULL);\n-\t} else {\n-\t\tfor (i = 0; i < want_obj.nr; i++) {\n-\t\t\tstruct object *o = want_obj.objects[i].item;\n-\t\t\t/* why??? */\n-\t\t\to->flags &= ~UNINTERESTING;\n-\t\t\tadd_pending_object(&revs, o, NULL);\n-\t\t}\n-\t\tfor (i = 0; i < have_obj.nr; i++) {\n-\t\t\tstruct object *o = have_obj.objects[i].item;\n-\t\t\to->flags |= UNINTERESTING;\n-\t\t\tadd_pending_object(&revs, o, NULL);\n-\t\t}\n-\t\tsetup_revisions(0, NULL, &revs, NULL);\n+\tfor (i = 0; i < want_obj.nr; i++) {\n+\t\tstruct object *o = want_obj.objects[i].item;\n+\t\t/* why??? */\n+\t\to->flags &= ~UNINTERESTING;\n+\t\tadd_pending_object(&revs, o, NULL);\n+\t}\n+\tfor (i = 0; i < have_obj.nr; i++) {\n+\t\tstruct object *o = have_obj.objects[i].item;\n+\t\to->flags |= UNINTERESTING;\n+\t\tadd_pending_object(&revs, o, NULL);\n \t}\n+\tsetup_revisions(0, NULL, &revs, NULL);\n \tif (prepare_revision_walk(&revs))\n \t\tdie(\"revision walk setup failed\");\n \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n-- \n1.7.2.1.g83744\n"},{"id":"146344","messageId":"7vr5iqaje0.fsf@alter.siamese.dyndns.org","threadId":"24498","inReplyTo":"1280002151-7527-1-git-send-email-newren@gmail.com","subject":"Re: [PATCH] upload-pack: Remove some unused code","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-07-26T05:42:47Z","receivedAt":"2010-07-26T05:42:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Elijah Newren <newren@gmail.com> writes:\n\n> Signed-off-by: Elijah Newren <newren@gmail.com>\n> ---\n> I'm not sure if this is too trivial to submit, but I noticed it while\n> poking around in upload-pack.\n>\n> Ever since f0cea83f631689331fce73b51f22707e897f7939, the\n> 'create_full_pack' argument has been unused and is always false.  We\n> can make it clear that this argument is currently unused and remove some\n> associated code from an 'if' block.\n\nWhile it is true that currently we memset(&rev_list) to always pass NULL\nthere, I wonder if that was an unintended behaviour change introduced by\nthe commit you mentioned...\n\n>  upload-pack.c |   29 ++++++++++++-----------------\n>  1 files changed, 12 insertions(+), 17 deletions(-)\n>\n> diff --git a/upload-pack.c b/upload-pack.c\n> index dc464d7..c2e4eae 100644\n> --- a/upload-pack.c\n> +++ b/upload-pack.c\n> @@ -105,7 +105,7 @@ static void show_edge(struct commit *commit)\n>  \tfprintf(pack_pipe, \"-%s\\n\", sha1_to_hex(commit->object.sha1));\n>  }\n>  \n> -static int do_rev_list(int in, int out, void *create_full_pack)\n> +static int do_rev_list(int in, int out, void *unused)\n>  {\n>  \tint i;\n>  \tstruct rev_info revs;\n> @@ -118,23 +118,18 @@ static int do_rev_list(int in, int out, void *create_full_pack)\n>  \tif (use_thin_pack)\n>  \t\trevs.edge_hint = 1;\n>  \n> -\tif (create_full_pack) {\n> -\t\tconst char *args[] = {\"rev-list\", \"--all\", NULL};\n> -\t\tsetup_revisions(2, args, &revs, NULL);\n> -\t} else {\n> -\t\tfor (i = 0; i < want_obj.nr; i++) {\n> -\t\t\tstruct object *o = want_obj.objects[i].item;\n> -\t\t\t/* why??? */\n> -\t\t\to->flags &= ~UNINTERESTING;\n> -\t\t\tadd_pending_object(&revs, o, NULL);\n> -\t\t}\n> -\t\tfor (i = 0; i < have_obj.nr; i++) {\n> -\t\t\tstruct object *o = have_obj.objects[i].item;\n> -\t\t\to->flags |= UNINTERESTING;\n> -\t\t\tadd_pending_object(&revs, o, NULL);\n> -\t\t}\n> -\t\tsetup_revisions(0, NULL, &revs, NULL);\n> +\tfor (i = 0; i < want_obj.nr; i++) {\n> +\t\tstruct object *o = want_obj.objects[i].item;\n> +\t\t/* why??? */\n> +\t\to->flags &= ~UNINTERESTING;\n> +\t\tadd_pending_object(&revs, o, NULL);\n> +\t}\n> +\tfor (i = 0; i < have_obj.nr; i++) {\n> +\t\tstruct object *o = have_obj.objects[i].item;\n> +\t\to->flags |= UNINTERESTING;\n> +\t\tadd_pending_object(&revs, o, NULL);\n>  \t}\n> +\tsetup_revisions(0, NULL, &revs, NULL);\n>  \tif (prepare_revision_walk(&revs))\n>  \t\tdie(\"revision walk setup failed\");\n>  \tmark_edges_uninteresting(revs.commits, &revs, show_edge);\n> -- \n> 1.7.2.1.g83744\n"},{"id":"146410","messageId":"alpine.LFD.2.00.1007261424490.30890@xanadu.home","threadId":"24498","inReplyTo":"7vr5iqaje0.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] upload-pack: Remove some unused code","fromName":"Nicolas Pitre","fromEmail":"nico@fluxnic.net","sentAt":"2010-07-26T18:29:44Z","receivedAt":"2010-07-26T18:29:44Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 25 Jul 2010, Junio C Hamano wrote:\n\n> Elijah Newren <newren@gmail.com> writes:\n> \n> > Signed-off-by: Elijah Newren <newren@gmail.com>\n> > ---\n> > I'm not sure if this is too trivial to submit, but I noticed it while\n> > poking around in upload-pack.\n> >\n> > Ever since f0cea83f631689331fce73b51f22707e897f7939, the\n> > 'create_full_pack' argument has been unused and is always false.  We\n> > can make it clear that this argument is currently unused and remove some\n> > associated code from an 'if' block.\n> \n> While it is true that currently we memset(&rev_list) to always pass NULL\n> there, I wonder if that was an unintended behaviour change introduced by\n> the commit you mentioned...\n\nI don't think so.  Especially since now do_rev_list() is only used for \nshallow clones/fetches, there can never be a full pack in that case.  \nThe create_full_pack logic remans present and local to \ncreate_pack_file().\n\n\nNicolas\n"}]}