-
Notifications
You must be signed in to change notification settings - Fork 76
TST: harden against environment variables affecting meson-python #761
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @dnicolodi! LGTM, and confirmed locally that it fixes the failures in test_tags.py
that show up on main
when MACOSX_DEPLOYMENT_TARGET
is set.
It's not entirely clear to me why tests in test_wheel.py
aren't also failing for the same reason, but they do seem to pass in the Nix CI. So let's get this in and call it good - in case another issue gets reported we can revisit.
Indeed they should. I wonder if they did not show up in the Nix CI as failing because they were disabled already. |
The tests fail locally if I set I've also found an error in the documentation. https://mesonbuild.com/meson-python/reference/environment-variables.html#envvar-MACOSX_DEPLOYMENT_TARGET says that |
We have (or had?) some compat code to ensure that both |
I don't remember having it and I don't find trace of it in the git history. There are a lot of details in the comments in the implementation Lines 94 to 160 in 1e69e7a
The issue is that here Line 143 in 1e69e7a
11 has only one.
I don't know whether it is better to fix the implementation or the docs. I don't know how to check which values are accepted by clang. |
That was actually easy: clang errors out when it does not understand the env var format. I'll fix the implementation to accept the |
Fixes #760.