mesonbuild
Add build target keyword parameter 'build_subdir' [v3]
This is a replacement for #4037 as I want to switch the source branch. This would also replace my need for #13960.
Place the build products in a directory of the specified name somewhere within the build directory. This allows use of the target that includes a specific directory name:
#include <subdir/configure.h>
This also allows creating targets with the same basename by using different subdirectory names.
v2: Move build_subdir to Target class. Error if path separator in build_dir
v3: Rename to 'build_subdir' to make it clear that the name is appended to a meson-specific build directory, and does not provide the user with a way to define the overall meson build heirarchy. Allow build_subdir to include path separators. Support 'build_subdir' for configure_file.
This has the unfortunate knock-on effect that it breaks the established layout. Specifically whenever you ask "where did this file in my build dir get defined?" the answer is "from the corresponding source dir's build file". Now with this the answer basically becomes "anywhere at all".
I think there was a different MR with something similar to custom targets or something and the result we ended up with there was that while you could define a separate path it would need to be both A) below the current dir (so no ../ shenanigans) and B) there must not be a directory of that name in the source directory. In this way there can be no name collisions.
This has the unfortunate knock-on effect that it breaks the established layout. Specifically whenever you ask "where did this file in my build dir get defined?" the answer is "from the corresponding source dir's build file". Now with this the answer basically becomes "anywhere at all".
Not quite -- the subdir in question is always created beneath the directory corresponding to the original source dir. That ensures the same general relation between source dir and build dir. That's why I called it 'build_subdir' -- it's a subdirectory of the build directory, not some arbitrary directory within the build heirarchy.
I think there was a different MR with something similar to custom targets or something and the result we ended up with there was that while you could define a separate path it would need to be both A) below the current dir (so no
../shenanigans) and B) there must not be a directory of that name in the source directory. In this way there can be no name collisions.
Those both sound like good restrictions; I'll add validation for them.
I've evaluated the difference for my project between using this patch and #13960.
Here's using #13960:
https://github.com/picolibc/picolibc/commit/be3b85126170427982747d6556f3c174ccc62bbb
$ git diff main..rename | diffstat
dummyhost/meson.build | 5 ++++-
newlib/meson.build | 24 ++++++++++++++++++++----
picocrt/meson.build | 55 +++++++++++++++++++++++++++++++++++++++++++++----------
semihost/meson.build | 5 ++++-
4 files changed, 73 insertions(+), 16 deletions(-)
And here's using build_subdir:
https://github.com/picolibc/picolibc/commit/a6c8334491481412ad332055698aa2d9de69a3ec
$ git diff main..build-subdir | diffstat
dummyhost/meson.build | 8 +++-----
newlib/meson.build | 21 ++++++++++-----------
picocrt/meson.build | 43 ++++++++++++++++++++-----------------------
semihost/meson.build | 8 +++-----
4 files changed, 36 insertions(+), 44 deletions(-)
Diffstat shows that this version results in shorter code for this case. Let's look at the changes to meson itself. Here's the diffstat for rename:
$ git diff master..target-rename | diffstat
docs/yaml/functions/_build_target_base.yaml | 10 ++++++++++
docs/yaml/functions/configure_file.yaml | 9 +++++++++
docs/yaml/functions/custom_target.yaml | 10 ++++++++++
docs/yaml/functions/executable.yaml | 16 ++++++++++++++++
docs/yaml/functions/shared_library.yaml | 16 ++++++++++++++++
docs/yaml/functions/shared_module.yaml | 16 ++++++++++++++++
mesonbuild/backend/backends.py | 24 ++++++++++++++----------
mesonbuild/build.py | 68 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---
mesonbuild/interpreter/interpreter.py | 21 +++++++++++++++++++--
mesonbuild/interpreter/kwargs.py | 1 +
mesonbuild/interpreter/type_checking.py | 16 +++++++++++++++-
mesonbuild/minstall.py | 23 ++++++++++++++---------
mesonbuild/mintro.py | 4 ++--
test cases/common/109 custom target capture/meson.build | 12 ++++++++++++
test cases/common/109 custom target capture/test.json | 3 ++-
test cases/common/117 shared module/meson.build | 23 +++++++++++++++++++++++
test cases/common/117 shared module/test.json | 5 ++++-
test cases/common/14 configure file/meson.build | 7 +++++++
test cases/common/14 configure file/test.json | 3 ++-
19 files changed, 257 insertions(+), 30 deletions(-)
and here's build_subdir:
$ git diff master..keithp-build-dir | diffstat
docs/yaml/functions/_build_target_base.yaml | 17 +++++++++++++++++
docs/yaml/functions/configure_file.yaml | 17 +++++++++++++++++
mesonbuild/backend/backends.py | 13 ++++++++-----
mesonbuild/build.py | 24 +++++++++++++++++++++---
mesonbuild/interpreter/interpreter.py | 31 +++++++++++++++++++++++++++----
mesonbuild/interpreter/type_checking.py | 1 +
test cases/common/109 custom target capture/meson.build | 10 ++++++++++
test cases/common/109 custom target capture/test.json | 3 ++-
test cases/common/117 shared module/meson.build | 8 ++++++++
test cases/common/117 shared module/test.json | 5 ++++-
test cases/common/14 configure file/meson.build | 7 +++++++
test cases/common/14 configure file/test.json | 3 ++-
12 files changed, 124 insertions(+), 15 deletions(-)
The build_subdir version is also less fragile for users -- the rename version requires that the meson.build file include complete filenames for generated files, including the 'lib' prefix and '.a' suffix for libraries. The build-subdir version inherits all of that from the existing code. Similarly, import libraries and debuginfo files would be automatically generated and named correctly with the build-subdir version, while the rename version requires that the user provide correct names for each of those.
The lint failure from CI is not caused by new code. I can't explain mypy's complaints though; the code seems correct.
The lint check shows mypy failing against existing code where it calls shutil.chown. The code looks correct to me; I'm not sure why mypy is unhappy.
there must not be a directory of that name in the source directory. In this way there can be no name collisions.
That seems too much of a limitation to make this useful for VLC. Right now we build a lot of plugins, based on a big dictionary so all targets are expanded at the same location but I would like to have plugins go into subdirs based on where in the source dirs they actually are.
there must not be a directory of that name in the source directory. In this way there can be no name collisions.
That seems too much of a limitation to make this useful for VLC. Right now we build a lot of plugins, based on a big dictionary so all targets are expanded at the same location but I would like to have plugins go into subdirs based on where in the source dirs they actually are.
Maybe I just described it badly. The subdir is relative to the build directory for the related source directory, so if you're not using a 'flat' build (which isn't the default), that will have the same shape as the source hierarchy. In concrete terms, if you have a source tree like this:
src/foo
src/foo/bar
src/bar
and if you create a build_dir of bar in each of these, then you will get these results:
src/foo builddir/src/foo/bar fails because src/foo/bar exists in the source tree
src/foo/bar builddir/src/foo/bar/bar
src/bar builddir/src/bar/bar
src/foo builddir/src/foo/bar fails because src/foo/bar exists in the source tree
This is what I want but would fail. Here an example:modules/audio has the actual module and meson file defining it but the definition of the target is in modules/meson.build, I would not be able to have it end up in build/modules/audio because the audio subfolder exists in the src folder.
This is what I want but would fail. Here an example:
modules/audiohas the actual module and meson file defining it but the definition of the target is inmodules/meson.build, I would not be able to have it end up inbuild/modules/audiobecause theaudiosubfolder exists in the src folder.
Yeah in this case you are expected to stick a meson.build file in the audio subfolder and have that contain the build rules. Is there some reason you can't make that work?
The goal of this option is to allow you to create additional build directories that don't exist in the source hierarchy. It's certainly easier to skip this check, but it was requested to ensure no collisions between meson-generated paths in those common directories.
However, if you don't have a meson.build file that you're using in that subdirectory, then there wouldn't be a problem. Would it be helpful to relax this condition and only require that the subdirectory not be the target of a meson subdir command?
Yeah in this case you are expected to stick a meson.build file in the audio subfolder and have that contain the build rules. Is there some reason you can't make that work?
Yes because we use a big array of dicts where we assemble all modules build info before actually expanding to the full targets, to avoid a ton of boilerplate and also being able to easier handle test dependencies on modules and such.
However, if you don't have a meson.build file that you're using in that subdirectory, then there wouldn't be a problem. > Would it be helpful to relax this condition and only require that the subdirectory not be the target of a meson subdir command?
No, because we do have meson build files in the subdirs.
No, because we do have meson build files in the subdirs.
I'm not opposed to allowing this, but I'm unsure how to prevent file name collisions in the subdir build directory. Maybe just make it subdir/subdir instead of subdir so you still get the right name, but nested one more level down to avoid the name conflict?
I rather have it with the restriction (and unusable for my use-case) than with the weird double subdirectory tbh
I rather have it with the restriction (and unusable for my use-case) than with the weird double subdirectory tbh
Weird double-subdirectory hack would be supported with the current code...
However, making sure meson can easily manage filenames in the build directory seems important enough to keep the restriction in place. Thanks for thinking about how this might help with other use cases than mine.
Instead of a double subdir you could, for example, append a _mod to the directory name.
Perhaps with #10714, it would be possible to perform validation from the type_checking module. It is a pity that that pull request seems to be stuck for a while already.
I don't think the current pull request should be delayed, but once #10714 lands, it means that the current pull request adds a bit more work to reworking the type safety of the interpreter (cf. #14214).
Sorry that this got lost in the shuffle. To get things back on track
- Is this still desired?
- Is appending a, say,
_modto the subdirectory name enough to make things work for VLC or is something more drastic needed?
@jpakkane
Is appending a, say, _mod to the subdirectory name enough to make things work for VLC or is something more drastic needed?
That would be a solution but it would still be a bit confusing to not find the built library in the same-name directory in the build tree as in the source tree where it is defined.
However I do not want to block this just because it doesn't nicely work for our case, so I would be fine with proceeding with this PR with the mentioned restrictions in place.
Sorry that this got lost in the shuffle. To get things back on track
* Is this still desired?
Still desperately needed, yes. Picolibc continues to emit thousands of warnings when running meson and I have no way to avoid them without this change. Thank you for not breaking this over the last decade.
* Is appending a, say, `_mod` to the subdirectory name enough to make things work for VLC or is something more drastic needed?
That hardly matters at all -- I'd just create a fake _mod name and then stick the real directory name underneath that. I need the final portion of the path to contain the correct directory names, and those names are computed from data supplied during the meson configuration process.
Okay, good. To get things moving then:
- needs a rebase
- needs a failing test that tries to create a
build_subdirthat exists in source tree - the test only uses a
shared_module, it would be nice if it had some other target, too - one of the commit messages uses heirarchy, but the correct spelling is hierarchy.
* needs a rebase
Done.
* needs a failing test that tries to create a `build_subdir` that exists in source tree
Added test cases/failing/136 invalid build_subdir
* the test only uses a `shared_module`, it would be nice if it had some other target, too
There are three test cases -- 109 custom target capture, 117 shared module and 14 configure file that should hit all of the new code paths.
* one of the commit messages uses _heirarchy_, but the correct spelling is _hierarchy_.
Oops. Fixed.
I'm late to the game, but why adding a new keyword argument instead of allowing path as output: 'subdir/foo.txt', as suggested in https://github.com/mesonbuild/meson/issues/2320#issuecomment-370418738. That seems more flexible and natural, IMHO.
I'm late to the game, but why adding a new keyword argument instead of allowing path as
output: 'subdir/foo.txt', as suggested in #2320 (comment). That seems more flexible and natural, IMHO.
That breaks things like libraries and other files which have additional mangling to their basename values (libraries get prefixed with 'lib', many files have extensions added by default). Using build_subdir made the result integrate into the existing meson environment a lot cleaner, at least for my use where I've been using the unsupported hack of including a directory name in the target name for about seven years.
For build targets that makes sense indeed, but for custom_target() and configure_file() that's less ideal. OTOH, I agree that since we need a kwarg for build targets, it also makes sense for consistency to have it on custom_target and configure_file...
What I'm a bit worried is the case of custom_target() where each output could be in a different subdir. If we allow custom_target(..., output: ['subdir/out.txt', 'subdir2/out.txt']) as requested in https://github.com/mesonbuild/meson/issues/2320, that would conflict with custom_target(..., build_subdir: ...).
I'm wondering if we are better to only allow build_subdir for build targets for now?
Note we could also allow library('subdir/foo', ...) and just split dirname/basename immediately, before doing prefixes stuff. Implementation would be very similar to what you did here. But to be fair, for build targets build_subdir seems nicer.
Sorry, just thinking out loud here.
What I'm a bit worried is the case of custom_target() where each output could be in a different subdir. If we allow
custom_target(..., output: ['subdir/out.txt', 'subdir2/out.txt'])as requested in #2320, that would conflict withcustom_target(..., build_subdir: ...).
Yup, which is why I suggested that custom_target could accept an array of strings for build_subdir and then associate each array element with one output element. That way, you'd be able to place each output in a separate directory as desired.
I toyed with that while implementing this function, but didn't want to make it that much harder to review.
custom_target() already has the precedent of doing that for other things, and for doing the helpful thing of treating an array of length == 1 as if all entries are the same, so that you can write things like:
custom_target(
...
output : ['1', '2, '3'],
install_dir : get_option('datadir') / meson.project_name(),
install_tag : 'data',
)
so that would allow you to write things like:
custom_target(
output : ['1', '2', '3'],
build_subdir : 'my_subdir'
)
instead of having to write ['mysubdir/1', 'mysubdir/2', 'mysubdir/3']
