BUG: honor MACOSX_DEPLOYMENT_TARGET

[FFY00]

Signed-off-by: Filipe Laíns [email protected]

[FFY00]

Should we also fallback to sysconfig.get_config_var('MACOSX_DEPLOYMENT_TARGET')?

[FFY00]

@rgommers thoughts on this? I am unsure about the implications.

[rgommers]

I was just looking at it. I think this is correct as is. I do not think we need to use sysconfig.get_config_var('MACOSX_DEPLOYMENT_TARGET'). That is the platform for which the Python interpreter was built. I don't see a reason why that would be relevant.

[FFY00]

Alright, thanks!

[rgommers]

Let me try to play with this a bit locally. I want to make sure that I understand where the original input comes from.

[FFY00]

Okay. distutils/setuptools does fallback to it, that's why I was asking.

https://github.com/pypa/setuptools/blob/a86428cfef419e9ef3f04a04bde2d64d395936b8/setuptools/_distutils/util.py#L88

After looking into it a bit more, I think we only have to mimic Meson's behavior though.

[rgommers]

Ah I see, right now we start from sysconfig.get_platform(). That does not look correct to me. On my machine, which is on macOS 12.4:

>>> sysconfig.get_platform()
'macosx-11.0-arm64'

The logic should probably be:

• If MACOSX_DEPLOYMENT target is set, honor that
• Otherwise, build for the current OS version (to check if minor versions are to be taken into account or not)

Otherwise we get strange issues when we different Python interpreters are built for different targets. What needs checking first though is how Meson makes its choices. Because in the end the tags need to match what the packaged extension modules are actually built for.

[FFY00]

We could potentially just set MACOSX_DEPLOYMENT to the sysconfig value when invoking Meson if not set, this would force Meson to target that version and would, I hope, override any fallback logic they may have. What do you think?

[rgommers]

This is a little nontrivial, we should add to the docs how this works. We discussed this previously in https://github.com/FFY00/meson-python/issues/91#issuecomment-1175477386.

There is more to it though. MACOSX_DEPLOYMENT_TARGET is an environment variable, it has to be translated to what a compiler understands somewhere. There are differences per compiler:

• GCC actually honors MACOSX_DEPLOYMENT_TARGET itself, no flag needed
• Clang does not, it wants a flag: --macosx-version-min. EDIT: this was wrong, see this test
• For other compilers, YMMV

On modern macOS systems, Clang is the default and is what the majority of folks are going to be building with. Extra complication: Clang has no good Fortran compiler, so we often end up with clang/clang++/gfortran as a combo. Which may explain build warnings about target mismatches I've been seen in the SciPy builds.

So that leaves the question for Clang: who is going to be translating the env var to a compiler flag? There's various options:

• Meson does nothing here
• CMake does: https://cmake.org/cmake/help/latest/envvar/MACOSX_DEPLOYMENT_TARGET.html
• It may also be compiler packagers; conda-forge does handle the env var in its activation script for Clang: https://github.com/conda-forge/clang-compiler-activation-feedstock/blob/b52f54576e9947e65196adef73ee5d550eafe1f1/recipe/activate-clang.sh#L101. What is done there is:

if [ "${MACOSX_DEPLOYMENT_TARGET:-0}" != "0" ]; then
  CPPFLAGS_USED="$CPPFLAGS_USED -mmacosx-version-min=${MACOSX_DEPLOYMENT_TARGET}"
fi

_CMAKE_ARGS="${_CMAKE_ARGS} -DCMAKE_OSX_DEPLOYMENT_TARGET=${MACOSX_DEPLOYMENT_TARGET}"

I don't know why it only adds -mmacosx-version-min to clang++ and not to clang.

So I think that we cannot know for sure what will happen when MACOSX_DEPLOYMENT_TARGET is not set. Most likely, the build does not use any SDK and is therefore for the local OS version. If the env var is set, then we should assume that it is honored somewhere, and this is up to the compiler & compiler packager to do.

What the deployment target for the Python interpreter was at the time that that interpreter was built should not be relevant at all for us. That distutils code was probably written with the assumption that everyone is only using the python.org provided Python, and hence let's all target that. But that's a bad assumption.

[rgommers]

We could potentially just set MACOSX_DEPLOYMENT to the sysconfig value when invoking Meson if not set, this would force Meson to target that version and would, I hope, override any fallback logic they may have. What do you think?

