Handle xwin cache paths containing spaces

.github/workflows/CI.yml

@@ -79,6 +79,11 @@ jobs:
         run: |
           # Test static CRT linkage using hello-rustls
           target/release/cargo-xwin build --target x86_64-pc-windows-msvc --manifest-path tests/hello-rustls/Cargo.toml
+      - name: xwin build with cache path containing spaces
+        if: startsWith(matrix.os, 'ubuntu') && matrix.toolchain == 'stable' && matrix.cross-compiler == 'clang-cl'
+        env:
+          XWIN_CACHE_DIR: ${{ runner.temp }}/xwin cache
+        run: target/release/cargo-xwin build --target x86_64-pc-windows-msvc --manifest-path tests/hello-tls/Cargo.toml
       - name: xwin run - x86_64
         if: startsWith(matrix.os, 'ubuntu')
         run: |

README.md

@@ -91,6 +91,8 @@ The Microsoft CRT and Windows SDK can be customized using the following environm
 | `XWIN_INCLUDE_DEBUG_SYMBOLS` | `--xwin-include-debug-symbols` | Whether or not to include debug symbols (PDBs) in installation (default false).                                    |
 | `XWIN_HTTP_RETRIES`          | `--xwin-http-retries`          | Number of times to retry HTTP requests when downloading (default 3).                                               |
 
