qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] meson: set test programs to not build by default
@ 2025-08-19 15:49 Eli Schwartz
  2025-08-19 16:31 ` Daniel P. Berrangé
  0 siblings, 1 reply; 4+ messages in thread
From: Eli Schwartz @ 2025-08-19 15:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Michael Roth, Kostiantyn Kostiuk, Manos Pitsidianakis,
	Aurelien Jarno, Peter Maydell, Alex Bennée, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:Rust-related patc...

Rather, they are built when running `meson test`. This change is inert
unless building with meson 1.7, as previous versions of meson build all
`meson test` dependencies as part of `ninja all` as well.

See:
https://mesonbuild.com/Release-notes-for-1-7-0.html#test-targets-no-longer-built-by-default

An existing comment references this meson issue, with an included bug
reference, but was written before meson 1.7 fixed the bug. Update the
comment to change the advice from "if the bug gets fixed" to "when
bumping the minimum meson version".

Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
---
 qga/meson.build              | 1 +
 rust/qemu-api/meson.build    | 1 +
 tests/fp/meson.build         | 2 ++
 tests/functional/meson.build | 9 ++++-----
 tests/qtest/meson.build      | 2 +-
 tests/unit/meson.build       | 2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/qga/meson.build b/qga/meson.build
index 89a4a8f713..44175ccedd 100644
--- a/qga/meson.build
+++ b/qga/meson.build
@@ -197,6 +197,7 @@ if host_os != 'windows' and not get_option('fuzzing')
     i = i + 1
   endforeach
   qga_ssh_test = executable('qga-ssh-test', srcs,
+                            build_by_default: false,
                             dependencies: [qemuutil],
                             c_args: ['-DQGA_BUILD_UNIT_TEST'])
 
diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
index a090297c45..fec38d6726 100644
--- a/rust/qemu-api/meson.build
+++ b/rust/qemu-api/meson.build
@@ -102,6 +102,7 @@ test('rust-qemu-api-integration',
     executable(
         'rust-qemu-api-integration',
         files('tests/tests.rs', 'tests/vmstate_tests.rs'),
+        build_by_default: false,
         override_options: ['rust_std=2021', 'build.rust_std=2021'],
         rust_args: ['--test'],
         install: false,
diff --git a/tests/fp/meson.build b/tests/fp/meson.build
index 9059a24752..9c50452bf1 100644
--- a/tests/fp/meson.build
+++ b/tests/fp/meson.build
@@ -57,6 +57,7 @@ fpcflags = [
 fptest = executable(
   'fp-test',
   ['fp-test.c', '../../fpu/softfloat.c'],
+  build_by_default: false,
   dependencies: [qemuutil, libsoftfloat, libtestfloat, libslowfloat],
   c_args: fpcflags,
 )
@@ -149,6 +150,7 @@ executable(
 fptestlog2 = executable(
   'fp-test-log2',
   ['fp-test-log2.c', '../../fpu/softfloat.c'],
+  build_by_default: false,
   dependencies: [qemuutil, libsoftfloat],
   c_args: fpcflags,
 )
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 311c6f1806..6d7dc91954 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -408,11 +408,10 @@ foreach speed : ['quick', 'thorough']
       # Ideally we would add 'precache' to 'depends' here, such that
       # 'build_by_default: false' lets the pre-caching automatically
       # run immediately before the test runs. In practice this is
-      # broken in meson, with it running the pre-caching in the normal
-      # compile phase https://github.com/mesonbuild/meson/issues/2518
-      # If the above bug ever gets fixed, when QEMU changes the min
-      # meson version, add the 'depends' and remove the custom
-      # 'run_target' logic below & in Makefile.include
+      # broken in older versions of meson, with it running the
+      # pre-caching in the normal compile phase. When QEMU changes
+      # the min meson version to >=1.7, add the 'depends' and remove
+      # the custom 'run_target' logic below & in Makefile.include
       test('func-' + testname,
            python,
            depends: [test_deps, test_emulator, emulator_modules, plugin_modules],
diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 669d07c06b..0a5309005f 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -447,7 +447,7 @@ foreach dir : target_dirs
         deps += test_ss.all_dependencies()
       endif
       qtest_executables += {
-        test: executable(test, src, dependencies: deps)
+        test: executable(test, src, build_by_default: false, dependencies: deps)
       }
     endif
 
diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index d5248ae51d..af9725a3f8 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -189,7 +189,7 @@ foreach test_name, extra: tests
     src += test_ss.all_sources()
     deps += test_ss.all_dependencies()
   endif
-  exe = executable(test_name, src, genh, dependencies: deps)
+  exe = executable(test_name, src, genh, build_by_default: false, dependencies: deps)
 
   test(test_name, exe,
        depends: test_deps.get(test_name, []),
-- 
2.49.1



^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] meson: set test programs to not build by default
  2025-08-19 15:49 [PATCH] meson: set test programs to not build by default Eli Schwartz
@ 2025-08-19 16:31 ` Daniel P. Berrangé
  2025-08-20  5:12   ` Eli Schwartz
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel P. Berrangé @ 2025-08-19 16:31 UTC (permalink / raw)
  To: Eli Schwartz
  Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, Manos Pitsidianakis,
	Aurelien Jarno, Peter Maydell, Alex Bennée, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:Rust-related patc...

On Tue, Aug 19, 2025 at 11:49:40AM -0400, Eli Schwartz wrote:
> Rather, they are built when running `meson test`. This change is inert
> unless building with meson 1.7, as previous versions of meson build all
> `meson test` dependencies as part of `ninja all` as well.
> 
> See:
> https://mesonbuild.com/Release-notes-for-1-7-0.html#test-targets-no-longer-built-by-default
> 
> An existing comment references this meson issue, with an included bug
> reference, but was written before meson 1.7 fixed the bug. Update the
> comment to change the advice from "if the bug gets fixed" to "when
> bumping the minimum meson version".

I am very much not a fan of projects that do not build test
programs by default.

If a dev is changing code and running 'ninja'/'make' everything
that is affected should be rebuilt to identify any build failures
that may have introduced immediately. This should include all
test binaries, regardless of whether the dev is intending to run
the tests at that time or not.

We already have too many occassions when contributors submit
patches that have tests which fail to pass & this will expand
that problem with contributors submitting patches that fail
to even pass compilation of the tests.

If we want the ability to opt-out of building tests by default
so reduce build time, IMHO that should be behind a 'configure'
flag / meson_options.txt setting.

> 
> Signed-off-by: Eli Schwartz <eschwartz@gentoo.org>
> ---
>  qga/meson.build              | 1 +
>  rust/qemu-api/meson.build    | 1 +
>  tests/fp/meson.build         | 2 ++
>  tests/functional/meson.build | 9 ++++-----
>  tests/qtest/meson.build      | 2 +-
>  tests/unit/meson.build       | 2 +-
>  6 files changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/qga/meson.build b/qga/meson.build
> index 89a4a8f713..44175ccedd 100644
> --- a/qga/meson.build
> +++ b/qga/meson.build
> @@ -197,6 +197,7 @@ if host_os != 'windows' and not get_option('fuzzing')
>      i = i + 1
>    endforeach
>    qga_ssh_test = executable('qga-ssh-test', srcs,
> +                            build_by_default: false,
>                              dependencies: [qemuutil],
>                              c_args: ['-DQGA_BUILD_UNIT_TEST'])
>  
> diff --git a/rust/qemu-api/meson.build b/rust/qemu-api/meson.build
> index a090297c45..fec38d6726 100644
> --- a/rust/qemu-api/meson.build
> +++ b/rust/qemu-api/meson.build
> @@ -102,6 +102,7 @@ test('rust-qemu-api-integration',
>      executable(
>          'rust-qemu-api-integration',
>          files('tests/tests.rs', 'tests/vmstate_tests.rs'),
> +        build_by_default: false,
>          override_options: ['rust_std=2021', 'build.rust_std=2021'],
>          rust_args: ['--test'],
>          install: false,
> diff --git a/tests/fp/meson.build b/tests/fp/meson.build
> index 9059a24752..9c50452bf1 100644
> --- a/tests/fp/meson.build
> +++ b/tests/fp/meson.build
> @@ -57,6 +57,7 @@ fpcflags = [
>  fptest = executable(
>    'fp-test',
>    ['fp-test.c', '../../fpu/softfloat.c'],
> +  build_by_default: false,
>    dependencies: [qemuutil, libsoftfloat, libtestfloat, libslowfloat],
>    c_args: fpcflags,
>  )
> @@ -149,6 +150,7 @@ executable(
>  fptestlog2 = executable(
>    'fp-test-log2',
>    ['fp-test-log2.c', '../../fpu/softfloat.c'],
> +  build_by_default: false,
>    dependencies: [qemuutil, libsoftfloat],
>    c_args: fpcflags,
>  )
> diff --git a/tests/functional/meson.build b/tests/functional/meson.build
> index 311c6f1806..6d7dc91954 100644
> --- a/tests/functional/meson.build
> +++ b/tests/functional/meson.build
> @@ -408,11 +408,10 @@ foreach speed : ['quick', 'thorough']
>        # Ideally we would add 'precache' to 'depends' here, such that
>        # 'build_by_default: false' lets the pre-caching automatically
>        # run immediately before the test runs. In practice this is
> -      # broken in meson, with it running the pre-caching in the normal
> -      # compile phase https://github.com/mesonbuild/meson/issues/2518
> -      # If the above bug ever gets fixed, when QEMU changes the min
> -      # meson version, add the 'depends' and remove the custom
> -      # 'run_target' logic below & in Makefile.include
> +      # broken in older versions of meson, with it running the
> +      # pre-caching in the normal compile phase. When QEMU changes
> +      # the min meson version to >=1.7, add the 'depends' and remove
> +      # the custom 'run_target' logic below & in Makefile.include
>        test('func-' + testname,
>             python,
>             depends: [test_deps, test_emulator, emulator_modules, plugin_modules],
> diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
> index 669d07c06b..0a5309005f 100644
> --- a/tests/qtest/meson.build
> +++ b/tests/qtest/meson.build
> @@ -447,7 +447,7 @@ foreach dir : target_dirs
>          deps += test_ss.all_dependencies()
>        endif
>        qtest_executables += {
> -        test: executable(test, src, dependencies: deps)
> +        test: executable(test, src, build_by_default: false, dependencies: deps)
>        }
>      endif
>  
> diff --git a/tests/unit/meson.build b/tests/unit/meson.build
> index d5248ae51d..af9725a3f8 100644
> --- a/tests/unit/meson.build
> +++ b/tests/unit/meson.build
> @@ -189,7 +189,7 @@ foreach test_name, extra: tests
>      src += test_ss.all_sources()
>      deps += test_ss.all_dependencies()
>    endif
> -  exe = executable(test_name, src, genh, dependencies: deps)
> +  exe = executable(test_name, src, genh, build_by_default: false, dependencies: deps)
>  
>    test(test_name, exe,
>         depends: test_deps.get(test_name, []),
> -- 
> 2.49.1
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] meson: set test programs to not build by default
  2025-08-19 16:31 ` Daniel P. Berrangé
