Skip to content

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

Merged
merged 1 commit into from
Jun 8, 2025

Conversation

dnicolodi
Copy link
Member

Fixes #760.

@rgommers rgommers added the maintenance Regular code improvements that are not new features nor end-user-visible bugs label Jun 8, 2025
Copy link
Contributor

@rgommers rgommers left a 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.

@rgommers rgommers added this to the v0.19.0 milestone Jun 8, 2025
@rgommers rgommers merged commit 1e69e7a into mesonbuild:main Jun 8, 2025
41 checks passed
@dnicolodi
Copy link
Member Author

It's not entirely clear to me why tests in test_wheel.py aren't also failing for the same reason

Indeed they should. I wonder if they did not show up in the Nix CI as failing because they were disabled already.

@dnicolodi
Copy link
Member Author

dnicolodi commented Jun 8, 2025

The tests fail locally if I set $MACOSX_DEPLOYMENT_TARGET. I'll extend the fix to these tests too.

I've also found an error in the documentation. https://mesonbuild.com/meson-python/reference/environment-variables.html#envvar-MACOSX_DEPLOYMENT_TARGET says that 11 should be a valid value for this environment variable, but it is not parsed correctly, it should be 11.0.

@rgommers
Copy link
Contributor

rgommers commented Jun 8, 2025

We have (or had?) some compat code to ensure that both 11 and 11.0 are valid. packaging made a mess in the 10.x -> >=11 transition, and we had users using both. 11.0 is canonical indeed, but that's also weird given that packaging disallows 11.x with x != 0.

@dnicolodi
Copy link
Member Author

We have (or had?) some compat code to ensure that both 11 and 11.0 are valid.

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

def _get_macosx_platform_tag() -> str:
ver, _, arch = platform.mac_ver()
# Override the architecture with the one provided in the
# _PYTHON_HOST_PLATFORM environment variable. This environment
# variable affects the sysconfig.get_platform() return value and
# is used to cross-compile python extensions on macOS for a
# different architecture. We base the platform tag computation on
# platform.mac_ver() but respect the content of the environment
# variable.
try:
arch = os.environ.get('_PYTHON_HOST_PLATFORM', '').split('-')[2]
except IndexError:
pass
# Override the macOS version if one is provided via the
# MACOSX_DEPLOYMENT_TARGET environment variable.
try:
version = tuple(map(int, os.environ.get('MACOSX_DEPLOYMENT_TARGET', '').split('.')))[:2]
except ValueError:
version = tuple(map(int, ver.split('.')))[:2]
# Python built with older macOS SDK on macOS 11, reports an
# unexising macOS 10.16 version instead of the real version.
#
# The packaging module introduced a workaround
# https://github.com/pypa/packaging/commit/67c4a2820c549070bbfc4bfbf5e2a250075048da
#
# This results in packaging versions up to 21.3 generating
# platform tags like "macosx_10_16_x86_64" and later versions
# generating "macosx_11_0_x86_64". Using the latter would be more
# correct but prevents the resulting wheel from being installed on
# systems using packaging 21.3 or earlier (pip 22.3 or earlier).
#
# Fortunately packaging versions carrying the workaround still
# accepts "macosx_10_16_x86_64" as a compatible platform tag. We
# can therefore ignore the issue and generate the slightly
# incorrect tag.
# The minimum macOS ABI version on arm64 is 11.0. The macOS SDK
# on arm64 silently bumps any compatibility version specified via
# the MACOSX_DEPLOYMENT_TARGET environment variable to 11.0.
# Despite the platform ABI tag being intended to be a minimum
# compatibility version, pip refuses to install wheels with a
# platform tag specifying an ABI version lower than 11.0. Use
# 11.0 as minimum ABI version on arm64.
if arch == 'arm64' and version < (11, 0):
version = (11, 0)
major, minor = version
if major >= 11:
# For macOS reelases up to 10.15, the major version number is
# actually part of the OS name and the minor version is the
# actual OS release. Starting with macOS 11, the major
# version number is the OS release and the minor version is
# the patch level. Reset the patch level to zero.
minor = 0
if _32_BIT_INTERPRETER:
# 32-bit Python running on a 64-bit kernel.
if arch == 'ppc64':
arch = 'ppc'
if arch == 'x86_64':
arch = 'i386'
return f'macosx_{major}_{minor}_{arch}'

The issue is that here

major, minor = version
we expect the version to have two components, and 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.

@dnicolodi
Copy link
Member Author

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. 11, 11.0, 11.0.0 are all valid.

I'll fix the implementation to accept the 11 variant too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Regular code improvements that are not new features nor end-user-visible bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests/test_tags.py:43: AssertionError on Darwin
2 participants