Given what I wrote above about how MACOSX_DEPLOYMENT_TARGET is translated into -mmacosx-version-min for Clang, I don't think that will do anything as of right now. The compiler activation script has already run. We may want to check if Meson should start handling the env var - that would probably be a good idea. After that is done, we could potentially do what you suggest here.

[rgommers]

I'm still not sure if we should be doing that though. At that point, it becomes impossible to build a wheel only for the current OS version - which is what should be fastest and most performant. If you build for yourself, you don't want to have compat shims or older versions of functions linked in for you.

The right design I think is to build for the current OS version, and redistributors should explicitly target the minimum OS they want or can support. We already had problems with what setuptools does now in SciPy - we need macosx_12_0_arm64 wheels because 11.0 did not work. But setuptools doesn't listen, so we manually have to rename the wheels afterwards.

[rgommers]

For pypa/build to set MACOSX_DEPLOYMENT_TARGET is perhaps more reasonable, because that is often the tool of choice for packagers. However, we don't want pip install . to target older versions.

[rgommers]

I believe this is a clear improvement as is. We can merge it in its current form, and the rest can be done as a follow up in Meson, and a docs PR.

[FFY00]

Given what I wrote above about how MACOSX_DEPLOYMENT_TARGET is translated into -mmacosx-version-min for Clang, I don't think that will do anything as of right now.

IIRC from my previous research about this, clang did used to honor MACOSX_DEPLOYMENT_TARGET but started not to in newer versions in favor of -mmacosx-version-min.

The compiler activation script has already run.

I don't follow, we can set this env var before running Meson.

We may want to check if Meson should start handling the env var - that would probably be a good idea.

Agreed.

After that is done, we could potentially do what you suggest here.

Given that GCC already does respect it, and if I am correct older Clang versions too, I think if we want to do this, we can start doing it right now. I think MACOSX_DEPLOYMENT_TARGET not being considered for clang should essentially be as a Meson bug.

If Meson doesn't handle this correctly, this PR itself will be problematic.

[FFY00]

However, we don't want pip install . to target older versions.

Why not? Older version should be compatible with newer ones AFAIK.

[rgommers]

I don't follow, we can set this env var before running Meson.

Activation scripts are run on environment activation, not each time the binary for the compiler is called.

Why not? Older version should be compatible with newer ones AFAIK.

It's extra overhead and compile flags, and possibly lower performance. It's also not good design from first principles. If I'm building on my system, for my OS, and not for redistribution (which is the vast majority of builds I think), a packaging library should not force me to target an older OS version.

I think MACOSX_DEPLOYMENT_TARGET not being considered for clang should essentially be as a Meson bug.

Yes, I agree. I need to verify with a non-conda Python/environment. We need it to be correct and consistent for at least python.org installers, system Python, conda and Homebrew.

If Meson doesn't handle this correctly, this PR itself will be problematic.

This PR does improve things (it will fix things for conda-forge and probably also for Homebrew clang), but yes it's still wrong.

[rgommers]

Did some testing:

# Homebrew clang was already installed; can be done with `brew install llvm`
% export MACOSX_DEPLOYMENT_TARGET=11.4
% export CC=/opt/homebrew/Cellar/llvm/14.0.6_1/bin/clang
% export CXX=/opt/homebrew/Cellar/llvm/14.0.6_1/bin/clang++
% /opt/homebrew/Cellar/[email protected]/3.10.6_2/bin/pip3 install ninja
% cd ~/code/meson/test cases/osx/1 basic
% meson setup build

Then searching build/build.ninja shows no -mmacosx-version-min version used. Full output:

# This is the build file for project "osx fundamentals"
# It is autogenerated by the Meson build system.
# Do not edit by hand.

ninja_required_version = 1.8.2

# Rules for module scanning.

# Rules for compiling.

rule c_COMPILER
 command = /opt/homebrew/Cellar/llvm/14.0.6_1/bin/clang $ARGS -MD -MQ $out -MF $DEPFILE -o $out -c $in
 deps = gcc
 depfile = $DEPFILE_UNQUOTED
 description = Compiling C object $out

# Rules for linking.

rule c_LINKER
 command = /opt/homebrew/Cellar/llvm/14.0.6_1/bin/clang $ARGS -o $out $in $LINK_ARGS
 description = Linking target $out

# Other rules

rule CUSTOM_COMMAND
 command = $COMMAND
 description = $DESC
 restat = 1

