mesonbuild / meson

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

subprojects ignore `shared_module`'s `build_by_default : false` option when `install : true` and build regardless #13498

Open lf- opened 4 months ago

lf- commented 4 months ago

Describe the bug If you set build_by_default : false in a subproject, one would expect the target to not be built by default. I later discovered this is because install : true, which is not documented to be related to whether it will be be built by default.

The use case that is leading to wanting this is that this is a clang-tidy plugin that I would like to build only on demand when you actually run the clang-tidy custom target, and it is in a subproject because I would like to be able to also pass it in externally to easily use Nix build caching. Actually it would be even more ideal if the subproject were not even configured if the dependent target is not demanded, since there is no reason to need it to be, but I would also accept skipping the build.

To Reproduce

https://git.jade.fyi/jade/meson-repro-20240731

or, reproduce the below file structure:

meson/repro » tree .
.
├── meson.build
└── subprojects
    └── subproj
        ├── lib.c
        └── meson.build

3 directories, 3 files

meson/repro » cat meson.build
project('repro')
subproject('subproj')
meson/repro » cat subprojects/subproj/meson.build
project('lix-clang-tidy', ['c'])

sources = files('lib.c')
module = shared_module('lix-clang-tidy', sources, install : true, build_by_default : false)

meson/repro » cat subprojects/subproj/lib.c

Repro:

meson/repro » meson setup build
....

meson/repro » rg 'build all' build/build.ninja
115:build all: phony meson-test-prereq meson-benchmark-prereq subprojects/subproj/liblix-clang-tidy.so

Observe that liblix-clang-tidy.so is incorrectly included in all in spite of being explicitly set to not be built by default.

Expected behavior

I expect the subproject target to not be built by default if it is not set to be built by default.

system parameters

eyalitki commented 6 days ago

This is a larger bug, as it isn't directly tied to subprojects. Fix will be posted soon. Edit: While this does globally impact the build_by_default flag (for all targets, regardless of subproject), there is an updated comment below, with additional scope around this issue.

eyalitki commented 6 days ago

Well, it seems I was overly optimistic, and I couldn't find a step in which the "install" phase can trigger the build of targets that were skipped during the "compile" phase. While this might be possible, it is beyond my understanding of the project.

This also means that "build_by_default" currently only makes sense when install is "false". If this is a concise decision, the documentation (and comments in the code) should probably be updated accordingly.

eli-schwartz commented 5 days ago

That is broadly correct, yes. In the distant past, we didn't build those files and instead raised incomprehensible tracebacks during meson install because required files weren't found.

There is a related issue somewhere but I can't remember where off the top of my head.

Note: it's fairly common for build systems to provide a default target (usually called "all" that compiles all build rules which are needed for, well, installing the results. The current meson behavior is, in a sense, not surprising.

(In another sense, it's arguably surprising that you can try setting build_by_default: false without triggering a warning/error "this setting has no effect because the target is installed.)

eyalitki commented 5 days ago

Ack, understood.

  1. Should I instead provide a pull request to update the documentation accordingly?
  2. Should said pull request also add a warning about this setting having no effect?
eyalitki commented 5 days ago

I tracked the following related issues: #4370, #12530 and #13453.