-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Always set MACOSX_DEPLOYMENT_TARGET
in build scripts
#13115
Comments
Can you say why this would not be handled in rustc? Generally we try to avoid having cargo have explicit target knowledge. Fixing this in rustc is tracked in rust-lang/rust#118204. |
Hmm, that issue is only tangentially related: this is about setting |
Ah, thanks for the clarification, I misunderstood. |
So one thing that concerns me about doing this (though I know it would be extremely helpful) is compatibility. As cc has seen, most people don't actually set their deployment target variables and instead rely on the active SDK's Since subprocess C compilers would inherit variables and that rustc's |
Yeah, that's a reasonable fear. I would argue that most of those users would benefit from this, as they'd get to realize that their build doesn't actually work on older macOS versions, and that they need to either do more work to support that, or explicitly raise their supported version. Though without something like rust-lang/rfcs#3379, it will be kinda hard to know what to tell users to do, even if we do the breakage across an edition. |
So perhaps that's also a viable option for Cargo? This wouldn't have the same backwards compatibility issues, and I think it would provide a step forwards for better Cargo integration with deployment targets. |
This might also be a good candidate for the build scipt API: #12432. |
…t, r=Mark-Simulacrum Always set the deployment target when building std `cc` has [a bug/feature](rust-lang/cc-rs#1171) (I guess depending on how you look at it) where the default deployment target is taken from the SDK instead of from `rustc`. This causes `compiler-builtins` to build `compiler-rt` with the wrong deployment target on iOS. I've been meaning to change how `cc` works in this regard, but that's a lengthy process, so let's fix it in bootstrap for now. The behaviour be seen locally with `./x build library --set build.optimized-compiler-builtins=true` for various target triples, and then inspecting with `otool -l build/host/stage1/lib/rustlib/*/lib/libcompiler_builtins-*.rlib | rg 'minos|version'`. I have added a rmake test that ensures that we now have the same version everywhere. Fixes rust-lang#128419 Fixes rust-lang/compiler-builtins#650 See also rust-lang/cargo#13115 `@rustbot` label O-apple
Problem
The environment variables
MACOSX_DEPLOYMENT_TARGET
,IPHONEOS_DEPLOYMENT_TARGET
,TVOS_DEPLOYMENT_TARGET
andWATCHOS_DEPLOYMENT_TARGET
are standard environment variables on Apple targets, and are used by compilers to get the desired minimum supported operating system version.When not specified, compilers usually choose some default. The default that
rustc
chooses can be retrieved withrustc --target x86_64-apple-darwin --print deployment-target
, and e.g. thecc
crate has support for detecting this, and passing it on to a C compiler.The problem(s) is that:
cc
has, has to be implemented by everybuild.rs
script that wants to call an external compiler.rustc
process to determine this is inefficient (although probably negligible).Proposed Solution
Cargo always sets these in build scripts when building for the relevant Apple targets.
That is, it sets
MACOSX_DEPLOYMENT_TARGET
when building for macOS,IPHONEOS_DEPLOYMENT_TARGET
when building for iOS, and so on.As an example, as an author of a
build.rs
script, I would like to be able to do the following (once a version of Cargo that supports this is in my MSRV):(Note: In contrast to all other environment variables that Cargo sets for build scripts, these are explicitly target dependent).
Notes
CC @BlackHoleFox whom implemented the
rustc --print deployment-target
flag.I'd volunteer to do the implementation work if this feature is desired?
The text was updated successfully, but these errors were encountered: