Karthik Nayak <karthik.188@xxxxxxxxx> writes:
> 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).
As we use qsort(), if two members compare as equal, their order in
the sorted array is undefined. I think this is a good change.
> Hence, fallback to alphabetical comparison based on the refname
> whenever the other criterion is equal. Fix the test in t3203 in this
> regard.
It is unclear what "in this regard" is. Do you mean this (I am not
suggesting you to spell these out in a very detailed way in the
final log message; I am deliberately being detailed here to help me
understand what you really mean)?
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.
I am not sure about "HEAD" and "branch-two" in the above (it may be
comparison between "HEAD" and "refs/heads/branch-two", for example).
> Reported-by: Johannes Sixt <j6t@xxxxxxxx>
> Signed-off-by: Karthik Nayak <Karthik.188@xxxxxxxxx>
> ---
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html