Andrea Bolognani (6): meson: Do less when not building from git meson: Move all handling of test options together meson: Handle -Dtests=enabled with Clang meson: Make -Dexpensive_tests depend on -Dtests meson: Disable all tests when tests are disabled meson: Rename build_tests -> tests_enabled build-aux/meson.build | 99 ++++++++++---------- meson.build | 75 +++++++++------ meson_options.txt | 2 +- src/access/meson.build | 16 ++-- src/meson.build | 204 +++++++++++++++++++++-------------------- 5 files changed, 207 insertions(+), 189 deletions(-) -- 2.41.0
As explained in the comment, the syntax-check machinery uses git to figure out the list of files it should operate on, so we can only enable it when building from git. Despite only registering the various tests with meson in that case, however, we unconditionally perform a bunch of preparation that is only useful for the purpose of registering and running the tests. If we're not going to do that, we can skip a few steps and save a bit of time. Best viewed with 'git show -w'. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 99 +++++++++++++++++++++---------------------- 1 file changed, 49 insertions(+), 50 deletions(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index 16d085505d..b5d88a4c44 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -1,63 +1,62 @@ -flake8_path = '' -if flake8_prog.found() - flake8_path = flake8_prog.full_path() -endif +# Skip syntax-check if not building from git because we get the list of files +# to check using git commands and it fails if we are not in git repository. +if git + flake8_path = '' + if flake8_prog.found() + flake8_path = flake8_prog.full_path() + endif -if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin' - make_prog = find_program('gmake') - sed_prog = find_program('gsed') -else - make_prog = find_program('make') - sed_prog = find_program('sed') -endif + if host_machine.system() == 'freebsd' or host_machine.system() == 'darwin' + make_prog = find_program('gmake') + sed_prog = find_program('gsed') + else + make_prog = find_program('make') + sed_prog = find_program('sed') + endif -if host_machine.system() == 'freebsd' - grep_prog = find_program('grep') - 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() - error('GNU grep not found') + if host_machine.system() == 'freebsd' + grep_prog = find_program('grep') + 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() + error('GNU grep not found') + endif endif + elif host_machine.system() == 'darwin' + grep_prog = find_program('ggrep') + else + grep_prog = find_program('grep') endif -elif host_machine.system() == 'darwin' - grep_prog = find_program('ggrep') -else - grep_prog = find_program('grep') -endif - -awk_prog = find_program('awk') -syntax_check_conf = configuration_data({ - 'top_srcdir': meson.project_source_root(), - 'top_builddir': meson.project_build_root(), - 'flake8_path': flake8_path, - 'runutf8': ' '.join(runutf8), - 'PYTHON3': python3_prog.full_path(), - 'GREP': grep_prog.full_path(), - 'SED': sed_prog.full_path(), - 'AWK': awk_prog.full_path(), -}) + awk_prog = find_program('awk') -configure_file( - input: 'Makefile.in', - output: '@BASENAME@', - configuration: syntax_check_conf, -) + syntax_check_conf = configuration_data({ + 'top_srcdir': meson.project_source_root(), + 'top_builddir': meson.project_build_root(), + 'flake8_path': flake8_path, + 'runutf8': ' '.join(runutf8), + 'PYTHON3': python3_prog.full_path(), + 'GREP': grep_prog.full_path(), + 'SED': sed_prog.full_path(), + 'AWK': awk_prog.full_path(), + }) -rc = run_command( - 'sed', '-n', - 's/^sc_\\([a-zA-Z0-9_-]*\\):.*/\\1/p', - meson.current_source_dir() / 'syntax-check.mk', - check: true, -) + configure_file( + input: 'Makefile.in', + output: '@BASENAME@', + configuration: syntax_check_conf, + ) -sc_tests = rc.stdout().strip().split() + rc = run_command( + 'sed', '-n', + 's/^sc_\\([a-zA-Z0-9_-]*\\):.*/\\1/p', + meson.current_source_dir() / 'syntax-check.mk', + check: true, + ) + sc_tests = rc.stdout().strip().split() -# Skip syntax-check if not building from git because we get the list of files -# to check using git commands and it fails if we are not in git repository. -if git foreach target : sc_tests test( target, -- 2.41.0
This will make future patches nicer. Note that we need to handle these somewhat late because of the dependency on information about the compiler and the flags it supports. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 57 +++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 28 deletions(-) diff --git a/meson.build b/meson.build index 6fa1f74670..ef0b5641de 100644 --- a/meson.build +++ b/meson.build @@ -151,23 +151,6 @@ if packager_version != '' endif -# test options - -if get_option('expensive_tests').auto() - use_expensive_tests = not git -else - use_expensive_tests = get_option('expensive_tests').enabled() -endif - -coverage_flags = [] -if get_option('test_coverage') - coverage_flags = [ - '-fprofile-arcs', - '-ftest-coverage', - ] -endif - - # Add RPATH information when building for a non-standard prefix, or # when explicitly requested to do so @@ -2028,6 +2011,35 @@ if conf.has('WITH_DECL_SYS_PIDFD_OPEN') conf.set('WITH_NBDKIT', 1) endif + +# test options + +build_tests = [ not get_option('tests').disabled() ] +if build_tests[0] and \ + cc.get_id() == 'clang' and \ + not supported_cc_flags.contains('-fsemantic-interposition') \ + and get_option('optimization') != '0' + # If CLang doesn't support -fsemantic-interposition then our + # mocking doesn't work. The best we can do is to not run the + # test suite. + build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks -fsemantic-interposition. Update CLang or disable optimization !!!' ] +endif + +if get_option('expensive_tests').auto() + use_expensive_tests = not git +else + use_expensive_tests = get_option('expensive_tests').enabled() +endif + +coverage_flags = [] +if get_option('test_coverage') + coverage_flags = [ + '-fprofile-arcs', + '-ftest-coverage', + ] +endif + + # Various definitions # Python3 < 3.7 treats the C locale as 7-bit only. We must force env vars so @@ -2051,17 +2063,6 @@ subdir('src') subdir('tools') -build_tests = [ not get_option('tests').disabled() ] -if build_tests[0] and \ - cc.get_id() == 'clang' and \ - not supported_cc_flags.contains('-fsemantic-interposition') \ - and get_option('optimization') != '0' - # If CLang doesn't support -fsemantic-interposition then our - # mocking doesn't work. The best we can do is to not run the - # test suite. - build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks -fsemantic-interposition. Update CLang or disable optimization !!!' ] -endif - if build_tests[0] subdir('tests') endif -- 2.41.0
There are some cases in which we automatically disable tests when using Clang as the compiler. If the user has explicitly asked for tests to be enabled, however, we should error out instead of silently disabling things. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index ef0b5641de..ed9a07942d 100644 --- a/meson.build +++ b/meson.build @@ -2022,7 +2022,11 @@ if build_tests[0] and \ # If CLang doesn't support -fsemantic-interposition then our # mocking doesn't work. The best we can do is to not run the # test suite. - build_tests = [ false, '!!! Forcibly disabling tests because CLang lacks -fsemantic-interposition. Update CLang or disable optimization !!!' ] + msg = 'Forcibly disabling tests because CLang lacks -fsemantic-interposition. Update CLang or disable optimization' + if get_option('tests').enabled() + error(msg) + endif + build_tests = [ false, '!!! @0@ !!!'.format(msg) ] endif if get_option('expensive_tests').auto() -- 2.41.0
It only makes sense to enable expensive tests when tests are enabled. Disallow invalid configurations. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- meson.build | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/meson.build b/meson.build index ed9a07942d..7ec2a6fd1e 100644 --- a/meson.build +++ b/meson.build @@ -2030,9 +2030,12 @@ if build_tests[0] and \ endif if get_option('expensive_tests').auto() - use_expensive_tests = not git + use_expensive_tests = not git and build_tests[0] else use_expensive_tests = get_option('expensive_tests').enabled() + if use_expensive_tests and not build_tests[0] + error('cannot enable expensive tests when tests are disabled') + endif endif coverage_flags = [] -- 2.41.0
Currently, passing -Dtests=disabled only disables a subset of tests: those that are written in C and thus require compilation. Other tests, such as the syntax-check ones and those that are implemented as scripts, are always enabled. There's a potentially dangerous consequence of this behavior: when tests are disabled, 'meson test' will succeed as if they had been enabled. No indication of this will be shown, so the user will likely make the reasonable assumption that everything is fine when in fact the significantly reduced coverage might be hiding failures. To solve this issues, disable *all* tests when asked to do so, and inject an intentionally failing test to ensure that 'meson test' doesn't succeed. Best viewed with 'git show -w'. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 2 +- meson.build | 7 ++ src/access/meson.build | 16 ++-- src/meson.build | 204 +++++++++++++++++++++-------------------- 4 files changed, 120 insertions(+), 109 deletions(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index b5d88a4c44..84405c5ec8 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -1,6 +1,6 @@ # Skip syntax-check if not building from git because we get the list of files # to check using git commands and it fails if we are not in git repository. -if git +if git and build_tests[0] flake8_path = '' if flake8_prog.found() flake8_path = flake8_prog.full_path() diff --git a/meson.build b/meson.build index 7ec2a6fd1e..5a1a81d087 100644 --- a/meson.build +++ b/meson.build @@ -2072,6 +2072,13 @@ subdir('tools') if build_tests[0] subdir('tests') +else + # Ensure that 'meson test' fails when tests are disabled, as opposed to + # misleadingly succeeding at doing absolutely nothing + test( + 'tests-are-disabled', + python3_prog, args: [ '-c', 'raise Exception("tests are disabled")' ], + ) endif subdir('examples') diff --git a/src/access/meson.build b/src/access/meson.build index e65f17c0a2..6ca953c932 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -105,10 +105,12 @@ access_dep = declare_dependency( generated_sym_files += access_gen_sym -test( - 'check-aclperms', - python3_prog, - args: [ check_aclperms_prog.full_path(), access_perm_h, files('viraccessperm.c') ], - env: runutf8, - suite: 'script' -) +if build_tests[0] + test( + 'check-aclperms', + python3_prog, + args: [ check_aclperms_prog.full_path(), access_perm_h, files('viraccessperm.c') ], + env: runutf8, + suite: 'script' + ) +endif diff --git a/src/meson.build b/src/meson.build index 144f24e526..0c64cef04e 100644 --- a/src/meson.build +++ b/src/meson.build @@ -942,121 +942,123 @@ meson.add_install_script( # Check driver files -if host_machine.system() == 'linux' - test( - 'check-symfile', - python3_prog, - args: [ check_symfile_prog.full_path(), libvirt_syms, libvirt_lib ], - env: runutf8, - suite: 'script' - ) - - if conf.has('WITH_REMOTE') +if build_tests[0] + if host_machine.system() == 'linux' test( - 'check-admin-symfile', + 'check-symfile', python3_prog, - args: [ check_symfile_prog.full_path(), libvirt_admin_syms, libvirt_admin_lib ], + args: [ check_symfile_prog.full_path(), libvirt_syms, libvirt_lib ], env: runutf8, suite: 'script' ) + + if conf.has('WITH_REMOTE') + test( + 'check-admin-symfile', + python3_prog, + args: [ check_symfile_prog.full_path(), libvirt_admin_syms, libvirt_admin_lib ], + env: runutf8, + suite: 'script' + ) + endif endif -endif -test( - 'check-symsorting', - python3_prog, - args: [ - check_symsorting_prog.full_path(), - meson.current_source_dir(), - files(sym_files, used_sym_files), - ], - env: runutf8, - suite: 'script' -) + test( + 'check-symsorting', + python3_prog, + args: [ + check_symsorting_prog.full_path(), + meson.current_source_dir(), + files(sym_files, used_sym_files), + ], + env: runutf8, + suite: 'script' + ) -test( - 'check-admin-symsorting', - python3_prog, - args: [ - check_symsorting_prog.full_path(), - meson.current_source_dir(), - libvirt_admin_private_syms, - ], - env: runutf8, - suite: 'script' -) + test( + 'check-admin-symsorting', + python3_prog, + args: [ + check_symsorting_prog.full_path(), + meson.current_source_dir(), + libvirt_admin_private_syms, + ], + env: runutf8, + suite: 'script' + ) -test( - 'check-drivername', - python3_prog, - args: [ - check_drivername_prog.full_path(), files(driver_headers), - files('libvirt_public.syms'), libvirt_qemu_syms, libvirt_lxc_syms, - ], - env: runutf8, - suite: 'script' -) + test( + 'check-drivername', + python3_prog, + args: [ + check_drivername_prog.full_path(), files(driver_headers), + files('libvirt_public.syms'), libvirt_qemu_syms, libvirt_lxc_syms, + ], + env: runutf8, + suite: 'script' + ) -test( - 'check-admin-drivername', - python3_prog, - args: [ - check_drivername_prog.full_path(), libvirt_admin_public_syms, - ], - env: runutf8, - suite: 'script' -) + test( + 'check-admin-drivername', + python3_prog, + args: [ + check_drivername_prog.full_path(), libvirt_admin_public_syms, + ], + env: runutf8, + suite: 'script' + ) -test( - 'check-driverimpls', - python3_prog, - args: [ check_driverimpls_prog.full_path(), driver_source_files ], - env: runutf8, - suite: 'script' -) + test( + 'check-driverimpls', + python3_prog, + args: [ check_driverimpls_prog.full_path(), driver_source_files ], + env: runutf8, + suite: 'script' + ) -test( - 'check-aclrules', - python3_prog, - args: [ check_aclrules_prog.full_path(), files('remote/remote_protocol.x'), stateful_driver_source_files ], - env: runutf8, - suite: 'script' -) + test( + 'check-aclrules', + python3_prog, + args: [ check_aclrules_prog.full_path(), files('remote/remote_protocol.x'), stateful_driver_source_files ], + env: runutf8, + suite: 'script' + ) -if augparse_prog.found() - foreach data : augeas_test_data - test( - 'check-augeas-@0@'.format(data['name']), - augparse_prog, - args: [ - '-I', data['srcdir'], - '-I', data['builddir'], - data['file'].full_path(), - ], - suite: 'script' - ) - endforeach -endif + if augparse_prog.found() + foreach data : augeas_test_data + test( + 'check-augeas-@0@'.format(data['name']), + augparse_prog, + args: [ + '-I', data['srcdir'], + '-I', data['builddir'], + data['file'].full_path(), + ], + suite: 'script' + ) + endforeach + endif -if pdwtags_prog.found() and cc.get_id() != 'clang' - foreach proto : check_protocols - lib = proto['lib'] - test( - 'check-@0@'.format(proto['name']), - python3_prog, - args: [ - check_remote_protocol_prog.full_path(), - proto['name'], - lib.name(), - lib.full_path(), - pdwtags_prog.full_path(), - files('@0@-structs'.format(proto['name'])), - ], - env: runutf8, - depends: [ lib ], - suite: 'script' - ) - endforeach + if pdwtags_prog.found() and cc.get_id() != 'clang' + foreach proto : check_protocols + lib = proto['lib'] + test( + 'check-@0@'.format(proto['name']), + python3_prog, + args: [ + check_remote_protocol_prog.full_path(), + proto['name'], + lib.name(), + lib.full_path(), + pdwtags_prog.full_path(), + files('@0@-structs'.format(proto['name'])), + ], + env: runutf8, + depends: [ lib ], + suite: 'script' + ) + endforeach + endif endif # configure pkg-config files for run script -- 2.41.0
Given that this variable now controls not just whether C tests are built, but also whether any test at all is executed, the new name is more appropriate. Update the description for the corresponding meson option accordingly. Signed-off-by: Andrea Bolognani <abologna@redhat.com> --- build-aux/meson.build | 2 +- meson.build | 14 +++++++------- meson_options.txt | 2 +- src/access/meson.build | 2 +- src/meson.build | 2 +- 5 files changed, 11 insertions(+), 11 deletions(-) diff --git a/build-aux/meson.build b/build-aux/meson.build index 84405c5ec8..f96d46c445 100644 --- a/build-aux/meson.build +++ b/build-aux/meson.build @@ -1,6 +1,6 @@ # Skip syntax-check if not building from git because we get the list of files # to check using git commands and it fails if we are not in git repository. -if git and build_tests[0] +if git and tests_enabled[0] flake8_path = '' if flake8_prog.found() flake8_path = flake8_prog.full_path() diff --git a/meson.build b/meson.build index 5a1a81d087..397315a77d 100644 --- a/meson.build +++ b/meson.build @@ -2014,8 +2014,8 @@ endif # test options -build_tests = [ not get_option('tests').disabled() ] -if build_tests[0] and \ +tests_enabled = [ not get_option('tests').disabled() ] +if tests_enabled[0] and \ cc.get_id() == 'clang' and \ not supported_cc_flags.contains('-fsemantic-interposition') \ and get_option('optimization') != '0' @@ -2026,14 +2026,14 @@ if build_tests[0] and \ if get_option('tests').enabled() error(msg) endif - build_tests = [ false, '!!! @0@ !!!'.format(msg) ] + tests_enabled = [ false, '!!! @0@ !!!'.format(msg) ] endif if get_option('expensive_tests').auto() - use_expensive_tests = not git and build_tests[0] + use_expensive_tests = not git and tests_enabled[0] else use_expensive_tests = get_option('expensive_tests').enabled() - if use_expensive_tests and not build_tests[0] + if use_expensive_tests and not tests_enabled[0] error('cannot enable expensive tests when tests are disabled') endif endif @@ -2070,7 +2070,7 @@ subdir('src') subdir('tools') -if build_tests[0] +if tests_enabled[0] subdir('tests') else # Ensure that 'meson test' fails when tests are disabled, as opposed to @@ -2294,7 +2294,7 @@ endif misc_summary = { 'Warning Flags': supported_cc_flags, 'docs': gen_docs, - 'tests': build_tests, + 'tests': tests_enabled, 'DTrace': conf.has('WITH_DTRACE_PROBES'), 'firewalld': conf.has('WITH_FIREWALLD'), 'firewalld-zone': conf.has('WITH_FIREWALLD_ZONE'), diff --git a/meson_options.txt b/meson_options.txt index ba6e49afc5..16812f7ade 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -10,7 +10,7 @@ option('git_werror', type: 'feature', value: 'auto', description: 'use -Werror i option('rpath', type: 'feature', value: 'auto', description: 'whether to include rpath information in installed binaries and libraries') option('docdir', type: 'string', value: '', description: 'documentation installation directory') option('docs', type: 'feature', value: 'auto', description: 'whether to generate documentation') -option('tests', type: 'feature', value: 'auto', description: 'whether to build tests') +option('tests', type: 'feature', value: 'auto', description: 'whether to build and run tests') # build dependencies options diff --git a/src/access/meson.build b/src/access/meson.build index 6ca953c932..fc5ba5b342 100644 --- a/src/access/meson.build +++ b/src/access/meson.build @@ -105,7 +105,7 @@ access_dep = declare_dependency( generated_sym_files += access_gen_sym -if build_tests[0] +if tests_enabled[0] test( 'check-aclperms', python3_prog, diff --git a/src/meson.build b/src/meson.build index 0c64cef04e..14686065bf 100644 --- a/src/meson.build +++ b/src/meson.build @@ -942,7 +942,7 @@ meson.add_install_script( # Check driver files -if build_tests[0] +if tests_enabled[0] if host_machine.system() == 'linux' test( 'check-symfile', -- 2.41.0
On Tue, Oct 03, 2023 at 04:56:31PM +0200, Andrea Bolognani wrote:
Andrea Bolognani (6): meson: Do less when not building from git meson: Move all handling of test options together meson: Handle -Dtests=enabled with Clang meson: Make -Dexpensive_tests depend on -Dtests meson: Disable all tests when tests are disabled meson: Rename build_tests -> tests_enabled
build-aux/meson.build | 99 ++++++++++---------- meson.build | 75 +++++++++------ meson_options.txt | 2 +- src/access/meson.build | 16 ++-- src/meson.build | 204 +++++++++++++++++++++-------------------- 5 files changed, 207 insertions(+), 189 deletions(-)
Ping. This no longer applies cleanly after 7cbd8c423057 but the conflict is easy enough to handle, so I don't think it's worth posting v2 because of it. -- Andrea Bolognani / Red Hat / Virtualization
On 10/3/23 16:56, Andrea Bolognani wrote:
Andrea Bolognani (6): meson: Do less when not building from git meson: Move all handling of test options together meson: Handle -Dtests=enabled with Clang meson: Make -Dexpensive_tests depend on -Dtests meson: Disable all tests when tests are disabled meson: Rename build_tests -> tests_enabled
build-aux/meson.build | 99 ++++++++++---------- meson.build | 75 +++++++++------ meson_options.txt | 2 +- src/access/meson.build | 16 ++-- src/meson.build | 204 +++++++++++++++++++++-------------------- 5 files changed, 207 insertions(+), 189 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
On Wed, Oct 25, 2023 at 09:41:40AM +0200, Michal Prívozník wrote:
On 10/3/23 16:56, Andrea Bolognani wrote:
Andrea Bolognani (6): meson: Do less when not building from git meson: Move all handling of test options together meson: Handle -Dtests=enabled with Clang meson: Make -Dexpensive_tests depend on -Dtests meson: Disable all tests when tests are disabled meson: Rename build_tests -> tests_enabled
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Thanks a lot for the review! Unfortunately I have realized that this, merged as-is, would break things quite badly on macOS and consequently result in every single pipeline failing. Wouldn't want that :) Luckily the fix is not complicated and doesn't invalidate most of the series either. v2 here: https://listman.redhat.com/archives/libvir-list/2023-October/242876.html -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Michal Prívozník