{"thread":{"id":"8353","subject":"Re: [PATCH] rev-parse: Identify short sha1 sums correctly.","startedAt":"2007-05-29T23:40:37Z","lastAt":"2007-05-30T02:02:36Z","messageCount":3,"participants":["Junio C Hamano","Shawn O. Pearce"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"43589","messageId":"7vabvn5hca.fsf@assigned-by-dhcp.cox.net","threadId":"8353","inReplyTo":"1180478596243-git-send-email-jbowes@dangerouslyinc.com","subject":"Re: [PATCH] rev-parse: Identify short sha1 sums correctly.","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-29T23:40:37Z","receivedAt":"2007-05-29T23:40:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"James Bowes writes:\n\n> find_short_packed_object was not loading the pack index files.\n> Teach it to do so.\n>\n> Signed-off-by: James Bowes \n\nI think this is the proper fix of the problem I was unhappy\nabout with 'next', rather than reverting the lazy index\nloading. But I wonder how many _other_ places like this there\nare that we might be missing...\n\nShawn, an Ack, and any ideas for futureproofing?\n\n> sha1_name.c | 7 +++++--\n> 1 files changed, 5 insertions(+), 2 deletions(-)\n>\n> diff --git a/sha1_name.c b/sha1_name.c\n> index 8dfceb2..7df01af 100644\n> --- a/sha1_name.c\n> +++ b/sha1_name.c\n> @@ -76,8 +76,11 @@ static int find_short_packed_object(int len, const unsigned char *match, unsigne\n> \n> \tprepare_packed_git();\n> \tfor (p = packed_git; p && found < 2; p = p->next) {\n> -\t\tuint32_t num = p->num_objects;\n> -\t\tuint32_t first = 0, last = num;\n> +\t\tuint32_t num, last;\n> +\t\tuint32_t first = 0;\n> +\t\topen_pack_index(p);\n> +\t\tnum = p->num_objects;\n> +\t\tlast = num;\n> \t\twhile (first < last) {\n> \t\t\tuint32_t mid = (first + last) / 2;\n> \t\t\tconst unsigned char *now;\n> -- \n> 1.5.2.869.g6b3ba\n"},{"id":"43595","messageId":"20070530015843.GH7044@spearce.org","threadId":"8353","inReplyTo":"7vabvn5hca.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] rev-parse: Identify short sha1 sums correctly.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-30T01:58:43Z","receivedAt":"2007-05-30T01:58:43Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> James Bowes writes:\n> \n> > find_short_packed_object was not loading the pack index files.\n> > Teach it to do so.\n> >\n> > Signed-off-by: James Bowes \n> \n> I think this is the proper fix of the problem I was unhappy\n> about with 'next', rather than reverting the lazy index\n> loading. But I wonder how many _other_ places like this there\n> are that we might be missing...\n> \n> Shawn, an Ack, and any ideas for futureproofing?\n\nAck, though late. ;-)\n\nI actually found this exact same bug today. At first I thought it\nwas related to alternates, but when I dug into the code I came up\nwith basically the same patch as James did. His was sent in first\nand is logically identical, so I'm not going to send my version.\n\nWith regards to future proofing this, I have no idea. I'm going to\nwrite up a test case that catches this and submit that, to avoid a\nfuture regression here, but otherwise I'm not sure what we can do.\nI had thought I had visited every callsite and checked them, but\napparently that wasn't true.\n\nWhat would probably help is to change the name of the structure\nmember in the .h file when its initialization time changes (or its\nmeaning changes in a subtle way) and then go through and manually\nupdate every mention of the old name to the new name, then flip it\nback with a global search and replace when done.\n\nE.g. I should have done:\n\n\t* in cache.h rename num_objects to num_objects_SHAWNCHANGEHERE;\n\t* manually update every num_objects usage to my private name;\n\t* finally globally search-replace back to the standard name.\n\nIt would have forced me to more carefully visit every damn callsite.\n\nI'll go back through the code tonight and double check my work,\nbecause this bug was completely my fault. I'm hoping this was the\nonly bug however.\n\n-- \nShawn.\n"},{"id":"43596","messageId":"20070530020236.GI7044@spearce.org","threadId":"8353","inReplyTo":"7vabvn5hca.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] rev-parse: Identify short sha1 sums correctly.","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2007-05-30T02:02:36Z","receivedAt":"2007-05-30T02:02:36Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Junio C Hamano wrote:\n> James Bowes writes:\n> \n> > find_short_packed_object was not loading the pack index files.\n> > Teach it to do so.\n> >\n> > Signed-off-by: James Bowes \n> \n> I think this is the proper fix of the problem I was unhappy\n> about with 'next'\n\nActually this bug is exactly the reason why I *always* run 'next'\nin production use on all of my systems, and why I also get a number\nof coworkers to run it. Many people beating on the same code in\nweird ways notice things.\n\nToday I found this because I have a shell script that tried to\ndo something of the form \"git checkout v1.8.0-18-g18as1f' and Git\ncouldn't believe that was a revision... It held up day-job work\nfor an hour while I tracked it down, but I did catch it. ;-)\n\n-- \nShawn.\n"}]}