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

Add meson properties field, and specificy needs_exe_wrapper depending… #10846

Merged
merged 2 commits into from Mar 25, 2022

Conversation

avetiso
Copy link
Contributor

@avetiso avetiso commented Mar 21, 2022

… on cross conditions.

Changelog: Feature: Add detection in meson toolchain for cross conditions and requirement of needs_exe_wrapper. Users may specify the exe wrapper in their meson.build now.
Docs: omit

Issue: #10796

Not sure if docs are required, as this is probably expected behavior of the generator for someone who understands what meson is looking for. Happy to write some docs, though.

  • Refer to the issue that supports this Pull Request.
  • If the issue has missing info, explain the purpose/use case/pain/need that covers this Pull Request.
  • I've read the Contributing guide.
  • I've followed the PEP8 style guides for Python code.
  • I've opened another PR in the Conan docs repo to the develop branch, documenting this one.

Note: By default this PR will skip the slower tests and will use a limited set of python versions. Check here how to increase the testing level by writing some tags in the current PR body text.

@CLAassistant
Copy link

CLAassistant commented Mar 21, 2022

CLA assistant check
All committers have signed the CLA.

@eli-schwartz
Copy link

Users may specify the exe wrapper in their meson.build now.

Err... I assume you mean something other than the meson.build file which is read by meson setup?

@avetiso
Copy link
Contributor Author

avetiso commented Mar 21, 2022

Users may specify the exe wrapper in their meson.build now.

Err... I assume you mean something other than the meson.build file which is read by meson setup?

Maybe? That's where I've seen most examples of exe_wrappers being set. Feel free to correct me here.

@avetiso
Copy link
Contributor Author

avetiso commented Mar 21, 2022

Users may specify the exe wrapper in their meson.build now.

Err... I assume you mean something other than the meson.build file which is read by meson setup?

Also, for the case of (arch_build != arch_host), do you think it makes sense to be explicit here re: needs_exe_wrapper when meson probably guess this correctly?

Copy link
Member

@memsharded memsharded left a comment

Choose a reason for hiding this comment

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

Some tests that covers this would be necessary. I'd recommend an integration test, doesn't need to actually build anything, just check the generated file contents under several conditions.

conan/tools/meson/toolchain.py Outdated Show resolved Hide resolved
@avetiso avetiso force-pushed the feature/meson_tc_properties branch 2 times, most recently from 5653957 to 3b0d4c3 Compare March 22, 2022 02:18
@avetiso avetiso force-pushed the feature/meson_tc_properties branch from 3b0d4c3 to ab0a84c Compare March 22, 2022 03:28
@avetiso
Copy link
Contributor Author

avetiso commented Mar 22, 2022

Test for presence of fields in Android and iOS cross builds, and test for lack of presence in a native build. @memsharded let me know what you think. Seemed to make more sense to put the tests there rather in anything under integration/.

@avetiso
Copy link
Contributor Author

avetiso commented Mar 24, 2022

@memsharded any update feedback?

@memsharded memsharded added this to the 1.47 milestone Mar 25, 2022
@memsharded memsharded merged commit 4f7bb66 into conan-io:develop Mar 25, 2022
@memsharded
Copy link
Member

Merged for 1.47, thanks!

@avetiso avetiso deleted the feature/meson_tc_properties branch March 28, 2022 20:38
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.

None yet

4 participants