Re: [PATCH] describe: fix --exclude, --match with --contains and --all
- From
Jacob Keller <jacob.e.keller@intel.com>
- Date
- Jun 1, 2026, 22:35 UTC
- Message-ID
- <3ad3a7ad-14de-4972-acbd-433ad4ced7f8@intel.com>
- In-Reply-To
- <xmqq33z7ay9e.fsf@gitster.g>
On 5/31/2026 5:40 PM, Junio C Hamano wrote:
Show 63 quoted lines
> Tuomas Ahola <taahol@utu.fi> writes:
>
>> Junio C Hamano <gitster@pobox.com> 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.