Fix cargo-gpu in build script failing when called by Miri or Clippy

crates/spirv-builder/Cargo.toml

@@ -51,3 +51,4 @@ cargo_metadata = "0.19.2"
 notify = { version = "7.0", optional = true }
 # Pinning clap, as newer versions have raised min rustc version without being marked a breaking change
 clap = { version = "=4.5.37", optional = true, features = ["derive"] }
+log = { version = "0.4.22", features = ["std"] }

crates/spirv-builder/src/cargo_cmd.rs

@@ -0,0 +1,157 @@
+use std::collections::HashSet;
+use std::env;
+use std::ffi::{OsStr, OsString};
+use std::fmt::{Display, Formatter};
+use std::ops::{Deref, DerefMut};
+use std::process::Command;
+
+/// Filters the various env vars that a `cargo` child process would receive and reports back
+/// what was inherited and what was removed. By default, removes all env vars that influences
+/// the cargo invocations of a spirv-builder's build or cargo-gpu's install action.
+pub struct CargoCmd {
+    cargo: Command,
+    vars_os: Vec<(OsString, OsString)>,
+    removed: HashSet<OsString>,
+}
+
+impl CargoCmd {
+    pub fn new() -> Self {
+        let mut cargo = CargoCmd::new_no_filtering();
+
+        // Clear Cargo environment variables that we don't want to leak into the
+        // inner invocation of Cargo (because e.g. build scripts might read them),
+        // before we set any of our own below.
+        cargo.retain_vars_os(|(key, _)| {
+            !key.to_str()
+                .is_some_and(|s| s.starts_with("CARGO_FEATURES_") || s.starts_with("CARGO_CFG_"))
+        });
+
+        // NOTE(eddyb) Cargo caches some information it got from `rustc` in
+        // `.rustc_info.json`, and assumes it only depends on the `rustc` binary,
+        // but in our case, `rustc_codegen_spirv` changes are also relevant,
+        // so we turn off that caching with an env var, just to avoid any issues.
+        cargo.env("CARGO_CACHE_RUSTC_INFO", "0");
+
+        // NOTE(firestar99) If you call SpirvBuilder in a build script, it will
+        // set `RUSTC` before calling it. And if we were to propagate it to our
+        // cargo invocation, it will take precedence over the `+toolchain` we
+        // previously set.
+        cargo.env_remove("RUSTC");
+
+        // NOTE(tuguzT) Used by Cargo to call executables of Clippy, Miri
+        // (and maybe other Cargo subcommands) instead of `rustc`
+        // which could affect its functionality and break the build process.
+        cargo.env_remove("RUSTC_WRAPPER");
+
+        // overwritten by spirv-builder anyway
+        cargo.env_remove("CARGO_ENCODED_RUSTFLAGS");
+
+        cargo
+            .env_remove("RUSTC")
+            .env_remove("RUSTC_WRAPPER")
+            .env_remove("RUSTC_WORKSPACE_WRAPPER")
+            .env_remove("RUSTFLAGS")
+            .env_remove("CARGO")
+            .env_remove("RUSTUP_TOOLCHAIN");
+
+        // ignore any externally supplied target dir
+        // spirv-builder: we overwrite it with `SpirvBuilder.target_dir_path` anyway
+        // cargo-gpu: we want to build it in our cache dir
+        cargo.env_remove("CARGO_TARGET_DIR");
+
+        cargo
+    }
+
+    pub fn new_no_filtering() -> Self {
+        Self {
+            cargo: Command::new("cargo"),
+            vars_os: env::vars_os().collect(),
+            removed: HashSet::default(),
+        }
+    }
+
+    pub fn retain_vars_os(&mut self, mut f: impl FnMut((&OsString, &OsString)) -> bool) {
+        for (key, value) in &self.vars_os {
+            if !f((key, value)) {
+                self.removed.insert(key.clone());
+                self.cargo.env_remove(key);
+            }
+        }
+    }
+
+    pub fn env_remove(&mut self, key: impl AsRef<OsStr>) -> &mut Self {
+        self.removed.insert(key.as_ref().to_os_string());
+        self.cargo.env_remove(key);
+        self
+    }
+
+    pub fn env(&mut self, key: impl AsRef<OsStr>, val: impl AsRef<OsStr>) -> &mut Self {
+        self.removed.remove(key.as_ref());
+        self.cargo.env(key, val);
+        self
+    }
+
+    pub fn env_var_report(&self) -> EnvVarReport {
+        let mut inherited = self.vars_os.clone();
+        inherited.retain(|(key, _)| !self.removed.contains(key));
+        EnvVarReport {
+            inherited,
+            removed: self.removed.clone(),
+        }
+    }
+}
+
+impl Default for CargoCmd {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+impl Display for CargoCmd {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        f.debug_struct("CargoCmd")
+            .field("cargo", &self.cargo)
+            .field("env_vars", &self.env_var_report())
+            .finish()
+    }
+}
+
+impl Deref for CargoCmd {
+    type Target = Command;
+
+    fn deref(&self) -> &Self::Target {
+        &self.cargo
+    }
+}
+
+impl DerefMut for CargoCmd {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.cargo
+    }
+}
+
+#[derive(Clone, Debug, Default)]
+pub struct EnvVarReport {
+    pub inherited: Vec<(OsString, OsString)>,
+    pub removed: HashSet<OsString>,
+}
+
+impl Display for EnvVarReport {
+    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
+        let removed = self
+            .removed
+            .iter()
+            .map(|key| format!("{}", key.to_string_lossy()))
+            .collect::<Vec<_>>();
+        let inherited = self
+            .inherited
+            .iter()
+            .map(|(key, value)| format!("{}: {}", key.to_string_lossy(), value.to_string_lossy()))
+            .collect::<Vec<_>>();
+
+        f.debug_struct("EnvVarReport")
+            .field("removed", &removed)
+            .field("inherited", &inherited)
+            .finish()
+    }
+}

