Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: add preprocessor_definitions to Meson + CC/CXX from build requirements #8353

Merged
merged 5 commits into from Jun 23, 2021

Conversation

SSE4
Copy link
Contributor

@SSE4 SSE4 commented Jan 18, 2021

Changelog: Fix: The new MesonToolchain now takes the declared environment variables (CC, CXX...) from build-requires and profiles to set the variables c, cpp, c_ld, cpp_ld etc, into the conan_meson_native.ini
Changelog: Fix: Added new preprocessor_definitions to new Meson build helper.
Changelog: Fix: The new MesonToolchain now allows adjusting any variable before generating the conan_meson_native.ini file.
Docs: conan-io/docs#2139

Closes #8311

this is mostly to align interface with CMake, Make and MSBuild (feature parity)
important side change: c_args, c_link_args, cpp_args and cpp_link_args are now arrays as documented at https://mesonbuild.com/Builtin-options.html#compiler-options:

c_args |   | free-form comma-separated list | C compile arguments to use
-- | -- | -- | --
c_link_args |   | free-form comma-separated list | C link arguments to use

cpp_args |   | free-form comma-separated list | C++ compile arguments to use
-- | -- | -- | --
cpp_link_args |   | free-form comma-separated list | C++ link arguments to use

Signed-off-by: SSE4 <tomskside@gmail.com>
@SSE4
Copy link
Contributor Author

SSE4 commented Jan 19, 2021

blocked by #8311
actually, we don't capture ENV vars from the profile (and it causes build to fail), it's meson who captures them in the build method.
previously I didn't set c_args, so meson used CFLAGS
no I try to set c_args to CFLAGS + preprocessor_definitions, and CFLAGS are empty within generate.

Signed-off-by: SSE4 <tomskside@gmail.com>
conan/tools/meson/toolchain.py Outdated Show resolved Hide resolved
conan/tools/meson/toolchain.py Outdated Show resolved Hide resolved
@lasote lasote changed the title Fix: add preprocessor_definitions to Meson Fix: add preprocessor_definitions to Meson + CC/CXX from build requirements Jun 22, 2021
@lasote
Copy link
Contributor

lasote commented Jun 22, 2021

@memsharded @SSE4 I updated the PR to use the new environment model. Check if the changes are correct and respond to the requested in the #8311

@lasote lasote added this to the 1.38 milestone Jun 22, 2021
@memsharded memsharded merged commit a53ecec into conan-io:develop Jun 23, 2021
@@ -71,8 +74,44 @@ def __init__(self, conanfile, env=os.environ):
self._cppstd = cppstd_from_settings(self._conanfile.settings)
self._shared = self._conanfile.options.get_safe("shared")
self._fpic = self._conanfile.options.get_safe("fPIC")
self._build_env = VirtualEnv(self._conanfile).build_environment()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have merged this PR, because it is already an improvement.
But this is a bit "hardcoded". The VirtualEnv might be used in generate() in a different way, to customize what gets pulled into the environment, and users might prioritize dependencies, [conf], environment, or other stuff, but that will not be taken into account here.

It seems the ideal is that the MesonToolchain could use the conanbuildenv.xx script, or some other way to reuse the build environment. But I ignore how Meson works for this, I guess the activation of conanbuildenv.xx is still necessary?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bug] MesonToolchain does not pick up CC/CXX from build requirements (in build profile)
4 participants