mesonbuild / meson

The Meson Build System
http://mesonbuild.com
Apache License 2.0
5.47k stars 1.59k forks source link

Backward compatibility broken for build_by_default #12530

Open seingierf opened 9 months ago

seingierf commented 9 months ago

Describe the bug With Meson < 1.3.0, a build target declared with build_by_default: false and install: true would prevent the target from being built if meson compile was called without any arguments. For example, we could set build_by_default: not meson.is_subproject() to skip building time consuming targets if the project was used as a subproject. Change in behaviour was introduced by 507d8bf4d.

To Reproduce

cat > meson.build<<EOF
project('Test', 'cpp')
shared_module('module',
              'module.cc',
              build_by_default: false,
              install: true)
EOF
touch module.cc
meson-507d8bf4d^ setup build-old
meson-main setup build-main
diff -u <(meson introspect --targets --indent build-old) <(meson introspect --targets --indent build-main)
--- /dev/fd/63  2023-11-20 23:33:24.459984107 +0100
+++ /dev/fd/62  2023-11-20 23:33:24.459984107 +0100
@@ -5,9 +5,9 @@
         "type": "shared module",
         "defined_in": "/tmp/test-build-by_default/meson.build",
         "filename": [
-            "/tmp/test-build-by_default/build-old/libmodule.so"
+            "/tmp/test-build-by_default/build-main/libmodule.so"
         ],
-        "build_by_default": false,
+        "build_by_default": true,
         "target_sources": [
             {   
                 "language": "cpp",
@@ -15,8 +15,8 @@
                     "c++"
                 ],
                 "parameters": [
-                    "-I/tmp/test-build-by_default/build-old/libmodule.so.p",
-                    "-I/tmp/test-build-by_default/build-old",
+                    "-I/tmp/test-build-by_default/build-main/libmodule.so.p",
+                    "-I/tmp/test-build-by_default/build-main",
                     "-I/tmp/test-build-by_default",
                     "-fdiagnostics-color=always",
                     "-D_FILE_OFFSET_BITS=64",

Expected behavior Do not override user-set build_by_default.

system parameters

eli-schwartz commented 9 months ago

What do you expect to happen if you run meson install?

seingierf commented 9 months ago

We use meson install --skip-subprojects. Adding build_by_default: not meson.is_subproject() had allowed us to prevent time consuming targets from being built when the project was used as a subproject instead of e.g. adding a new option and change our build scripts. If the project was used in standalone mode, then both build_by_default and install would be true and meson install would work flawlessly.

Since compile and install are 2 different actions, we could probably debate of what is expected by the install command and I fully respect the choice meson has made. But to answer your question, I would equally have been ok if in my example above: meson compile => do not compile target libmodule.so meson install => depends on libmodule.so since install:true so trigger build and install

or even: meson compile => do not compile target libmodule.so meson compile libmodule.so => user explicitely decides to build this target meson install => build all targets marked with build_by_default:true and possibly user triggered targets marked with install:true

The thing is, I felt uncomfortable with the fact that meson silently overwrote the user supplied option build_by_default:false and the fact that the comment in the code did not seem quite aligned with the initial intent (i.e. overwrite build_by_default with the value of install iif user did not specify a build_by_default option for that target whereas now it might discard the user provided value false)

I thought it might be worth sharing this observation because it seems to me meson puts a fair amount of work into not making behavioral changes from one version to the next.

eli-schwartz commented 9 months ago

Our story here is a bit inconsistent as time goes by: #4370

As far as backwards compat of behavior goes, bugfixes are behavioral changes too. In this report, the build file is valid either way and produces the same results, and the only difference is that newer meson versions result in a slower build that builds an additional side target.

eli-schwartz commented 9 months ago

I'm a bit confused actually because I thought that the backend, when generating the default "all" target, uses the union of build_by_default targets as well as all targets that are marked for installation. Specifically to avoid crashing. So meson 1.3.0 should not have behaved differently...

phillipjohnston commented 3 months ago

While nominally "the only difference is that newer meson versions result in a slower build that builds an additional side target", it has been problematic with cross-compilation setups.

In several of our projects, a unit testing subproject (CMocka) was previously not being cross-compiled, but the update to the logic here caused that to be built once more, even though that is not desired (the requirements for cross-compiling that library aren't satisfied) and nothing in the subproject using that library.

I don't really have a position on what the right behavior is here. I'll be creating my own build patch files for this, rather than using the wrapdb version, since the main recourse is to disable install for my needs and that may not be what the majority of users or maintainers of that build rule want..

eli-schwartz commented 3 months ago

For the WrapDB in particular it makes a lot of sense to not unconditionally install test harnesses just because one builds a subproject as a testsuite dependency.

I'm amenable to solutions such as install: not meson.is_subproject()

eli-schwartz commented 3 months ago

It's also possible the answer is to avoid looking for a cmocka dependency at all, during cross compiled (if it's not expected to work under embedded anyway).

phillipjohnston commented 3 months ago

It's also possible the answer is to avoid looking for a cmocka dependency at all, during cross compiled (if it's not expected to work under embedded anyway).

I am only looking for the native dependency to use with the native: true test targets. But because it's marked install: true, the library is now being compiled for the target by default.