# [PATCH] upload-pack: Remove some unused code 3 messages from 2010-07-24 to 2010-07-26. Participants: Elijah Newren, Junio C Hamano, Nicolas Pitre. Thread: https://gitlist.dev/t/24498 ## Elijah Newren, 2010-07-24 20:09 Subject: [PATCH] upload-pack: Remove some unused code Message-ID: <1280002151-7527-1-git-send-email-newren@gmail.com> URL: https://gitlist.dev/e/1280002151-7527-1-git-send-email-newren%40gmail.com ``` Signed-off-by: Elijah Newren --- I'm not sure if this is too trivial to submit, but I noticed it while poking around in upload-pack. Ever since f0cea83f631689331fce73b51f22707e897f7939, the 'create_full_pack' argument has been unused and is always false. We can make it clear that this argument is currently unused and remove some associated code from an 'if' block. upload-pack.c | 29 ++++++++++++----------------- 1 files changed, 12 insertions(+), 17 deletions(-) diff --git a/upload-pack.c b/upload-pack.c index dc464d7..c2e4eae 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -105,7 +105,7 @@ static void show_edge(struct commit *commit) fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1)); } -static int do_rev_list(int in, int out, void *create_full_pack) +static int do_rev_list(int in, int out, void *unused) { int i; struct rev_info revs; @@ -118,23 +118,18 @@ static int do_rev_list(int in, int out, void *create_full_pack) if (use_thin_pack) revs.edge_hint = 1; - if (create_full_pack) { - const char *args[] = {"rev-list", "--all", NULL}; - setup_revisions(2, args, &revs, NULL); - } else { - for (i = 0; i < want_obj.nr; i++) { - struct object *o = want_obj.objects[i].item; - /* why??? */ - o->flags &= ~UNINTERESTING; - add_pending_object(&revs, o, NULL); - } - for (i = 0; i < have_obj.nr; i++) { - struct object *o = have_obj.objects[i].item; - o->flags |= UNINTERESTING; - add_pending_object(&revs, o, NULL); - } - setup_revisions(0, NULL, &revs, NULL); + for (i = 0; i < want_obj.nr; i++) { + struct object *o = want_obj.objects[i].item; + /* why??? */ + o->flags &= ~UNINTERESTING; + add_pending_object(&revs, o, NULL); + } + for (i = 0; i < have_obj.nr; i++) { + struct object *o = have_obj.objects[i].item; + o->flags |= UNINTERESTING; + add_pending_object(&revs, o, NULL); } + setup_revisions(0, NULL, &revs, NULL); if (prepare_revision_walk(&revs)) die("revision walk setup failed"); mark_edges_uninteresting(revs.commits, &revs, show_edge); -- 1.7.2.1.g83744 ``` ## Junio C Hamano, 2010-07-26 05:42 Subject: Re: [PATCH] upload-pack: Remove some unused code Message-ID: <7vr5iqaje0.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr5iqaje0.fsf%40alter.siamese.dyndns.org In-Reply-To: <1280002151-7527-1-git-send-email-newren@gmail.com> ``` Elijah Newren writes: > Signed-off-by: Elijah Newren > --- > I'm not sure if this is too trivial to submit, but I noticed it while > poking around in upload-pack. > > Ever since f0cea83f631689331fce73b51f22707e897f7939, the > 'create_full_pack' argument has been unused and is always false. We > can make it clear that this argument is currently unused and remove some > associated code from an 'if' block. While it is true that currently we memset(&rev_list) to always pass NULL there, I wonder if that was an unintended behaviour change introduced by the commit you mentioned... > upload-pack.c | 29 ++++++++++++----------------- > 1 files changed, 12 insertions(+), 17 deletions(-) > > diff --git a/upload-pack.c b/upload-pack.c > index dc464d7..c2e4eae 100644 > --- a/upload-pack.c > +++ b/upload-pack.c > @@ -105,7 +105,7 @@ static void show_edge(struct commit *commit) > fprintf(pack_pipe, "-%s\n", sha1_to_hex(commit->object.sha1)); > } > > -static int do_rev_list(int in, int out, void *create_full_pack) > +static int do_rev_list(int in, int out, void *unused) > { > int i; > struct rev_info revs; > @@ -118,23 +118,18 @@ static int do_rev_list(int in, int out, void *create_full_pack) > if (use_thin_pack) > revs.edge_hint = 1; > > - if (create_full_pack) { > - const char *args[] = {"rev-list", "--all", NULL}; > - setup_revisions(2, args, &revs, NULL); > - } else { > - for (i = 0; i < want_obj.nr; i++) { > - struct object *o = want_obj.objects[i].item; > - /* why??? */ > - o->flags &= ~UNINTERESTING; > - add_pending_object(&revs, o, NULL); > - } > - for (i = 0; i < have_obj.nr; i++) { > - struct object *o = have_obj.objects[i].item; > - o->flags |= UNINTERESTING; > - add_pending_object(&revs, o, NULL); > - } > - setup_revisions(0, NULL, &revs, NULL); > + for (i = 0; i < want_obj.nr; i++) { > + struct object *o = want_obj.objects[i].item; > + /* why??? */ > + o->flags &= ~UNINTERESTING; > + add_pending_object(&revs, o, NULL); > + } > + for (i = 0; i < have_obj.nr; i++) { > + struct object *o = have_obj.objects[i].item; > + o->flags |= UNINTERESTING; > + add_pending_object(&revs, o, NULL); > } > + setup_revisions(0, NULL, &revs, NULL); > if (prepare_revision_walk(&revs)) > die("revision walk setup failed"); > mark_edges_uninteresting(revs.commits, &revs, show_edge); > -- > 1.7.2.1.g83744 ``` ## Nicolas Pitre, 2010-07-26 18:29 Subject: Re: [PATCH] upload-pack: Remove some unused code Message-ID: URL: https://gitlist.dev/e/alpine.LFD.2.00.1007261424490.30890%40xanadu.home In-Reply-To: <7vr5iqaje0.fsf@alter.siamese.dyndns.org> ``` On Sun, 25 Jul 2010, Junio C Hamano wrote: > Elijah Newren writes: > > > Signed-off-by: Elijah Newren > > --- > > I'm not sure if this is too trivial to submit, but I noticed it while > > poking around in upload-pack. > > > > Ever since f0cea83f631689331fce73b51f22707e897f7939, the > > 'create_full_pack' argument has been unused and is always false. We > > can make it clear that this argument is currently unused and remove some > > associated code from an 'if' block. > > While it is true that currently we memset(&rev_list) to always pass NULL > there, I wonder if that was an unintended behaviour change introduced by > the commit you mentioned... I don't think so. Especially since now do_rev_list() is only used for shallow clones/fetches, there can never be a full pack in that case. The create_full_pack logic remans present and local to create_pack_file(). Nicolas ```