diff --git a/CHANGELOG.md b/CHANGELOG.md index 377594290b9..4dc5b6cfd09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Toolchain: `nightly-2026-08-15` (rustc 1.99.0) +### Added ⭐ + +- [PR#696](https://github.com/Rust-GPU/rust-gpu/pull/696) add `RUSTGPU_ENCODED_RUSTFLAGS` and `RUSTGPU_ENCODED_CARGOFLAGS` env vars, split on `\x1f` like Cargo's `CARGO_ENCODED_RUSTFLAGS`, to allow spaces within flags + ### Fixed 🩹 - [PR#678](https://github.com/Rust-GPU/rust-gpu/pull/678) fix missing reexports in `spirv-std-nightly` diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index cd19f00862e..825a46ff89f 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -883,57 +883,38 @@ fn join_checking_for_separators(strings: Vec>, sep: &str) -> St strings.join(sep) } -pub struct RustcOutput { - pub compile_result: CompileResult, - pub deps: Vec, +/// Overrides read from `RUSTGPU_*` environment variables, see [`SpirvBuilder::rustflags`]. +#[derive(Default)] +struct EnvOverrides { + /// `RUSTGPU_CODEGEN_ARGS`: replaces [`SpirvBuilder::extra_args`]. + codegen_args: Option, + /// `RUSTGPU_ENCODED_RUSTFLAGS` or `RUSTGPU_RUSTFLAGS`: appended to the rustflags. + rustflags: Option>, + /// `RUSTGPU_ENCODED_CARGOFLAGS` or `RUSTGPU_CARGOFLAGS`: appended to the cargo args. + cargoflags: Option>, } -impl SpirvBuilder { - fn invoke_rustc(&self) -> Result { - let path_to_crate = self - .path_to_crate - .as_ref() - .ok_or(SpirvBuilderError::MissingCratePath)?; - let target; - { - let target_str = self - .target - .as_ref() - .ok_or(SpirvBuilderError::MissingTarget)?; - target = SpirvTarget::parse(target_str)?; - if !path_to_crate.is_dir() { - return Err(SpirvBuilderError::CratePathDoesntExist( - path_to_crate.clone(), - )); - } - } - - let toolchain_rustc_version = - if let Some(toolchain_rustc_version) = &self.toolchain_rustc_version { - toolchain_rustc_version.clone() - } else { - query_rustc_version(self.toolchain_overwrite.as_deref())? - }; - - // Okay, this is a little bonkers: in a normal world, we'd have the user clone - // rustc_codegen_spirv and pass in the path to it, and then we'd invoke cargo to build it, grab - // the resulting .so, and pass it into -Z codegen-backend. But that's really gross: the user - // needs to clone rustc_codegen_spirv and tell us its path! So instead, we *directly reference - // rustc_codegen_spirv in spirv-builder's Cargo.toml*, which means that it will get built - // alongside build.rs, and cargo will helpfully add it to LD_LIBRARY_PATH for us! However, - // rustc expects a full path, instead of a filename looked up via LD_LIBRARY_PATH, so we need - // to copy cargo's understanding of library lookup and find the library and its full path. - let rustc_codegen_spirv = Ok(self.rustc_codegen_spirv_location.clone()) - .transpose() - .unwrap_or_else(find_rustc_codegen_spirv)?; - if !rustc_codegen_spirv.is_file() { - return Err(SpirvBuilderError::RustcCodegenSpirvDylibDoesNotExist( - rustc_codegen_spirv, - )); - } +/// Splits flags from an env var pair, like Cargo's `CARGO_ENCODED_RUSTFLAGS` and `RUSTFLAGS`: +/// the `encoded` one is split on `\x1f` (allowing spaces within flags) and takes precedence, +/// the `plain` one is split on whitespace. +fn split_env_flags(encoded: Option, plain: Option) -> Option> { + if let Some(encoded) = encoded { + // like Cargo, an empty value means no flags rather than one empty flag + return Some(if encoded.is_empty() { + Vec::new() + } else { + encoded.split('\x1f').map(str::to_owned).collect() + }); + } + plain.map(|plain| plain.split_whitespace().map(str::to_owned).collect()) +} +impl SpirvBuilder { + /// The `rustc` flags (for `CARGO_ENCODED_RUSTFLAGS`) to build the shader crate with the + /// `rustc_codegen_spirv` backend at `codegen_backend`. + fn rustflags(&self, codegen_backend: &Path, env_overrides: &EnvOverrides) -> Vec { let mut rustflags = vec![ - format!("-Zcodegen-backend={}", rustc_codegen_spirv.display()), + format!("-Zcodegen-backend={}", codegen_backend.display()), // Ensure the codegen backend is emitted in `.d` files to force Cargo // to rebuild crates compiled with it when it changes (this used to be // the default until https://github.com/rust-lang/rust/pull/93969). @@ -962,14 +943,29 @@ impl SpirvBuilder { "-Zshare-generics=off".to_string(), ]; - // Wrapper for `env::var` that appropriately informs Cargo of the dependency. - let tracked_env_var_get = |name| { - if self.build_script.get_dependency_info() { - println!("cargo:rerun-if-env-changed={name}"); - } - env::var(name) - }; + let llvm_args = join_checking_for_separators(self.llvm_args(env_overrides), " "); + if !llvm_args.is_empty() { + rustflags.push(["-Cllvm-args=", &llvm_args].concat()); + } + + let target_features = join_checking_for_separators(self.target_features(), ","); + if !target_features.is_empty() { + rustflags.push(["-Ctarget-feature=", &target_features].concat()); + } + + if self.deny_warnings { + rustflags.push("-Dwarnings".to_string()); + } + + if let Some(extra_rustflags) = &env_overrides.rustflags { + rustflags.extend(extra_rustflags.iter().cloned()); + } + rustflags + } + + /// The `rustc_codegen_spirv` args, passed via `-Cllvm-args`. + fn llvm_args(&self, env_overrides: &EnvOverrides) -> Vec { let mut llvm_args = vec![]; if self.multimodule { llvm_args.push("--module-output=multiple".to_string()); @@ -1002,52 +998,111 @@ impl SpirvBuilder { if self.optimizer.preserve_bindings { llvm_args.push("--preserve-bindings".to_string()); } - let mut target_features = vec![]; let abort_strategy = match self.shader_panic_strategy { ShaderPanicStrategy::SilentExit => None, ShaderPanicStrategy::DebugPrintfThenExit { print_inputs, print_backtrace, - } => { - target_features.push("+ext:SPV_KHR_non_semantic_info".into()); - Some(format!( - "debug-printf{}{}", - if print_inputs { "+inputs" } else { "" }, - if print_backtrace { "+backtrace" } else { "" } - )) - } + } => Some(format!( + "debug-printf{}{}", + if print_inputs { "+inputs" } else { "" }, + if print_backtrace { "+backtrace" } else { "" } + )), ShaderPanicStrategy::UNSOUND_DO_NOT_USE_UndefinedBehaviorViaUnreachable => { Some("unreachable".into()) } }; llvm_args.extend(abort_strategy.map(|strategy| format!("--abort-strategy={strategy}"))); - if let Ok(extra_codegen_args) = tracked_env_var_get("RUSTGPU_CODEGEN_ARGS") { + if let Some(extra_codegen_args) = &env_overrides.codegen_args { llvm_args.extend(extra_codegen_args.split_whitespace().map(|s| s.to_string())); } else { llvm_args.extend(self.extra_args.iter().cloned()); } + llvm_args + } - let llvm_args = join_checking_for_separators(llvm_args, " "); - if !llvm_args.is_empty() { - rustflags.push(["-Cllvm-args=", &llvm_args].concat()); + /// The target features (capabilities and extensions), passed via `-Ctarget-feature`. + fn target_features(&self) -> Vec { + let mut target_features = vec![]; + if let ShaderPanicStrategy::DebugPrintfThenExit { .. } = self.shader_panic_strategy { + target_features.push("+ext:SPV_KHR_non_semantic_info".into()); } - target_features.extend(self.capabilities.iter().map(|cap| format!("+{cap:?}"))); target_features.extend(self.extensions.iter().map(|ext| format!("+ext:{ext}"))); - let target_features = join_checking_for_separators(target_features, ","); - if !target_features.is_empty() { - rustflags.push(["-Ctarget-feature=", &target_features].concat()); - } + target_features + } +} - if self.deny_warnings { - rustflags.push("-Dwarnings".to_string()); +pub struct RustcOutput { + pub compile_result: CompileResult, + pub deps: Vec, +} + +impl SpirvBuilder { + fn invoke_rustc(&self) -> Result { + let path_to_crate = self + .path_to_crate + .as_ref() + .ok_or(SpirvBuilderError::MissingCratePath)?; + let target; + { + let target_str = self + .target + .as_ref() + .ok_or(SpirvBuilderError::MissingTarget)?; + target = SpirvTarget::parse(target_str)?; + if !path_to_crate.is_dir() { + return Err(SpirvBuilderError::CratePathDoesntExist( + path_to_crate.clone(), + )); + } } - if let Ok(extra_rustflags) = tracked_env_var_get("RUSTGPU_RUSTFLAGS") { - rustflags.extend(extra_rustflags.split_whitespace().map(|s| s.to_string())); + let toolchain_rustc_version = + if let Some(toolchain_rustc_version) = &self.toolchain_rustc_version { + toolchain_rustc_version.clone() + } else { + query_rustc_version(self.toolchain_overwrite.as_deref())? + }; + + // Okay, this is a little bonkers: in a normal world, we'd have the user clone + // rustc_codegen_spirv and pass in the path to it, and then we'd invoke cargo to build it, grab + // the resulting .so, and pass it into -Z codegen-backend. But that's really gross: the user + // needs to clone rustc_codegen_spirv and tell us its path! So instead, we *directly reference + // rustc_codegen_spirv in spirv-builder's Cargo.toml*, which means that it will get built + // alongside build.rs, and cargo will helpfully add it to LD_LIBRARY_PATH for us! However, + // rustc expects a full path, instead of a filename looked up via LD_LIBRARY_PATH, so we need + // to copy cargo's understanding of library lookup and find the library and its full path. + let rustc_codegen_spirv = Ok(self.rustc_codegen_spirv_location.clone()) + .transpose() + .unwrap_or_else(find_rustc_codegen_spirv)?; + if !rustc_codegen_spirv.is_file() { + return Err(SpirvBuilderError::RustcCodegenSpirvDylibDoesNotExist( + rustc_codegen_spirv, + )); } + // Wrapper for `env::var` that appropriately informs Cargo of the dependency. + let tracked_env_var_get = |name| { + if self.build_script.get_dependency_info() { + println!("cargo:rerun-if-env-changed={name}"); + } + env::var(name).ok() + }; + let env_overrides = EnvOverrides { + codegen_args: tracked_env_var_get("RUSTGPU_CODEGEN_ARGS"), + rustflags: split_env_flags( + tracked_env_var_get("RUSTGPU_ENCODED_RUSTFLAGS"), + tracked_env_var_get("RUSTGPU_RUSTFLAGS"), + ), + cargoflags: split_env_flags( + tracked_env_var_get("RUSTGPU_ENCODED_CARGOFLAGS"), + tracked_env_var_get("RUSTGPU_CARGOFLAGS"), + ), + }; + let rustflags = self.rustflags(&rustc_codegen_spirv, &env_overrides); + let target_dir_path = self .target_dir_path .clone() @@ -1090,8 +1145,8 @@ impl SpirvBuilder { cargo.args(["--crate-type", "dylib"]); } - if let Ok(extra_cargoflags) = tracked_env_var_get("RUSTGPU_CARGOFLAGS") { - cargo.args(extra_cargoflags.split_whitespace()); + if let Some(extra_cargoflags) = &env_overrides.cargoflags { + cargo.args(extra_cargoflags); } let target_spec_dir = target_dir.join("target-specs"); diff --git a/crates/spirv-builder/src/tests.rs b/crates/spirv-builder/src/tests.rs index 6b37072be07..3b694c29d4c 100644 --- a/crates/spirv-builder/src/tests.rs +++ b/crates/spirv-builder/src/tests.rs @@ -102,3 +102,121 @@ mod dylib_lookup { ); } } + +mod rustflags { + use crate::{Capability, EnvOverrides, ShaderPanicStrategy, SpirvBuilder, SpirvMetadata}; + use std::path::Path; + + const BACKEND: &str = "/backend/librustc_codegen_spirv.so"; + + fn builder() -> SpirvBuilder { + SpirvBuilder::new("/shader", "spirv-unknown-vulkan1.2") + } + + fn rustflags(builder: &SpirvBuilder, env: &EnvOverrides) -> Vec { + builder.rustflags(Path::new(BACKEND), env) + } + + /// A builder that emits no configurable flags at all (the default panic strategy does). + fn silent_builder() -> SpirvBuilder { + builder().shader_panic_strategy(ShaderPanicStrategy::SilentExit) + } + + /// The flags after the fixed ones every build gets. + fn extra_rustflags(builder: &SpirvBuilder, env: &EnvOverrides) -> Vec { + let fixed = rustflags(&silent_builder(), &EnvOverrides::default()).len(); + rustflags(builder, env)[fixed..].to_vec() + } + + #[test] + fn fixed_flags_start_with_the_backend() { + let flags = rustflags(&builder(), &EnvOverrides::default()); + assert_eq!(flags[0], format!("-Zcodegen-backend={BACKEND}")); + assert!(flags.contains(&"-Zbinary-dep-depinfo".to_string())); + assert!(flags.contains(&"-Zshare-generics=off".to_string())); + } + + #[test] + fn silent_exit_has_no_llvm_args_or_target_features() { + let flags = extra_rustflags(&silent_builder(), &EnvOverrides::default()); + assert_eq!(flags, Vec::::new()); + } + + #[test] + fn builder_options_map_to_flags() { + let builder = builder() + .multimodule(true) + .spirv_metadata(SpirvMetadata::Full) + .relax_block_layout(true) + .preserve_bindings(true) + .shader_panic_strategy(ShaderPanicStrategy::DebugPrintfThenExit { + print_inputs: true, + print_backtrace: false, + }) + .capability(Capability::Int8) + .extension("SPV_KHR_foo") + .extra_arg("--dump-post-link=/tmp/x") + .deny_warnings(true); + assert_eq!( + extra_rustflags(&builder, &EnvOverrides::default()), + [ + "-Cllvm-args=--module-output=multiple --spirv-metadata=full \ + --relax-block-layout --preserve-bindings \ + --abort-strategy=debug-printf+inputs --dump-post-link=/tmp/x", + "-Ctarget-feature=+ext:SPV_KHR_non_semantic_info,+Int8,+ext:SPV_KHR_foo", + "-Dwarnings", + ] + ); + } + + #[test] + fn env_codegen_args_replace_extra_args_and_env_rustflags_are_appended() { + let builder = silent_builder().extra_arg("--from-builder"); + let env = EnvOverrides { + codegen_args: Some("--from-env --and-more".into()), + rustflags: Some(vec!["-Cfoo".into(), "-Cbar".into()]), + cargoflags: None, + }; + assert_eq!( + extra_rustflags(&builder, &env), + ["-Cllvm-args=--from-env --and-more", "-Cfoo", "-Cbar"] + ); + } +} + +mod env_flags { + use crate::split_env_flags; + + fn split(encoded: Option<&str>, plain: Option<&str>) -> Option> { + split_env_flags(encoded.map(Into::into), plain.map(Into::into)) + } + + #[test] + fn unset() { + assert_eq!(split(None, None), None); + } + + #[test] + fn plain_splits_on_whitespace() { + assert_eq!( + split(None, Some(" -Cfoo -Zbar\t")), + Some(vec!["-Cfoo".into(), "-Zbar".into()]) + ); + } + + #[test] + fn encoded_keeps_spaces_and_takes_precedence() { + assert_eq!( + split( + Some("--config\x1fbuild.rustflags=['-C a b']"), + Some("-Cignored") + ), + Some(vec!["--config".into(), "build.rustflags=['-C a b']".into()]) + ); + } + + #[test] + fn empty_encoded_means_no_flags() { + assert_eq!(split(Some(""), Some("-Cignored")), Some(vec![])); + } +}