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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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? I
think we should keep it around as it was a bug in the first place that
we didn't have it.

I guess this is done because we didn't have the flag in "maint" yet. 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.

Patrick




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux