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@xxxxxx> 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@xxxxxx> > > + Acked-by: Patrick Steinhardt <ps@xxxxxx> > > + [jc: rebased on 'maint' to enable fast-tracking the change down] > > Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx> > > > > ## .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
