Rollup merge of rust-lang#94466 - jonhoo:bootstrap-proper-env-flags, r=Mark-Simulacrum

bootstrap: correct reading of flags for llvm

First, this reverts the `CFLAGS`/`CXXFLAGS` of rust-lang#93918. Those flags are
already read by `cc` and populated into `Build` earlier on in the
process. We shouldn't be overriding that based on `CFLAGS`, since `cc`
also respects overrides like `CFLAGS_{TARGET}` and `HOST_CFLAGS`, which
we want to take into account.

Second, this adds the same capability to specify target-specific
versions of `LDFLAGS` as we have through `cc` for the `C*` flags:
https://github.com/alexcrichton/cc-rs#external-configuration-via-environment-variables

Author: Dylan-DPC

src/bootstrap/builder.rs

@@ -26,7 +26,7 @@ use crate::run;
 use crate::test;
 use crate::tool::{self, SourceType};
 use crate::util::{self, add_dylib_path, add_link_lib_path, exe, libdir};
-use crate::{Build, DocTests, GitRepo, Mode};
+use crate::{Build, CLang, DocTests, GitRepo, Mode};
 
 pub use crate::Compiler;
 // FIXME: replace with std::lazy after it gets stabilized and reaches beta
@@ -1511,7 +1511,7 @@ impl<'a> Builder<'a> {
             let cc = ccacheify(&self.cc(target));
             cargo.env(format!("CC_{}", target.triple), &cc);
 
-            let cflags = self.cflags(target, GitRepo::Rustc).join(" ");
+            let cflags = self.cflags(target, GitRepo::Rustc, CLang::C).join(" ");
             cargo.env(format!("CFLAGS_{}", target.triple), &cflags);
 
             if let Some(ar) = self.ar(target) {
@@ -1523,9 +1523,10 @@ impl<'a> Builder<'a> {
 
             if let Ok(cxx) = self.cxx(target) {
                 let cxx = ccacheify(&cxx);
+                let cxxflags = self.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" ");
                 cargo
                     .env(format!("CXX_{}", target.triple), &cxx)
-                    .env(format!("CXXFLAGS_{}", target.triple), cflags);
+                    .env(format!("CXXFLAGS_{}", target.triple), cxxflags);
             }
         }
 

src/bootstrap/cc_detect.rs

@@ -29,7 +29,7 @@ use std::{env, iter};
 use build_helper::output;
 
 use crate::config::{Target, TargetSelection};
-use crate::{Build, GitRepo};
+use crate::{Build, CLang, GitRepo};
 
 // The `cc` crate doesn't provide a way to obtain a path to the detected archiver,
 // so use some simplified logic here. First we respect the environment variable `AR`, then
@@ -109,7 +109,7 @@ pub fn find(build: &mut Build) {
         };
 
         build.cc.insert(target, compiler.clone());
-        let cflags = build.cflags(target, GitRepo::Rustc);
+        let cflags = build.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
@@ -142,8 +142,9 @@ pub fn find(build: &mut Build) {
         build.verbose(&format!("CC_{} = {:?}", &target.triple, build.cc(target)));
         build.verbose(&format!("CFLAGS_{} = {:?}", &target.triple, cflags));
         if let Ok(cxx) = build.cxx(target) {
+            let cxxflags = build.cflags(target, GitRepo::Rustc, CLang::Cxx);
             build.verbose(&format!("CXX_{} = {:?}", &target.triple, cxx));
-            build.verbose(&format!("CXXFLAGS_{} = {:?}", &target.triple, cflags));
+            build.verbose(&format!("CXXFLAGS_{} = {:?}", &target.triple, cxxflags));
         }
         if let Some(ar) = ar {
             build.verbose(&format!("AR_{} = {:?}", &target.triple, ar));

src/bootstrap/compile.rs

@@ -28,7 +28,7 @@ use crate::native;
 use crate::tool::SourceType;
 use crate::util::{exe, is_debug_info, is_dylib, symlink_dir};
 use crate::LLVM_TOOLS;
-use crate::{Compiler, DependencyType, GitRepo, Mode};
+use crate::{CLang, Compiler, DependencyType, GitRepo, Mode};
 
 #[derive(Debug, PartialOrd, Ord, Copy, Clone, PartialEq, Eq, Hash)]
 pub struct Std {
@@ -249,7 +249,7 @@ fn copy_self_contained_objects(
         }
     } else if target.contains("windows-gnu") {
         for obj in ["crt2.o", "dllcrt2.o"].iter() {
-            let src = compiler_file(builder, builder.cc(target), target, obj);
+            let src = compiler_file(builder, builder.cc(target), target, CLang::C, obj);
             let target = libdir_self_contained.join(obj);
             builder.copy(&src, &target);
             target_deps.push((target, DependencyType::TargetSelfContained));
@@ -727,7 +727,13 @@ pub fn rustc_cargo_env(builder: &Builder<'_>, cargo: &mut Cargo, target: TargetS
             && !target.contains("msvc")
             && !target.contains("apple")
         {
-            let file = compiler_file(builder, builder.cxx(target).unwrap(), target, "libstdc++.a");
+            let file = compiler_file(
+                builder,
+                builder.cxx(target).unwrap(),
+                target,
+                CLang::Cxx,
+                "libstdc++.a",
+            );
             cargo.env("LLVM_STATIC_STDCPP", file);
         }
         if builder.config.llvm_link_shared {
@@ -948,10 +954,11 @@ pub fn compiler_file(
     builder: &Builder<'_>,
     compiler: &Path,
     target: TargetSelection,
+    c: CLang,
     file: &str,
 ) -> PathBuf {
     let mut cmd = Command::new(compiler);
-    cmd.args(builder.cflags(target, GitRepo::Rustc));
+    cmd.args(builder.cflags(target, GitRepo::Rustc, c));
     cmd.arg(format!("-print-file-name={}", file));
     let out = output(&mut cmd);
     PathBuf::from(out.trim())

src/bootstrap/lib.rs

@@ -339,6 +339,11 @@ impl Mode {
     }
 }
 
+pub enum CLang {
+    C,
+    Cxx,
+}
+
 impl Build {
     /// Creates a new set of build configuration from the `flags` on the command
     /// line and the filesystem `config`.
@@ -941,10 +946,15 @@ impl Build {
 
     /// Returns a list of flags to pass to the C compiler for the target
     /// specified.
-    fn cflags(&self, target: TargetSelection, which: GitRepo) -> Vec<String> {
+    fn cflags(&self, target: TargetSelection, which: GitRepo, c: CLang) -> Vec<String> {
+        let base = match c {
+            CLang::C => &self.cc[&target],
+            CLang::Cxx => &self.cxx[&target],
+        };
+
         // 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 = self.cc[&target]
+        let mut base = base
             .args()
             .iter()
             .map(|s| s.to_string_lossy().into_owned())

src/bootstrap/native.rs

@@ -21,7 +21,7 @@ use build_helper::{output, t};
 use crate::builder::{Builder, RunConfig, ShouldRun, Step};
 use crate::config::TargetSelection;
 use crate::util::{self, exe};
-use crate::GitRepo;
+use crate::{CLang, GitRepo};
 use build_helper::up_to_date;
 
 pub struct Meta {
@@ -529,7 +529,7 @@ fn configure_cmake(
     }
 
     cfg.build_arg("-j").build_arg(builder.jobs().to_string());
-    let mut cflags: OsString = builder.cflags(target, GitRepo::Llvm).join(" ").into();
+    let mut cflags: OsString = builder.cflags(target, GitRepo::Llvm, CLang::C).join(" ").into();
     if let Some(ref s) = builder.config.llvm_cflags {
         cflags.push(" ");
         cflags.push(s);
@@ -545,23 +545,15 @@ fn configure_cmake(
     if builder.config.llvm_clang_cl.is_some() {
         cflags.push(&format!(" --target={}", target));
     }
-    if let Some(flags) = env::var_os("CFLAGS") {
-        cflags.push(" ");
-        cflags.push(flags);
-    }
     cfg.define("CMAKE_C_FLAGS", cflags);
-    let mut cxxflags: OsString = builder.cflags(target, GitRepo::Llvm).join(" ").into();
+    let mut cxxflags: OsString = builder.cflags(target, GitRepo::Llvm, CLang::Cxx).join(" ").into();
     if let Some(ref s) = builder.config.llvm_cxxflags {
         cxxflags.push(" ");
         cxxflags.push(s);
     }
     if builder.config.llvm_clang_cl.is_some() {
         cxxflags.push(&format!(" --target={}", target));
     }
-    if let Some(flags) = env::var_os("CXXFLAGS") {
-        cxxflags.push(" ");
-        cxxflags.push(flags);
-    }
     cfg.define("CMAKE_CXX_FLAGS", cxxflags);
     if let Some(ar) = builder.ar(target) {
         if ar.is_absolute() {
@@ -583,7 +575,7 @@ fn configure_cmake(
         ldflags.push_all(flags);
     }
 
-    if let Some(flags) = env::var_os("LDFLAGS") {
+    if let Some(flags) = get_var("LDFLAGS", &builder.config.build.triple, &target.triple) {
         ldflags.push_all(&flags);
     }
 
@@ -596,6 +588,16 @@ fn configure_cmake(
     }
 }
 
+// Adapted from https://github.com/alexcrichton/cc-rs/blob/fba7feded71ee4f63cfe885673ead6d7b4f2f454/src/lib.rs#L2347-L2365
+fn get_var(var_base: &str, host: &str, target: &str) -> Option<OsString> {
+    let kind = if host == target { "HOST" } else { "TARGET" };
+    let target_u = target.replace("-", "_");
+    env::var_os(&format!("{}_{}", var_base, target))
+        .or_else(|| env::var_os(&format!("{}_{}", var_base, target_u)))
+        .or_else(|| env::var_os(&format!("{}_{}", kind, var_base)))
+        .or_else(|| env::var_os(var_base))
+}
+
 #[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
 pub struct Lld {
     pub target: TargetSelection,

src/bootstrap/test.rs

@@ -24,7 +24,7 @@ use crate::tool::{self, SourceType, Tool};
 use crate::toolstate::ToolState;
 use crate::util::{self, add_link_lib_path, dylib_path, dylib_path_var};
 use crate::Crate as CargoCrate;
-use crate::{envify, DocTests, GitRepo, Mode};
+use crate::{envify, CLang, DocTests, GitRepo, Mode};
 
 const ADB_TEST_DIR: &str = "/data/tmp/work";
 
@@ -1509,7 +1509,9 @@ note: if you're sure you want to do this, please open an issue as to why. In the
                 .arg("--cxx")
                 .arg(builder.cxx(target).unwrap())
                 .arg("--cflags")
-                .arg(builder.cflags(target, GitRepo::Rustc).join(" "));
+                .arg(builder.cflags(target, GitRepo::Rustc, CLang::C).join(" "))
+                .arg("--cxxflags")
+                .arg(builder.cflags(target, GitRepo::Rustc, CLang::Cxx).join(" "));
             copts_passed = true;
             if let Some(ar) = builder.ar(target) {
                 cmd.arg("--ar").arg(ar);
@@ -1520,7 +1522,14 @@ note: if you're sure you want to do this, please open an issue as to why. In the
             cmd.arg("--llvm-components").arg("");
         }
         if !copts_passed {
-            cmd.arg("--cc").arg("").arg("--cxx").arg("").arg("--cflags").arg("");
+            cmd.arg("--cc")
+                .arg("")
+                .arg("--cxx")
+                .arg("")
+                .arg("--cflags")
+                .arg("")
+                .arg("--cxxflags")
+                .arg("");
         }
 
         if builder.remote_tested(target) {

src/tools/compiletest/src/common.rs

@@ -357,6 +357,7 @@ pub struct Config {
     pub cc: String,
     pub cxx: String,
     pub cflags: String,
+    pub cxxflags: String,
     pub ar: String,
     pub linker: Option<String>,
     pub llvm_components: String,

src/tools/compiletest/src/header/tests.rs

@@ -52,6 +52,7 @@ fn config() -> Config {
         "--cc=c",
         "--cxx=c++",
         "--cflags=",
+        "--cxxflags=",
         "--llvm-components=",
         "--android-cross-path=",
         "--target=x86_64-unknown-linux-gnu",

src/tools/compiletest/src/main.rs

@@ -126,6 +126,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
         .reqopt("", "cc", "path to a C compiler", "PATH")
         .reqopt("", "cxx", "path to a C++ compiler", "PATH")
         .reqopt("", "cflags", "flags for the C compiler", "FLAGS")
+        .reqopt("", "cxxflags", "flags for the CXX compiler", "FLAGS")
         .optopt("", "ar", "path to an archiver", "PATH")
         .optopt("", "linker", "path to a linker", "PATH")
         .reqopt("", "llvm-components", "list of LLVM components built in", "LIST")
@@ -288,6 +289,7 @@ pub fn parse_config(args: Vec<String>) -> Config {
         cc: matches.opt_str("cc").unwrap(),
         cxx: matches.opt_str("cxx").unwrap(),
         cflags: matches.opt_str("cflags").unwrap(),
+        cxxflags: matches.opt_str("cxxflags").unwrap(),
         ar: matches.opt_str("ar").unwrap_or_else(|| String::from("ar")),
         linker: matches.opt_str("linker"),
         llvm_components: matches.opt_str("llvm-components").unwrap(),

src/tools/compiletest/src/runtest.rs

@@ -2919,15 +2919,22 @@ impl<'test> TestCx<'test> {
                 .map(|s| s.replace("/", "-"))
                 .collect::<Vec<_>>()
                 .join(" ");
+            let cxxflags = self
+                .config
+                .cxxflags
+                .split(' ')
+                .map(|s| s.replace("/", "-"))
+                .collect::<Vec<_>>()
+                .join(" ");
 
             cmd.env("IS_MSVC", "1")
                 .env("IS_WINDOWS", "1")
                 .env("MSVC_LIB", format!("'{}' -nologo", lib.display()))
                 .env("CC", format!("'{}' {}", self.config.cc, cflags))
-                .env("CXX", format!("'{}'", &self.config.cxx));
+                .env("CXX", format!("'{}' {}", &self.config.cxx, cxxflags));
         } else {
             cmd.env("CC", format!("{} {}", self.config.cc, self.config.cflags))
-                .env("CXX", format!("{} {}", self.config.cxx, self.config.cflags))
+                .env("CXX", format!("{} {}", self.config.cxx, self.config.cxxflags))
                 .env("AR", &self.config.ar);
 
             if self.config.target.contains("windows") {