[PATCH] meson: Explicitly specify run_command's check parameter

Martin Kletzander posted 1 patch 2 years, 3 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/feaf72186fe1c03f9ffd597c7c0f85f1144ef96c.1642888485.git.mkletzan@redhat.com
build-aux/meson.build |  2 +-
docs/meson.build      |  4 ++--
meson.build           | 22 ++++++++++++----------
3 files changed, 15 insertions(+), 13 deletions(-)
[PATCH] meson: Explicitly specify run_command's check parameter
Posted by Martin Kletzander 2 years, 3 months ago
An update to meson 0.61.1 meant that it started showing warnings due to the fact
that the default for run_command's 'check' parameter is going to change.  It
unveiled the fact that we were even missing that parameter in some calls where
we expected different outcome.  To make sure the behaviour does not change
specify the parameter explicitly.  In places where we check for the return code
the parameter should be 'false' so that meson does not fail.  In all other cases
the parameter should be set to 'true' to make sure possible failure also stops
meson.

The warning in meson was added in https://github.com/mesonbuild/meson/pull/9304

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
---
 build-aux/meson.build |  2 +-
 docs/meson.build      |  4 ++--
 meson.build           | 22 ++++++++++++----------
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/build-aux/meson.build b/build-aux/meson.build
index e491bdeebc3f..f4d0130e3bf2 100644
--- a/build-aux/meson.build
+++ b/build-aux/meson.build
@@ -20,7 +20,7 @@ endif
 
 if host_machine.system() == 'freebsd'
   grep_prog = find_program('grep')
-  grep_cmd = run_command(grep_prog, '--version')
+  grep_cmd = run_command(grep_prog, '--version', check: true)
   if grep_cmd.stdout().startswith('grep (BSD grep')
     grep_prog = find_program('/usr/local/bin/grep', required: false)
     if not grep_prog.found()
diff --git a/docs/meson.build b/docs/meson.build
index 50c12cc3c255..54bba5e1b1fc 100644
--- a/docs/meson.build
+++ b/docs/meson.build
@@ -132,7 +132,7 @@ aclperms_gen = custom_target(
 )
 
 docs_timestamp = run_command(
-  python3_prog, meson_timestamp_prog.path(), env: runutf8
+  python3_prog, meson_timestamp_prog.path(), env: runutf8, check: true,
 ).stdout().strip()
 
 site_xsl = files('site.xsl')
@@ -206,7 +206,7 @@ endforeach
 #
 # Use this knowledge to detect the version that we know doesn't work
 # for building libvirt and reject it
-rst2html5_version = run_command(rst2html5_prog, '--version')
+rst2html5_version = run_command(rst2html5_prog, '--version', check: true)
 rst2html5_version = rst2html5_version.stdout().split(' ')
 if rst2html5_version[1] != '(Docutils'
   error('Please uninstall the rst2html5 package and install the docutils package')
diff --git a/meson.build b/meson.build
index 70843afcd546..ccb2b73624e0 100644
--- a/meson.build
+++ b/meson.build
@@ -14,10 +14,10 @@ project(
 
 # figure out if we are building from git
 
-git = run_command('test', '-d', '.git').returncode() == 0
+git = run_command('test', '-d', '.git', check: false).returncode() == 0
 
 if git and not get_option('no_git')
-  run_command('git', 'submodule', 'update', '--init')
+  run_command('git', 'submodule', 'update', '--init', check: true)
 endif
 
 
@@ -45,7 +45,7 @@ endif
 if get_option('system')
   prefix = '/usr'
   libdir = prefix / 'lib64'
-  if run_command('test', '-d', libdir).returncode() != 0
+  if run_command('test', '-d', libdir, check: false).returncode() != 0
     libdir = prefix / 'lib'
   endif
   localstatedir = '/var'
@@ -222,6 +222,7 @@ size_max = cc.sizeof('size_t', prefix: '#include <stdint.h>')
 alloc_max = run_command(
   'python3', '-c',
   'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) - 1))'.format(ptrdiff_max, size_max),
+  check: true,
 )
 
 # sanitizer instrumentation may enlarge stack frames
@@ -1096,7 +1097,7 @@ if not get_option('nls').disabled()
   endforeach
 
   if xgettext_prog.found() and msgfmt_prog.found() and msgmerge_prog.found()
-    rc = run_command(msgfmt_prog, '--version')
+    rc = run_command(msgfmt_prog, '--version', check: false)
     if rc.returncode() == 0 and rc.stdout().contains('GNU gettext')
       have_gnu_gettext_tools = true
     endif
@@ -1199,7 +1200,7 @@ selinux_dep = dependency('libselinux', required: get_option('selinux'))
 if selinux_dep.found()
   selinux_mount = get_option('selinux_mount')
   if selinux_mount == ''
-    if run_command('test', '-d', '/sys/fs/selinux').returncode() == 0
+    if run_command('test', '-d', '/sys/fs/selinux', check: false).returncode() == 0
       selinux_mount = '/sys/fs/selinux'
     else
       selinux_mount = '/selinux'
@@ -1658,7 +1659,7 @@ if not get_option('driver_qemu').disabled()
       default_qemu_user = 'root'
       default_qemu_group = 'wheel'
     else
-      os_release = run_command('grep', '^ID=', '/etc/os-release').stdout()
+      os_release = run_command('grep', '^ID=', '/etc/os-release', check: true).stdout()
       if os_release.contains('arch')
         default_qemu_user = 'nobody'
         default_qemu_group = 'nobody'
@@ -1682,8 +1683,8 @@ if not get_option('driver_qemu').disabled()
       # If the expected user and group don't exist, or we haven't hit any
       # of the cases above bacuse we're running on an unknown OS, the only
       # sensible fallback is root:root
-      if (run_command('getent', 'passwd', default_qemu_user).returncode() != 0 and
-          run_command('getent', 'group', default_qemu_group).returncode() != 0)
+      if (run_command('getent', 'passwd', default_qemu_user, check: false).returncode() != 0 and
+          run_command('getent', 'group', default_qemu_group, check: false).returncode() != 0)
         default_qemu_user = 'root'
         default_qemu_group = 'root'
       endif
@@ -2178,7 +2179,8 @@ if git
     )
   endforeach
 
