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

From: Patrick Steinhardt <ps@pks.im>
Date: Apr 28, 2025, 07:47 UTC
Message-ID: <aA8ymUzWM2t0QkFP@pks.im>
In-Reply-To: <xmqqmsc4uv6d.fsf@gitster.g>

On Fri, Apr 25, 2025 at 08:18:02AM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
> writes:
> 
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > Since switching to `--vsenv`, the t7001-mv test consistently times out
> > after six hours in the CI builds on GitHub. This kind of waste is
> > inconsistent with my values.
> 
> With mine too and I would presume everybody else's.  I've been
> annoyed for a long time by one of those sharded Meson-Win test jobs
> that hang around until timeout.
> 
> Thank you very much for addressing the issue.

Indeed, thanks for fixing the issue! I haven't noticed these failing jobs yet on my end, probably because on GitLab we only execute the Win+Meson changes manually. Which is not great given that it makes me miss issues with Meson like this.

I'll send a patch to run these tests by default so that I can see the pain myself and address any issues that come up before others are forced to.

Show 15 quoted lines

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

Show 13 quoted lines

> > diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
> > index 83ca8e4182b..275240be5dc 100644
> > --- a/.github/workflows/main.yml
> > +++ b/.github/workflows/main.yml
> > @@ -265,7 +265,7 @@ jobs:
> >        run: pip install meson ninja
> >      - name: Setup
> >        shell: pwsh
> > -      run: meson setup build --vsenv -Dperl=disabled -Dcredential_helpers=wincred
> > +      run: meson setup build --vsenv -Dbuildtype=release -Dperl=disabled -Dcredential_helpers=wincred
> >      - name: Compile
> >        shell: pwsh
> >        run: meson compile -C build

So I'm fine with this trivial change as a band aid for now. The diff looks obviously good to me.

Patrick