From e790b92d72246c5dc2752285d1bb5605af3d2f64 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 31 Jan 2025 04:07:04 +0100 Subject: [PATCH 1/6] Bootstrap: Don't duplicate cc-rs flags Commands that end up invoking cc-rs, i.e. Cargo (through build scripts) and cmake-rs don't need the CFLAGS from cc-rs itself, as they will just end up as duplicates. We do still choose to pass them in certain places, but now it's at least clear which flags are default, and which flags are extra flags added on. --- src/bootstrap/src/core/build_steps/compile.rs | 3 ++- src/bootstrap/src/core/build_steps/llvm.rs | 11 ++++++++-- src/bootstrap/src/core/build_steps/test.rs | 8 +++++-- src/bootstrap/src/core/builder/cargo.rs | 4 ++-- src/bootstrap/src/lib.rs | 21 ++++++++++++------- src/bootstrap/src/utils/cc_detect.rs | 6 ++++-- 6 files changed, 36 insertions(+), 17 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index fd9bf47234c63..d76a15fa58311 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1541,7 +1541,8 @@ pub fn compiler_file( return PathBuf::new(); } let mut cmd = command(compiler); - cmd.args(builder.cflags(target, GitRepo::Rustc, c)); + cmd.args(builder.default_cflags(target, c)); + cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); let out = cmd.run_capture_stdout(builder).stdout(); PathBuf::from(out.trim()) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 3cf25373b8963..706bef521e0de 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -757,9 +757,15 @@ fn configure_cmake( } cfg.build_arg("-j").build_arg(builder.jobs().to_string()); + // FIXME(madsmtm): Allow `cmake-rs` to select flags by itself by passing + // our flags via `.cflag`/`.cxxflag` instead. + // + // Needs `suppressed_compiler_flag_prefixes` to be gone, and hence + // https://github.com/llvm/llvm-project/issues/88780 to be fixed. let mut cflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::C) + .default_cflags(target, CLang::C) .into_iter() + .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::C)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() @@ -778,8 +784,9 @@ fn configure_cmake( } cfg.define("CMAKE_C_FLAGS", cflags); let mut cxxflags: OsString = builder - .cflags(target, GitRepo::Llvm, CLang::Cxx) + .default_cflags(target, CLang::Cxx) .into_iter() + .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index 99b033db115ce..a5cad5b8414aa 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2005,14 +2005,18 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // Only pass correct values for these flags for the `run-make` suite as it // requires that a C++ compiler was configured which isn't always the case. if !builder.config.dry_run() && mode == "run-make" { + let mut cflags = builder.default_cflags(target, CLang::C); + cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C)); + let mut cxxflags = builder.default_cflags(target, CLang::Cxx); + cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); cmd.arg("--cc") .arg(builder.cc(target)) .arg("--cxx") .arg(builder.cxx(target).unwrap()) .arg("--cflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" ")) + .arg(cflags.join(" ")) .arg("--cxxflags") - .arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ")); + .arg(cxxflags.join(" ")); copts_passed = true; if let Some(ar) = builder.ar(target) { cmd.arg("--ar").arg(ar); diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index d418237a5689f..fb8ef2b57be60 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -317,7 +317,7 @@ impl Cargo { let cc = ccacheify(&builder.cc(target)); self.command.env(format!("CC_{triple_underscored}"), &cc); - let cflags = builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "); + let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" "); self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags); if let Some(ar) = builder.ar(target) { @@ -329,7 +329,7 @@ impl Cargo { if let Ok(cxx) = builder.cxx(target) { let cxx = ccacheify(&cxx); - let cxxflags = builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); + let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); self.command .env(format!("CXX_{triple_underscored}"), &cxx) .env(format!("CXXFLAGS_{triple_underscored}"), cxxflags); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 482e23cd04c31..1ce868316623a 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -246,6 +246,7 @@ impl Mode { } } +#[derive(Debug, Hash, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)] pub enum CLang { C, Cxx, @@ -1139,9 +1140,9 @@ Executed at: {executed_at}"#, self.cc.borrow()[&target].path().into() } - /// Returns a list of flags to pass to the C compiler for the target - /// specified. - fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + /// Returns C flags that `cc-rs` thinks should be enabled for the + /// specified target by default. + fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec { if self.config.dry_run() { return Vec::new(); } @@ -1150,14 +1151,18 @@ Executed at: {executed_at}"#, CLang::Cxx => self.cxx.borrow()[&target].clone(), }; - // Filter out -O and /O (the optimization flags) that we picked up from - // cc-rs because the build scripts will determine that for themselves. - let mut base = base - .args() + // Filter out -O and /O (the optimization flags) that we picked up + // from cc-rs, that's up to the caller to figure out. + base.args() .iter() .map(|s| s.to_string_lossy().into_owned()) .filter(|s| !s.starts_with("-O") && !s.starts_with("/O")) - .collect::>(); + .collect::>() + } + + /// Returns extra C flags that `cc-rs` doesn't handle. + fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + let mut base = Vec::new(); // If we're compiling C++ on macOS then we add a flag indicating that // we want libc++ (more filled out than libstdc++), ensuring that diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 4aec554b4321a..33b490e5ca1b2 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -142,7 +142,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { }; build.cc.borrow_mut().insert(target, compiler.clone()); - let cflags = build.cflags(target, GitRepo::Rustc, CLang::C); + let mut cflags = build.default_cflags(target, CLang::C); + cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C)); // If we use llvm-libunwind, we will need a C++ compiler as well for all targets // We'll need one anyways if the target triple is also a host triple @@ -168,7 +169,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple)); if let Ok(cxx) = build.cxx(target) { - let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx); + let mut cxxflags = build.default_cflags(target, CLang::Cxx); + cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple)); build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple)); } From 9318fbb33aa85d0467d05159623d0e56ef2e8f3e Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 31 Jan 2025 04:05:54 +0100 Subject: [PATCH 2/6] Always set the deployment target when building std --- src/bootstrap/src/core/build_steps/compile.rs | 37 ++++++++++++++++++- .../run-make/apple-deployment-target/rmake.rs | 23 +++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index d76a15fa58311..f137460e7ea55 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -443,8 +443,41 @@ fn compiler_rt_for_profiler(builder: &Builder<'_>) -> PathBuf { /// Configure cargo to compile the standard library, adding appropriate env vars /// and such. pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, cargo: &mut Cargo) { - if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { - cargo.env("MACOSX_DEPLOYMENT_TARGET", target); + // rustc already ensures that it builds with the minimum deployment + // target, so ideally we shouldn't need to do anything here. + // + // However, `cc` currently defaults to a higher version for backwards + // compatibility, which means that compiler-rt, which is built via + // compiler-builtins' build script, gets built with a higher deployment + // target. This in turn causes warnings while linking, and is generally + // a compatibility hazard. + // + // So, at least until https://github.com/rust-lang/cc-rs/issues/1171, or + // perhaps https://github.com/rust-lang/cargo/issues/13115 is resolved, we + // explicitly set the deployment target environment variables to avoid + // this issue. + // + // This place also serves as an extension point if we ever wanted to raise + // rustc's default deployment target while keeping the prebuilt `std` at + // a lower version, so it's kinda nice to have in any case. + if target.contains("apple") && !builder.config.dry_run() { + // Query rustc for the deployment target, and the associated env var. + // The env var is one of the standard `*_DEPLOYMENT_TARGET` vars, i.e. + // `MACOSX_DEPLOYMENT_TARGET`, `IPHONEOS_DEPLOYMENT_TARGET`, etc. + let mut cmd = command(builder.rustc(cargo.compiler())); + cmd.arg("--target").arg(target.rustc_target_arg()); + cmd.arg("--print=deployment-target"); + let output = cmd.run_capture_stdout(builder).stdout(); + + let (env_var, value) = output.split_once('=').unwrap(); + // Unconditionally set the env var (if it was set in the environment + // already, rustc should've picked that up). + cargo.env(env_var.trim(), value.trim()); + + // Allow CI to override the deployment target for `std`. + if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { + cargo.env("MACOSX_DEPLOYMENT_TARGET", target); + } } // Paths needed by `library/profiler_builtins/build.rs`. diff --git a/tests/run-make/apple-deployment-target/rmake.rs b/tests/run-make/apple-deployment-target/rmake.rs index 0ae95cb1f4b16..839e21b7496d9 100644 --- a/tests/run-make/apple-deployment-target/rmake.rs +++ b/tests/run-make/apple-deployment-target/rmake.rs @@ -7,7 +7,11 @@ //@ only-apple -use run_make_support::{apple_os, cmd, run_in_tmpdir, rustc, target}; +use std::collections::HashSet; + +use run_make_support::{ + apple_os, cmd, has_extension, path, regex, run_in_tmpdir, rustc, shallow_find_files, target, +}; /// Run vtool to check the `minos` field in LC_BUILD_VERSION. /// @@ -166,4 +170,21 @@ fn main() { rustc().env_remove(env_var).run(); minos("foo.o", default_version); }); + + // Test that all binaries in rlibs produced by `rustc` have the same version. + // Regression test for https://github.com/rust-lang/rust/issues/128419. + let sysroot = rustc().print("sysroot").run().stdout_utf8(); + let target_sysroot = path(sysroot.trim()).join("lib/rustlib").join(target()).join("lib"); + let rlibs = shallow_find_files(&target_sysroot, |path| has_extension(path, "rlib")); + + let output = cmd("otool").arg("-l").args(rlibs).run().stdout_utf8(); + let re = regex::Regex::new(r"(minos|version) ([0-9.]*)").unwrap(); + let mut versions = HashSet::new(); + for (_, [_, version]) in re.captures_iter(&output).map(|c| c.extract()) { + versions.insert(version); + } + // FIXME(madsmtm): See above for aarch64-apple-watchos. + if versions.len() != 1 && target() != "aarch64-apple-watchos" { + panic!("std rlibs contained multiple different deployment target versions: {versions:?}"); + } } From 0a3fd1f0626167dce2648680300df3c252fe7154 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 25 Nov 2024 08:00:22 +0100 Subject: [PATCH 3/6] Explain why MACOSX_STD_DEPLOYMENT_TARGET exist --- src/bootstrap/src/core/build_steps/compile.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index f137460e7ea55..61f404aecdc80 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -474,7 +474,15 @@ pub fn std_cargo(builder: &Builder<'_>, target: TargetSelection, stage: u32, car // already, rustc should've picked that up). cargo.env(env_var.trim(), value.trim()); - // Allow CI to override the deployment target for `std`. + // Allow CI to override the deployment target for `std` on macOS. + // + // This is useful because we might want the host tooling LLVM, `rustc` + // and Cargo to have a different deployment target than `std` itself + // (currently, these two versions are the same, but in the past, we + // supported macOS 10.7 for user code and macOS 10.8 in host tooling). + // + // It is not necessary on the other platforms, since only macOS has + // support for host tooling. if let Some(target) = env::var_os("MACOSX_STD_DEPLOYMENT_TARGET") { cargo.env("MACOSX_DEPLOYMENT_TARGET", target); } From 9fa819ef94ed831a410e5c66a156974c718670de Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Fri, 31 Jan 2025 04:26:19 +0100 Subject: [PATCH 4/6] Update deployment target setup in jobs.yml Note that specifying MACOSX_STD_DEPLOYMENT_TARGET is still completely unnecessary here, but it's nice to have for future changes where we might want to version `std` and `rustc`'s deployment target separately. --- src/ci/github-actions/jobs.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/ci/github-actions/jobs.yml b/src/ci/github-actions/jobs.yml index 7730d29d28f68..e5c648779b8b7 100644 --- a/src/ci/github-actions/jobs.yml +++ b/src/ci/github-actions/jobs.yml @@ -59,6 +59,7 @@ envs: SCRIPT: ./x.py --stage 2 test --skip tests/ui --skip tests/rustdoc -- --exact RUST_CONFIGURE_ARGS: --build=x86_64-apple-darwin --enable-sanitizers --enable-profiler --set rust.jemalloc RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 + # Ensure that host tooling is tested on our minimum supported macOS version. MACOSX_DEPLOYMENT_TARGET: 10.12 MACOSX_STD_DEPLOYMENT_TARGET: 10.12 SELECT_XCODE: /Applications/Xcode_15.2.app @@ -370,7 +371,9 @@ auto: SCRIPT: ./x.py dist bootstrap --include-default-paths --host=x86_64-apple-darwin --target=x86_64-apple-darwin RUST_CONFIGURE_ARGS: --enable-full-tools --enable-sanitizers --enable-profiler --set rust.jemalloc --set rust.lto=thin --set rust.codegen-units=1 RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 + # Ensure that host tooling is built to support our minimum support macOS version. MACOSX_DEPLOYMENT_TARGET: 10.12 + MACOSX_STD_DEPLOYMENT_TARGET: 10.12 SELECT_XCODE: /Applications/Xcode_15.2.app NO_LLVM_ASSERTIONS: 1 NO_DEBUG_ASSERTIONS: 1 @@ -386,7 +389,10 @@ auto: # https://github.com/rust-lang/rust/issues/129069 RUST_CONFIGURE_ARGS: --enable-sanitizers --enable-profiler --set rust.jemalloc --set target.aarch64-apple-ios-macabi.sanitizers=false --set target.x86_64-apple-ios-macabi.sanitizers=false RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 + # Ensure that host tooling is built to support our minimum support macOS version. + # FIXME(madsmtm): This might be redundant, as we're not building host tooling here (?) MACOSX_DEPLOYMENT_TARGET: 10.12 + MACOSX_STD_DEPLOYMENT_TARGET: 10.12 SELECT_XCODE: /Applications/Xcode_15.2.app NO_LLVM_ASSERTIONS: 1 NO_DEBUG_ASSERTIONS: 1 @@ -404,7 +410,6 @@ auto: <<: *env-x86_64-apple-tests <<: *job-macos-xl - # This target only needs to support 11.0 and up as nothing else supports the hardware - name: dist-aarch64-apple env: SCRIPT: ./x.py dist bootstrap --include-default-paths --host=aarch64-apple-darwin --target=aarch64-apple-darwin @@ -419,6 +424,8 @@ auto: RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 SELECT_XCODE: /Applications/Xcode_15.4.app USE_XCODE_CLANG: 1 + # Aarch64 tooling only needs to support macOS 11.0 and up as nothing else + # supports the hardware. MACOSX_DEPLOYMENT_TARGET: 11.0 MACOSX_STD_DEPLOYMENT_TARGET: 11.0 NO_LLVM_ASSERTIONS: 1 @@ -428,7 +435,6 @@ auto: CODEGEN_BACKENDS: llvm,cranelift <<: *job-macos-m1 - # This target only needs to support 11.0 and up as nothing else supports the hardware - name: aarch64-apple env: SCRIPT: ./x.py --stage 2 test --host=aarch64-apple-darwin --target=aarch64-apple-darwin @@ -439,6 +445,8 @@ auto: RUSTC_RETRY_LINKER_ON_SEGFAULT: 1 SELECT_XCODE: /Applications/Xcode_15.4.app USE_XCODE_CLANG: 1 + # Aarch64 tooling only needs to support macOS 11.0 and up as nothing else + # supports the hardware, so only need to test it there. MACOSX_DEPLOYMENT_TARGET: 11.0 MACOSX_STD_DEPLOYMENT_TARGET: 11.0 NO_LLVM_ASSERTIONS: 1 From d8e93c7e07ec3b299fae88e5f8b07e7a786b0082 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Wed, 5 Feb 2025 13:44:35 +0100 Subject: [PATCH 5/6] Rename [default|extra]_cflags -> cc_[|un]handled_clags Makes it explicit that these are in relation to the cc-rs crate. --- src/bootstrap/src/core/build_steps/compile.rs | 4 ++-- src/bootstrap/src/core/build_steps/llvm.rs | 8 ++++---- src/bootstrap/src/core/build_steps/test.rs | 8 ++++---- src/bootstrap/src/core/builder/cargo.rs | 5 +++-- src/bootstrap/src/lib.rs | 9 +++++++-- src/bootstrap/src/utils/cc_detect.rs | 8 ++++---- 6 files changed, 24 insertions(+), 18 deletions(-) diff --git a/src/bootstrap/src/core/build_steps/compile.rs b/src/bootstrap/src/core/build_steps/compile.rs index 61f404aecdc80..c12b138f06869 100644 --- a/src/bootstrap/src/core/build_steps/compile.rs +++ b/src/bootstrap/src/core/build_steps/compile.rs @@ -1582,8 +1582,8 @@ pub fn compiler_file( return PathBuf::new(); } let mut cmd = command(compiler); - cmd.args(builder.default_cflags(target, c)); - cmd.args(builder.extra_cflags(target, GitRepo::Rustc, c)); + cmd.args(builder.cc_handled_clags(target, c)); + cmd.args(builder.cc_unhandled_cflags(target, GitRepo::Rustc, c)); cmd.arg(format!("-print-file-name={file}")); let out = cmd.run_capture_stdout(builder).stdout(); PathBuf::from(out.trim()) diff --git a/src/bootstrap/src/core/build_steps/llvm.rs b/src/bootstrap/src/core/build_steps/llvm.rs index 706bef521e0de..13b3074f213df 100644 --- a/src/bootstrap/src/core/build_steps/llvm.rs +++ b/src/bootstrap/src/core/build_steps/llvm.rs @@ -763,9 +763,9 @@ fn configure_cmake( // Needs `suppressed_compiler_flag_prefixes` to be gone, and hence // https://github.com/llvm/llvm-project/issues/88780 to be fixed. let mut cflags: OsString = builder - .default_cflags(target, CLang::C) + .cc_handled_clags(target, CLang::C) .into_iter() - .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::C)) + .chain(builder.cc_unhandled_cflags(target, GitRepo::Llvm, CLang::C)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() @@ -784,9 +784,9 @@ fn configure_cmake( } cfg.define("CMAKE_C_FLAGS", cflags); let mut cxxflags: OsString = builder - .default_cflags(target, CLang::Cxx) + .cc_handled_clags(target, CLang::Cxx) .into_iter() - .chain(builder.extra_cflags(target, GitRepo::Llvm, CLang::Cxx)) + .chain(builder.cc_unhandled_cflags(target, GitRepo::Llvm, CLang::Cxx)) .filter(|flag| { !suppressed_compiler_flag_prefixes .iter() diff --git a/src/bootstrap/src/core/build_steps/test.rs b/src/bootstrap/src/core/build_steps/test.rs index a5cad5b8414aa..b537c8c6cd98e 100644 --- a/src/bootstrap/src/core/build_steps/test.rs +++ b/src/bootstrap/src/core/build_steps/test.rs @@ -2005,10 +2005,10 @@ NOTE: if you're sure you want to do this, please open an issue as to why. In the // Only pass correct values for these flags for the `run-make` suite as it // requires that a C++ compiler was configured which isn't always the case. if !builder.config.dry_run() && mode == "run-make" { - let mut cflags = builder.default_cflags(target, CLang::C); - cflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::C)); - let mut cxxflags = builder.default_cflags(target, CLang::Cxx); - cxxflags.extend(builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); + let mut cflags = builder.cc_handled_clags(target, CLang::C); + cflags.extend(builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C)); + let mut cxxflags = builder.cc_handled_clags(target, CLang::Cxx); + cxxflags.extend(builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::Cxx)); cmd.arg("--cc") .arg(builder.cc(target)) .arg("--cxx") diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index fb8ef2b57be60..d8d7877f34e8b 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -317,7 +317,7 @@ impl Cargo { let cc = ccacheify(&builder.cc(target)); self.command.env(format!("CC_{triple_underscored}"), &cc); - let cflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::C).join(" "); + let cflags = builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C).join(" "); self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags); if let Some(ar) = builder.ar(target) { @@ -329,7 +329,8 @@ impl Cargo { if let Ok(cxx) = builder.cxx(target) { let cxx = ccacheify(&cxx); - let cxxflags = builder.extra_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); + let cxxflags = + builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); self.command .env(format!("CXX_{triple_underscored}"), &cxx) .env(format!("CXXFLAGS_{triple_underscored}"), cxxflags); diff --git a/src/bootstrap/src/lib.rs b/src/bootstrap/src/lib.rs index 1ce868316623a..e04651c298c56 100644 --- a/src/bootstrap/src/lib.rs +++ b/src/bootstrap/src/lib.rs @@ -1142,7 +1142,7 @@ Executed at: {executed_at}"#, /// Returns C flags that `cc-rs` thinks should be enabled for the /// specified target by default. - fn default_cflags(&self, target: TargetSelection, c: CLang) -> Vec { + fn cc_handled_clags(&self, target: TargetSelection, c: CLang) -> Vec { if self.config.dry_run() { return Vec::new(); } @@ -1161,7 +1161,12 @@ Executed at: {executed_at}"#, } /// Returns extra C flags that `cc-rs` doesn't handle. - fn extra_cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec { + fn cc_unhandled_cflags( + &self, + target: TargetSelection, + which: GitRepo, + c: CLang, + ) -> Vec { let mut base = Vec::new(); // If we're compiling C++ on macOS then we add a flag indicating that diff --git a/src/bootstrap/src/utils/cc_detect.rs b/src/bootstrap/src/utils/cc_detect.rs index 33b490e5ca1b2..f6afd50afcef0 100644 --- a/src/bootstrap/src/utils/cc_detect.rs +++ b/src/bootstrap/src/utils/cc_detect.rs @@ -142,8 +142,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { }; build.cc.borrow_mut().insert(target, compiler.clone()); - let mut cflags = build.default_cflags(target, CLang::C); - cflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::C)); + let mut cflags = build.cc_handled_clags(target, CLang::C); + cflags.extend(build.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C)); // If we use llvm-libunwind, we will need a C++ compiler as well for all targets // We'll need one anyways if the target triple is also a host triple @@ -169,8 +169,8 @@ pub fn find_target(build: &Build, target: TargetSelection) { build.verbose(|| println!("CC_{} = {:?}", target.triple, build.cc(target))); build.verbose(|| println!("CFLAGS_{} = {cflags:?}", target.triple)); if let Ok(cxx) = build.cxx(target) { - let mut cxxflags = build.default_cflags(target, CLang::Cxx); - cxxflags.extend(build.extra_cflags(target, GitRepo::Rustc, CLang::Cxx)); + let mut cxxflags = build.cc_handled_clags(target, CLang::Cxx); + cxxflags.extend(build.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::Cxx)); build.verbose(|| println!("CXX_{} = {cxx:?}", target.triple)); build.verbose(|| println!("CXXFLAGS_{} = {cxxflags:?}", target.triple)); } From ea68a47103c72c17f894510d4ddc5d309369c272 Mon Sep 17 00:00:00 2001 From: Mads Marquart Date: Mon, 10 Feb 2025 10:18:02 +0100 Subject: [PATCH 6/6] bootstrap: Don't overwrite CFLAGS_* set in the environment --- src/bootstrap/src/core/builder/cargo.rs | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/src/bootstrap/src/core/builder/cargo.rs b/src/bootstrap/src/core/builder/cargo.rs index d8d7877f34e8b..c88b9b8101496 100644 --- a/src/bootstrap/src/core/builder/cargo.rs +++ b/src/bootstrap/src/core/builder/cargo.rs @@ -317,8 +317,15 @@ impl Cargo { let cc = ccacheify(&builder.cc(target)); self.command.env(format!("CC_{triple_underscored}"), &cc); - let cflags = builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C).join(" "); - self.command.env(format!("CFLAGS_{triple_underscored}"), &cflags); + // Extend `CXXFLAGS_$TARGET` with our extra flags. + let env = format!("CFLAGS_{triple_underscored}"); + let mut cflags = + builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::C).join(" "); + if let Ok(var) = std::env::var(&env) { + cflags.push(' '); + cflags.push_str(&var); + } + self.command.env(env, &cflags); if let Some(ar) = builder.ar(target) { let ranlib = format!("{} s", ar.display()); @@ -329,11 +336,17 @@ impl Cargo { if let Ok(cxx) = builder.cxx(target) { let cxx = ccacheify(&cxx); - let cxxflags = + self.command.env(format!("CXX_{triple_underscored}"), &cxx); + + // Extend `CXXFLAGS_$TARGET` with our extra flags. + let env = format!("CXXFLAGS_{triple_underscored}"); + let mut cxxflags = builder.cc_unhandled_cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "); - self.command - .env(format!("CXX_{triple_underscored}"), &cxx) - .env(format!("CXXFLAGS_{triple_underscored}"), cxxflags); + if let Ok(var) = std::env::var(&env) { + cxxflags.push(' '); + cxxflags.push_str(&var); + } + self.command.env(&env, cxxflags); } }