-  authors = run_command(python3_prog, meson_gen_authors_prog.path(), env: runutf8)
+  authors = run_command(python3_prog, meson_gen_authors_prog.path(),
+                        env: runutf8, check: true)
   authors_file = 'AUTHORS.rst.in'
 
   authors_conf = configuration_data()
@@ -2218,7 +2220,7 @@ configure_file(
   output: '@BASENAME@',
   configuration: run_conf,
 )
-run_command('chmod', 'a+x', meson.current_build_dir() / 'run')
+run_command('chmod', 'a+x', meson.current_build_dir() / 'run', check: true)
 
 
 # generate developer tooling files
-- 
2.34.1

Re: [PATCH] meson: Explicitly specify run_command's check parameter
Posted by Tim Wiederhake 2 years, 3 months ago
On Sat, 2022-01-22 at 22:54 +0100, Martin Kletzander wrote:
> An update to meson 0.61.1 meant that it started showing warnings due
> to the fact
> that the default for run_command's 'check' parameter is going to
> change.  It
> unveiled the fact that we were even missing that parameter in some
> calls where
> we expected different outcome.  To make sure the behaviour does not
> change
> specify the parameter explicitly.  In places where we check for the
> return code
> the parameter should be 'false' so that meson does not fail.  In all
> other cases
> the parameter should be set to 'true' to make sure possible failure
> also stops
> meson.
> 
> The warning in meson was added in
> https://github.com/mesonbuild/meson/pull/9304
> 
> Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
> ---
>  build-aux/meson.build |  2 +-
>  docs/meson.build      |  4 ++--
>  meson.build           | 22 ++++++++++++----------
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/build-aux/meson.build b/build-aux/meson.build
> index e491bdeebc3f..f4d0130e3bf2 100644
> --- a/build-aux/meson.build
> +++ b/build-aux/meson.build
> @@ -20,7 +20,7 @@ endif
>  
>  if host_machine.system() == 'freebsd'
>    grep_prog = find_program('grep')
> -  grep_cmd = run_command(grep_prog, '--version')
> +  grep_cmd = run_command(grep_prog, '--version', check: true)
>    if grep_cmd.stdout().startswith('grep (BSD grep')
>      grep_prog = find_program('/usr/local/bin/grep', required: false)
>      if not grep_prog.found()
> diff --git a/docs/meson.build b/docs/meson.build
> index 50c12cc3c255..54bba5e1b1fc 100644
> --- a/docs/meson.build
> +++ b/docs/meson.build
> @@ -132,7 +132,7 @@ aclperms_gen = custom_target(
>  )
>  
>  docs_timestamp = run_command(
> -  python3_prog, meson_timestamp_prog.path(), env: runutf8
> +  python3_prog, meson_timestamp_prog.path(), env: runutf8, check:
> true,
>  ).stdout().strip()
>  
>  site_xsl = files('site.xsl')
> @@ -206,7 +206,7 @@ endforeach
>  #
>  # Use this knowledge to detect the version that we know doesn't work
>  # for building libvirt and reject it
> -rst2html5_version = run_command(rst2html5_prog, '--version')
> +rst2html5_version = run_command(rst2html5_prog, '--version', check:
> true)
>  rst2html5_version = rst2html5_version.stdout().split(' ')
>  if rst2html5_version[1] != '(Docutils'
>    error('Please uninstall the rst2html5 package and install the
> docutils package')
> diff --git a/meson.build b/meson.build
> index 70843afcd546..ccb2b73624e0 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -14,10 +14,10 @@ project(
>  
>  # figure out if we are building from git
>  
> -git = run_command('test', '-d', '.git').returncode() == 0
> +git = run_command('test', '-d', '.git', check: false).returncode()
> == 0
>  
>  if git and not get_option('no_git')
> -  run_command('git', 'submodule', 'update', '--init')
> +  run_command('git', 'submodule', 'update', '--init', check: true)
>  endif
>  
>  
> @@ -45,7 +45,7 @@ endif
>  if get_option('system')
>    prefix = '/usr'
>    libdir = prefix / 'lib64'
> -  if run_command('test', '-d', libdir).returncode() != 0
> +  if run_command('test', '-d', libdir, check: false).returncode() !=
> 0
>      libdir = prefix / 'lib'
>    endif
>    localstatedir = '/var'
> @@ -222,6 +222,7 @@ size_max = cc.sizeof('size_t', prefix: '#include
> <stdint.h>')
>  alloc_max = run_command(
>    'python3', '-c',
>    'print(min(2**(@0@ * 8 - 1) - 1, 2**(@1@ * 8) -
> 1))'.format(ptrdiff_max, size_max),
> +  check: true,
>  )
>  
>  # sanitizer instrumentation may enlarge stack frames
> @@ -1096,7 +1097,7 @@ if not get_option('nls').disabled()
>    endforeach
>  
>    if xgettext_prog.found() and msgfmt_prog.found() and
> msgmerge_prog.found()
> -    rc = run_command(msgfmt_prog, '--version')
> +    rc = run_command(msgfmt_prog, '--version', check: false)
>      if rc.returncode() == 0 and rc.stdout().contains('GNU gettext')
>        have_gnu_gettext_tools = true
>      endif
> @@ -1199,7 +1200,7 @@ selinux_dep = dependency('libselinux',
> required: get_option('selinux'))
>  if selinux_dep.found()
>    selinux_mount = get_option('selinux_mount')
>    if selinux_mount == ''
> -    if run_command('test', '-d', '/sys/fs/selinux').returncode() ==
> 0
> +    if run_command('test', '-d', '/sys/fs/selinux', check:
> false).returncode() == 0
>        selinux_mount = '/sys/fs/selinux'
>      else
>        selinux_mount = '/selinux'
> @@ -1658,7 +1659,7 @@ if not get_option('driver_qemu').disabled()
>        default_qemu_user = 'root'
>        default_qemu_group = 'wheel'
>      else
> -      os_release = run_command('grep', '^ID=', '/etc/os-
> release').stdout()
> +      os_release = run_command('grep', '^ID=', '/etc/os-release',
> check: true).stdout()
>        if os_release.contains('arch')
>          default_qemu_user = 'nobody'
>          default_qemu_group = 'nobody'
> @@ -1682,8 +1683,8 @@ if not get_option('driver_qemu').disabled()
>        # If the expected user and group don't exist, or we haven't
> hit any
>        # of the cases above bacuse we're running on an unknown OS,
> the only
>        # sensible fallback is root:root
> -      if (run_command('getent', 'passwd',
> default_qemu_user).returncode() != 0 and
> -          run_command('getent', 'group',
> default_qemu_group).returncode() != 0)
> +      if (run_command('getent', 'passwd', default_qemu_user, check:
> false).returncode() != 0 and
> +          run_command('getent', 'group', default_qemu_group, check:
> false).returncode() != 0)
>          default_qemu_user = 'root'
>          default_qemu_group = 'root'
>        endif
> @@ -2178,7 +2179,8 @@ if git
>      )
>    endforeach
>  
> -  authors = run_command(python3_prog, meson_gen_authors_prog.path(),
> env: runutf8)
> +  authors = run_command(python3_prog, meson_gen_authors_prog.path(),
> +                        env: runutf8, check: true)
>    authors_file = 'AUTHORS.rst.in'
>  
>    authors_conf = configuration_data()
> @@ -2218,7 +2220,7 @@ configure_file(
>    output: '@BASENAME@',
>    configuration: run_conf,
>  )
> -run_command('chmod', 'a+x', meson.current_build_dir() / 'run')
> +run_command('chmod', 'a+x', meson.current_build_dir() / 'run',
> check: true)
>  
>  
>  # generate developer tooling files

Reviewed-by: Tim Wiederhake <twiederh@redhat.com>