@ 2025-08-20  5:12   ` Eli Schwartz
  2025-08-20 10:23     ` Daniel P. Berrangé
  0 siblings, 1 reply; 4+ messages in thread
From: Eli Schwartz @ 2025-08-20  5:12 UTC (permalink / raw)
  To: Daniel P. Berrangé
  Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, Manos Pitsidianakis,
	Aurelien Jarno, Peter Maydell, Alex Bennée, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:Rust-related patc...


[-- Attachment #1.1: Type: text/plain, Size: 2955 bytes --]

On 8/19/25 12:31 PM, Daniel P. Berrangé wrote:
> On Tue, Aug 19, 2025 at 11:49:40AM -0400, Eli Schwartz wrote:
>> Rather, they are built when running `meson test`. This change is inert
>> unless building with meson 1.7, as previous versions of meson build all
>> `meson test` dependencies as part of `ninja all` as well.
>>
>> See:
>> https://mesonbuild.com/Release-notes-for-1-7-0.html#test-targets-no-longer-built-by-default
>>
>> An existing comment references this meson issue, with an included bug
>> reference, but was written before meson 1.7 fixed the bug. Update the
>> comment to change the advice from "if the bug gets fixed" to "when
>> bumping the minimum meson version".
> 
> I am very much not a fan of projects that do not build test
> programs by default.
> 
> If a dev is changing code and running 'ninja'/'make' everything
> that is affected should be rebuilt to identify any build failures
> that may have introduced immediately. This should include all
> test binaries, regardless of whether the dev is intending to run
> the tests at that time or not.
> 
> We already have too many occassions when contributors submit
> patches that have tests which fail to pass & this will expand
> that problem with contributors submitting patches that fail
> to even pass compilation of the tests.


I don't really understand this division between people who submit broken
untested patches and people who submit broken untested patches. You do
still need to test the patches and reject them for not passing the tests
so no time is really saved. It strikes me as some kind of confusing
optics-based objection, like this is about the exact embarrassment level
of a broken patch.

But your proposal runs counter to existing code already in qemu's build
system, i.e. the comment I've updated. So I don't think it's entirely
fair to raise an objection to my patch at all -- better you should raise
a meta-discussion about this to discuss changing the existing
build_by_defaults.


> If we want the ability to opt-out of building tests by default
> so reduce build time, IMHO that should be behind a 'configure'
> flag / meson_options.txt setting.


There is anyways no point in this being a meson_options.txt setting and
complexifying all test programs via conditionals. As explained in my
link, you can have Makefile itself read a configure option and run:

`ninja all meson-test-prereq`

instead of

`ninja all`

In fact users submitting patches could as well -- but then too, they
could simply run tests for code they touched.

Either way it is definitely of interest to users running a build +
install of a tagged release, to skip building tests they won't run. e.g.
many linux distros make the test phase of packaging be optional.

Automake works the same way -- tests can be excluded from `make && make
install`, but get built on demand by `make check`.


-- 
Eli Schwartz

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 963 bytes --]

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] meson: set test programs to not build by default
  2025-08-20  5:12   ` Eli Schwartz
@ 2025-08-20 10:23     ` Daniel P. Berrangé
  0 siblings, 0 replies; 4+ messages in thread
