Use CARGO_ENCODED_RUSTFLAGS to support paths with spaces

bjorn3 · bjorn3 · commit 68f7b826be24 · 2023-08-02T11:06:51.000+02:00
Fixes #1391
diff --git a/build_system/build_backend.rs b/build_system/build_backend.rs
@@ -1,8 +1,8 @@
-use std::env;
 use std::path::PathBuf;
 
 use crate::path::{Dirs, RelPath};
 use crate::rustc_info::get_file_name;
+use crate::shared_utils::{rustflags_from_env, rustflags_to_cmd_env};
 use crate::utils::{is_ci, is_ci_opt, maybe_incremental, CargoProject, Compiler, LogGroup};
 
 pub(crate) static CG_CLIF: CargoProject = CargoProject::new(&RelPath::SOURCE, "cg_clif");
@@ -18,11 +18,11 @@ pub(crate) fn build_backend(
     let mut cmd = CG_CLIF.build(&bootstrap_host_compiler, dirs);
     maybe_incremental(&mut cmd);
 
-    let mut rustflags = env::var("RUSTFLAGS").unwrap_or_default();
+    let mut rustflags = rustflags_from_env("RUSTFLAGS");
 
     if is_ci() {
         // Deny warnings on CI
-        rustflags += " -Dwarnings";
+        rustflags.push("-Dwarnings".to_owned());
 
         if !is_ci_opt() {
             cmd.env("CARGO_PROFILE_RELEASE_DEBUG_ASSERTIONS", "true");
@@ -42,7 +42,7 @@ pub(crate) fn build_backend(
         _ => unreachable!(),
     }
 
-    cmd.env("RUSTFLAGS", rustflags);
+    rustflags_to_cmd_env(&mut cmd, "RUSTFLAGS", &rustflags);
 
     eprintln!("[BUILD] rustc_codegen_cranelift");
     crate::utils::spawn_and_wait(cmd);
diff --git a/build_system/build_sysroot.rs b/build_system/build_sysroot.rs
@@ -128,8 +128,8 @@ pub(crate) fn build_sysroot(
             cargo: bootstrap_host_compiler.cargo.clone(),
             rustc: rustc_clif.clone(),
             rustdoc: rustdoc_clif.clone(),
-            rustflags: String::new(),
-            rustdocflags: String::new(),
+            rustflags: vec![],
+            rustdocflags: vec![],
             triple: target_triple,
             runner: vec![],
         }
@@ -241,25 +241,25 @@ fn build_clif_sysroot_for_triple(
     }
 
     // Build sysroot
-    let mut rustflags = " -Zforce-unstable-if-unmarked -Cpanic=abort".to_string();
+    let mut rustflags = vec!["-Zforce-unstable-if-unmarked".to_owned(), "-Cpanic=abort".to_owned()];
     match cg_clif_dylib_path {
         CodegenBackend::Local(path) => {
-            rustflags.push_str(&format!(" -Zcodegen-backend={}", path.to_str().unwrap()));
+            rustflags.push(format!("-Zcodegen-backend={}", path.to_str().unwrap()));
         }
         CodegenBackend::Builtin(name) => {
-            rustflags.push_str(&format!(" -Zcodegen-backend={name}"));
+            rustflags.push(format!("-Zcodegen-backend={name}"));
         }
     };
     // Necessary for MinGW to find rsbegin.o and rsend.o
-    rustflags
-        .push_str(&format!(" --sysroot {}", RTSTARTUP_SYSROOT.to_path(dirs).to_str().unwrap()));
+    rustflags.push("--sysroot".to_owned());
+    rustflags.push(RTSTARTUP_SYSROOT.to_path(dirs).to_str().unwrap().to_owned());
     if channel == "release" {
         // Incremental compilation by default disables mir inlining. This leads to both a decent
         // compile perf and a significant runtime perf regression. As such forcefully enable mir
         // inlining.
-        rustflags.push_str(" -Zinline-mir");
+        rustflags.push("-Zinline-mir".to_owned());
     }
-    compiler.rustflags += &rustflags;
+    compiler.rustflags.extend(rustflags);
     let mut build_cmd = STANDARD_LIBRARY.build(&compiler, dirs);
     maybe_incremental(&mut build_cmd);
     if channel == "release" {
diff --git a/build_system/main.rs b/build_system/main.rs
@@ -16,6 +16,7 @@ mod config;
 mod path;
 mod prepare;
 mod rustc_info;
+mod shared_utils;
 mod tests;
 mod utils;
 
@@ -169,8 +170,8 @@ fn main() {
             cargo,
             rustc,
             rustdoc,
-            rustflags: String::new(),
-            rustdocflags: String::new(),
+            rustflags: vec![],
+            rustdocflags: vec![],
             triple,
             runner: vec![],
         }
diff --git a/build_system/shared_utils.rs b/build_system/shared_utils.rs
@@ -0,0 +1,26 @@
+// This file is used by both the build system as well as cargo-clif.rs
+
+// Adapted from https://github.com/rust-lang/cargo/blob/6dc1deaddf62c7748c9097c7ea88e9ec77ff1a1a/src/cargo/core/compiler/build_context/target_info.rs#L750-L77
+pub(crate) fn rustflags_from_env(kind: &str) -> Vec<String> {
+    // First try CARGO_ENCODED_RUSTFLAGS from the environment.
+    // Prefer this over RUSTFLAGS since it's less prone to encoding errors.
+    if let Ok(a) = std::env::var(format!("CARGO_ENCODED_{}", kind)) {
+        if a.is_empty() {
+            return Vec::new();
+        }
+        return a.split('\x1f').map(str::to_string).collect();
+    }
+
+    // Then try RUSTFLAGS from the environment
+    if let Ok(a) = std::env::var(kind) {
+        let args = a.split(' ').map(str::trim).filter(|s| !s.is_empty()).map(str::to_string);
+        return args.collect();
+    }
+
+    // No rustflags to be collected from the environment
+    Vec::new()
+}
+
+pub(crate) fn rustflags_to_cmd_env(cmd: &mut std::process::Command, kind: &str, flags: &[String]) {
+    cmd.env(format!("CARGO_ENCODED_{}", kind), flags.join("\x1f"));
+}
diff --git a/build_system/tests.rs b/build_system/tests.rs
@@ -1,4 +1,3 @@
-use std::env;
 use std::ffi::OsStr;
 use std::fs;
 use std::path::PathBuf;
@@ -9,6 +8,7 @@ use crate::config;
 use crate::path::{Dirs, RelPath};
 use crate::prepare::{apply_patches, GitRepo};
 use crate::rustc_info::get_default_sysroot;
+use crate::shared_utils::rustflags_from_env;
 use crate::utils::{spawn_and_wait, spawn_and_wait_with_input, CargoProject, Compiler, LogGroup};
 use crate::{CodegenBackend, SysrootKind};
 
@@ -307,7 +307,7 @@ pub(crate) fn run_tests(
         );
         // Rust's build system denies a couple of lints that trigger on several of the test
         // projects. Changing the code to fix them is not worth it, so just silence all lints.
-        target_compiler.rustflags += " --cap-lints=allow";
+        target_compiler.rustflags.push("--cap-lints=allow".to_owned());
 
         let runner = TestRunner::new(
             dirs.clone(),
@@ -351,18 +351,15 @@ impl<'a> TestRunner<'a> {
         is_native: bool,
         stdlib_source: PathBuf,
     ) -> Self {
-        if let Ok(rustflags) = env::var("RUSTFLAGS") {
-            target_compiler.rustflags.push(' ');
-            target_compiler.rustflags.push_str(&rustflags);
-        }
-        if let Ok(rustdocflags) = env::var("RUSTDOCFLAGS") {
-            target_compiler.rustdocflags.push(' ');
-            target_compiler.rustdocflags.push_str(&rustdocflags);
-        }
+        target_compiler.rustflags.extend(rustflags_from_env("RUSTFLAGS"));
+        target_compiler.rustdocflags.extend(rustflags_from_env("RUSTDOCFLAGS"));
 
         // FIXME fix `#[linkage = "extern_weak"]` without this
         if target_compiler.triple.contains("darwin") {
-            target_compiler.rustflags.push_str(" -Clink-arg=-undefined -Clink-arg=dynamic_lookup");
+            target_compiler.rustflags.extend([
+                "-Clink-arg=-undefined".to_owned(),
+                "-Clink-arg=dynamic_lookup".to_owned(),
+            ]);
         }
 
         let jit_supported = use_unstable_features
@@ -471,7 +468,7 @@ impl<'a> TestRunner<'a> {
         S: AsRef<OsStr>,
     {
         let mut cmd = Command::new(&self.target_compiler.rustc);
-        cmd.args(self.target_compiler.rustflags.split_whitespace());
+        cmd.args(&self.target_compiler.rustflags);
         cmd.arg("-L");
         cmd.arg(format!("crate={}", BUILD_EXAMPLE_OUT_DIR.to_path(&self.dirs).display()));
         cmd.arg("--out-dir");
diff --git a/build_system/utils.rs b/build_system/utils.rs
@@ -6,14 +6,15 @@ use std::process::{self, Command, Stdio};
 use std::sync::atomic::{AtomicBool, Ordering};
 
 use crate::path::{Dirs, RelPath};
+use crate::shared_utils::rustflags_to_cmd_env;
 
 #[derive(Clone, Debug)]
 pub(crate) struct Compiler {
     pub(crate) cargo: PathBuf,
     pub(crate) rustc: PathBuf,
     pub(crate) rustdoc: PathBuf,
-    pub(crate) rustflags: String,
-    pub(crate) rustdocflags: String,
+    pub(crate) rustflags: Vec<String>,
+    pub(crate) rustdocflags: Vec<String>,
     pub(crate) triple: String,
     pub(crate) runner: Vec<String>,
 }
@@ -23,8 +24,8 @@ impl Compiler {
         match self.triple.as_str() {
             "aarch64-unknown-linux-gnu" => {
                 // We are cross-compiling for aarch64. Use the correct linker and run tests in qemu.
-                self.rustflags += " -Clinker=aarch64-linux-gnu-gcc";
-                self.rustdocflags += " -Clinker=aarch64-linux-gnu-gcc";
+                self.rustflags.push("-Clinker=aarch64-linux-gnu-gcc".to_owned());
+                self.rustdocflags.push("-Clinker=aarch64-linux-gnu-gcc".to_owned());
                 self.runner = vec![
                     "qemu-aarch64".to_owned(),
                     "-L".to_owned(),
@@ -33,8 +34,8 @@ impl Compiler {
             }
             "s390x-unknown-linux-gnu" => {
                 // We are cross-compiling for s390x. Use the correct linker and run tests in qemu.
-                self.rustflags += " -Clinker=s390x-linux-gnu-gcc";
-                self.rustdocflags += " -Clinker=s390x-linux-gnu-gcc";
+                self.rustflags.push("-Clinker=s390x-linux-gnu-gcc".to_owned());
+                self.rustdocflags.push("-Clinker=s390x-linux-gnu-gcc".to_owned());
                 self.runner = vec![
                     "qemu-s390x".to_owned(),
                     "-L".to_owned(),
@@ -100,8 +101,8 @@ impl CargoProject {
 
         cmd.env("RUSTC", &compiler.rustc);
         cmd.env("RUSTDOC", &compiler.rustdoc);
-        cmd.env("RUSTFLAGS", &compiler.rustflags);
-        cmd.env("RUSTDOCFLAGS", &compiler.rustdocflags);
+        rustflags_to_cmd_env(&mut cmd, "RUSTFLAGS", &compiler.rustflags);
+        rustflags_to_cmd_env(&mut cmd, "RUSTDOCFLAGS", &compiler.rustdocflags);
         if !compiler.runner.is_empty() {
             cmd.env(
                 format!("CARGO_TARGET_{}_RUNNER", compiler.triple.to_uppercase().replace('-', "_")),
diff --git a/scripts/cargo-clif.rs b/scripts/cargo-clif.rs
@@ -3,34 +3,28 @@ use std::env;
 use std::os::unix::process::CommandExt;
 use std::process::Command;
 
+include!("../build_system/shared_utils.rs");
+
 fn main() {
     let current_exe = env::current_exe().unwrap();
     let mut sysroot = current_exe.parent().unwrap();
     if sysroot.file_name().unwrap().to_str().unwrap() == "bin" {
         sysroot = sysroot.parent().unwrap();
     }
 
-    let mut rustflags = String::new();
-    rustflags.push_str(" -Cpanic=abort -Zpanic-abort-tests -Zcodegen-backend=");
+    let mut rustflags = vec!["-Cpanic=abort".to_owned(), "-Zpanic-abort-tests".to_owned()];
     if let Some(name) = option_env!("BUILTIN_BACKEND") {
-        rustflags.push_str(name);
+        rustflags.push(format!("-Zcodegen-backend={name}"));
     } else {
-        rustflags.push_str(
-            sysroot
-                .join(if cfg!(windows) { "bin" } else { "lib" })
-                .join(
-                    env::consts::DLL_PREFIX.to_string()
-                        + "rustc_codegen_cranelift"
-                        + env::consts::DLL_SUFFIX,
-                )
-                .to_str()
-                .unwrap(),
+        let dylib = sysroot.join(if cfg!(windows) { "bin" } else { "lib" }).join(
+            env::consts::DLL_PREFIX.to_string()
+                + "rustc_codegen_cranelift"
+                + env::consts::DLL_SUFFIX,
         );
+        rustflags.push(format!("-Zcodegen-backend={}", dylib.to_str().unwrap()));
     }
-    rustflags.push_str(" --sysroot ");
-    rustflags.push_str(sysroot.to_str().unwrap());
-    env::set_var("RUSTFLAGS", env::var("RUSTFLAGS").unwrap_or(String::new()) + &rustflags);
-    env::set_var("RUSTDOCFLAGS", env::var("RUSTDOCFLAGS").unwrap_or(String::new()) + &rustflags);
+    rustflags.push("--sysroot".to_owned());
+    rustflags.push(sysroot.to_str().unwrap().to_owned());
 
     let cargo = if let Some(cargo) = option_env!("CARGO") {
         cargo
@@ -49,10 +43,7 @@ fn main() {
 
     let args: Vec<_> = match args.get(0).map(|arg| &**arg) {
         Some("jit") => {
-            env::set_var(
-                "RUSTFLAGS",
-                env::var("RUSTFLAGS").unwrap_or(String::new()) + " -Cprefer-dynamic",
-            );
+            rustflags.push("-Cprefer-dynamic".to_owned());
             args.remove(0);
             IntoIterator::into_iter(["rustc".to_string()])
                 .chain(args)
@@ -64,10 +55,7 @@ fn main() {
                 .collect()
         }
         Some("lazy-jit") => {
-            env::set_var(
-                "RUSTFLAGS",
-                env::var("RUSTFLAGS").unwrap_or(String::new()) + " -Cprefer-dynamic",
-            );
+            rustflags.push("-Cprefer-dynamic".to_owned());
             args.remove(0);
             IntoIterator::into_iter(["rustc".to_string()])
                 .chain(args)
@@ -81,11 +69,28 @@ fn main() {
         _ => args,
     };
 
+    let mut cmd = Command::new(cargo);
+    cmd.args(args);
+    rustflags_to_cmd_env(
+        &mut cmd,
+        "RUSTFLAGS",
+        &rustflags_from_env("RUSTFLAGS")
+            .into_iter()
+            .chain(rustflags.iter().map(|flag| flag.clone()))
+            .collect::<Vec<_>>(),
+    );
+    rustflags_to_cmd_env(
+        &mut cmd,
+        "RUSTDOCFLAGS",
+        &rustflags_from_env("RUSTDOCFLAGS")
+            .into_iter()
+            .chain(rustflags.iter().map(|flag| flag.clone()))
+            .collect::<Vec<_>>(),
+    );
+
     #[cfg(unix)]
-    panic!("Failed to spawn cargo: {}", Command::new(cargo).args(args).exec());
+    panic!("Failed to spawn cargo: {}", cmd.exec());
 
     #[cfg(not(unix))]
-    std::process::exit(
-        Command::new(cargo).args(args).spawn().unwrap().wait().unwrap().code().unwrap_or(1),
-    );
+    std::process::exit(cmd.spawn().unwrap().wait().unwrap().code().unwrap_or(1));
 }
