{"thread":{"id":"2156","subject":"[PATCH] Do not send \"want\" lines for complete objects","startedAt":"2005-10-19T22:07:41Z","lastAt":"2005-10-21T15:44:14Z","messageCount":11,"participants":["Johannes Schindelin","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"10309","messageId":"Pine.LNX.4.63.0510200004290.7689@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2156","inReplyTo":null,"subject":"[PATCH] Do not send \"want\" lines for complete objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-10-19T22:07:41Z","receivedAt":"2005-10-19T22:07:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"It was all good and well to check if all remote refs are complete (local \nrefs or descendants thereof), but we can just as easily use the same \ninformation to avoid sending \"want\" lines just for the complete objects in \nthe case that not all remote refs are complete (or their names differ).\n\nAlso, git-fetch-pack does not have to ask for descendants of remote refs \nwhich are complete (for now, git-rev-list is told to ignore only the first \nparent). That change also eliminates a code path where a popen()ed handle \nwas not pclose()ed.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tWith this patch, in a particular setup, git-fetch got 805 objects \n\tinstead of 8404, i.e. 10 times less! This setup involves 691 \n\ttags...\n\n\tNOTE: I'd rather have git-rev-list circumvented, so that \n\tdescendants of ack'ed objects are not sent via \"have\" lines.\n\n\t2nd NOTE: If this patch reminds you of my \"has_ref()\" patch: Yes,\n\tit is the next generation of that idea.\n\n fetch-pack.c | 33 +++++++++++++++++++++++++--------\n 1 files changed, 25 insertions(+), 8 deletions(-)\n\napplies-to: f76de7f266b0b7feb38c73c7ccb635d6da18a582\nf22af070becd6546eeff44895769b8299a6ddb5e\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 969e72a..9dfd072 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -12,31 +12,49 @@ static const char fetch_pack_usage[] =\n \"git-fetch-pack [-q] [-v] [--exec=upload-pack] [host:]directory ...\";\n static const char *exec = \"git-upload-pack\";\n \n+#define COMPLETE\t(1U << 0)\n+\n static int find_common(int fd[2], unsigned char *result_sha1,\n \t\t struct ref *refs)\n {\n \tint fetching;\n \tstatic char line[1000];\n-\tint count = 0, flushes = 0, retval;\n+\tstatic char rev_command[1024];\n+\tint count = 0, flushes = 0, retval, rev_command_len;\n \tFILE *revs;\n \n-\trevs = popen(\"git-rev-list $(git-rev-parse --all)\", \"r\");\n-\tif (!revs)\n-\t\tdie(\"unable to run 'git-rev-list'\");\n-\n+\tstrcpy(rev_command, \"git-rev-list $(git-rev-parse --all)\");\n+\trev_command_len = strlen(rev_command);\n \tfetching = 0;\n \tfor ( ; refs ; refs = refs->next) {\n \t\tunsigned char *remote = refs->old_sha1;\n-\t\tunsigned char *local = refs->new_sha1;\n \n-\t\tif (!memcmp(remote, local, 20))\n+\t\t/*\n+\t\t If that object is complete (i.e. it is a descendant of a\n+\t\t local ref), we don't want it, nor its descendants.\n+\t\t*/\n+\t\tif (has_sha1_file(remote)\n+\t\t\t\t&& parse_object(remote)->flags & COMPLETE) {\n+\t\t\tif (rev_command_len + 44 < sizeof(rev_command)) {\n+\t\t\t\tsnprintf(rev_command + rev_command_len, 44,\n+\t\t\t\t\t\" ^%s^\", sha1_to_hex(remote));\n+\t\t\t\trev_command_len += 43;\n+\t\t\t}\n+\n \t\t\tcontinue;\n+\t\t}\n+\n \t\tpacket_write(fd[1], \"want %s\\n\", sha1_to_hex(remote));\n \t\tfetching++;\n \t}\n \tpacket_flush(fd[1]);\n \tif (!fetching)\n \t\treturn 1;\n+\n+\trevs = popen(rev_command, \"r\");\n+\tif (!revs)\n+\t\tdie(\"unable to run 'git-rev-list'\");\n+\n \tflushes = 1;\n \tretval = -1;\n \twhile (fgets(line, sizeof(line), revs) != NULL) {\n@@ -81,7 +99,6 @@ static int find_common(int fd[2], unsign\n \treturn retval;\n }\n \n-#define COMPLETE\t(1U << 0)\n static struct commit_list *complete = NULL;\n \n static int mark_complete(const char *path, const unsigned char *sha1)\n---\n0.99.8.GIT\n"},{"id":"10315","messageId":"7vzmp5xfwu.fsf@assigned-by-dhcp.cox.net","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510200004290.7689@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-19T23:09:37Z","receivedAt":"2005-10-19T23:09:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Also, git-fetch-pack does not have to ask for descendants of remote refs \n> which are complete (for now, git-rev-list is told to ignore only the first \n> parent).\n\nI should interpret what you said with s/descendant/ancestor/\napplied, I think. I would like to make sure I understand what\nyou are doing here.\n\n> +\t\tif (has_sha1_file(remote)\n> +\t\t\t\t&& parse_object(remote)->flags & COMPLETE) {\n> +\t\t\tif (rev_command_len + 44 < sizeof(rev_command)) {\n> +\t\t\t\tsnprintf(rev_command + rev_command_len, 44,\n> +\t\t\t\t\t\" ^%s^\", sha1_to_hex(remote));\n> +\t\t\t\trev_command_len += 43;\n> +\t\t\t}\n\nThis rev-list command is to generate the list of \"have\", and we\nlearned that the other side says she has remote -- we choose not\nto tell her that we have ancestors of it, but we do tell her\nabout the remote head itself.\n\nLet's draw a single strand of pearl case to illustrate. You\nhave a commit chain A->B->C->D, and the other end says she has\nC. At this point, telling her that you have C is enough, and by\nnot telling her about A and B, you would save her from depreting\nMAX_HAVE slots. Of course, if the other end has D as another\nhead, then the above logic would give \"^D^\" to rev-list as well,\ntelling it not to tell her about C, but that is what we want --\nbecause she already knows you have C too when you tell her that\nyou have D. I think I like this optimization.\n\nOne thing that might help, when we are telling the other end\nabout what we have, is an output ordering option to get-rev-list\nthat shows not in chronological order, but in the order of\ndistance from the tip. That may give the other end a better\nchance to find the latest (in commit order) common commit in\neach branch without running out is MAX_HAS buffer.\n\n> \tNOTE: I'd rather have git-rev-list circumvented, so that \n> \tdescendants of ack'ed objects are not sent via \"have\" lines.\n\nI think since we do the traversal ourselves this may start to\nmake more sense.\n"},{"id":"10324","messageId":"7vwtk9vvhg.fsf@assigned-by-dhcp.cox.net","threadId":"2156","inReplyTo":"7vzmp5xfwu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-20T01:16:11Z","receivedAt":"2005-10-20T01:16:11Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Let's draw a single strand of pearl case to illustrate. You\n> have a commit chain A->B->C->D, and the other end says she has\n> C. At this point, telling her that you have C is enough, and by\n> not telling her about A and B, you would save her from depreting\n> MAX_HAVE slots. Of course, if the other end has D as another\n> head, then the above logic would give \"^D^\" to rev-list as well,\n> telling it not to tell her about C, but that is what we want --\n> because she already knows you have C too when you tell her that\n> you have D. I think I like this optimization.\n\nThis was subtly wrong. ^D^ would barf if D is a tag that points\nat a non commit (refs/tags/v2.6.11-tree). Also it would do a\nsuboptimal thing for a merge commit, since it will not cull the\nsecond and later parents.\n\nMaybe something like this on top of your patch? This is turning\nout to be quite ugly.\n\n---\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex 9dfd072..5cc3766 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -28,20 +28,29 @@ static int find_common(int fd[2], unsign\n \tfetching = 0;\n \tfor ( ; refs ; refs = refs->next) {\n \t\tunsigned char *remote = refs->old_sha1;\n-\n+\t\tstruct object *o;\n+\t\tstruct commit *commit;\n \t\t/*\n-\t\t If that object is complete (i.e. it is a descendant of a\n-\t\t local ref), we don't want it, nor its descendants.\n-\t\t*/\n-\t\tif (has_sha1_file(remote)\n-\t\t\t\t&& parse_object(remote)->flags & COMPLETE) {\n-\t\t\tif (rev_command_len + 44 < sizeof(rev_command)) {\n+\t\t * If that object is complete (i.e. it is an ancestor of a\n+\t\t * local ref), we tell them we have it but do not have to\n+\t\t * tell them about its ancestors, which they already know\n+\t\t * about.\n+\t\t */\n+\t\tif (has_sha1_file(remote) &&\n+\t\t ((o = parse_object(remote)) != NULL) &&\n+\t\t (o->flags & COMPLETE) &&\n+\t\t ((commit = (struct commit *) deref_tag(o)) != NULL) &&\n+\t\t (commit->object.type = commit_type)) {\n+\t\t\tstruct commit_list *p = commit->parents;\n+\t\t\twhile (p && rev_command_len + 44 < sizeof(rev_command)) {\n \t\t\t\tsnprintf(rev_command + rev_command_len, 44,\n-\t\t\t\t\t\" ^%s^\", sha1_to_hex(remote));\n+\t\t\t\t\t \" ^%s\",\n+\t\t\t\t\t sha1_to_hex(p->item->object.sha1));\n \t\t\t\trev_command_len += 43;\n+\t\t\t\tp = p->next;\n \t\t\t}\n-\n-\t\t\tcontinue;\n+\t\t\tif (!p)\n+\t\t\t\tcontinue;\n \t\t}\n \n \t\tpacket_write(fd[1], \"want %s\\n\", sha1_to_hex(remote));\n"},{"id":"10329","messageId":"Pine.LNX.4.63.0510200348410.26594@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2156","inReplyTo":"7vzmp5xfwu.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-10-20T01:51:18Z","receivedAt":"2005-10-20T01:51:18Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 19 Oct 2005, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > Also, git-fetch-pack does not have to ask for descendants of remote refs \n> > which are complete (for now, git-rev-list is told to ignore only the first \n> > parent).\n> \n> I should interpret what you said with s/descendant/ancestor/\n> applied, I think. I would like to make sure I understand what\n> you are doing here.\n\nSure. You are thinking along chronological lines. Okay.\n\n> > +\t\tif (has_sha1_file(remote)\n> > +\t\t\t\t&& parse_object(remote)->flags & COMPLETE) {\n> > +\t\t\tif (rev_command_len + 44 < sizeof(rev_command)) {\n> > +\t\t\t\tsnprintf(rev_command + rev_command_len, 44,\n> > +\t\t\t\t\t\" ^%s^\", sha1_to_hex(remote));\n> > +\t\t\t\trev_command_len += 43;\n> > +\t\t\t}\n> \n> This rev-list command is to generate the list of \"have\", and we\n> learned that the other side says she has remote -- we choose not\n> to tell her that we have ancestors of it, but we do tell her\n> about the remote head itself.\n> \n> Let's draw a single strand of pearl case to illustrate. You\n> have a commit chain A->B->C->D, and the other end says she has\n> C. At this point, telling her that you have C is enough, and by\n> not telling her about A and B, you would save her from depreting\n> MAX_HAVE slots. Of course, if the other end has D as another\n> head, then the above logic would give \"^D^\" to rev-list as well,\n> telling it not to tell her about C, but that is what we want --\n> because she already knows you have C too when you tell her that\n> you have D. I think I like this optimization.\n> \n> One thing that might help, when we are telling the other end\n> about what we have, is an output ordering option to get-rev-list\n> that shows not in chronological order, but in the order of\n> distance from the tip. That may give the other end a better\n> chance to find the latest (in commit order) common commit in\n> each branch without running out is MAX_HAS buffer.\n\nThat was my idea.\n\nCiao,\nDscho\n"},{"id":"10330","messageId":"Pine.LNX.4.63.0510200351260.26594@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2156","inReplyTo":"7vwtk9vvhg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-10-20T02:04:20Z","receivedAt":"2005-10-20T02:04:20Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 19 Oct 2005, Junio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Let's draw a single strand of pearl case to illustrate. You\n> > have a commit chain A->B->C->D, and the other end says she has\n> > C. At this point, telling her that you have C is enough, and by\n> > not telling her about A and B, you would save her from depreting\n> > MAX_HAVE slots. Of course, if the other end has D as another\n> > head, then the above logic would give \"^D^\" to rev-list as well,\n> > telling it not to tell her about C, but that is what we want --\n> > because she already knows you have C too when you tell her that\n> > you have D. I think I like this optimization.\n> \n> This was subtly wrong. ^D^ would barf if D is a tag that points\n> at a non commit (refs/tags/v2.6.11-tree). Also it would do a\n> suboptimal thing for a merge commit, since it will not cull the\n> second and later parents.\n\nRight, I keep forgetting about tags. And again right, like I said, it is \nsuboptimal but helps the common case.\n\n> +\t\tif (has_sha1_file(remote) &&\n> +\t\t ((o = parse_object(remote)) != NULL) &&\n> +\t\t (o->flags & COMPLETE) &&\n\nWhy not split it here, and do a separate block here:\n\n> +\t\t ((commit = (struct commit *) deref_tag(o)) != NULL) &&\n> +\t\t (commit->object.type = commit_type)) {\n> +\t\t\tstruct commit_list *p = commit->parents;\n> +\t\t\twhile (p && rev_command_len + 44 < sizeof(rev_command)) {\n> \t\t\t\tsnprintf(rev_command + rev_command_len, 44,\n> -\t\t\t\t\t\" ^%s^\", sha1_to_hex(remote));\n> +\t\t\t\t\t \" ^%s\",\n> +\t\t\t\t\t sha1_to_hex(p->item->object.sha1));\n> \t\t\t\trev_command_len += 43;\n> +\t\t\t\tp = p->next;\n> \t\t\t}\n\nAnd here I am at a loss: why only continue if p is empty? I mean, remote \ncould be a tag, and still be complete, no?\n\n> -\n> -\t\t\tcontinue;\n> +\t\t\tif (!p)\n> +\t\t\t\tcontinue;\n> \t\t}\n> \n> \t\tpacket_write(fd[1], \"want %s\\n\", sha1_to_hex(remote));\n> \n\nIf the git-rev-list call goes, this probably gets prettier. I'll try to \ncome up with a patch.\n\nCiao,\nDscho\n"},{"id":"10332","messageId":"Pine.LNX.4.63.0510200559540.3394@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510200351260.26594@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-10-20T04:08:48Z","receivedAt":"2005-10-20T04:08:48Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\n> > +\t\t ((o = parse_object(remote)) != NULL) &&\n> > +\t\t (o->flags & COMPLETE) &&\n\nI just realized that parse_object() always reads the file, then does a \nlookup (which makes the above code work), and then parses the file. It \nalways does all of these steps, even if the object was already parsed. Any \nreason for this?\n\nCiao,\nDscho\n"},{"id":"10333","messageId":"7v8xwovn5d.fsf@assigned-by-dhcp.cox.net","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510200559540.3394@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2005-10-20T04:16:14Z","receivedAt":"2005-10-20T04:16:14Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> Hi,\n>\n>> > +\t\t ((o = parse_object(remote)) != NULL) &&\n>> > +\t\t (o->flags & COMPLETE) &&\n>\n> I just realized that parse_object() always reads the file, then does a \n> lookup (which makes the above code work), and then parses the file. It \n> always does all of these steps, even if the object was already parsed. Any \n> reason for this?\n\nYou are right. We should be using lookup_object() for this part\nof the code.\n"},{"id":"10370","messageId":"Pine.LNX.4.64.0510201630000.25300@iabervon.org","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510200559540.3394@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-10-20T20:32:08Z","receivedAt":"2005-10-20T20:32:08Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 20 Oct 2005, Johannes Schindelin wrote:\n\n> Hi,\n> \n> > > +\t\t ((o = parse_object(remote)) != NULL) &&\n> > > +\t\t (o->flags & COMPLETE) &&\n> \n> I just realized that parse_object() always reads the file, then does a \n> lookup (which makes the above code work), and then parses the file. It \n> always does all of these steps, even if the object was already parsed. Any \n> reason for this?\n\nI'm lazy and haven't sent in a patch to clean that up. There's no reason \nit couldn't check whether the value it gets is already parsed.\n\n\t-Daniel\n*This .sig left intentionally blank*\n"},{"id":"10375","messageId":"Pine.LNX.4.63.0510210114370.10503@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"2156","inReplyTo":"Pine.LNX.4.64.0510201630000.25300@iabervon.org","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2005-10-20T23:16:58Z","receivedAt":"2005-10-20T23:16:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 20 Oct 2005, Daniel Barkalow wrote:\n\n> On Thu, 20 Oct 2005, Johannes Schindelin wrote:\n> \n> > Hi,\n> > \n> > > > +\t\t ((o = parse_object(remote)) != NULL) &&\n> > > > +\t\t (o->flags & COMPLETE) &&\n> > \n> > I just realized that parse_object() always reads the file, then does a \n> > lookup (which makes the above code work), and then parses the file. It \n> > always does all of these steps, even if the object was already parsed. Any \n> > reason for this?\n> \n> I'm lazy and haven't sent in a patch to clean that up. There's no reason \n> it couldn't check whether the value it gets is already parsed.\n\nActually, you don't have to... Junio already replaced parse_object() by \nlookup_object() in this case. I did not use it originally, because lines \n24-25 of commit.h say\n\n\t/** Internal only **/\n\tstruct object *lookup_object(const unsigned char *sha1);\n\nIs this obsolete?\n\nCiao,\nDscho\n"},{"id":"10391","messageId":"loom.20051021T024100-352@post.gmane.org","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510210114370.10503@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Junio C Hamano","fromEmail":"junkio@twinsun.com","sentAt":"2005-10-21T00:43:04Z","receivedAt":"2005-10-21T00:43:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin gmx.de> writes:\n\n> ... I did not use it originally, because lines \n> 24-25 of commit.h say\n> \n> \t/** Internal only **/\n> \tstruct object *lookup_object(const unsigned char *sha1);\n> \n> Is this obsolete?\n\nFor this particular application, it should be OK, because:\n\n - it checks if we already have that object, and returns a pointer to it;\n - if we haven't seen that object, we get a NULL back;\n\nand\n\n - what we care about is to see if we have checked and verified if it is\n reachable from our refs. We do not want the check to actively go read\n and parse that object.\n\n-jc\n"},{"id":"10429","messageId":"Pine.LNX.4.64.0510211139590.25300@iabervon.org","threadId":"2156","inReplyTo":"Pine.LNX.4.63.0510210114370.10503@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] Do not send \"want\" lines for complete objects","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2005-10-21T15:44:14Z","receivedAt":"2005-10-21T15:44:14Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Fri, 21 Oct 2005, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Thu, 20 Oct 2005, Daniel Barkalow wrote:\n> \n> > On Thu, 20 Oct 2005, Johannes Schindelin wrote:\n> > \n> > > Hi,\n> > > \n> > > > > +\t\t ((o = parse_object(remote)) != NULL) &&\n> > > > > +\t\t (o->flags & COMPLETE) &&\n> > > \n> > > I just realized that parse_object() always reads the file, then does a \n> > > lookup (which makes the above code work), and then parses the file. It \n> > > always does all of these steps, even if the object was already parsed. Any \n> > > reason for this?\n> > \n> > I'm lazy and haven't sent in a patch to clean that up. There's no reason \n> > it couldn't check whether the value it gets is already parsed.\n> \n> Actually, you don't have to... Junio already replaced parse_object() by \n> lookup_object() in this case.\n\nIt often gets used in cases where it isn't completely obvious that the \nobject has already been parsed; this is the second place that it's been \nreplaced.\n\n> I did not use it originally, because lines 24-25 of commit.h say\n> \n> \t/** Internal only **/\n> \tstruct object *lookup_object(const unsigned char *sha1);\n> \n> Is this obsolete?\n\nI'd intended it to be internal, for use by things like lookup_tree(), but \nI suppose that it could be used more generally, if it got a comment that \nsays that, unlike lookup_tree() and such, it returns NULL if the object \nhas not already been read (because it doesn't know how to create it).\n\n\t-Daniel\n*This .sig left intentionally blank*\n"}]}