From: Daniel P. Berrangé @ 2025-08-20 10:23 UTC (permalink / raw)
  To: Eli Schwartz
  Cc: qemu-devel, Michael Roth, Kostiantyn Kostiuk, Manos Pitsidianakis,
	Aurelien Jarno, Peter Maydell, Alex Bennée, Fabiano Rosas,
	Laurent Vivier, Paolo Bonzini, open list:Rust-related patc...

On Wed, Aug 20, 2025 at 01:12:35AM -0400, Eli Schwartz wrote:
> But your proposal runs counter to existing code already in qemu's build
> system, i.e. the comment I've updated. So I don't think it's entirely
> fair to raise an objection to my patch at all -- better you should raise
> a meta-discussion about this to discuss changing the existing
> build_by_defaults.

The comment in tests/functional/meson.build is a special case rule
that isn't building anything. Rather that precache logic is using a
custom target to download several GB of disk images needed by tests,
such that the tests themselves won't hit a timeout downloading images
on first run. That use of  "build_by_default: false" would be fine.

> > If we want the ability to opt-out of building tests by default
> > so reduce build time, IMHO that should be behind a 'configure'
> > flag / meson_options.txt setting.
> 
> 
> There is anyways no point in this being a meson_options.txt setting and
> complexifying all test programs via conditionals. As explained in my
> link, you can have Makefile itself read a configure option and run:

AFAICT, it shouldn't be more complex than anything else we're doing
with meson option handling. eg instead of hardcoding 'false', do
something like:

  build_by_default: get_option('tests')

where the option defaults to 'true'.

> Either way it is definitely of interest to users running a build +
> install of a tagged release, to skip building tests they won't run. e.g.
> many linux distros make the test phase of packaging be optional.

FWIW, I would really strongly suggest to *always* run QEMU's tests
from release tarball, and most especially for distro packaging.
There are just way too many moving parts in our 3rd party deps to
be confident everything in QEMU works correctly without having run
the tests.

> Automake works the same way -- tests can be excluded from `make && make
> install`, but get built on demand by `make check`.

When we still used automake in libvirt, we overrode that behaviuor to
always build tests, as it was causing too frequent problems with
contributors overlooking that a patch had broken test compile.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-20 10:24 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-19 15:49 [PATCH] meson: set test programs to not build by default Eli Schwartz
2025-08-19 16:31 ` Daniel P. Berrangé
2025-08-20  5:12   ` Eli Schwartz
2025-08-20 10:23     ` Daniel P. Berrangé

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).