threads / discuss / 40627
Subject: t3203: --sort=objectsize failure on Windows
Johannes SixtOn Windows, I observe a failure of the test case 'git branch `--sort` option' introduced by aedcb7dc (branch.c: use 'ref-filter' APIs). The reason is that the resulting order generated by qsort is unspecified for entries that compare equal. In fact, the test case sorts 4 entries where there are only 2 different values.
To achieve a deterministic order, perhaps cmp_ref_sorting() should fall back to alphabetic comparison if the other criterion (when used) compares equal?
-- Hannes
Karthik NayakRe: t3203: --sort=objectsize failure on Windows
On Sat, Oct 24, 2015 at 1:19 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> On Windows, I observe a failure of the test case 'git branch `--sort` > option' introduced by aedcb7dc (branch.c: use 'ref-filter' APIs). The reason > is that the resulting order generated by qsort is unspecified for entries > that compare equal. In fact, the test case sorts 4 entries where there are > only 2 different values.
Thanks for reporting the error. Although I couldn't reproduce this on a Linux environment. I get what the problem is.
> > To achieve a deterministic order, perhaps cmp_ref_sorting() should fall back > to alphabetic comparison if the other criterion (when used) compares equal? >
Seems like a simple and good way to go around this. will reply with a patch.
-- Regards, Karthik Nayak
Karthik Nayak[PATCH] ref-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 as pointed out by Johannes Sixt ($gmane/280117).
Hence, fallback to alphabetical comparison based on the refname whenever the other criterion is equal. Fix the test in t3203 in this regard.
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
Eric SunshineRe: [PATCH] ref-filter: fallback on alphabetical comparison
On Sat, Oct 24, 2015 at 10:42 AM, Karthik Nayak <karthik.188@gmail.com> wrote:
> 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 as pointed out > by Johannes Sixt ($gmane/280117).
Please make the commit message self-contained by describing the "unexpected results" here rather than directing readers to chase down the information elsewhere themselves.
> Hence, fallback to alphabetical comparison based on the refname whenever > the other criterion is equal. Fix the test in t3203 in this regard. > > Signed-off-by: Karthik Nayak <Karthik.188@gmail.com>
It would not be amiss to add a Reported-by: to credit j6t.
> --- > 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
Karthik NayakRe: [PATCH] ref-filter: fallback on alphabetical comparison
On Sun, Oct 25, 2015 at 5:48 AM, Eric Sunshine <sunshine@sunshineco.com> wrote:
> On Sat, Oct 24, 2015 at 10:42 AM, Karthik Nayak <karthik.188@gmail.com> wrote: >> 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 as pointed out >> by Johannes Sixt ($gmane/280117). > > Please make the commit message self-contained by describing the > "unexpected results" here rather than directing readers to chase down > the information elsewhere themselves. >
Will do.
>> Hence, fallback to alphabetical comparison based on the refname whenever >> the other criterion is equal. Fix the test in t3203 in this regard. >> >> Signed-off-by: Karthik Nayak <Karthik.188@gmail.com> > > It would not be amiss to add a Reported-by: to credit j6t. >
Definitely. Thanks for the quick review :)
-- Regards, Karthik Nayak