The `check` argument of `run_command` should be `True` by default #9300

Volker-Weissmann · 2021-09-26T16:48:35Z

The run_command argument takes a check bool-argument. If check: true, is given and the command fails, meson will error out.

May I ask, who thought that it was a good idea to let check default to false? Ignoring errors by default goes against everything I learned.

So essentially, I want something like this, (plus documentation updates)

diff --git a/mesonbuild/interpreter/interpreterobjects.py b/mesonbuild/interpreter/interpreterobjects.py
index fca371cbc..3b3218c54 100644
--- a/mesonbuild/interpreter/interpreterobjects.py
+++ b/mesonbuild/interpreter/interpreterobjects.py
@@ -163,7 +163,7 @@ class RunProcess(MesonInterpreterObject):
                  subdir: str,
                  mesonintrospect: T.List[str],
                  in_builddir: bool = False,
-                 check: bool = False,
+                 check: bool = True,
                  capture: bool = True) -> None:
         super().__init__()
         if not isinstance(cmd, ExternalProgram):
@@ -184,7 +184,7 @@ class RunProcess(MesonInterpreterObject):
                     subdir: str,
                     mesonintrospect: T.List[str],
                     in_builddir: bool,
-                    check: bool = False) -> T.Tuple[int, str, str]:
+                    check: bool = True) -> T.Tuple[int, str, str]:
         command_array = cmd.get_command() + args
         menv = {'MESON_SOURCE_ROOT': source_dir,
                 'MESON_BUILD_ROOT': build_dir,

The text was updated successfully, but these errors were encountered:

xclaesse · 2021-09-26T17:06:32Z

I think it's like that for historical reasons, changing this would not be backward compatible. But I agree it's not ideal, we should warn when check argument is not set for a few releases, then do the flip eventually.

Volker-Weissmann · 2021-09-26T17:37:07Z

I think it's like that for historical reasons, changing this would not be backward compatible.

I can understand that.

we should warn when check argument is not set

Should I write the PR?

eli-schwartz · 2021-09-26T17:57:40Z

I agree that this change would be nice and yes, if you would write the PR that would be great. ;)

eli-schwartz · 2021-11-01T16:43:29Z

Fixed via linked PR.

xclaesse · 2021-11-02T21:34:33Z

FWIW, I checked GStreamer and they have plenty of places where it assumes that check is false by default. I still agree the default should be true, but we'll probably need a long deprecation period because that seems a common thing.

eli-schwartz · 2021-11-02T21:36:22Z

Sure, baby steps for now though. :D

keszybz · 2021-11-17T12:56:16Z

Please, please, always include the source line in messages like this. In large projects, hunting for the right line to fix is a pita.

meson-0.59.4-1.fc35.noarch says: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

eli-schwartz · 2021-11-17T13:50:41Z

You're entirely correct and I apologize for not thinking of this during the review process.

It should be easy to fix -- the mlog functions take an optional location= parameter. Would you like to submit a PR?

meson-0.59.4-1.fc35.noarch says: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

Volker-Weissmann · 2021-11-17T20:41:06Z

Please, please, always include the source line in messages like this. In large projects, hunting for the right line to fix is a pita.

You're right, sorry I forget.

Volker-Weissmann · 2021-11-17T20:41:52Z

You're entirely correct and I apologize for not thinking of this during the review process.

Yeah, it was a real slip up of you to accept this crap. /s

eli-schwartz · 2021-11-17T21:01:54Z

It was a good PR :p it just could have used one specific touch-up.

keszybz · 2021-11-17T22:17:41Z

OK, OK, I'm sorry that I mentioned it. The change is good.

Volker-Weissmann · 2021-11-17T22:23:25Z

OK, OK, I'm sorry that I mentioned it. The change is good.

No, No, it's good that you mentioned it.

Good error messages are important.

meson-0.59.4-1.fc35.noarch says: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

This is required since Meson 0.61.0, and causes a warning to be emitted otherwise: mesonbuild/meson@2c079d8 mesonbuild/meson#9300 This exposed a bunch of places where we had broken run_command() calls, unnecessary run_command() calls, and places where check: true should be used. Part-of: <https://gitlab.freedesktop.org/gstreamer/gstreamer/-/merge_requests/1507>

The include tests are not critical, hence there is no need to halt the build if, for some reason, `find` fails. See mesonbuild/meson#9300

Warning from meson: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 All of the run_command() calls currently use need to succeed for the build to work properly.

As per: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

This explicitly checks the command before assigning to the `version` variable. This also addresses the following warning during building: ``` WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 ```

Fixes the following warning: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

``` Program test-cc-supports-hidden-visibility.py found: YES (/usr/bin/python3 /path/to/libffi/test-cc-supports-hidden-visibility.py) WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 ``` Part-of: <https://gitlab.freedesktop.org/gstreamer/meson-ports/libffi/-/merge_requests/16>

This fixes warnings: Program xsltproc found: YES (/usr/bin/xsltproc) WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 Signed-off-by: Petr Vorel <pvorel@suse.cz>

