{"thread":{"id":"40639","subject":"[PATCH v2] ref-filter: fallback on alphabetical comparison","startedAt":"2015-10-25T16:29:00Z","lastAt":"2015-10-26T17:13:50Z","messageCount":3,"participants":["Karthik Nayak","Eric Sunshine"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"272202","messageId":"1445790540-22764-1-git-send-email-Karthik.188@gmail.com","threadId":"40639","inReplyTo":null,"subject":"[PATCH v2] ref-filter: fallback on alphabetical comparison","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-25T16:29:00Z","receivedAt":"2015-10-25T16:29:00Z","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\nrefs hold the same value. This can cause unexpected results (i.e. the\norder of listting refs with equal values cannot be pre-determined) as\npointed out by Johannes Sixt ($gmane/280117).\n\nHence, fallback to alphabetical comparison based on the refname\nwhenever the other criterion is equal. Fix the test in t3203 in this\nregard.\n\nReported-by: Johannes Sixt \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":"272214","messageId":"CAPig+cRRB5MNxuBc-5rM_8cJh69PjH5-OSmkRNw_6OFQdQOoTw@mail.gmail.com","threadId":"40639","inReplyTo":"1445790540-22764-1-git-send-email-Karthik.188@gmail.com","subject":"Re: [PATCH v2] ref-filter: fallback on alphabetical comparison","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-10-26T01:13:41Z","receivedAt":"2015-10-26T01:13:41Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Sun, Oct 25, 2015 at 12:29 PM, 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\n> refs hold the same value. This can cause unexpected results (i.e. the\n> order of listting refs with equal values cannot be pre-determined) as\n\ns/listting/listing/\n\n> pointed out by Johannes Sixt ($gmane/280117).\n>\n> Hence, fallback to alphabetical comparison based on the refname\n> whenever the other criterion is equal. Fix the test in t3203 in this\n> regard.\n>\n> Reported-by: Johannes Sixt \n> Signed-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>\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":"272247","messageId":"CAOLa=ZQGwrQrie+Bp0B61LdG37-UeEdyHqvQEtq26nX9VjYu7Q@mail.gmail.com","threadId":"40639","inReplyTo":"CAPig+cRRB5MNxuBc-5rM_8cJh69PjH5-OSmkRNw_6OFQdQOoTw@mail.gmail.com","subject":"Re: [PATCH v2] ref-filter: fallback on alphabetical comparison","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2015-10-26T17:13:50Z","receivedAt":"2015-10-26T17:13:50Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"On Mon, Oct 26, 2015 at 6:43 AM, Eric Sunshine wrote:\n> On Sun, Oct 25, 2015 at 12:29 PM, 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\n>> refs hold the same value. This can cause unexpected results (i.e. the\n>> order of listting refs with equal values cannot be pre-determined) as\n>\n> s/listting/listing/\n\nThanks for pointing out. Should I re-roll?\n\n-- \nRegards,\nKarthik Nayak\n"}]}