git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Kristoffer Haugsbakk <code@khaugsbakk.name>
Subject: Re: [PATCH v2] ci(win+Meson): build in Release mode
Date: Tue, 6 May 2025 09:57:16 +0200	[thread overview]
Message-ID: <aBnA3KNOfAo9EbpS@pks.im> (raw)
In-Reply-To: <xmqqmsbroxxs.fsf@gitster.g>

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

  reply	other threads:[~2025-05-06  7:57 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25 15:05 [PATCH] ci(win+Meson): build in Release mode, avoiding t7001-mv hangs Johannes Schindelin via GitGitGadget
2025-04-25 15:18 ` Junio C Hamano
2025-04-28  7:47   ` Patrick Steinhardt
2025-04-28 17:01     ` Junio C Hamano
2025-04-29 12:20       ` Patrick Steinhardt
2025-04-29 20:48         ` Junio C Hamano
2025-04-30  8:58           ` Patrick Steinhardt
2025-04-30 12:46             ` Patrick Steinhardt
2025-04-29 20:57 ` Kristoffer Haugsbakk
2025-05-03 14:25 ` [PATCH v2] ci(win+Meson): build in Release mode Johannes Schindelin via GitGitGadget
2025-05-05  6:06   ` Patrick Steinhardt
2025-05-05  7:27     ` Johannes Schindelin
2025-05-05  9:43       ` Patrick Steinhardt
2025-05-05 15:54         ` Junio C Hamano
2025-05-06  7:57           ` Patrick Steinhardt [this message]
2025-05-06  8:32             ` Johannes Schindelin
2025-05-08 18:19               ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aBnA3KNOfAo9EbpS@pks.im \
    --to=ps@pks.im \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=code@khaugsbakk.name \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).