Subject: Re: [PATCH] ci(win+Meson): build in Release mode, avoiding t7001-mv hangs

From: Junio C Hamano <[email-masked]>
Date: Mon, 28 Apr 2025 10:01:34 -0700

Patrick Steinhardt <[email-masked]> writes:

>> > The reason for this timeout is the test case 'nonsense mv triggers
>> > assertion failure and partially updated index' in t7001-mv (which is
>> > not even a regression test, but instead merely demonstrates a bug that
>> > someone thought someone else should fix at some time). As the name
>> > suggests, it triggers an assertion. The problem with this is that an
>> > assertion on Windows, at least when run in Debug mode, will open a modal
>> > dialog that patiently awaits some buttons to be clicked. Which never
>> > happens in automated builds.
>> 
>> Interesting.
>> 
>> So another viable fix (no, I am not suggesting a counter-proposal,
>> but asking a pure question to see if I understand the issue
>> correctly) is to rewrite "assert(cond)" to "if (cond) BUG(...)"
>> or something like that, so that it truly fails?
>
> On the surface this sounds like a reasonable thing to do, but I don't
> have enough context to be really able to tell.

Interesting again ;-) I didn't realize that it was a fairly recent
development.  0fcd473f (t7001: add failure test which triggers
assertion, 2024-10-22) is what adds the questionable test.

And I do agree with Dscho's assessment that this is "show a bug
without bothering to fix it", which is not what we usually take
without first exploring how involved the necessary fix would be.

I wonder in what bad status would a production build that simply
disabled the assert() is leaving the resulting repository.

Quoting from the last part of my response [*] to the initial report
that eventually turned into the test after 9 months:

 [*] https://lore.kernel.org/git/[email-masked]/

---- snip snap ----
Thanks for reporting, Kristoffer.

Any takers?

$ git shortlog --since=3.years -s -n -e --no-merges v2.43.0 builtin/mv.c
    15	Shaoxuan Yuan <[email-masked]>
    10	Elijah Newren <[email-masked]>
     5	Ævar Arnfjörð Bjarmason <[email-masked]>
     2	Junio C Hamano <[email-masked]>
     1	Andrzej Hunt <[email-masked]>
     1	Calvin Wan <[email-masked]>
     1	Derrick Stolee <[email-masked]>
     1	Sebastian Thiel <[email-masked]>
     1	Torsten Bögershausen <[email-masked]>