* [PATCH v4] ref-filter: fallback on alphabetical comparison
@ 2015-10-30 8:45 Karthik Nayak
2015-10-30 22:51 ` Johannes Sixt
0 siblings, 1 reply; 3+ messages in thread
From: Karthik Nayak @ 2015-10-30 8:45 UTC (permalink / raw)
To: git; +Cc: j6t, gitster, Karthik Nayak
In ref-filter.c the comparison of refs while sorting is handled by
cmp_ref_sorting() function. When sorting as per numerical values
(e.g. --sort=objectsize) there is no fallback comparison when both
refs hold the same value. This can cause unexpected results (i.e. the
order of listing refs with equal values cannot be pre-determined) as
pointed out by Johannes Sixt ($gmane/280117).
Hence, fallback to alphabetical comparison based on the refname
whenever the other criterion is equal.
A test in t3203 was expecting that branch-two sorts before HEAD, which
happened to be how qsort(3) on Linux sorted the array, but (1) that
outcome was not even guaranteed, and (2) once we start breaking ties
with the refname, "HEAD" should sort before "branch-two" so the
original expectation was inconsistent with the criterion we now use.
Update it to match the new world order, which we can now depend on
being stable.
Helped-by: Junio C Hamano <gitster@pobox.com>
Reported-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
---
ref-filter.c | 2 +-
t/t3203-branch-output.sh | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 046e73b..7b33cb8 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1698,7 +1698,7 @@ static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, stru
if (va->ul < vb->ul)
cmp = -1;
else if (va->ul == vb->ul)
- cmp = 0;
+ cmp = strcmp(a->refname, b->refname);
else
cmp = 1;
}
diff --git a/t/t3203-branch-output.sh b/t/t3203-branch-output.sh
index f77971c..9f2d482 100755
--- a/t/t3203-branch-output.sh
+++ b/t/t3203-branch-output.sh
@@ -158,8 +158,8 @@ EOF
test_expect_success 'git branch `--sort` option' '
cat >expect <<-\EOF &&
- branch-two
* (HEAD detached from fromtag)
+ branch-two
branch-one
master
EOF
--
2.6.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v4] ref-filter: fallback on alphabetical comparison
2015-10-30 8:45 [PATCH v4] ref-filter: fallback on alphabetical comparison Karthik Nayak
@ 2015-10-30 22:51 ` Johannes Sixt
2015-10-30 22:54 ` Junio C Hamano
0 siblings, 1 reply; 3+ messages in thread
From: Johannes Sixt @ 2015-10-30 22:51 UTC (permalink / raw)
To: Karthik Nayak, git; +Cc: gitster
Am 30.10.2015 um 09:45 schrieb Karthik Nayak:
> In ref-filter.c the comparison of refs while sorting is handled by
> cmp_ref_sorting() function. When sorting as per numerical values
> (e.g. --sort=objectsize) there is no fallback comparison when both
> refs hold the same value. This can cause unexpected results (i.e. the
> order of listing refs with equal values cannot be pre-determined) as
> pointed out by Johannes Sixt ($gmane/280117).
>
> Hence, fallback to alphabetical comparison based on the refname
> whenever the other criterion is equal.
>
> A test in t3203 was expecting that branch-two sorts before HEAD, which
> happened to be how qsort(3) on Linux sorted the array, but (1) that
> outcome was not even guaranteed, and (2) once we start breaking ties
> with the refname, "HEAD" should sort before "branch-two" so the
> original expectation was inconsistent with the criterion we now use.
>
> Update it to match the new world order, which we can now depend on
> being stable.
Needless to say that the patch fixes the test failure on Windows. (I
tested v2 of the patch.)
-- Hannes
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v4] ref-filter: fallback on alphabetical comparison
2015-10-30 22:51 ` Johannes Sixt
@ 2015-10-30 22:54 ` Junio C Hamano
0 siblings, 0 replies; 3+ messages in thread
From: Junio C Hamano @ 2015-10-30 22:54 UTC (permalink / raw)
To: Johannes Sixt; +Cc: Karthik Nayak, git
Johannes Sixt <j6t@kdbg.org> writes:
>> Update it to match the new world order, which we can now depend on
>> being stable.
>
> Needless to say that the patch fixes the test failure on Windows. (I
> tested v2 of the patch.)
Thanks, both. Queued.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-10-30 22:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-30 8:45 [PATCH v4] ref-filter: fallback on alphabetical comparison Karthik Nayak
2015-10-30 22:51 ` Johannes Sixt
2015-10-30 22:54 ` Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).