Skip to content

Conversation

giordano
Copy link
Member

@giordano giordano commented Nov 4, 2022

Needs tests

@giordano giordano force-pushed the mg/macos-ld-sdk-version branch from f963fa6 to 4f27972 Compare November 4, 2022 22:36
src/Runner.jl Outdated
function min_macos_version_flags(p::AbstractPlatform)
ver = macos_version(p)
# Ask compilers to compile for a minimum macOS version, targeting that SDK.
return ("-mmacosx-version-min=$(ver)", "-Wl,-sdk_version,$(ver)")
Copy link
Member Author

@giordano giordano Nov 4, 2022

Choose a reason for hiding this comment

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

@staticfloat what do you think about using here

("-mmacosx-version-min=\${MACOSX_DEPLOYMENT_TARGET}", "-Wl,-sdk_version,\${MACOSX_DEPLOYMENT_TARGET}")

? The idea is that in the cases where we use a different SDK we can still set the same value by overriding the environment variable MACOSX_DEPLOYMENT_TARGET, which is otherwise set to the default macOS SDK version:

mapping["MACOSX_DEPLOYMENT_TARGET"] = macos_version(platform)

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at the value of MACOSX_DEPLOYMENT_TARGET is also what cctools linker should in principle do (even though I didn't manage to make it work): https://github.com/tpoechtrager/cctools-port/blob/634a084377ee2e2932c66459b0396edf76da2e9f/cctools/ld64/src/ld/Options.cpp#L5378-L5394.

Therefore I'm going with this option because it's more consistent (the environment variable MACOSX_DEPLOYMENT_TARGET is used by build systems, like CMake) and offers users an escape hatch when they use a different SDK.

@giordano giordano marked this pull request as ready for review November 4, 2022 22:41
@giordano giordano force-pushed the mg/macos-ld-sdk-version branch from 0f0c89d to 0d03d9b Compare November 5, 2022 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant