From c945329d9e13966e0f52b4141709e318a5d1563d Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 21 Aug 2024 17:31:21 +0200 Subject: [PATCH] Apple: Rebuild when deployment target changes Rebuild when the `*_DEPLOYMENT_TARGET` variables change when building Apple targets. This is done by: 1. Adding it as a tracked variable to `Options` to make sure it clears the incremental cache. 2. Emitting the variable in `--emit=dep-info` (`.d`) files, so that Cargo can pick up changes to it, and correctly trigger a rebuild. --- compiler/rustc_interface/src/passes.rs | 25 +-- compiler/rustc_session/src/config.rs | 2 + compiler/rustc_session/src/options.rs | 9 ++ compiler/rustc_session/src/session.rs | 9 +- .../rustc_target/src/spec/base/apple/mod.rs | 31 ++-- compiler/rustc_target/src/spec/mod.rs | 1 + src/tools/run-make-support/src/lib.rs | 2 +- src/tools/run-make-support/src/targets.rs | 18 +++ .../tidy/src/allowed_run_make_makefiles.txt | 1 - tests/run-make/apple-deployment-target/foo.rs | 1 + .../run-make/apple-deployment-target/rmake.rs | 142 ++++++++++++++++++ .../run-make/macos-deployment-target/Makefile | 21 --- .../with_deployment_target.rs | 4 - 13 files changed, 217 insertions(+), 49 deletions(-) create mode 100644 tests/run-make/apple-deployment-target/foo.rs create mode 100644 tests/run-make/apple-deployment-target/rmake.rs delete mode 100644 tests/run-make/macos-deployment-target/Makefile delete mode 100644 tests/run-make/macos-deployment-target/with_deployment_target.rs diff --git a/compiler/rustc_interface/src/passes.rs b/compiler/rustc_interface/src/passes.rs index 96a6f52d60b6c..8151ae98cdb9e 100644 --- a/compiler/rustc_interface/src/passes.rs +++ b/compiler/rustc_interface/src/passes.rs @@ -391,8 +391,7 @@ fn escape_dep_filename(filename: &str) -> String { // Makefile comments only need escaping newlines and `\`. // The result can be unescaped by anything that can unescape `escape_default` and friends. -fn escape_dep_env(symbol: Symbol) -> String { - let s = symbol.as_str(); +fn escape_dep_env(s: &str) -> String { let mut escaped = String::with_capacity(s.len()); for c in s.chars() { match c { @@ -492,16 +491,22 @@ fn write_out_deps(tcx: TyCtxt<'_>, outputs: &OutputFilenames, out_filenames: &[P // Emit special comments with information about accessed environment variables. let env_depinfo = sess.psess.env_depinfo.borrow(); + + // We will soon sort, so the initial order does not matter. + #[allow(rustc::potential_query_instability)] + let mut env_depinfo: Vec<_> = env_depinfo + .iter() + .map(|(k, v)| (escape_dep_env(k.as_str()), v.map(|v| escape_dep_env(v.as_str())))) + .chain(tcx.sess.target.is_like_osx.then(|| { + // On Apple targets, we also depend on the deployment target environment variable. + let name = rustc_target::spec::apple_deployment_target_env(&tcx.sess.target.os); + (name.into(), std::env::var(name).ok().map(|var| escape_dep_env(&var))) + })) + .collect(); + env_depinfo.sort_unstable(); if !env_depinfo.is_empty() { - // We will soon sort, so the initial order does not matter. - #[allow(rustc::potential_query_instability)] - let mut envs: Vec<_> = env_depinfo - .iter() - .map(|(k, v)| (escape_dep_env(*k), v.map(escape_dep_env))) - .collect(); - envs.sort_unstable(); writeln!(file)?; - for (k, v) in envs { + for (k, v) in env_depinfo { write!(file, "# env-dep:{k}")?; if let Some(v) = v { write!(file, "={v}")?; diff --git a/compiler/rustc_session/src/config.rs b/compiler/rustc_session/src/config.rs index 95d171409d86d..6a259ccb272f3 100644 --- a/compiler/rustc_session/src/config.rs +++ b/compiler/rustc_session/src/config.rs @@ -1164,6 +1164,7 @@ impl Default for Options { color: ColorConfig::Auto, logical_env: FxIndexMap::default(), verbose: false, + apple_deployment_target_env: None, } } } @@ -2710,6 +2711,7 @@ pub fn build_session_options(early_dcx: &mut EarlyDiagCtxt, matches: &getopts::M color, logical_env, verbose, + apple_deployment_target_env: None, } } diff --git a/compiler/rustc_session/src/options.rs b/compiler/rustc_session/src/options.rs index df72e2430fd50..7ed20059f24ae 100644 --- a/compiler/rustc_session/src/options.rs +++ b/compiler/rustc_session/src/options.rs @@ -222,6 +222,15 @@ top_level_options!( color: ColorConfig [UNTRACKED], verbose: bool [TRACKED_NO_CRATE_HASH], + + /// The raw value of the `*_DEPLOYMENT_TARGET` environment variable + /// for the selected target OS. + /// + /// The exact environment variable to use depends on the target that + /// the user has chosen, and we do not want to re-compile if an + /// unrelated deployment target environment variable changed, so we + /// defer the initialization of this to `build_session`. + apple_deployment_target_env: Option [TRACKED], } ); diff --git a/compiler/rustc_session/src/session.rs b/compiler/rustc_session/src/session.rs index 693867c3853da..4f269c9fda280 100644 --- a/compiler/rustc_session/src/session.rs +++ b/compiler/rustc_session/src/session.rs @@ -1006,7 +1006,7 @@ fn default_emitter( #[allow(rustc::untranslatable_diagnostic)] // FIXME: make this translatable pub fn build_session( early_dcx: EarlyDiagCtxt, - sopts: config::Options, + mut sopts: config::Options, io: CompilerIO, bundle: Option>, registry: rustc_errors::registry::Registry, @@ -1110,6 +1110,13 @@ pub fn build_session( let asm_arch = if target.allow_asm { InlineAsmArch::from_str(&target.arch).ok() } else { None }; + // Configure the deployment target for change-tracking, now that target + // details are available. + if target.is_like_osx { + let name = rustc_target::spec::apple_deployment_target_env(&target.os); + sopts.apple_deployment_target_env = std::env::var(name).ok(); + } + let sess = Session { target, host, diff --git a/compiler/rustc_target/src/spec/base/apple/mod.rs b/compiler/rustc_target/src/spec/base/apple/mod.rs index 9262399818f98..37d47f29ca205 100644 --- a/compiler/rustc_target/src/spec/base/apple/mod.rs +++ b/compiler/rustc_target/src/spec/base/apple/mod.rs @@ -274,6 +274,19 @@ pub fn platform(target: &Target) -> Option { }) } +/// Name of the environment variable used to fetch the deployment target on +/// the given OS. +pub fn deployment_target_env(os: &str) -> &'static str { + match os { + "macos" => "MACOSX_DEPLOYMENT_TARGET", + "ios" => "IPHONEOS_DEPLOYMENT_TARGET", + "watchos" => "WATCHOS_DEPLOYMENT_TARGET", + "tvos" => "TVOS_DEPLOYMENT_TARGET", + "visionos" => "XROS_DEPLOYMENT_TARGET", + _ => unreachable!("tried to get deployment target env var for non-Apple platform"), + } +} + /// Hack for calling `deployment_target` outside of this module. pub fn deployment_target_for_target(target: &Target) -> (u16, u8, u8) { let arch = if target.llvm_target.starts_with("arm64e") { @@ -323,17 +336,13 @@ fn deployment_target(os: &str, arch: Arch, abi: TargetAbi) -> (u16, u8, u8) { _ => os_min, }; - // The environment variable used to fetch the deployment target. - let env_var = match os { - "macos" => "MACOSX_DEPLOYMENT_TARGET", - "ios" => "IPHONEOS_DEPLOYMENT_TARGET", - "watchos" => "WATCHOS_DEPLOYMENT_TARGET", - "tvos" => "TVOS_DEPLOYMENT_TARGET", - "visionos" => "XROS_DEPLOYMENT_TARGET", - _ => unreachable!("tried to get deployment target env var for non-Apple platform"), - }; - - if let Ok(deployment_target) = env::var(env_var) { + // NOTE: We access the deployment target environment variable here, which + // makes the variable an **implicit** input which affects compilation. + // + // We make sure to rebuild when the variable changes, both by busting the + // incremental cache, and by telling Cargo that it is a dependency. + // Search for usages of `deployment_target_env` to see how. + if let Ok(deployment_target) = env::var(deployment_target_env(os)) { match parse_version(&deployment_target) { // It is common that the deployment target is set too low, e.g. on // macOS Aarch64 to also target older x86_64, the user may set a diff --git a/compiler/rustc_target/src/spec/mod.rs b/compiler/rustc_target/src/spec/mod.rs index 5d0d68c26a3f9..27f5ed093a965 100644 --- a/compiler/rustc_target/src/spec/mod.rs +++ b/compiler/rustc_target/src/spec/mod.rs @@ -60,6 +60,7 @@ pub mod crt_objects; mod base; pub use base::apple::{ + deployment_target_env as apple_deployment_target_env, deployment_target_for_target as current_apple_deployment_target, platform as current_apple_platform, sdk_version as current_apple_sdk_version, }; diff --git a/src/tools/run-make-support/src/lib.rs b/src/tools/run-make-support/src/lib.rs index fc20fd3b2e86a..db76a354682a3 100644 --- a/src/tools/run-make-support/src/lib.rs +++ b/src/tools/run-make-support/src/lib.rs @@ -70,7 +70,7 @@ pub use env::{env_var, env_var_os}; pub use run::{cmd, run, run_fail, run_with_args}; /// Helpers for checking target information. -pub use targets::{is_darwin, is_msvc, is_windows, llvm_components_contain, target, uname}; +pub use targets::{is_darwin, is_msvc, is_windows, llvm_components_contain, target, uname, apple_os}; /// Helpers for building names of output artifacts that are potentially target-specific. pub use artifact_names::{ diff --git a/src/tools/run-make-support/src/targets.rs b/src/tools/run-make-support/src/targets.rs index 5dcb0b83430f9..896abb73fc108 100644 --- a/src/tools/run-make-support/src/targets.rs +++ b/src/tools/run-make-support/src/targets.rs @@ -28,6 +28,24 @@ pub fn is_darwin() -> bool { target().contains("darwin") } +/// Get the target OS on Apple operating systems. +#[must_use] +pub fn apple_os() -> &'static str { + if target().contains("darwin") { + "macos" + } else if target().contains("ios") { + "ios" + } else if target().contains("tvos") { + "tvos" + } else if target().contains("watchos") { + "watchos" + } else if target().contains("visionos") { + "visionos" + } else { + panic!("not an Apple OS") + } +} + /// Check if `component` is within `LLVM_COMPONENTS` #[must_use] pub fn llvm_components_contain(component: &str) -> bool { diff --git a/src/tools/tidy/src/allowed_run_make_makefiles.txt b/src/tools/tidy/src/allowed_run_make_makefiles.txt index f55abb513b80d..1d45a52d3e8f3 100644 --- a/src/tools/tidy/src/allowed_run_make_makefiles.txt +++ b/src/tools/tidy/src/allowed_run_make_makefiles.txt @@ -7,7 +7,6 @@ run-make/issue-84395-lto-embed-bitcode/Makefile run-make/jobserver-error/Makefile run-make/libs-through-symlinks/Makefile run-make/libtest-thread-limit/Makefile -run-make/macos-deployment-target/Makefile run-make/split-debuginfo/Makefile run-make/symbol-mangling-hashed/Makefile run-make/translation/Makefile diff --git a/tests/run-make/apple-deployment-target/foo.rs b/tests/run-make/apple-deployment-target/foo.rs new file mode 100644 index 0000000000000..f328e4d9d04c3 --- /dev/null +++ b/tests/run-make/apple-deployment-target/foo.rs @@ -0,0 +1 @@ +fn main() {} diff --git a/tests/run-make/apple-deployment-target/rmake.rs b/tests/run-make/apple-deployment-target/rmake.rs new file mode 100644 index 0000000000000..562263a5d085a --- /dev/null +++ b/tests/run-make/apple-deployment-target/rmake.rs @@ -0,0 +1,142 @@ +//! Test codegen when setting deployment targets on Apple platforms. +//! +//! This is important since its a compatibility hazard. The linker will +//! generate load commands differently based on what minimum OS it can assume. +//! +//! See https://github.com/rust-lang/rust/pull/105123. + +//@ only-apple + +use run_make_support::{apple_os, cmd, run_in_tmpdir, rustc, target}; + +/// Run vtool to check the `minos` field in LC_BUILD_VERSION. +/// +/// On lower deployment targets, LC_VERSION_MIN_MACOSX, LC_VERSION_MIN_IPHONEOS and similar +/// are used instead of LC_BUILD_VERSION - these have a `version` field, so also check that. +#[track_caller] +fn minos(file: &str, version: &str) { + cmd("vtool") + .arg("-show-build") + .arg(file) + .run() + .assert_stdout_contains_regex(format!("(minos|version) {version}")); +} + +fn main() { + // These versions should generally be higher than the default versions + let (env_var, example_version, higher_example_version) = match apple_os() { + "macos" => ("MACOSX_DEPLOYMENT_TARGET", "12.0", "13.0"), + // armv7s-apple-ios and i386-apple-ios only supports iOS 10.0 + "ios" if target() == "armv7s-apple-ios" || target() == "i386-apple-ios" => { + ("IPHONEOS_DEPLOYMENT_TARGET", "10.0", "10.0") + } + "ios" => ("IPHONEOS_DEPLOYMENT_TARGET", "15.0", "16.0"), + "watchos" => ("WATCHOS_DEPLOYMENT_TARGET", "7.0", "9.0"), + "tvos" => ("TVOS_DEPLOYMENT_TARGET", "14.0", "15.0"), + "visionos" => ("XROS_DEPLOYMENT_TARGET", "1.1", "1.2"), + _ => unreachable!(), + }; + let default_version = + rustc().target(target()).env_remove(env_var).print("deployment-target").run().stdout_utf8(); + let default_version = default_version.strip_prefix("deployment_target=").unwrap().trim(); + + // Test that version makes it to the object file. + run_in_tmpdir(|| { + let mut rustc = rustc(); + rustc.target(target()); + rustc.crate_type("lib"); + rustc.emit("obj"); + rustc.input("foo.rs"); + rustc.output("foo.o"); + + rustc.env(env_var, example_version).run(); + minos("foo.o", example_version); + + // FIXME(madsmtm): Doesn't work on Mac Catalyst and the simulator. + if !target().contains("macabi") && !target().contains("sim") { + rustc.env_remove(env_var).run(); + minos("foo.o", default_version); + } + }); + + // Test that version makes it to the linker when linking dylibs. + run_in_tmpdir(|| { + // Certain watchOS targets don't support dynamic linking, so we disable the test on those. + if apple_os() == "watchos" { + return; + } + + let mut rustc = rustc(); + rustc.target(target()); + rustc.crate_type("dylib"); + rustc.input("foo.rs"); + rustc.output("libfoo.dylib"); + + rustc.env(env_var, example_version).run(); + minos("libfoo.dylib", example_version); + + // FIXME(madsmtm): Deployment target is not currently passed properly to linker + // rustc.env_remove(env_var).run(); + // minos("libfoo.dylib", default_version); + + // Test with ld64 instead + rustc.arg("-Clinker-flavor=ld"); + + rustc.env(env_var, example_version).run(); + minos("libfoo.dylib", example_version); + + rustc.env_remove(env_var).run(); + minos("libfoo.dylib", default_version); + }); + + // Test that version makes it to the linker when linking executables. + run_in_tmpdir(|| { + let mut rustc = rustc(); + rustc.target(target()); + rustc.crate_type("bin"); + rustc.input("foo.rs"); + rustc.output("foo"); + + // FIXME(madsmtm): Doesn't work on watchOS for some reason? + if !target().contains("watchos") { + rustc.env(env_var, example_version).run(); + minos("foo", example_version); + } + + // FIXME(madsmtm): Deployment target is not currently passed properly to linker + // rustc.env_remove(env_var).run(); + // minos("foo", default_version); + + // Test with ld64 instead + rustc.arg("-Clinker-flavor=ld"); + + rustc.env(env_var, example_version).run(); + minos("foo", example_version); + + rustc.env_remove(env_var).run(); + minos("foo", default_version); + }); + + // Test that changing the deployment target busts the incremental cache. + run_in_tmpdir(|| { + let mut rustc = rustc(); + rustc.target(target()); + rustc.incremental("incremental"); + rustc.crate_type("lib"); + rustc.emit("obj"); + rustc.input("foo.rs"); + rustc.output("foo.o"); + + rustc.env(env_var, example_version).run(); + minos("foo.o", example_version); + + rustc.env(env_var, higher_example_version).run(); + minos("foo.o", higher_example_version); + + // FIXME(madsmtm): Doesn't work on Mac Catalyst and the simulator. + if !target().contains("macabi") && !target().contains("sim") { + rustc.env_remove(env_var).run(); + minos("foo.o", default_version); + } + }); +} diff --git a/tests/run-make/macos-deployment-target/Makefile b/tests/run-make/macos-deployment-target/Makefile deleted file mode 100644 index 757ca6995350f..0000000000000 --- a/tests/run-make/macos-deployment-target/Makefile +++ /dev/null @@ -1,21 +0,0 @@ -# only-macos -# -# Check that a set deployment target actually makes it to the linker. -# This is important since its a compatibility hazard. The linker will -# generate load commands differently based on what minimum OS it can assume. - -include ../tools.mk - -ifeq ($(strip $(shell uname -m)),arm64) - GREP_PATTERN = "minos 11.0" -else - GREP_PATTERN = "version 10.13" -endif - -OUT_FILE=$(TMPDIR)/with_deployment_target.dylib -all: - env MACOSX_DEPLOYMENT_TARGET=10.13 $(RUSTC) with_deployment_target.rs -o $(OUT_FILE) -# XXX: The check is for either the x86_64 minimum OR the aarch64 minimum (M1 starts at macOS 11). -# They also use different load commands, so we let that change with each too. The aarch64 check -# isn't as robust as the x86 one, but testing both seems unneeded. - vtool -show-build $(OUT_FILE) | $(CGREP) -e $(GREP_PATTERN) diff --git a/tests/run-make/macos-deployment-target/with_deployment_target.rs b/tests/run-make/macos-deployment-target/with_deployment_target.rs deleted file mode 100644 index 342fe0ecbcfcd..0000000000000 --- a/tests/run-make/macos-deployment-target/with_deployment_target.rs +++ /dev/null @@ -1,4 +0,0 @@ -#![crate_type = "cdylib"] - -#[allow(dead_code)] -fn something_and_nothing() {}