crates/spirv-builder/src/lib.rs

@@ -71,6 +71,7 @@
 // #![allow()]
 #![doc = include_str!("../README.md")]
 
+pub mod cargo_cmd;
 mod depfile;
 #[cfg(feature = "watch")]
 mod watch;
@@ -961,7 +962,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
 
     let profile = if builder.release { "release" } else { "dev" };
 
-    let mut cargo = Command::new("cargo");
+    let mut cargo = cargo_cmd::CargoCmd::new();
     if let Some(toolchain) = &builder.toolchain_overwrite {
         cargo.arg(format!("+{toolchain}"));
     }
@@ -1014,30 +1015,6 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
 
     cargo.arg("--target-dir").arg(target_dir);
 
-    // Clear Cargo environment variables that we don't want to leak into the
-    // inner invocation of Cargo (because e.g. build scripts might read them),
-    // before we set any of our own below.
-    for (key, _) in env::vars_os() {
-        let remove = key
-            .to_str()
-            .is_some_and(|s| s.starts_with("CARGO_FEATURES_") || s.starts_with("CARGO_CFG_"));
-        if remove {
-            cargo.env_remove(key);
-        }
-    }
-
-    // NOTE(eddyb) Cargo caches some information it got from `rustc` in
-    // `.rustc_info.json`, and assumes it only depends on the `rustc` binary,
-    // but in our case, `rustc_codegen_spirv` changes are also relevant,
-    // so we turn off that caching with an env var, just to avoid any issues.
-    cargo.env("CARGO_CACHE_RUSTC_INFO", "0");
-
-    // NOTE(firestar99) If you call SpirvBuilder in a build script, it will
-    // set `RUSTC` before calling it. And if we were to propagate it to our
-    // cargo invocation, it will take precedence over the `+toolchain` we
-    // previously set.
-    cargo.env_remove("RUSTC");
-
     // NOTE(eddyb) this used to be just `RUSTFLAGS` but at some point Cargo
     // added a separate environment variable using `\x1f` instead of spaces,
     // which allows us to have spaces within individual `rustc` flags.
@@ -1046,21 +1023,18 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result<PathBuf, SpirvBuilderError> {
         join_checking_for_separators(rustflags, "\x1f"),
     );
 
-    let profile_in_env_var = profile.replace('-', "_").to_ascii_uppercase();
-
     // NOTE(eddyb) there's no parallelism to take advantage of multiple CGUs,
     // and inter-CGU duplication can be wasteful, so this forces 1 CGU for now.
+    let profile_in_env_var = profile.replace('-', "_").to_ascii_uppercase();
     let num_cgus = 1;
     cargo.env(
         format!("CARGO_PROFILE_{profile_in_env_var}_CODEGEN_UNITS"),
         num_cgus.to_string(),
     );
 
-    let build = cargo
-        .stderr(Stdio::inherit())
-        .current_dir(path_to_crate)
-        .output()
-        .expect("failed to execute cargo build");
+    cargo.stderr(Stdio::inherit()).current_dir(path_to_crate);
+    log::debug!("building shaders with `{cargo}`");
+    let build = cargo.output().expect("failed to execute cargo build");
 
     // `get_last_artifact` has the side-effect of printing invalid lines, so
     // we do that even in case of an error, to let through any useful messages