{"thread":{"id":"40627","subject":"t3203: --sort=objectsize failure on Windows","startedAt":"2015-10-24T07:49:27Z","lastAt":"2015-10-25T07:15:28Z","messageCount":5,"participants":["Johannes Sixt","Karthik Nayak","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"272162","messageId":"562B3807.2030203@kdbg.org","threadId":"40627","inReplyTo":null,"subject":"t3203: --sort=objectsize failure on Windows","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-10-24T07:49:27Z","receivedAt":"2015-10-24T07:49:27Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"On Windows, I observe a failure of the test case 'git branch `--sort` \noption' introduced by aedcb7dc (branch.c: use 'ref-filter' APIs). The \nreason is that the resulting order generated by qsort is unspecified for \nentries that compare equal. In fact, the test case sorts 4 entries where \nthere are only 2 different values.\n\nTo achieve a deterministic order, perhaps cmp_ref_sorting() should fall \nback to alphabetic comparison if the other criterion (when used) \ncompares equal?\n\n-- Hannes\n"},{"id":"272166","messageId":"CAOLa=ZRB6_jyyg-M7h8EMtNt96VhXF2hvvKp9OGMvXyJEZB1mw@mail.gmail.com","threadId":"40627","inReplyTo":"562B3807.2030203@kdbg.org","subject":"Re: t3203: --sort=objectsize failure on Windows","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-24T14:25:58Z","receivedAt":"2015-10-24T14:25:58Z","isPatch":false,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sat, Oct 24, 2015 at 1:19 PM, Johannes Sixt wrote:\n> On Windows, I observe a failure of the test case 'git branch `--sort`\n> option' introduced by aedcb7dc (branch.c: use 'ref-filter' APIs). The reason\n> is that the resulting order generated by qsort is unspecified for entries\n> that compare equal. In fact, the test case sorts 4 entries where there are\n> only 2 different values.\n\nThanks for reporting the error. Although I couldn't reproduce this on\na Linux environment.\nI get what the problem is.\n\n>\n> To achieve a deterministic order, perhaps cmp_ref_sorting() should fall back\n> to alphabetic comparison if the other criterion (when used) compares equal?\n>\n\nSeems like a simple and good way to go around this. will reply with a patch.\n\n-- \nRegards,\nKarthik Nayak\n"},{"id":"272167","messageId":"1445697770-29331-1-git-send-email-Karthik.188@gmail.com","threadId":"40627","inReplyTo":"562B3807.2030203@kdbg.org","subject":"[PATCH] ref-filter: fallback on alphabetical comparison","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-24T14:42:50Z","receivedAt":"2015-10-24T14:42:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In ref-filter.c the comparison of refs while sorting is handled by\ncmp_ref_sorting() function. When sorting as per numerical values\n(e.g. --sort=objectsize) there is no fallback comparison when both refs\nhold the same value. This can cause unexpected results as pointed out\nby Johannes Sixt ($gmane/280117).\n\nHence, fallback to alphabetical comparison based on the refname whenever\nthe other criterion is equal. Fix the test in t3203 in this regard.\n\nSigned-off-by: Karthik Nayak \n---\n ref-filter.c | 2 +-\n t/t3203-branch-output.sh | 2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/ref-filter.c b/ref-filter.c\nindex 046e73b..7b33cb8 100644\n--- a/ref-filter.c\n+++ b/ref-filter.c\n@@ -1698,7 +1698,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n \t\tif (va->ul < vb->ul)\n \t\t\tcmp = -1;\n \t\telse if (va->ul == vb->ul)\n-\t\t\tcmp = 0;\n+\t\t\tcmp = strcmp(a->refname, b->refname);\n \t\telse\n \t\t\tcmp = 1;\n \t}\ndiff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\nindex f77971c..9f2d482 100755\n--- a/t/t3203-branch-output.sh\n+++ b/t/t3203-branch-output.sh\n@@ -158,8 +158,8 @@ EOF\n \n test_expect_success 'git branch `--sort` option' '\n \tcat >expect <<-\\EOF &&\n-\t branch-two\n \t* (HEAD detached from fromtag)\n+\t branch-two\n \t branch-one\n \t master\n \tEOF\n-- \n2.6.2\n"},{"id":"272179","messageId":"CAPig+cRnDNFCQs6qspKDEz6C0Rum8W0tSMxejw+WOBD=XHrDxw@mail.gmail.com","threadId":"40627","inReplyTo":"1445697770-29331-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH] ref-filter: fallback on alphabetical comparison","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-25T00:18:19Z","receivedAt":"2015-10-25T00:18:19Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sat, Oct 24, 2015 at 10:42 AM, Karthik Nayak wrote:\n> In ref-filter.c the comparison of refs while sorting is handled by\n> cmp_ref_sorting() function. When sorting as per numerical values\n> (e.g. --sort=objectsize) there is no fallback comparison when both refs\n> hold the same value. This can cause unexpected results as pointed out\n> by Johannes Sixt ($gmane/280117).\n\nPlease make the commit message self-contained by describing the\n\"unexpected results\" here rather than directing readers to chase down\nthe information elsewhere themselves.\n\n> Hence, fallback to alphabetical comparison based on the refname whenever\n> the other criterion is equal. Fix the test in t3203 in this regard.\n>\n> Signed-off-by: Karthik Nayak \n\nIt would not be amiss to add a Reported-by: to credit j6t.\n\n> ---\n> diff --git a/ref-filter.c b/ref-filter.c\n> index 046e73b..7b33cb8 100644\n> --- a/ref-filter.c\n> +++ b/ref-filter.c\n> @@ -1698,7 +1698,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru\n> if (va->ul < vb->ul)\n> cmp = -1;\n> else if (va->ul == vb->ul)\n> - cmp = 0;\n> + cmp = strcmp(a->refname, b->refname);\n> else\n> cmp = 1;\n> }\n> diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh\n> index f77971c..9f2d482 100755\n> --- a/t/t3203-branch-output.sh\n> +++ b/t/t3203-branch-output.sh\n> @@ -158,8 +158,8 @@ EOF\n>\n> test_expect_success 'git branch `--sort` option' '\n> cat >expect <<-\\EOF &&\n> - branch-two\n> * (HEAD detached from fromtag)\n> + branch-two\n> branch-one\n> master\n> EOF\n> --\n> 2.6.2\n"},{"id":"272183","messageId":"CAOLa=ZSS3QK8wkKUGCtLqhoZATCxJxpYBvPCUf37KHVwZeRsOg@mail.gmail.com","threadId":"40627","inReplyTo":"CAPig+cRnDNFCQs6qspKDEz6C0Rum8W0tSMxejw+WOBD=XHrDxw@mail.gmail.com","subject":"Re: [PATCH] ref-filter: fallback on alphabetical comparison","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-25T07:15:28Z","receivedAt":"2015-10-25T07:15:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Sun, Oct 25, 2015 at 5:48 AM, Eric Sunshine wrote:\n> On Sat, Oct 24, 2015 at 10:42 AM, Karthik Nayak wrote:\n>> In ref-filter.c the comparison of refs while sorting is handled by\n>> cmp_ref_sorting() function. When sorting as per numerical values\n>> (e.g. --sort=objectsize) there is no fallback comparison when both refs\n>> hold the same value. This can cause unexpected results as pointed out\n>> by Johannes Sixt ($gmane/280117).\n>\n> Please make the commit message self-contained by describing the\n> \"unexpected results\" here rather than directing readers to chase down\n> the information elsewhere themselves.\n>\n\nWill do.\n\n>> Hence, fallback to alphabetical comparison based on the refname whenever\n>> the other criterion is equal. Fix the test in t3203 in this regard.\n>>\n>> Signed-off-by: Karthik Nayak \n>\n> It would not be amiss to add a Reported-by: to credit j6t.\n>\n\nDefinitely. Thanks for the quick review :)\n\n-- \nRegards,\nKarthik Nayak\n"}]}