search

ThrowTheSwitch / Unity

Simple Unit Testing for C
ThrowTheSwitch.org
MIT License
4.02k stars 969 forks source link

Meson update #658

Closed amcnulty-fermat closed 1 year ago

amcnulty-fermat commented 1 year ago

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 :

This updates the meson build scripts to bring them more-or-less up to parity with the CMake script and adds a meson build script to one of the project examples.

Build script additions:

Library version is now retrieved from unity.h instead of being left as undefined. Extensions (fixture and memory) are now supported. The library, headers, and package configuration files (using pkg-config instead of CMake's config format) are now installed with ninja install / meson compile install.

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 use meson.project_source_root() which is introduced in Meson version 0.56.0 in place of the deprecated function meson.source_root().

amcnulty-fermat commented 1 year ago

@maintainers, many thanks for the CI approval. Given that it seems to pass the CI check I will mark this as ready for review.

amcnulty-fermat commented 1 year ago

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.

amcnulty-fermat commented 1 year ago

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 meson.build in order to avoid generating warnings. Please let me know what you think.

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.

eli-schwartz commented 1 year ago

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.

amcnulty-fermat commented 1 year ago

You need at least 0.47.0 for the check: argument to run_command(), but that version is practically everywhere anyway.

Interesting. Currently when I set meson_version: '0.37.0' and specify check: true in run_command(), Meson 1.0.0 does not seem to generate a warning. Should it generate one?

Using other functionality like summary() and dict while setting meson_version to '0.37.0' did cause Meson to warn about there being a mismatch between the features used and the version of meson required.

eli-schwartz commented 1 year ago

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

amcnulty-fermat commented 1 year ago

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 project() call to make meson_version be the first kwarg, thus parsed before the run_command, and in this case as well Meson does not issue a warning. It seems as though in the implementation, it should issue one, right?

Thank you for your time and insight!

eli-schwartz commented 1 year ago

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.

amcnulty-fermat commented 1 year ago

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?