The `run_command()` invocations in the iniparser meson.build lacked the `check` parameter: ``` iniparser| WARNING: You should add the boolean check kwarg to the run_command call. iniparser| It currently defaults to false, iniparser| but it will default to true in future releases of meson. iniparser| See also: mesonbuild/meson#9300 ``` Add `check` and set it to true to make sure the build configuration only succeeds if these commands also succeed. Fixes: 1e04f44 ("use iniparser dependency for config file parsing") Change-Id: I97841398fc899e876b97e4e28e18cd8dd3b13222 Signed-off-by: Andrew Jeffery <andrew@codeconstruct.com.au>

Set the "check" parameter in run_command() calls, instead of relying on the default value, which might get changed at some point in the fuiture. This silences the folloeing Meson warning: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

Set the "check" parameter in run_command() calls, instead of relying on the default value, which might get changed at some point in the fuiture. This silences the following Meson warning: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

Set the "check" parameter in run_command() calls, instead of relying on the default value, which might get changed at some point in the future. This silences the following Meson warning: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

Set the "check" parameter in run_command() calls, instead of relying on the default value, which might get changed at some point in the future. This silences the following Meson warning: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 (cherry picked from commit f486b18)

"mesonbuild#9300"

meson complains: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

Meson warns about this when the "check" keyword is absent, WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 and the command should indeed succeed, so we now pass "check: true" to run_command().

To make meson happy and get rid of: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300

run_command() wants a check kwarg now: WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>

This addresses the following warning: ``` WARNING: You should add the boolean check kwarg to the run_command call. It currently defaults to false, but it will default to true in future releases of meson. See also: mesonbuild/meson#9300 ```

Volker-Weissmann mentioned this issue Sep 27, 2021

Added warning if check kwarg of run_command is not given. #9304

Merged

eli-schwartz closed this as completed Nov 1, 2021

keszybz mentioned this issue Nov 17, 2021

meson: add check:true/false to all run_command() invocations systemd/systemd#21417

Merged

evverx mentioned this issue Nov 21, 2021

ci: pin python dependencies and let Dependabot keep track of them systemd/systemd#21456

Merged

thompon mentioned this issue Jan 14, 2022

undefined reference to `vmaf_cpu_cpuid Netflix/vmaf#971

Closed

alekgr mentioned this issue Apr 26, 2024

lastest commit doesn't build hyprwm/Hyprland#5760

Closed

vmihalko mentioned this issue May 27, 2024

Fix meson deprecation warnings polkit-org/polkit#447

Merged

AlanSong641 mentioned this issue Jun 19, 2024

Installation with arm64 TheochemUI/eOn#111

Open

cyango mentioned this issue Aug 16, 2024

docker build fails when trying to install pyshtools mpetroff/pannellum#1054

Closed

Ikaros520544 mentioned this issue Aug 22, 2024

试过几十次了，老是卡在这个地方 coolsnowwolf/lede#12429

Closed

1 task

Tymon3310 mentioned this issue Aug 27, 2024

[BUG] yay install fails mylinuxforwork/dotfiles#197

Closed

kittenvr mentioned this issue Sep 8, 2024

[Issue] end-4/dots-hyprland#798

Open

christophecvr added a commit to christophecvr/meson that referenced this issue Sep 23, 2024

The check argument run com should be true

ad02d46

"mesonbuild#9300"

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

The `check` argument of `run_command` should be `True` by default #9300

The `check` argument of `run_command` should be `True` by default #9300

Volker-Weissmann commented Sep 26, 2021

xclaesse commented Sep 26, 2021 •

edited

Loading

Volker-Weissmann commented Sep 26, 2021 •

edited

Loading

eli-schwartz commented Sep 26, 2021

eli-schwartz commented Nov 1, 2021

xclaesse commented Nov 2, 2021

eli-schwartz commented Nov 2, 2021

keszybz commented Nov 17, 2021

eli-schwartz commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

eli-schwartz commented Nov 17, 2021

keszybz commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

The check argument of run_command should be True by default #9300

The check argument of run_command should be True by default #9300

Comments

Volker-Weissmann commented Sep 26, 2021

xclaesse commented Sep 26, 2021 • edited Loading

Volker-Weissmann commented Sep 26, 2021 • edited Loading

eli-schwartz commented Sep 26, 2021

eli-schwartz commented Nov 1, 2021

xclaesse commented Nov 2, 2021

eli-schwartz commented Nov 2, 2021

keszybz commented Nov 17, 2021

eli-schwartz commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

eli-schwartz commented Nov 17, 2021

keszybz commented Nov 17, 2021

Volker-Weissmann commented Nov 17, 2021

The `check` argument of `run_command` should be `True` by default #9300

The `check` argument of `run_command` should be `True` by default #9300

xclaesse commented Sep 26, 2021 •

edited

Loading

Volker-Weissmann commented Sep 26, 2021 •

edited

Loading