Re: [PATCH v2] ci(win+Meson): build in Release mode

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Patrick,

On Tue, 6 May 2025, Patrick Steinhardt wrote:

> On Mon, May 05, 2025 at 08:54:23AM -0700, Junio C Hamano wrote:
> > Patrick Steinhardt <ps@pks.im> writes:
> > 
> > >> I am afraid that getting rid of asserts in Git's codebase won't ever be
> > >> able to address the challenge that Git -- despite much reluctance --
> > >> relies on a couple of external dependencies that might at any point in
> > >> time cause `assert()` to be called, e.g. due to unexpected changes in the
> > >> CI runner images.
> > >
> > > Good point indeed, I haven't considered this.
> > 
> > Thanks both for a discussion.  Let's replace and queue this, and
> > fast track it down to 'maint'.
> > 
> > Here is a range-diff for my tentative rebasing the patch on 'maint';
> > I'll make sure merging it up to 'master' would match exactly the
> > result of applying the original patch directly to 'master' before
> > queuing.
> > 
> > Thanks!
> > 
> > 
> > 1:  f3ae94b175 ! 1:  184abdcf05 ci(win+Meson): build in Release mode
> >     @@ Commit message
> >          patch is still needed.
> >      
> >          Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> >     +    Acked-by: Patrick Steinhardt <ps@pks.im>
> >     +    [jc: rebased on 'maint' to enable fast-tracking the change down]
> >          Signed-off-by: Junio C Hamano <gitster@pobox.com>
> >      
> >       ## .github/workflows/main.yml ##
> >     @@ .github/workflows/main.yml: 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
> >     +-      run: meson setup build -Dperl=disabled -Dcredential_helpers=wincred
> >     ++      run: meson setup build -Dbuildtype=release -Dperl=disabled -Dcredential_helpers=wincred
> >           - name: Compile
> >             shell: pwsh
> >             run: meson compile -C build
> 
> Am I reading this diff correctly that we drop the `--vsenv` flag?

That's similar to my reading as well, but...

> I think we should keep it around as it was a bug in the first place that
> we didn't have it.

Correct.

> I guess this is done because we didn't have the flag in "maint" yet.

... I think this is the reason. It would probably make more sense to
backport the `--vsenv` patch first, seeing as having `win+Meson` without
that flag defeats the purpose of `win+Meson`.

> But I'm not even sure whether the fix would be needed in case we don't
> build with Visual Studio, as the MinGW toolchain probably doesn't have
> the same behaviour with asserts (but please correct me if I'm wrong,
> Dscho). So maybe we should also cherry-pick 85e1d6819fb (ci: use Visual
> Studio for win+meson job on GitHub Workflows, 2025-03-31) at the same
> time.

Indeed. If we keep both bugs unfixed in `maint` (the bug that `win+Meson`
does not build with Visual Studio, plus the bug that `win+Meson` builds in
Debug mode), the CI will continue to pass... just not for the intended
reasons.

Ciao,
Johannes