{"thread":{"id":"7704","subject":"[PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","startedAt":"2007-04-17T01:42:50Z","lastAt":"2007-04-19T08:49:42Z","messageCount":12,"participants":["Julian Phillips","Linus Torvalds","Junio C Hamano","Johannes Sixt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"39584","messageId":"20070417014307.12486.28930.julian@quantumfyre.co.uk","threadId":"7704","inReplyTo":null,"subject":"[PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T01:42:50Z","receivedAt":"2007-04-17T01:42:50Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"Rather than sorting the refs list while building it, sort in one go\nafter it is built using a merge sort.  This has a large performance\nboost with large numbers of refs.\n\nSigned-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n---\n\nHaving got builtin fetch to the point of generating a correct FETCH_HEAD (for a certain path through the code at least), I revisted the speed issue I brought up a while back with the sorting of refs.\n\nRunning fetch (builtin version) on a repo with >9000 refs which is up-to-date, using the old sort-on-add I get (best of 5, warm cache):\n\nreal    0m4.351s\nuser    0m4.068s\nsys     0m0.219s\n\nWith this patch the same fetch gives (worst of 5, warm cache):\n\nreal    0m2.196s\nuser    0m1.870s\nsys     0m0.212s\n\nSince this is orthogonal to making fetch a builtin, I don't see that it needs to wait ...\n\n refs.c |   95 +++++++++++++++++++++++++++++++++++++++++++++++++--------------\n 1 files changed, 74 insertions(+), 21 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex d2b7b7f..23982fc 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -47,22 +47,7 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,\n \t\t\t\tstruct ref_list **new_entry)\n {\n \tint len;\n-\tstruct ref_list **p = &list, *entry;\n-\n-\t/* Find the place to insert the ref into.. */\n-\twhile ((entry = *p) != NULL) {\n-\t\tint cmp = strcmp(entry->name, name);\n-\t\tif (cmp > 0)\n-\t\t\tbreak;\n-\n-\t\t/* Same as existing entry? */\n-\t\tif (!cmp) {\n-\t\t\tif (new_entry)\n-\t\t\t\t*new_entry = entry;\n-\t\t\treturn list;\n-\t\t}\n-\t\tp = &entry->next;\n-\t}\n+\tstruct ref_list *entry;\n \n \t/* Allocate it and add it in.. */\n \tlen = strlen(name) + 1;\n@@ -71,11 +56,79 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,\n \thashclr(entry->peeled);\n \tmemcpy(entry->name, name, len);\n \tentry->flag = flag;\n-\tentry->next = *p;\n-\t*p = entry;\n+\tentry->next = list;\n \tif (new_entry)\n \t\t*new_entry = entry;\n-\treturn list;\n+\treturn entry;\n+}\n+\n+/* merge sort the ref list */\n+static struct ref_list *sort_ref_list(struct ref_list *list)\n+{\n+\tint psize, qsize, last_merge_count;\n+\tstruct ref_list *p, *q, *l, *e;\n+\tstruct ref_list *new_list = list;\n+\tint k = 1;\n+\tint merge_count = 0;\n+\n+\tif (!list)\n+\t\treturn list;\n+\n+\tdo {\n+\t\tlast_merge_count = merge_count;\n+\t\tmerge_count = 0;\n+\n+\t\tpsize = 0;\n+\n+\t\tp = new_list;\n+\t\tq = new_list;\n+\t\tnew_list = NULL;\n+\t\tl = NULL;\n+\n+\t\twhile (p) {\n+\t\t\tmerge_count++;\n+\n+\t\t\twhile (psize < k && q->next) {\n+\t\t\t\tq = q->next;\n+\t\t\t\tpsize++;\n+\t\t\t}\n+\t\t\tqsize = k;\n+\n+\t\t\twhile ((psize > 0) || (qsize > 0 && q)) {\n+\t\t\t\tif (qsize == 0 || !q) {\n+\t\t\t\t\te = p;\n+\t\t\t\t\tp = p->next;\n+\t\t\t\t\tpsize--;\n+\t\t\t\t} else if (psize == 0) {\n+\t\t\t\t\te = q;\n+\t\t\t\t\tq = q->next;\n+\t\t\t\t\tqsize--;\n+\t\t\t\t} else if (strcmp(q->name, p->name) < 0) {\n+\t\t\t\t\te = q;\n+\t\t\t\t\tq = q->next;\n+\t\t\t\t\tqsize--;\n+\t\t\t\t} else {\n+\t\t\t\t\te = p;\n+\t\t\t\t\tp = p->next;\n+\t\t\t\t\tpsize--;\n+\t\t\t\t}\n+\n+\t\t\t\te->next = NULL;\n+\n+\t\t\t\tif (l)\n+\t\t\t\t\tl->next = e;\n+\t\t\t\tif (!new_list)\n+\t\t\t\t\tnew_list = e;\n+\t\t\t\tl = e;\n+\t\t\t}\n+\n+\t\t\tp = q;\n+\t\t};\n+\n+\t\tk = k * 2;\n+\t} while ((last_merge_count != merge_count) || (last_merge_count != 1));\n+\n+\treturn new_list;\n }\n \n /*\n@@ -142,7 +195,7 @@ static void read_packed_refs(FILE *f, struct cached_refs *cached_refs)\n \t\t    !get_sha1_hex(refline + 1, sha1))\n \t\t\thashcpy(last->peeled, sha1);\n \t}\n-\tcached_refs->packed = list;\n+\tcached_refs->packed = sort_ref_list(list);\n }\n \n static struct ref_list *get_packed_refs(void)\n@@ -201,7 +254,7 @@ static struct ref_list *get_ref_dir(const char *base, struct ref_list *list)\n \t\tfree(ref);\n \t\tclosedir(dir);\n \t}\n-\treturn list;\n+\treturn sort_ref_list(list);\n }\n \n static struct ref_list *get_loose_refs(void)\n-- \n1.5.1.1\n"},{"id":"39667","messageId":"Pine.LNX.4.64.0704170901170.5473@woody.linux-foundation.org","threadId":"7704","inReplyTo":"20070417014307.12486.28930.julian@quantumfyre.co.uk","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T16:03:44Z","receivedAt":"2007-04-17T16:03:44Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Julian Phillips wrote:\n>\n> Rather than sorting the refs list while building it, sort in one go\n> after it is built using a merge sort.  This has a large performance\n> boost with large numbers of refs.\n> \n> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n\nAcked-by: Linus Torvalds <torvalds@linux-foundation.org>\n\nLooks fine. I think that even your new times are a bit high (over two \nseconds?) but things are clearly better. Have you looked at what takes so \nlong now? \n\n\t\tLinus\n"},{"id":"39692","messageId":"Pine.LNX.4.64.0704172114040.8164@beast.quantumfyre.co.uk","threadId":"7704","inReplyTo":"Pine.LNX.4.64.0704170901170.5473@woody.linux-foundation.org","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T20:17:14Z","receivedAt":"2007-04-17T20:17:14Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Tue, 17 Apr 2007, Linus Torvalds wrote:\n\n>\n>\n> On Tue, 17 Apr 2007, Julian Phillips wrote:\n>>\n>> Rather than sorting the refs list while building it, sort in one go\n>> after it is built using a merge sort.  This has a large performance\n>> boost with large numbers of refs.\n>>\n>> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n>\n> Acked-by: Linus Torvalds <torvalds@linux-foundation.org>\n>\n> Looks fine. I think that even your new times are a bit high (over two\n> seconds?) but things are clearly better. Have you looked at what takes so\n> long now?\n\nIt's the tag auto-following code, I'm calling read_ref to see if I already \nhave that tag - and it appears that doing that a few thousand times takes \na while.\n\nIf I comment out that one line (so the code will _always_ think I have \nthe tags - but I do have them, so ...) I get:\n\nreal    0m0.472s\nuser    0m0.277s\nsys     0m0.181s\n\nLooks like read_ref is the wrong thing to be using ...\n\n-- \nJulian\n\n  ---\n\"Life is like a buffet; it's not good but there's plenty of it.\"\n"},{"id":"39700","messageId":"Pine.LNX.4.64.0704171350000.5473@woody.linux-foundation.org","threadId":"7704","inReplyTo":"Pine.LNX.4.64.0704172114040.8164@beast.quantumfyre.co.uk","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Linus Torvalds","fromEmail":"torvalds@linux-foundation.org","sentAt":"2007-04-17T20:51:31Z","receivedAt":"2007-04-17T20:51:31Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Tue, 17 Apr 2007, Julian Phillips wrote:\n> \n> It's the tag auto-following code, I'm calling read_ref to see if I already\n> have that tag - and it appears that doing that a few thousand times takes a\n> while.\n\nHeh. I think we should probably call read_refs() just once to read them \nall (when most of them are packed, that's cheap), and then after that, \nhave some way to just check for a match on the refs we have cached.\n\n\t\tLinus\n"},{"id":"39703","messageId":"Pine.LNX.4.64.0704172156590.9988@beast.quantumfyre.co.uk","threadId":"7704","inReplyTo":"20070417014307.12486.28930.julian@quantumfyre.co.uk","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T21:01:15Z","receivedAt":"2007-04-17T21:01:15Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Tue, 17 Apr 2007, Julian Phillips wrote:\n\n> Rather than sorting the refs list while building it, sort in one go\n> after it is built using a merge sort.  This has a large performance\n> boost with large numbers of refs.\n>\n> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n> ---\n>\n> Having got builtin fetch to the point of generating a correct FETCH_HEAD (for a certain path through the code at least), I revisted the speed issue I brought up a while back with the sorting of refs.\n>\n> Running fetch (builtin version) on a repo with >9000 refs which is up-to-date, using the old sort-on-add I get (best of 5, warm cache):\n>\n> real    0m4.351s\n> user    0m4.068s\n> sys     0m0.219s\n>\n> With this patch the same fetch gives (worst of 5, warm cache):\n>\n> real    0m2.196s\n> user    0m1.870s\n> sys     0m0.212s\n>\n> Since this is orthogonal to making fetch a builtin, I don't see that it needs to wait ...\n\nIn case anyone is curious, doing the same fetch with master \n(v1.5.1.1-135-gf948792):\n\nmaster (best of 5, warm cache):\nreal    0m33.962s\nuser    0m23.992s\nsys     0m9.986s\n\nmaster + patch (worst of 5, warm cache):\nreal    0m20.821s\nuser    0m10.390s\nsys     0m9.799s\n\n-- \nJulian\n\n  ---\nNANCY!!  Why is everything RED?!\n"},{"id":"39714","messageId":"7vslaymzk3.fsf@assigned-by-dhcp.cox.net","threadId":"7704","inReplyTo":"Pine.LNX.4.64.0704170901170.5473@woody.linux-foundation.org","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-17T22:00:28Z","receivedAt":"2007-04-17T22:00:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@linux-foundation.org> writes:\n\n> On Tue, 17 Apr 2007, Julian Phillips wrote:\n>>\n>> Rather than sorting the refs list while building it, sort in one go\n>> after it is built using a merge sort.  This has a large performance\n>> boost with large numbers of refs.\n>> \n>> Signed-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n>\n> Acked-by: Linus Torvalds <torvalds@linux-foundation.org>\n>\n> Looks fine. I think that even your new times are a bit high (over two \n> seconds?) but things are clearly better. Have you looked at what takes so \n> long now? \n\nI wonder why the loss of \"we are replacing the same one\" case in\nthe original add_ref() was not compensated for in the new\nsort_ref_list().\n\nI think we would not call add_ref() to the same list with\nduplicate names, unless (1) filesystem is grossly corrupt, (2)\nsomebody added a new ref while we are walking (how does\nreaddir() behave in such a case???), or (3) packed-refs file is\ncorrupt.\n"},{"id":"39715","messageId":"Pine.LNX.4.64.0704172300170.16435@beast.quantumfyre.co.uk","threadId":"7704","inReplyTo":"Pine.LNX.4.64.0704171350000.5473@woody.linux-foundation.org","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T22:03:14Z","receivedAt":"2007-04-17T22:03:14Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Tue, 17 Apr 2007, Linus Torvalds wrote:\n\n>\n>\n> On Tue, 17 Apr 2007, Julian Phillips wrote:\n>>\n>> It's the tag auto-following code, I'm calling read_ref to see if I already\n>> have that tag - and it appears that doing that a few thousand times takes a\n>> while.\n>\n> Heh. I think we should probably call read_refs() just once to read them\n> all (when most of them are packed, that's cheap), and then after that,\n> have some way to just check for a match on the refs we have cached.\n\nI had a look at the exclude_existing function in show-ref.  That uses \nfor_each_ref to build a path_list, and path_list_has_path to do the \nfiltering...\n\nUsing that I get (worst of 5, warm cache):\nreal    0m0.526s\nuser    0m0.302s\nsys     0m0.176s\n\n-- \nJulian\n\n  ---\nThe descent to Hades is the same from every place.\n \t\t-- Anaxagoras\n"},{"id":"39717","messageId":"Pine.LNX.4.64.0704172304380.16435@beast.quantumfyre.co.uk","threadId":"7704","inReplyTo":"7vslaymzk3.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-17T22:43:30Z","receivedAt":"2007-04-17T22:43:30Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Tue, 17 Apr 2007, Junio C Hamano wrote:\n\n> I wonder why the loss of \"we are replacing the same one\" case in\n> the original add_ref() was not compensated for in the new\n> sort_ref_list().\n\nThat would be because it didn't bite me during testing, and I forgot to \ndouble check the code I was removing after I was happy that the sort was \nproducing sane output (it was getting rather late).\n\nThing is, I'm not sure what to do about it anyway.  You could drop the \nduplicate in the sort, but since add_ref returns a pointer to the added \nentry (and used to simply return a pointer to the one already in the list) \nit's possible that you would be dropping something that someone had a \npointer to.\n\n> I think we would not call add_ref() to the same list with\n> duplicate names, unless (1) filesystem is grossly corrupt, (2)\n> somebody added a new ref while we are walking (how does\n> readdir() behave in such a case???), or (3) packed-refs file is\n> corrupt.\n\nThis combined with the fact that the old code didn't check that the sha1 \nwas the same suggests to me that this behaviour may actually have been a \nsubtle bug?  Perhaps the best thing to do is die if we find two entries \nwith the same name when sorting?\n\n-- \nJulian\n\n  ---\nWhat passes for optimism is most often the effect of an intellectual error.\n \t\t-- Raymond Aron, \"The Opium of the Intellectuals\"\n"},{"id":"39812","messageId":"7vodllfpaj.fsf@assigned-by-dhcp.cox.net","threadId":"7704","inReplyTo":"Pine.LNX.4.64.0704172304380.16435@beast.quantumfyre.co.uk","subject":"Re: [PATCH] refs.c: add a function to sort a ref list, rather then sorting on add","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-04-18T19:36:20Z","receivedAt":"2007-04-18T19:36:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Julian Phillips <julian@quantumfyre.co.uk> writes:\n\n> On Tue, 17 Apr 2007, Junio C Hamano wrote:\n> ...\n>> I think we would not call add_ref() to the same list with\n>> duplicate names, unless (1) filesystem is grossly corrupt, (2)\n>> somebody added a new ref while we are walking (how does\n>> readdir() behave in such a case???), or (3) packed-refs file is\n>> corrupt.\n>\n> This combined with the fact that the old code didn't check that the\n> sha1 was the same suggests to me that this behaviour may actually have\n> been a subtle bug?  Perhaps the best thing to do is die if we find two\n> entries with the same name when sorting?\n\nI am not sure what readdir() does if somebody adds a new ref\nwhile we are walking the directory; I am hoping we would not get\nthe same thing in duplicates, but I dunno.\n\nI think the most sensible thing to do is to check for\nduplicates, discarding if the SHA-1 match and otherwise dying.\n"},{"id":"39831","messageId":"20070418212749.4842.62822.julian@quantumfyre.co.uk","threadId":"7704","inReplyTo":"7vodllfpaj.fsf@assigned-by-dhcp.cox.net","subject":"[PATCH] refs.c: drop duplicate entries in sort_ref_list","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-18T21:27:28Z","receivedAt":"2007-04-18T21:27:28Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"It shouldn't happen that we read duplicate entries into the same list,\nbut just in case make sort_ref_list drop them.  If the SHA1s don't\nmatch then die instead, as we have no way of knowing which one is the\ncorrect one.\n\nSigned-off-by: Julian Phillips <julian@quantumfyre.co.uk>\n---\n\nOn Wed, 18 Apr 2007, Junio C Hamano wrote:\n\n> Julian Phillips <julian@quantumfyre.co.uk> writes:\n>\n>> On Tue, 17 Apr 2007, Junio C Hamano wrote:\n>> ...\n>>> I think we would not call add_ref() to the same list with\n>>> duplicate names, unless (1) filesystem is grossly corrupt, (2)\n>>> somebody added a new ref while we are walking (how does\n>>> readdir() behave in such a case???), or (3) packed-refs file is\n>>> corrupt.\n>>\n>> This combined with the fact that the old code didn't check that the\n>> sha1 was the same suggests to me that this behaviour may actually have\n>> been a subtle bug?  Perhaps the best thing to do is die if we find two\n>> entries with the same name when sorting?\n>\n> I am not sure what readdir() does if somebody adds a new ref\n> while we are walking the directory; I am hoping we would not get\n> the same thing in duplicates, but I dunno.\n>\n> I think the most sensible thing to do is to check for\n> duplicates, discarding if the SHA-1 match and otherwise dying.\n>\n\nSomething like this (on top of my previous patch)?\n\n refs.c |   34 ++++++++++++++++++++++++++--------\n 1 files changed, 26 insertions(+), 8 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 23982fc..5736a0e 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -65,7 +65,7 @@ static struct ref_list *add_ref(const char *name, const unsigned char *sha1,\n /* merge sort the ref list */\n static struct ref_list *sort_ref_list(struct ref_list *list)\n {\n-\tint psize, qsize, last_merge_count;\n+\tint psize, qsize, last_merge_count, cmp;\n \tstruct ref_list *p, *q, *l, *e;\n \tstruct ref_list *new_list = list;\n \tint k = 1;\n@@ -103,14 +103,32 @@ static struct ref_list *sort_ref_list(struct ref_list *list)\n \t\t\t\t\te = q;\n \t\t\t\t\tq = q->next;\n \t\t\t\t\tqsize--;\n-\t\t\t\t} else if (strcmp(q->name, p->name) < 0) {\n-\t\t\t\t\te = q;\n-\t\t\t\t\tq = q->next;\n-\t\t\t\t\tqsize--;\n \t\t\t\t} else {\n-\t\t\t\t\te = p;\n-\t\t\t\t\tp = p->next;\n-\t\t\t\t\tpsize--;\n+\t\t\t\t\tcmp = strcmp(q->name, p->name);\n+\t\t\t\t\tif (cmp < 0) {\n+\t\t\t\t\t\te = q;\n+\t\t\t\t\t\tq = q->next;\n+\t\t\t\t\t\tqsize--;\n+\t\t\t\t\t} else if (cmp > 0) {\n+\t\t\t\t\t\te = p;\n+\t\t\t\t\t\tp = p->next;\n+\t\t\t\t\t\tpsize--;\n+\t\t\t\t\t} else {\n+\t\t\t\t\t\tif (hashcmp(q->sha1, p->sha1))\n+\t\t\t\t\t\t\tdie(\"Duplicated ref, \"\n+\t\t\t\t\t\t\t    \"and SHA1s don't \"\n+\t\t\t\t\t\t\t    \"match: %s\",\n+\t\t\t\t\t\t\t    q->name);\n+\t\t\t\t\t\twarning(\"Duplicated ref: %s\",\n+\t\t\t\t\t\t\tq->name);\n+\t\t\t\t\t\te = q;\n+\t\t\t\t\t\tq = q->next;\n+\t\t\t\t\t\tqsize--;\n+\t\t\t\t\t\tfree(e);\n+\t\t\t\t\t\te = p;\n+\t\t\t\t\t\tp = p->next;\n+\t\t\t\t\t\tpsize--;\n+\t\t\t\t\t}\n \t\t\t\t}\n \n \t\t\t\te->next = NULL;\n-- \n1.5.1.1\n"},{"id":"39869","messageId":"4627262A.463AA87D@eudaptics.com","threadId":"7704","inReplyTo":"20070418212749.4842.62822.julian@quantumfyre.co.uk","subject":"Re: [PATCH] refs.c: drop duplicate entries in sort_ref_list","fromName":"Johannes Sixt","fromEmail":"j.sixt@eudaptics.com","sentAt":"2007-04-19T08:19:54Z","receivedAt":"2007-04-19T08:19:54Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Julian Phillips wrote:\n> \n> It shouldn't happen that we read duplicate entries into the same list,\n> but just in case make sort_ref_list drop them.  If the SHA1s don't\n> match then die instead, as we have no way of knowing which one is the\n> correct one.\n\nClone a random repository that has tags, then\n\n$ rm .git/refs/tags/*\n$ git fetch origin\n\nnow lists each tag twice: for the tag object itself and the commit it\npoints to. Is this related to the problem you are solving here? If so,\ndying is probably not the best in this situation since the tags are\nstill unique.\n\n(rm .git/refs/tags/* may be insane, but just the other day I wanted to\nclean up my repo from a number of unneeded branches as well as the tags\nthat they contained - to find out that it is not enough to remove the\nrefs: the objects must go away too, otherwise git fetch auto-follows\nthem and the tags come back even though the branch heads are not there\nanymore.)\n\n$ git version\ngit version 1.5.1.1.27.g91776\n\n-- Hannes\n"},{"id":"39874","messageId":"Pine.LNX.4.64.0704190934480.30526@reaper.quantumfyre.co.uk","threadId":"7704","inReplyTo":"4627262A.463AA87D@eudaptics.com","subject":"Re: [PATCH] refs.c: drop duplicate entries in sort_ref_list","fromName":"Julian Phillips","fromEmail":"julian@quantumfyre.co.uk","sentAt":"2007-04-19T08:49:42Z","receivedAt":"2007-04-19T08:49:42Z","isPatch":true,"sender":{"key":"julian@quantumfyre.co.uk","avatar":"https://avatars.githubusercontent.com/u/948888?v=4"},"body":"On Thu, 19 Apr 2007, Johannes Sixt wrote:\n\n> Julian Phillips wrote:\n>>\n>> It shouldn't happen that we read duplicate entries into the same list,\n>> but just in case make sort_ref_list drop them.  If the SHA1s don't\n>> match then die instead, as we have no way of knowing which one is the\n>> correct one.\n>\n> Clone a random repository that has tags, then\n>\n> $ rm .git/refs/tags/*\n> $ git fetch origin\n>\n> now lists each tag twice: for the tag object itself and the commit it\n> points to. Is this related to the problem you are solving here? If so,\n> dying is probably not the best in this situation since the tags are\n> still unique.\n\nThe sort function is only looking at refs.  So if you rm .git/refs/tags/* \nthen you don't have any tag refs (assuming no packed-refs).  What it is \ntalking about is having two instances of (e.g.) refs/tags/foo.\n\nIn the situation you mention above you still shouldn't see duplicated \nrefs.\n\n-- \nJulian\n\n  ---\n\nAh, so that's what's been wrong with the little fella.  He misses\ncasual sex.\n\n \t\t-- Homer Simpson\n \t\t   Two Dozen and One Greyhounds\n"}]}