From 56a46c37542366e6861d07d476792aa119a5abd0 Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 15 Dec 2025 18:27:34 +0100 Subject: [PATCH 1/2] build_script: replace `enum MetadataPrintout` with `struct BuildScriptConfig` for more flexibility --- crates/spirv-builder/README.md | 11 +- crates/spirv-builder/src/lib.rs | 126 ++++++++++-------- crates/spirv-builder/src/watch.rs | 2 +- examples/multibuilder/src/main.rs | 12 +- examples/runners/ash/src/main.rs | 3 +- examples/runners/wgpu/builder/src/main.rs | 20 +-- examples/runners/wgpu/src/lib.rs | 3 +- .../src/scaffold/shader/rust_gpu_shader.rs | 1 - 8 files changed, 97 insertions(+), 81 deletions(-) diff --git a/crates/spirv-builder/README.md b/crates/spirv-builder/README.md index 69711c6dab..9445f74fc3 100644 --- a/crates/spirv-builder/README.md +++ b/crates/spirv-builder/README.md @@ -11,12 +11,13 @@ It takes care of pulling in the `SPIR-V` backend for Rust, `rustc_codegen_spirv` ## Example ```rust,no_run -use spirv_builder::{MetadataPrintout, SpirvBuilder}; - +# use spirv_builder::SpirvBuilder; +# fn main() -> Result<(), Box> { - SpirvBuilder::new("my_shaders", "spirv-unknown-vulkan1.1") - .print_metadata(MetadataPrintout::Full) - .build()?; + let mut builder = SpirvBuilder::new("my_shaders", "spirv-unknown-vulkan1.3"); + builder.build_script.defaults = true; + builder.build_script.env_shader_spv_path = Some(true); + builder.build()?; Ok(()) } ``` diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 3bd3c18dec..509f4ac474 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -123,8 +123,10 @@ pub enum SpirvBuilderError { MissingTargetSpec, #[error("build failed")] BuildFailed, - #[error("multi-module build cannot be used with print_metadata = MetadataPrintout::Full")] - MultiModuleWithPrintMetadata, + #[error( + "`multimodule: true` build cannot be used together with `build_script.env_shader_spv_path: true`" + )] + MultiModuleWithEnvShaderSpvPath, #[error("multi-module metadata file missing")] MetadataFileMissing(#[from] std::io::Error), #[error("unable to parse multi-module metadata file")] @@ -138,21 +140,6 @@ pub enum SpirvBuilderError { const SPIRV_TARGET_PREFIX: &str = "spirv-unknown-"; -#[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] -#[cfg_attr(feature = "clap", derive(clap::ValueEnum))] -#[non_exhaustive] -pub enum MetadataPrintout { - /// Print no cargo metadata. - #[default] - None, - /// Print only dependency information (eg for multiple modules). - DependencyOnly, - /// Print all cargo metadata. - /// - /// Includes dependency information and spirv environment variable. - Full, -} - #[derive(Debug, PartialEq, Eq, Clone, Copy, Default, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::ValueEnum))] #[non_exhaustive] @@ -396,6 +383,50 @@ impl Default for ShaderCrateFeatures { } } +/// Configuration for build scripts +#[derive(Clone, Debug, Default)] +#[non_exhaustive] +pub struct BuildScriptConfig { + /// Enable this if you are using `spirv-builder` from a build script to apply some recommended default options, such + /// as [`Self::dependency_info`]. + pub defaults: bool, + + /// Print dependency information for cargo build scripts (with `cargo::rerun-if-changed={}` and such). + /// Dependency information makes cargo rerun the build script is rerun when shader source files change, thus + /// rebuilding the shader. + /// + /// Default: [`Self::defaults`] + pub dependency_info: Option, + + /// Whether to emit an env var pointing to the shader module file (via `cargo::rustc-env={}`). The name of the env + /// var is the crate name with `.spv` appended, e.g. `sky_shader.spv`. + /// Not supported together with `multimodule=true` or `.watch()`. + /// + /// Some examples on how to include the shader module in the source code: + /// * wgpu: + /// ```rust,ignore + /// let shader: ShaderModuleDescriptorPassthrough = include_spirv_raw!(env!("my_shader.spv")); + /// ``` + /// * ash + /// ```rust,ignore + /// let bytes: &[u8] = include_bytes!(env!("my_shader.spv")) + /// let words = ash::util::read_spv(&mut std::io::Cursor::new(bytes)).unwrap(); + /// ``` + /// + /// Default: `false` + pub env_shader_spv_path: Option, +} + +/// these all have the prefix `get` so the doc items link to the members, not these private fns +impl BuildScriptConfig { + fn get_dependency_info(&self) -> bool { + self.dependency_info.unwrap_or(self.defaults) + } + fn get_env_shader_spv_path(&self) -> bool { + self.env_shader_spv_path.unwrap_or(false) + } +} + #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] #[cfg_attr(feature = "clap", derive(clap::Parser))] #[non_exhaustive] @@ -410,10 +441,10 @@ pub struct SpirvBuilder { /// `--crate-type dylib`. Defaults to true if `cargo_cmd` is `None` or `Some("rustc")`. #[cfg_attr(feature = "clap", clap(skip))] pub cargo_cmd_like_rustc: Option, - /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::None`]. - /// Within build scripts, set it to [`MetadataPrintout::DependencyOnly`] or [`MetadataPrintout::Full`] to ensure the build script is rerun on code changes. + /// Configuration for build scripts #[cfg_attr(feature = "clap", clap(skip))] - pub print_metadata: MetadataPrintout, + #[serde(skip)] + pub build_script: BuildScriptConfig, /// Build in release. Defaults to true. #[cfg_attr(feature = "clap", clap(long = "debug", default_value = "true", action = clap::ArgAction::SetFalse))] pub release: bool, @@ -507,7 +538,7 @@ impl Default for SpirvBuilder { path_to_crate: None, cargo_cmd: None, cargo_cmd_like_rustc: None, - print_metadata: MetadataPrintout::default(), + build_script: BuildScriptConfig::default(), release: true, target: None, deny_warnings: false, @@ -548,13 +579,6 @@ impl SpirvBuilder { self } - /// Whether to print build.rs cargo metadata (e.g. cargo:rustc-env=var=val). Defaults to [`MetadataPrintout::Full`]. - #[must_use] - pub fn print_metadata(mut self, v: MetadataPrintout) -> Self { - self.print_metadata = v; - self - } - #[must_use] pub fn deny_warnings(mut self, v: bool) -> Self { self.deny_warnings = v; @@ -700,19 +724,15 @@ impl SpirvBuilder { self } - /// Builds the module. If `print_metadata` is [`MetadataPrintout::Full`], you usually don't have to inspect the path - /// in the result, as the environment variable for the path to the module will already be set. + /// Builds the module pub fn build(&self) -> Result { let metadata_file = invoke_rustc(self)?; - match self.print_metadata { - MetadataPrintout::Full | MetadataPrintout::DependencyOnly => { - leaf_deps(&metadata_file, |artifact| { - println!("cargo:rerun-if-changed={artifact}"); - }) - // Close enough - .map_err(SpirvBuilderError::MetadataFileMissing)?; - } - MetadataPrintout::None => (), + if self.build_script.get_dependency_info() { + leaf_deps(&metadata_file, |artifact| { + println!("cargo:rerun-if-changed={artifact}"); + }) + // Close enough + .map_err(SpirvBuilderError::MetadataFileMissing)?; } let metadata = self.parse_metadata_file(&metadata_file)?; @@ -731,17 +751,17 @@ impl SpirvBuilder { match &metadata.module { ModuleResult::SingleModule(spirv_module) => { assert!(!self.multimodule); - let env_var = format!( - "{}.spv", - at.file_name() - .unwrap() - .to_str() - .unwrap() - .strip_suffix(ARTIFACT_SUFFIX) - .unwrap() - ); - if self.print_metadata == MetadataPrintout::Full { - println!("cargo:rustc-env={}={}", env_var, spirv_module.display()); + if self.build_script.get_env_shader_spv_path() { + let env_var = format!( + "{}.spv", + at.file_name() + .unwrap() + .to_str() + .unwrap() + .strip_suffix(ARTIFACT_SUFFIX) + .unwrap() + ); + println!("cargo::rustc-env={}={}", env_var, spirv_module.display()); } } ModuleResult::MultiModule(_) => { @@ -839,8 +859,8 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { } } - if (builder.print_metadata == MetadataPrintout::Full) && builder.multimodule { - return Err(SpirvBuilderError::MultiModuleWithPrintMetadata); + if builder.build_script.get_env_shader_spv_path() && builder.multimodule { + return Err(SpirvBuilderError::MultiModuleWithEnvShaderSpvPath); } if !path_to_crate.is_dir() { return Err(SpirvBuilderError::CratePathDoesntExist( @@ -905,7 +925,7 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { // Wrapper for `env::var` that appropriately informs Cargo of the dependency. let tracked_env_var_get = |name| { - if let MetadataPrintout::Full | MetadataPrintout::DependencyOnly = builder.print_metadata { + if builder.build_script.get_dependency_info() { println!("cargo:rerun-if-env-changed={name}"); } env::var(name) diff --git a/crates/spirv-builder/src/watch.rs b/crates/spirv-builder/src/watch.rs index 08d468eff5..0e4fd33d02 100644 --- a/crates/spirv-builder/src/watch.rs +++ b/crates/spirv-builder/src/watch.rs @@ -55,7 +55,7 @@ impl SpirvWatcher { .as_ref() .ok_or(SpirvBuilderError::MissingCratePath)? .clone(); - if !matches!(builder.print_metadata, crate::MetadataPrintout::None) { + if builder.build_script.get_env_shader_spv_path() { return Err(SpirvWatcherError::WatchWithPrintMetadata.into()); } diff --git a/examples/multibuilder/src/main.rs b/examples/multibuilder/src/main.rs index e694b72ee3..e64e254f3a 100644 --- a/examples/multibuilder/src/main.rs +++ b/examples/multibuilder/src/main.rs @@ -1,13 +1,11 @@ -use spirv_builder::{MetadataPrintout, SpirvBuilder}; +use spirv_builder::SpirvBuilder; fn main() { - let result = SpirvBuilder::new( + let mut builder = SpirvBuilder::new( concat!(env!("CARGO_MANIFEST_DIR"), "/../shaders/sky-shader"), "spirv-unknown-spv1.3", - ) - .print_metadata(MetadataPrintout::DependencyOnly) - .multimodule(true) - .build() - .unwrap(); + ); + builder.multimodule = true; + let result = builder.build().unwrap(); println!("{result:#?}"); } diff --git a/examples/runners/ash/src/main.rs b/examples/runners/ash/src/main.rs index 7f97c281ad..a62d58709f 100644 --- a/examples/runners/ash/src/main.rs +++ b/examples/runners/ash/src/main.rs @@ -78,7 +78,7 @@ use ash::util::read_spv; use clap::{Parser, ValueEnum}; use raw_window_handle::HasDisplayHandle as _; use shared::ShaderConstants; -use spirv_builder::{MetadataPrintout, SpirvBuilder}; +use spirv_builder::SpirvBuilder; use std::{ fs::File, path::PathBuf, @@ -259,7 +259,6 @@ pub fn compile_shaders(shader: &RustGPUShader) -> anyhow::Result> { .collect::(); let compile_result = SpirvBuilder::new(crate_path, "spirv-unknown-vulkan1.3") - .print_metadata(MetadataPrintout::None) .shader_panic_strategy(spirv_builder::ShaderPanicStrategy::DebugPrintfThenExit { print_inputs: true, print_backtrace: true, diff --git a/examples/runners/wgpu/builder/src/main.rs b/examples/runners/wgpu/builder/src/main.rs index ba94e42f64..5f91cdb293 100644 --- a/examples/runners/wgpu/builder/src/main.rs +++ b/examples/runners/wgpu/builder/src/main.rs @@ -1,18 +1,18 @@ -use spirv_builder::{MetadataPrintout, SpirvBuilder}; +use spirv_builder::SpirvBuilder; use std::env; use std::error::Error; use std::fs; -use std::path::Path; +use std::path::{Path, PathBuf}; fn build_shader(path_to_crate: &str, codegen_names: bool) -> Result<(), Box> { - let builder_dir = &Path::new(env!("CARGO_MANIFEST_DIR")); - let path_to_crate = builder_dir.join(path_to_crate); - let result = SpirvBuilder::new(path_to_crate, "spirv-unknown-vulkan1.1") - .print_metadata(MetadataPrintout::Full) - // Give this spirv-builder a unique target dir, so that rebuilding android and the main wgpu app's target dir - // don't clash and break each other's incremental - .target_dir_path("example-runner-wgpu-builder") - .build()?; + let path_to_crate = Path::new(env!("CARGO_MANIFEST_DIR")).join(path_to_crate); + let mut builder = SpirvBuilder::new(path_to_crate, "spirv-unknown-vulkan1.1"); + builder.build_script.defaults = true; + builder.build_script.env_shader_spv_path = Some(true); + // Give this spirv-builder a unique target dir, so that rebuilding android and the main wgpu app's target dir + // don't clash and break each other's incremental + builder.target_dir_path = Some(PathBuf::from("example-runner-wgpu-builder")); + let result = builder.build()?; if codegen_names { let out_dir = env::var_os("OUT_DIR").unwrap(); let dest_path = Path::new(&out_dir).join("entry_points.rs"); diff --git a/examples/runners/wgpu/src/lib.rs b/examples/runners/wgpu/src/lib.rs index 7faa69d89b..f95db5eb87 100644 --- a/examples/runners/wgpu/src/lib.rs +++ b/examples/runners/wgpu/src/lib.rs @@ -124,7 +124,7 @@ fn maybe_watch( ) -> CompiledShaderModules { #[cfg(not(any(target_os = "android", target_arch = "wasm32")))] { - use spirv_builder::{CompileResult, MetadataPrintout, SpirvBuilder}; + use spirv_builder::{CompileResult, SpirvBuilder}; use std::path::PathBuf; let crate_name = match options.shader { @@ -142,7 +142,6 @@ fn maybe_watch( let has_debug_printf = options.force_spirv_passthru; let builder = SpirvBuilder::new(crate_path, "spirv-unknown-vulkan1.1") - .print_metadata(MetadataPrintout::None) .shader_panic_strategy(if has_debug_printf { spirv_builder::ShaderPanicStrategy::DebugPrintfThenExit { print_inputs: true, diff --git a/tests/difftests/lib/src/scaffold/shader/rust_gpu_shader.rs b/tests/difftests/lib/src/scaffold/shader/rust_gpu_shader.rs index 59d853e731..e92f02a847 100644 --- a/tests/difftests/lib/src/scaffold/shader/rust_gpu_shader.rs +++ b/tests/difftests/lib/src/scaffold/shader/rust_gpu_shader.rs @@ -38,7 +38,6 @@ impl RustComputeShader { impl SpirvShader for RustComputeShader { fn spirv_bytes(&self) -> anyhow::Result<(Vec, String)> { let mut builder = SpirvBuilder::new(&self.path, &self.target) - .print_metadata(spirv_builder::MetadataPrintout::None) .release(true) .multimodule(false) .shader_panic_strategy(spirv_builder::ShaderPanicStrategy::SilentExit) From 8b83d585333140ee887f3a3d0e9e84269b3d2cd3 Mon Sep 17 00:00:00 2001 From: firestar99 Date: Mon, 15 Dec 2025 18:36:59 +0100 Subject: [PATCH 2/2] build_script: add forwarding of rustc warnings, with color support --- crates/spirv-builder/src/lib.rs | 42 ++++++++++++++++++++++++++++++++- 1 file changed, 41 insertions(+), 1 deletion(-) diff --git a/crates/spirv-builder/src/lib.rs b/crates/spirv-builder/src/lib.rs index 509f4ac474..48e055ba88 100644 --- a/crates/spirv-builder/src/lib.rs +++ b/crates/spirv-builder/src/lib.rs @@ -415,6 +415,20 @@ pub struct BuildScriptConfig { /// /// Default: `false` pub env_shader_spv_path: Option, + + /// Forwards any warnings or errors by rustc as build script warnings (via `cargo::warning=`). Not enabling this + /// option may hide warnings if the build succeeds. + /// + /// Default: [`Self::defaults`] + pub forward_rustc_warnings: Option, + + /// Pass `--color always` to cargo to force enable colorful error messages. Particularly in build scripts, these + /// are disabled by default, even though we'll forward them to your console. Should your console not support colors, + /// then the outer cargo executing the build script will filter out all ansi escape sequences anyway, so we're free + /// to always emit them. + /// + /// Default: [`Self::defaults`] + pub cargo_color_always: Option, } /// these all have the prefix `get` so the doc items link to the members, not these private fns @@ -425,6 +439,12 @@ impl BuildScriptConfig { fn get_env_shader_spv_path(&self) -> bool { self.env_shader_spv_path.unwrap_or(false) } + fn get_forward_rustc_warnings(&self) -> bool { + self.forward_rustc_warnings.unwrap_or(self.defaults) + } + fn get_cargo_color_always(&self) -> bool { + self.cargo_color_always.unwrap_or(self.defaults) + } } #[derive(Clone, Debug, serde::Deserialize, serde::Serialize)] @@ -1090,6 +1110,16 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { cargo.arg("--target-dir").arg(target_dir); + // Args for warning and error forwarding + if builder.build_script.get_forward_rustc_warnings() { + // Quiet to remove all the status messages and only emit errors and warnings + cargo.args(["--quiet"]); + } + if builder.build_script.get_cargo_color_always() { + // Always emit color, since the outer cargo will remove ascii escape sequences if color is turned off + cargo.args(["--color", "always"]); + } + // 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. @@ -1107,10 +1137,20 @@ fn invoke_rustc(builder: &SpirvBuilder) -> Result { num_cgus.to_string(), ); - cargo.stderr(Stdio::inherit()).current_dir(path_to_crate); + if !builder.build_script.get_forward_rustc_warnings() { + cargo.stderr(Stdio::inherit()); + } + cargo.current_dir(path_to_crate); log::debug!("building shaders with `{cargo:?}`"); let build = cargo.output().expect("failed to execute cargo build"); + if builder.build_script.get_forward_rustc_warnings() { + let stderr = String::from_utf8(build.stderr).unwrap(); + for line in stderr.lines() { + println!("cargo::warning={line}"); + } + } + // `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 // that ended up on stdout instead of stderr.