threads / patch / 40675

v4ref-filter: fallback on alphabetical comparison

Subject: [PATCH v4] ref-filter: fallback on alphabetical comparison

## tl;dr

3 messages between Oct 30, 2015 and Oct 30, 2015. Diffs are folded; open one to read it.

replies: 2people: 3as markdown or json

Karthik Nayak· Oct 30, 2015, 08:45 UTC · lore

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(-)
Show changes to 2 files +2 −2

ref-filter.c, t/t3203-branch-output.sh

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
Johannes Sixt· Oct 30, 2015, 22:51 UTC · re: Karthik Nayak · lore

Re: [PATCH v4] ref-filter: fallback on alphabetical comparison

Am 30.10.2015 um 09:45 schrieb Karthik Nayak:
Show 18 quoted lines
> 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
Junio C Hamano· Oct 30, 2015, 22:54 UTC · re: Johannes Sixt · lore

Re: [PATCH v4] ref-filter: fallback on alphabetical comparison

Johannes Sixt <j6t@kdbg.org> writes:
Show 5 quoted lines
>> 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.

← back to recent threads