v3ref-filter: fallback on alphabetical comparison

> 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).

> 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

>> 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).
>

> On Wed, Oct 28, 2015 at 1:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>> 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.
>>
>
> Exactly what you're saying, they happened to have the same objectsize.
> Hence sorting them would put them together, but since we compare the
> refname's the "HEAD" ref would come before "branch-two".
>
>>     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).
>
> It actually is, we consider "refs/heads/branch-two rather then the shortened
> version of this. It makes sense to classify refs this way, even though this
> was a side effect of this commit.

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> On Wed, Oct 28, 2015 at 1:20 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> 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.
>>>
>>
>> Exactly what you're saying, they happened to have the same objectsize.
>> Hence sorting them would put them together, but since we compare the
>> refname's the "HEAD" ref would come before "branch-two".
>>
>>>     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).
>>
>> It actually is, we consider "refs/heads/branch-two rather then the shortened
>> version of this. It makes sense to classify refs this way, even though this
>> was a side effect of this commit.
>
> Now these are enough bits of info, that can and needs to be
> condenced into an updated log message to help future readers.
>
> Thanks.