cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Always set `MACOSX_DEPLOYMENT_TARGET` in build scripts

Open madsmtm opened this issue 7 months ago • 7 comments

Problem

The environment variables MACOSX_DEPLOYMENT_TARGET, IPHONEOS_DEPLOYMENT_TARGET, TVOS_DEPLOYMENT_TARGET and WATCHOS_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 with rustc --target x86_64-apple-darwin --print deployment-target, and e.g. the cc crate has support for detecting this, and passing it on to a C compiler.

The problem(s) is that:

  • This kind of logic that cc has, has to be implemented by every build.rs script that wants to call an external compiler.
  • Spawning a new rustc process to determine this is inefficient (although probably negligible).
  • It is not really discoverable for users. (IMO the most important)

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):

if std::env::var("TARGET").unwrap() == "x86_64-apple-darwin" {
    // Note that I'm allowed to `unwrap` here because Cargo always sets it for this target.
    let deployment_target = std::env::var("MACOSX_DEPLOYMENT_TARGET").unwrap();
    // ...
}

(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?

madsmtm avatar Dec 05 '23 08:12 madsmtm

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 https://github.com/rust-lang/rust/issues/118204.

ehuss avatar Dec 05 '23 14:12 ehuss

Hmm, that issue is only tangentially related: this is about setting MACOSX_DEPLOYMENT_TARGET when running the build script, which is something that only Cargo can do, since it's an input to the build script.

madsmtm avatar Dec 05 '23 14:12 madsmtm

Ah, thanks for the clarification, I misunderstood.

ehuss avatar Dec 05 '23 15:12 ehuss

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 DefaultDeploymentTarget. As a result, @thomcc and I have been thinking about removing the --print sourcing from cc and relying on the SDK instead to avoid widespread breakage. That's not very ontopic for this issue though so I digress.

Since subprocess C compilers would inherit variables and that rustc's --print deployment-target is assumed always lower then the SDK's default (what clang uses if you don't specify MACOSX_DEPLOYMENT_TARGET), I get the fear this is going to break people's builds.

BlackHoleFox avatar Dec 06 '23 03:12 BlackHoleFox

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 https://github.com/rust-lang/rfcs/pull/3379, it will be kinda hard to know what to tell users to do, even if we do the breakage across an edition.

madsmtm avatar Dec 12 '23 03:12 madsmtm

cc went with using the current platform SDK's DefaultDeploymentTarget, see https://github.com/rust-lang/cc-rs/pull/943.

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.

madsmtm avatar Feb 13 '24 13:02 madsmtm

This might also be a good candidate for the build scipt API: https://github.com/rust-lang/cargo/issues/12432.

weihanglo avatar Apr 10 '24 20:04 weihanglo