rule REGENERATE_BUILD
 command = /Users/rgommers/mambaforge/bin/meson --internal regenerate '/Users/rgommers/code/meson/test$ cases/osx/1$ basic' '/Users/rgommers/code/meson/test$ cases/osx/1$ basic/build'
 description = Regenerating build files.
 generator = 1

# Phony build target, always out of date

build PHONY: phony

# Build rules for targets

build prog.p/main.c.o: c_COMPILER ../main.c
 DEPFILE = prog.p/main.c.o.d
 DEPFILE_UNQUOTED = prog.p/main.c.o.d
 ARGS = -Iprog.p -I. -I.. -fcolor-diagnostics -Wall -Winvalid-pch -O0 -g

build prog: c_LINKER prog.p/main.c.o
 LINK_ARGS = -Wl,-dead_strip_dylibs -Wl,-headerpad_max_install_names -Wl,-undefined,error

# Test rules

build test: phony meson-internal__test

build meson-internal__test: CUSTOM_COMMAND all PHONY
 COMMAND = /Users/rgommers/mambaforge/bin/meson test --no-rebuild --print-errorlogs
 DESC = Running$ all$ tests.
 pool = console

build benchmark: phony meson-internal__benchmark

build meson-internal__benchmark: CUSTOM_COMMAND all PHONY
 COMMAND = /Users/rgommers/mambaforge/bin/meson test --benchmark --logbase benchmarklog --num-processes=1 --no-rebuild
 DESC = Running$ benchmark$ suite.
 pool = console

# Install rules

build install: phony meson-internal__install

build meson-internal__install: CUSTOM_COMMAND PHONY | all
 DESC = Installing$ files.
 COMMAND = /Users/rgommers/mambaforge/bin/meson install --no-rebuild
 pool = console

build dist: phony meson-internal__dist

build meson-internal__dist: CUSTOM_COMMAND PHONY
 DESC = Creating$ source$ packages
 COMMAND = /Users/rgommers/mambaforge/bin/meson dist
 pool = console

# Suffix

build ctags: phony meson-internal__ctags

build meson-internal__ctags: CUSTOM_COMMAND PHONY
 COMMAND = /Users/rgommers/mambaforge/bin/meson --internal tags ctags '/Users/rgommers/code/meson/test$ cases/osx/1$ basic'
 pool = console

build uninstall: phony meson-internal__uninstall

build meson-internal__uninstall: CUSTOM_COMMAND PHONY
 COMMAND = /Users/rgommers/mambaforge/bin/meson --internal uninstall
 pool = console

build all: phony meson-test-prereq meson-benchmark-prereq prog

build meson-test-prereq: phony prog

build meson-benchmark-prereq: phony

build clean: phony meson-internal__clean

build meson-internal__clean: CUSTOM_COMMAND PHONY
 COMMAND = /opt/homebrew/bin/ninja -t clean
 description = Cleaning

build build.ninja: REGENERATE_BUILD ../meson.build meson-private/coredata.dat
 pool = console

build reconfigure: REGENERATE_BUILD PHONY
 pool = console

build ../meson.build meson-private/coredata.dat: phony

default all

Now what I can't check is if the produced binary works on an older OS version. It shouldn't though.

[rgommers]

I guess a pain point is that MACOSX_DEPLOYMENT_TARGET used to be honored by XCode / Apple tools, but is no longer. It seems that Apple has decided that it's not a thing. So what we are left with is a lot of outdated info, and an environment variable that is basically specific to GCC and some build/packaging tools that support it in an inconsistent way (CMake/distutils/conda-forge do, Make/Meson/Clang/Homebrew/XCode do not).

Having every project reinvent the wheel by passing a compiler flag is not a reasonable thing to do I think. So it has to be handled somewhere, somehow. Argh ....

[rgommers]

There are no hits in Meson for MACOSX_DEPLOYMENT_TARGET, but there are a bunch for -mmacosx-version-min: https://github.com/mesonbuild/meson/search?q=%22-mmacosx-version-min%22. I think the assumption made there is that users should pass the right compiler flags. But that's going to be a major pain.

xref https://cibuildwheel.readthedocs.io/en/stable/cpp_standards/#macos-and-deployment-target-versions for some useful info

[rgommers]

The cibuildwheel link above shows another problem with always setting it to an older version: you may break usage of modern C++ standard. The page shows what is needed up to C++17, but probably C++20 requires something newer than what sysconfig.get_platform() returns.