From: Jacob Keller Date: Mon, 01 Jun 2026 22:35:08 GMT Subject: Re: [PATCH] describe: fix --exclude, --match with --contains and --all Message-ID: <3ad3a7ad-14de-4972-acbd-433ad4ced7f8@intel.com> In-Reply-To: On 5/31/2026 5:40 PM, Junio C Hamano wrote: > Tuomas Ahola writes: > >> Junio C Hamano wrote: >> >>> It is curious that this fails in some but not all CI jobs, and even >>> more curious that these failures look the same. >>> >>> e.g., https://github.com/git/git/actions/runs/26671595367/job/78615760984#step:4:1984 >>> >>> +++ diff -u expect actual >>> --- expect 2026-05-30 02:21:23 >>> +++ actual 2026-05-30 02:21:23 >>> @@ -1 +1 @@ >>> -branch_A >>> +remotes/origin/remote_branch_A >>> error: last command exited with $?=1 >>> not ok 70 - describe --contains --all --exclude >>> # >>> # echo "branch_A" >expect && >>> # tagged_commit=$(git rev-parse "refs/tags/A^0") && >>> # git describe --contains --all --exclude="A" --exclude="c" --exclude="test*" $tagged_commit >actual && >>> # test_cmp expect actual >>> >>> Rings any bell? >> >> That's way out of my wheelhouse but this seems to fix the failure >> for Alpine at least: >> >> -----8<----- >> >> diff --git a/builtin/name-rev.c b/builtin/name-rev.c >> index d6594ada53..1776ffab46 100644 >> --- a/builtin/name-rev.c >> +++ b/builtin/name-rev.c >> @@ -416,7 +416,7 @@ static void name_tips(struct mem_pool *string_pool) >> * Try to set better names first, so that worse ones spread >> * less. >> */ >> - QSORT(tip_table.table, tip_table.nr, cmp_by_tag_and_age); >> + STABLE_QSORT(tip_table.table, tip_table.nr, cmp_by_tag_and_age); >> for (i = 0; i < tip_table.nr; i++) { >> struct tip_table_entry *e = &tip_table.table[i]; >> if (e->commit) { > > Ah, OK, when the test has multiple candidates with the same score, > of course emitting any one of them as the answer is a valid and > correctly working program. > > So switching to stable-qsort here may "fix" the test breakage, but > it makes the real-world use cases worse, doesn't it? When any one > of the solutions with the same "goodness" is acceptable, the change > makes the code behave as if the elements in the table before they > are sorted have an "if same score, earlier the better" kind of > relationship between them. > > I would have preferred to see a tweak on the test side to avoid > having more than one answer of the same goodness, or perhaps list > all the possible acceptable answers and instead of using test_cmp to > check for the exact answer, take any of the acceptable ones, or > something like that. > > Thanks. > Ya something like that is probably better. I'll look at cooking up a v2 which improves the test here. I think part of the issue is that the previous tests setup a bunch of tags and branches, so figuring out what all the possible outputs are is tricky. Probably I can just add additional excludes until there is only one answer.