Re: [PATCH] describe: fix --exclude, --match with --contains and --all

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 1, 2026, 00:40 UTC
Message-ID: <xmqq33z7ay9e.fsf@gitster.g>
In-Reply-To: <20260531234644.97LRl%taahol@utu.fi>

Tuomas Ahola <taahol@utu.fi> writes:

Show 41 quoted lines

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