+The `clang-cl` backend supports cache paths containing whitespace for Rust linker flags and C, C++, or bindgen include paths. Projects that compile C or C++ through [`cc`](https://crates.io/crates/cc) require `cc` 1.1.11 or newer for such paths because cargo-xwin enables `CC_SHELL_ESCAPED_FLAGS` only in that case. Windows resource compilation still requires a cache path without whitespace because the `RCFLAGS` quoting contract is not defined here.
+
 ### CMake Support
 
 Some Rust crates use the [cmake](https://github.com/alexcrichton/cmake-rs) crate to build C/C++ dependencies,

src/bench.rs

@@ -6,7 +6,7 @@ use std::process::{self, Command};
 use anyhow::{Context, Result};
 use clap::Parser;
 
-use crate::options::XWinOptions;
+use crate::options::{RustflagsMode, XWinOptions, append_cargo_configs};
 
 /// Execute all benchmarks of a local package
 #[derive(Clone, Debug, Default, Parser)]
@@ -58,12 +58,24 @@ impl Bench {
 
     /// Generate cargo subcommand
     pub fn build_command(&self) -> Result<Command> {
-        let mut build = self.cargo.command();
-        self.xwin.apply_command_env(
+        let mut cargo = self.cargo.clone();
+        let bench_name = cargo.bench.bench_name.take();
+        let args = std::mem::take(&mut cargo.bench.args);
+        let mut build = cargo.command();
+        let cargo_configs = self.xwin.prepare_command_env(
             self.manifest_path.as_deref(),
-            &self.cargo.common,
+            &cargo.common,
             &mut build,
+            RustflagsMode::CargoConfig,
         )?;
+        append_cargo_configs(&mut build, cargo_configs);
+        if bench_name.is_some() || !args.is_empty() {
+            build.arg("--");
+            if let Some(bench_name) = bench_name {
+                build.arg(bench_name);
+            }
+            build.args(args);
+        }
         Ok(build)
     }
 }

src/compiler/clang_cl.rs

@@ -19,7 +19,34 @@ use crate::compiler::common::{
     is_static_crt_enabled, resolve_target_info, setup_cmake_env, setup_env_path, setup_llvm_tools,
     setup_target_compiler_and_linker_env,
 };
-use crate::options::XWinOptions;
+use crate::options::{RustflagsMode, XWinOptions};
+
+const MSVC_INCLUDE_DIRS: [&str; 5] = [
+    "crt/include",
+    "sdk/include/ucrt",
+    "sdk/include/um",
+    "sdk/include/shared",
+    "sdk/include/winrt",
+];
+
+fn include_flags(xwin_dir: &str, prefix: &str, quote_paths: bool) -> Vec<String> {
+    MSVC_INCLUDE_DIRS
+        .iter()
+        .map(|include_dir| {
+            if quote_paths {
+                format!(r#"{prefix}"{xwin_dir}/{include_dir}""#)
+            } else {
+                format!("{prefix}{xwin_dir}/{include_dir}")
+            }
+        })
+        .collect()
+}
+
+fn target_rustflags_config(target: &str, rustflags: &cargo_config2::Flags) -> Result<String> {
+    let target = serde_json::to_string(target).context("Failed to encode target name")?;
+    let flags = serde_json::to_string(&rustflags.flags).context("Failed to encode target flags")?;
+    Ok(format!("target.{target}.rustflags={flags}"))
+}
 
 #[derive(Debug)]
 pub struct ClangCl<'a> {
@@ -37,7 +64,9 @@ impl<'a> ClangCl<'a> {
         cargo: &cargo_options::CommonOptions,
         cache_dir: PathBuf,
         cmd: &mut Command,
-    ) -> Result<()> {
+        rustflags_mode: RustflagsMode,
+    ) -> Result<Vec<String>> {
+        let mut cargo_configs = Vec::new();
         let env_path = setup_env_path(&cache_dir)?;
 
         let xwin_cache_dir = prepare_xwin_cache_dir(cache_dir.clone())
@@ -84,21 +113,21 @@ impl<'a> ClangCl<'a> {
                 };
 
                 let xwin_dir = adjust_canonicalization(xwin_cache_dir.to_slash_lossy().to_string());
+                let quote_include_paths = xwin_dir.chars().any(char::is_whitespace);
                 let mut cl_flags = vec![
                     format!("--target={llvm_target}"),
                     "-Wno-unused-command-line-argument".to_string(),
                     "-fuse-ld=lld-link".to_string(),
-                    format!("/imsvc {dir}/crt/include", dir = xwin_dir),
-                    format!("/imsvc {dir}/sdk/include/ucrt", dir = xwin_dir),
-                    format!("/imsvc {dir}/sdk/include/um", dir = xwin_dir),
-                    format!("/imsvc {dir}/sdk/include/shared", dir = xwin_dir),
-                    format!("/imsvc {dir}/sdk/include/winrt", dir = xwin_dir),
                 ];
+                cl_flags.extend(include_flags(&xwin_dir, "/imsvc ", quote_include_paths));
                 if !user_set_cl_flags.is_empty() {
                     cl_flags.push(user_set_cl_flags.clone());
                 }
                 let cl_flags = cl_flags.join(" ");
                 cmd.env("CL_FLAGS", &cl_flags);
+                if quote_include_paths {
+                    cmd.env("CC_SHELL_ESCAPED_FLAGS", "1");
+                }
                 cmd.env(
                     format!("CFLAGS_{env_target}"),
                     format!("{cl_flags} {user_set_c_flags}",),
@@ -110,19 +139,10 @@ impl<'a> ClangCl<'a> {
 
                 cmd.env(
                     format!("BINDGEN_EXTRA_CLANG_ARGS_{env_target}"),
-                    format!(
-                        "-I{dir}/crt/include -I{dir}/sdk/include/ucrt -I{dir}/sdk/include/um -I{dir}/sdk/include/shared -I{dir}/sdk/include/winrt",
-                        dir = xwin_dir
-                    )
+                    include_flags(&xwin_dir, "-I", quote_include_paths).join(" "),
                 );
 
-                cmd.env(
-                    "RCFLAGS",
-                    format!(
-                        "-I{dir}/crt/include -I{dir}/sdk/include/ucrt -I{dir}/sdk/include/um -I{dir}/sdk/include/shared -I{dir}/sdk/include/winrt",
-                        dir = xwin_dir
-                    )
-                );
+                cmd.env("RCFLAGS", include_flags(&xwin_dir, "-I", false).join(" "));
 
                 // Set LIB environment variable for clang-cl library path resolution
                 let lib_paths = [
@@ -189,18 +209,28 @@ impl<'a> ClangCl<'a> {
                     dir = xwin_dir,
                     arch = xwin_arch
                 ));
-                // Remove RUSTFLAGS from environment so that the spawned Cargo respects our
-                // CARGO_TARGET_<triple>_RUSTFLAGS. When RUSTFLAGS is present, Cargo prioritizes
-                // it over CARGO_TARGET_<triple>_RUSTFLAGS. The flags from RUSTFLAGS are already
-                // included in `rustflags` via cargo-config2's resolution.
+                // cargo-config2 has already folded inherited rustflags into this resolved list.
+                // Remove their global forms so they do not affect cross-target artifact
+                // dependencies when the target-scoped replacement is applied below.
                 cmd.env_remove("RUSTFLAGS");
-
-                // Use `CARGO_TARGET_<TRIPLE>_RUSTFLAGS` to avoid the flags being passed to artifact
-                // dependencies built for other targets.
-                cmd.env(
-                    format!("CARGO_TARGET_{}_RUSTFLAGS", env_target.to_uppercase()),
-                    rustflags.encode_space_separated()?,
-                );
+                match rustflags_mode {
+                    RustflagsMode::CargoConfig => {
+                        cmd.env_remove("CARGO_ENCODED_RUSTFLAGS");
+                        cmd.env_remove("CARGO_BUILD_RUSTFLAGS");
+                        cmd.env_remove(format!(
+                            "CARGO_TARGET_{}_RUSTFLAGS",
+                            env_target.to_uppercase()
+                        ));
+                        cargo_configs
+                            .push(target_rustflags_config(&cargo_target_name, &rustflags)?);
+                    }
+                    RustflagsMode::Environment => {
+                        cmd.env(
+                            format!("CARGO_TARGET_{}_RUSTFLAGS", env_target.to_uppercase()),
+                            rustflags.encode_space_separated()?,
+                        );
+                    }
+                }
                 cmd.env("PATH", &env_path);
 
                 // CMake support
@@ -210,7 +240,7 @@ impl<'a> ClangCl<'a> {
                 setup_cmake_env(cmd, target, cmake_toolchain);
             }
         }
-        Ok(())
+        Ok(cargo_configs)
     }
 
     fn setup_msvc_crt_with_retry(&self, cache_dir: PathBuf) -> Result<()> {
@@ -629,6 +659,54 @@ pub fn setup_clang_cl_symlink(env_path: &OsStr, cache_dir: &Path) -> Result<()>
     Ok(())
 }
 
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn include_flags_quote_cache_paths_with_spaces() {
+        let xwin_dir = "/tmp/xwin cache";
+
+        let cl_flags = include_flags(xwin_dir, "/imsvc ", true);
+        assert_eq!(cl_flags.len(), MSVC_INCLUDE_DIRS.len());
+        assert_eq!(cl_flags[0], r#"/imsvc "/tmp/xwin cache/crt/include""#);
+
+        let include_flags = include_flags(xwin_dir, "-I", true).join(" ");
+        assert!(include_flags.contains(r#"-I"/tmp/xwin cache/sdk/include/winrt""#));
+        assert!(!include_flags.contains("-I/tmp/xwin cache"));
+    }
+
+    #[test]
+    fn include_flags_preserve_legacy_format_without_spaces() {
+        let xwin_dir = "/tmp/xwin-cache";
+
+        assert_eq!(
+            include_flags(xwin_dir, "/imsvc ", false)[0],
+            "/imsvc /tmp/xwin-cache/crt/include"
+        );
+        assert_eq!(
+            include_flags(xwin_dir, "-I", false)[0],
+            "-I/tmp/xwin-cache/crt/include"
+        );
+    }
+
+    #[test]
+    fn target_rustflags_preserve_paths_with_spaces_and_target_scope() {
+        let mut rustflags = cargo_config2::Flags::default();
+        rustflags
+            .flags
+            .push("-Lnative=/tmp/xwin cache/crt/lib/x86_64".into());
+
+        let config = target_rustflags_config("x86_64-pc-windows-msvc", &rustflags).unwrap();
+        let (key, value) = config.split_once('=').unwrap();
+        let decoded: Vec<String> = serde_json::from_str(value).unwrap();
+
+        assert_eq!(key, r#"target."x86_64-pc-windows-msvc".rustflags"#);
+        assert_eq!(decoded, rustflags.flags);
+        assert!(value.contains("xwin cache"));
+    }
+}
+
 #[cfg(not(target_os = "macos"))]
 pub fn setup_clang_cl_symlink(env_path: &OsStr, cache_dir: &Path) -> Result<()> {
     if let Ok(clang) = which_in("clang", Some(env_path), env::current_dir()?) {

src/macros.rs

@@ -1,7 +1,15 @@
 use paste::paste;
 
 macro_rules! cargo_command {
-    ($command: ident) => {
+    (@prepare_cargo $this:ident) => {
+        ($this.cargo.clone(), Vec::<String>::new())
+    };
+    (@prepare_cargo $this:ident, $trailing:ident) => {{
+        let mut cargo = $this.cargo.clone();
+        let trailing = std::mem::take(&mut cargo.$trailing);
+        (cargo, trailing)
+    }};
+    ($command: ident $(, $trailing:ident)?) => {
         paste! {
             pub mod [<$command:lower>] {
                 use std::ops::{Deref, DerefMut};
@@ -11,7 +19,7 @@ macro_rules! cargo_command {
                 use anyhow::{Context, Result};
                 use clap::Parser;
 
-                use crate::options::XWinOptions;
+                use crate::options::{RustflagsMode, XWinOptions, append_cargo_configs};
 
                 #[derive(Clone, Debug, Default, Parser)]
                 #[command(
@@ -50,12 +58,18 @@ macro_rules! cargo_command {
 
                     /// Generate cargo subcommand
                     pub fn build_command(&self) -> Result<Command> {
-                        let mut build = self.cargo.command();
-                        self.xwin.apply_command_env(
+                        let (cargo, trailing) = cargo_command!(@prepare_cargo self $(, $trailing)?);
+                        let mut build = cargo.command();
+                        let cargo_configs = self.xwin.prepare_command_env(
                             self.manifest_path.as_deref(),
-                            &self.cargo.common,
+                            &cargo.common,
                             &mut build,
+                            RustflagsMode::CargoConfig,
                         )?;
+                        append_cargo_configs(&mut build, cargo_configs);
+                        if !trailing.is_empty() {
+                            build.arg("--").args(trailing);
+                        }
                         Ok(build)
                     }
                 }
@@ -90,6 +104,6 @@ macro_rules! cargo_command {
 
 cargo_command!(Build);
 cargo_command!(Check);
-cargo_command!(Clippy);
+cargo_command!(Clippy, args);
 cargo_command!(Doc);
-cargo_command!(Rustc);
+cargo_command!(Rustc, args);

src/options.rs

@@ -18,6 +18,18 @@ pub enum CrossCompiler {
     Clang,
 }
 
+#[derive(Clone, Copy, Debug)]
+pub(crate) enum RustflagsMode {
+    CargoConfig,
+    Environment,
+}
+
+pub(crate) fn append_cargo_configs(cmd: &mut Command, configs: Vec<String>) {
+    for config in configs {
+        cmd.arg("--config").arg(config);
+    }
+}
+
 /// common xwin options
 #[derive(Clone, Debug, Parser)]
 pub struct XWinOptions {
@@ -109,6 +121,17 @@ impl XWinOptions {
         cargo: &cargo_options::CommonOptions,
         cmd: &mut Command,
     ) -> Result<()> {
+        self.prepare_command_env(manifest_path, cargo, cmd, RustflagsMode::Environment)?;
+        Ok(())
+    }
+
+    pub(crate) fn prepare_command_env(
+        &self,
+        manifest_path: Option<&Path>,
+        cargo: &cargo_options::CommonOptions,
+        cmd: &mut Command,
+        rustflags_mode: RustflagsMode,
+    ) -> Result<Vec<String>> {
         let cache_dir = {
             let cache_dir = self.xwin_cache_dir.clone().unwrap_or_else(|| {
                 dirs::cache_dir()
@@ -121,13 +144,40 @@ impl XWinOptions {
         match self.cross_compiler {
             CrossCompiler::ClangCl => {
                 let clang_cl = crate::compiler::clang_cl::ClangCl::new(self);
-                clang_cl.apply_command_env(manifest_path, cargo, cache_dir, cmd)?;
+                clang_cl.apply_command_env(manifest_path, cargo, cache_dir, cmd, rustflags_mode)
             }
             CrossCompiler::Clang => {
                 let clang = crate::compiler::clang::Clang::new();
                 clang.apply_command_env(manifest_path, cargo, cache_dir, cmd)?;
+                Ok(Vec::new())
             }
         }
-        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn cargo_configs_are_appended_before_trailing_arguments() {
+        let mut cmd = Command::new("cargo");
+        cmd.args(["test", "--locked"]);
+
+        append_cargo_configs(&mut cmd, vec!["target.test.rustflags=[]".into()]);
+        cmd.args(["--", "test_filter"]);
+
+        let args: Vec<_> = cmd.get_args().collect();
+        assert_eq!(
+            args,
+            [
+                "test",
+                "--locked",
+                "--config",
+                "target.test.rustflags=[]",
+                "--",
+                "test_filter",
+            ]
+        );
     }
 }