Meson update #658
Meson update #658
Conversation
1. Use cross-platform `/` operator for path construction. 2. Use `meson.project_source_root()` for correct path resolution of generate_test_runner.rb path when used as a subproject. 3. Bump the minimum required Meson version to '0.56.0' as this is needed for the above changes.
The following features from the CMake build have been implemented: * Library version retrieved from unity.h. * Extension support. * Library, header, and package configuration file installation. This commit is entirely based on existing work by Owen Torres.
|
@maintainers, many thanks for the CI approval. Given that it seems to pass the CI check I will mark this as ready for review. |
|
I can't set reviewers but it would be good to get feedback from @optorres, and @pmembrey as their Meson build scripts are being modified here. @eli-schwartz, being a Meson developer, is well-placed to offer good feedback. |
meson.build
Outdated
| # Set project version to value extracted from unity.h header | ||
| version: run_command( | ||
| [ | ||
| find_program('python', 'python3'), |
There was a problem hiding this comment.
You don't need to find python here, because the extract_version.py script is runnable on its own as a regular program (it's executable and has a shebang). On Windows, Meson will scan the shebang and detect that it needs to be run with python.
There was a problem hiding this comment.
This has been implemented. I didn't know about Meson doing the right thing on Windows.
The version in #546 resulted in Meson not finding python3 on Ubuntu Jammy with Meson 1.0.0 installed via pip3, hence why I added 'python3' to the find_program() invokation, although this might be because the python-is-python3 package was not installed.
I had also tried import('python').find_installation('python3') as a cross-platform way to find the Python interpreter but this caused an internal Meson error. I will see if I can reproduce it and then check if it hasn't already been reported before reporting it to Meson upstream. EDIT: Meson bug report.
meson.build
Outdated
| build_fixture = get_option('extension_fixture').enabled() | ||
| build_memory = get_option('extension_memory').enabled() |
There was a problem hiding this comment.
You're just using these like booleans, it seems misleading to allow people to specify either =auto or =disabled and have auto really just mean disabled.
What about defining the options as type boolean, and just checking the option value directly?
There was a problem hiding this comment.
This makes sense. Implemented.
meson.build
Outdated
| # Create a generator that can be used by consumers of our build system to generate | ||
| # test runners. | ||
| gen_test_runner = generator( | ||
| find_program(meson.project_source_root() / 'auto' / 'generate_test_runner.rb'), |
There was a problem hiding this comment.
| find_program(meson.project_source_root() / 'auto' / 'generate_test_runner.rb'), | |
| find_program('auto/generate_test_runner.rb'), |
find_program() will already search for local files relative to the current context meson.build file so you don't need to prepend an absolute path.
This also means you don't need to bump the minimum version just to use this.
There was a problem hiding this comment.
Done. This simplifies things a lot.
1. Call the version extraction script directly instead of through a Python returned from `find_program()`. 2. We don't need to use `meson.project_source_root()` as `find_program()` will search relative to the current meson.build script. 3. Lower the required version back to `>= 0.37.0`, and modify some things to get rid of warnings with this version selected. The use of `summary()`, `dict`, and positional arguments in `pkgconfig.generate()` generate warnings with this version so remove `summary()` and dict()`, also pass keyword arguments to `pkgconfig.generate()`.
|
Thanks @eli-schwartz for your review! I have implemented your feedback. In the process of lowering the minimum required version of Meson, I have made some other changes to the toplevel Given that most users of Meson with Unity will be using it as a subproject, I thought it wise to avoid as many warnings as possible to avoid causing noise or confusion for users. Let me know if this is the right call or not, @mvandervoord. |
|
You need at least 0.47.0 for the check: argument to run_command(), but that version is practically everywhere anyway. It's not present in distros that are as old as, for example, Debian Jessie or Ubuntu 18.04, but I don't think that's a serious downside. |
Interesting. Currently when I set Using other functionality like |
|
I think that just happens because it's parsed before the meson_version is set, since it's inline in the project() function. :) It's a hard problem to solve, but then again, we don't typically recommend inlining functions into project() -- with the exception of, well, the "version" kwarg (run_command() or files()) |
So I was curious about this and modified the Thank you for your time and insight! |
The use of the check kwarg in run_command() was introduced in meson version 0.47.0
|
No, the issue is that run_command() is evaluated before project() is evaluated, and then project() just sees a string. :) The warning is issued if the run_command occurs on any line after the project() function has run. Possible solutions for handling this could include double parsing or remembering non-issued warnings and repeating them later. |
Ah that makes sense. Thanks for clarifying it for me. Could a Unity maintainer take a look at this PR and let me know if there's anything left to do before it's ready for merging? |
The reason for the unwanted unity installation is unity PR[1] enabling installation of the static library, pkgconfig file, and headers. We got this commit because we use `head` instead of a latest release. Fix by pinning unity to v2.5.2, which does not have the issue, and remove the unneeded --skip-subprojects option. [1] ThrowTheSwitch/Unity#658
This PR is heavily based on existing work by Owen Torres (@optorres) in #546. I aim to get his excellent work merged.
As stated by @optorres in #546 :
My additions to his work are mainly around hygiene for Meson. I try to use the cross-platform
/operator to join paths, which was introduced in version '0.49.0' and usemeson.project_source_root()which is introduced in Meson version0.56.0in place of the deprecated functionmeson.source_root().