diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml index 708cac3..87ede62 100644 --- a/.github/workflows/CI.yml +++ b/.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: | diff --git a/README.md b/README.md index bdbb362..824b060 100644 --- a/README.md +++ b/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, diff --git a/src/bench.rs b/src/bench.rs index 3914a1a..5a72854 100644 --- a/src/bench.rs +++ b/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 { - 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) } } diff --git a/src/compiler/clang_cl.rs b/src/compiler/clang_cl.rs index 807c87a..6a69902 100644 --- a/src/compiler/clang_cl.rs +++ b/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 { + 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 { + 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> { + 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__RUSTFLAGS. When RUSTFLAGS is present, Cargo prioritizes - // it over CARGO_TARGET__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__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 = 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()?) { diff --git a/src/macros.rs b/src/macros.rs index d69c925..6ccecab 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -1,7 +1,15 @@ use paste::paste; macro_rules! cargo_command { - ($command: ident) => { + (@prepare_cargo $this:ident) => { + ($this.cargo.clone(), Vec::::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 { - 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); diff --git a/src/options.rs b/src/options.rs index 0edf20c..91bcfab 100644 --- a/src/options.rs +++ b/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) { + 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> { 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", + ] + ); } } diff --git a/src/run.rs b/src/run.rs index 21f4a18..f900af8 100644 --- a/src/run.rs +++ b/src/run.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}; /// Run a binary or example of the local package #[derive(Clone, Debug, Default, Parser)] @@ -58,12 +58,19 @@ impl Run { /// Generate cargo subcommand pub fn build_command(&self) -> Result { - let mut build = self.cargo.command(); - self.xwin.apply_command_env( + let mut cargo = self.cargo.clone(); + let args = std::mem::take(&mut cargo.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 !args.is_empty() { + build.arg("--").args(args); + } Ok(build) } } diff --git a/src/test.rs b/src/test.rs index 7921285..b4a1247 100644 --- a/src/test.rs +++ b/src/test.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 unit and integration tests and build examples of a local package #[derive(Clone, Debug, Default, Parser)] @@ -58,12 +58,24 @@ impl Test { /// Generate cargo subcommand pub fn build_command(&self) -> Result { - let mut build = self.cargo.command(); - self.xwin.apply_command_env( + let mut cargo = self.cargo.clone(); + let test_name = cargo.test_name.take(); + let args = std::mem::take(&mut cargo.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 test_name.is_some() || !args.is_empty() { + build.arg("--"); + if let Some(test_name) = test_name { + build.arg(test_name); + } + build.args(args); + } Ok(build) } }