From 848a1983bd259c6d46737f6777c537a8f4e1290b Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 28 May 2025 15:00:22 +1000 Subject: [PATCH 01/13] show-targets comments --- crates/cargo-gpu/src/show.rs | 48 ++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 3b0e2f8..56f1096 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -1,5 +1,7 @@ //! Display various information about `cargo gpu`, eg its cache directory. +use std::process::{Command, Stdio}; + use crate::cache_dir; /// Show the computed source of the spirv-std dependency. @@ -21,6 +23,9 @@ pub enum Info { Commitsh, /// All the available SPIR-V capabilities that can be set with `--capabilities` Capabilities, + + /// All available SPIR-V targets + Targets, } /// `cargo gpu show` @@ -63,6 +68,10 @@ impl Show { println!(" {capability:?}"); } } + Info::Targets => { + let target_info = get_spirv_targets()?.join("\n"); + println!("{}", target_info); + } } Ok(()) @@ -77,3 +86,42 @@ impl Show { (0..=last_capability).filter_map(spirv_builder::Capability::from_u32) } } + +/// Gets available SPIR-V targets by calling the `spirv-tools`' validator: +/// ```sh +/// $ spirv-val --version +/// SPIRV-Tools v2022.2-dev unknown hash, 2022-02-16T16:37:15 +/// Targets: +/// SPIR-V 1.0 +/// SPIR-V 1.1 +/// SPIR-V 1.2 +/// ... snip for brevity +/// SPIR-V 1.6 (under Vulkan 1.3 semantics) +/// ``` +fn get_spirv_targets() -> anyhow::Result> { + // Defaults that have been tested, 1.2 is the existing default in the shader-crate-template.toml + let mut targets = vec![ + "spirv-unknown-vulkan1.0", + "spirv-unknown-vulkan1.1", + "spirv-unknown-vulkan1.2", + ]; + + let output = Command::new("spirv-val") + .arg("--version") + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .output(); + + if let Ok(output) = output { + let version_info = String::from_utf8_lossy(&output.stdout); + if version_info.contains("SPIR-V 1.3") { + targets.push("spirv-unknown-vulkan1.3"); + } + if version_info.contains("SPIR-V 1.4") { + targets.push("spirv-unknown-vulkan1.4"); + } + // Exhaustively, manually put in all possible versions? or regex them out? + } + + Ok(targets.into_iter().map(String::from).collect()) +} From 97b56dd5795f5f65f2b840acb60b03e7f7770124 Mon Sep 17 00:00:00 2001 From: Jer Date: Tue, 3 Jun 2025 17:14:33 +1000 Subject: [PATCH 02/13] TARGET_SPECS has them all at comp time --- crates/cargo-gpu/src/show.rs | 50 +++++++----------------------------- 1 file changed, 9 insertions(+), 41 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 804edc6..5657633 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -1,7 +1,5 @@ //! Display various information about `cargo gpu`, eg its cache directory. -use std::process::{Command, Stdio}; - use crate::cache_dir; /// Show the computed source of the spirv-std dependency. @@ -69,8 +67,9 @@ impl Show { } } Info::Targets => { - let target_info = get_spirv_targets()?.join("\n"); - println!("{}", target_info); + for target in Self::available_spirv_targets_iter() { + println!("{target}"); + } } } @@ -85,43 +84,12 @@ impl Show { let last_capability = spirv_builder::Capability::CacheControlsINTEL as u32; (0..=last_capability).filter_map(spirv_builder::Capability::from_u32) } -} - -/// Gets available SPIR-V targets by calling the `spirv-tools`' validator: -/// ```sh -/// $ spirv-val --version -/// SPIRV-Tools v2022.2-dev unknown hash, 2022-02-16T16:37:15 -/// Targets: -/// SPIR-V 1.0 -/// SPIR-V 1.1 -/// SPIR-V 1.2 -/// ... snip for brevity -/// SPIR-V 1.6 (under Vulkan 1.3 semantics) -/// ``` -fn get_spirv_targets() -> anyhow::Result> { - // Defaults that have been tested, 1.2 is the existing default in the shader-crate-template.toml - let mut targets = vec![ - "spirv-unknown-vulkan1.0", - "spirv-unknown-vulkan1.1", - "spirv-unknown-vulkan1.2", - ]; - let output = Command::new("spirv-val") - .arg("--version") - .stdout(Stdio::piped()) - .stderr(Stdio::piped()) - .output(); - - if let Ok(output) = output { - let version_info = String::from_utf8_lossy(&output.stdout); - if version_info.contains("SPIR-V 1.3") { - targets.push("spirv-unknown-vulkan1.3"); - } - if version_info.contains("SPIR-V 1.4") { - targets.push("spirv-unknown-vulkan1.4"); - } - // Exhaustively, manually put in all possible versions? or regex them out? + /// All supported spirv targets at the time cargo-gpu was compiled. + fn available_spirv_targets_iter() -> impl Iterator { + legacy_target_specs::TARGET_SPECS + .iter() + .filter(|(spec, _src)| spec.contains("vulkan")) + .map(|(spec, _src)| spec.replace(".json", "")) } - - Ok(targets.into_iter().map(String::from).collect()) } From a6170295506d8fefbe6904a17f3d38a2c8edb1d0 Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:30:11 +1000 Subject: [PATCH 03/13] Update methodology of retrieving targets. use the `cache_dir`, but fallback on the legacy const as it's known to be there, currently the cache_dir's presence is not guaranteed. --- crates/cargo-gpu/src/show.rs | 76 +++++++++++++++++++++++++++++++++--- 1 file changed, 70 insertions(+), 6 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 5657633..ab18092 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -1,5 +1,7 @@ //! Display various information about `cargo gpu`, eg its cache directory. +use std::fs; + use crate::cache_dir; /// Show the computed source of the spirv-std dependency. @@ -67,7 +69,7 @@ impl Show { } } Info::Targets => { - for target in Self::available_spirv_targets_iter() { + for target in Self::available_spirv_targets_iter()? { println!("{target}"); } } @@ -85,11 +87,73 @@ impl Show { (0..=last_capability).filter_map(spirv_builder::Capability::from_u32) } - /// All supported spirv targets at the time cargo-gpu was compiled. - fn available_spirv_targets_iter() -> impl Iterator { - legacy_target_specs::TARGET_SPECS + // List all available spirv targets, note: the targets from compile time of cargo-gpu and those + // in the cache-directory will be picked up. + fn available_spirv_targets_iter( + ) -> Result, Box> { + let legacy_targets = legacy_target_specs::TARGET_SPECS .iter() - .filter(|(spec, _src)| spec.contains("vulkan")) - .map(|(spec, _src)| spec.replace(".json", "")) + .map(|(spec, _src)| spec.to_string()); // Convert to String + + let cache_dir = cache_dir()?; + let entries = fs::read_dir(&cache_dir).map_err(|e| { + format!( + "Failed to read cache directory {}: {}", + cache_dir.display(), + e + ) + })?; + + let cached_targets: Vec = entries + .filter_map(|entry| entry.ok()) + .flat_map(|entry| { + let path = entry.path(); + if path.is_dir() { + fs::read_dir(path) + .ok() + .into_iter() + .flatten() + .filter_map(|e| e.ok()) + .filter_map(|e| { + e.path() + .file_stem() + .and_then(|s| s.to_str()) + .map(str::to_owned) + }) + .collect::>() + } else if path.extension().and_then(|s| s.to_str()) == Some("json") { + path.file_stem() + .and_then(|s| s.to_str()) + .map(str::to_owned) + .into_iter() + .collect() + } else { + Vec::new() + } + }) + .collect(); + + if cached_targets.is_empty() { + if !cache_dir.exists() { + log::error!( + "SPIR-V cache directory does not exist: {}", + cache_dir.display() + ); + } + log::error!( + "Cache directory exists but contains no valid SPIR-V target files (*.json): {}", + cache_dir.display() + ); + } + + let mut targets: Vec = legacy_targets + .chain(cached_targets) + .collect::>() + .into_iter() + .filter(|t| t.contains("vulkan")) + .collect(); + + targets.sort(); + Ok(targets.into_iter()) } } From b4ca3e8d1bd571e3021c2626026ec2c9a7b7602c Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:30:48 +1000 Subject: [PATCH 04/13] filter earlier on to do less hashing. --- crates/cargo-gpu/src/show.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index ab18092..a2d9b07 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -148,9 +148,9 @@ impl Show { let mut targets: Vec = legacy_targets .chain(cached_targets) + .filter(|t| t.contains("vulkan")) .collect::>() .into_iter() - .filter(|t| t.contains("vulkan")) .collect(); targets.sort(); From 243b1dda06b2191cc125155f2de4284754b4f22b Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:32:50 +1000 Subject: [PATCH 05/13] check dir exists earlier on. --- crates/cargo-gpu/src/show.rs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index a2d9b07..678e6ca 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -96,6 +96,13 @@ impl Show { .map(|(spec, _src)| spec.to_string()); // Convert to String let cache_dir = cache_dir()?; + if !cache_dir.exists() { + log::error!( + "SPIR-V cache directory does not exist: {}", + cache_dir.display() + ); + } + let entries = fs::read_dir(&cache_dir).map_err(|e| { format!( "Failed to read cache directory {}: {}", @@ -134,12 +141,6 @@ impl Show { .collect(); if cached_targets.is_empty() { - if !cache_dir.exists() { - log::error!( - "SPIR-V cache directory does not exist: {}", - cache_dir.display() - ); - } log::error!( "Cache directory exists but contains no valid SPIR-V target files (*.json): {}", cache_dir.display() From c96b613e0a33ae93ec37186dd7632b9508a97a68 Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:34:49 +1000 Subject: [PATCH 06/13] use same error conventions as already in codebase (anyhow) --- crates/cargo-gpu/src/show.rs | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 678e6ca..27f4bbb 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -2,6 +2,8 @@ use std::fs; +use anyhow::Context; + use crate::cache_dir; /// Show the computed source of the spirv-std dependency. @@ -89,8 +91,7 @@ impl Show { // List all available spirv targets, note: the targets from compile time of cargo-gpu and those // in the cache-directory will be picked up. - fn available_spirv_targets_iter( - ) -> Result, Box> { + fn available_spirv_targets_iter() -> anyhow::Result> { let legacy_targets = legacy_target_specs::TARGET_SPECS .iter() .map(|(spec, _src)| spec.to_string()); // Convert to String @@ -103,13 +104,7 @@ impl Show { ); } - let entries = fs::read_dir(&cache_dir).map_err(|e| { - format!( - "Failed to read cache directory {}: {}", - cache_dir.display(), - e - ) - })?; + let entries = fs::read_dir(&cache_dir)?; let cached_targets: Vec = entries .filter_map(|entry| entry.ok()) From 39774f1c6fb8a01835f7d5916c483c7a9b9ffef6 Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:39:57 +1000 Subject: [PATCH 07/13] docs, cleanups --- crates/cargo-gpu/src/show.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 27f4bbb..a6866e7 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -2,8 +2,6 @@ use std::fs; -use anyhow::Context; - use crate::cache_dir; /// Show the computed source of the spirv-std dependency. @@ -89,12 +87,12 @@ impl Show { (0..=last_capability).filter_map(spirv_builder::Capability::from_u32) } - // List all available spirv targets, note: the targets from compile time of cargo-gpu and those - // in the cache-directory will be picked up. + /// List all available spirv targets, note: the targets from compile time of cargo-gpu and those + /// in the cache-directory will be picked up. fn available_spirv_targets_iter() -> anyhow::Result> { let legacy_targets = legacy_target_specs::TARGET_SPECS .iter() - .map(|(spec, _src)| spec.to_string()); // Convert to String + .map(|(spec, _src)| (*spec).to_string()); let cache_dir = cache_dir()?; if !cache_dir.exists() { @@ -103,11 +101,9 @@ impl Show { cache_dir.display() ); } - let entries = fs::read_dir(&cache_dir)?; - let cached_targets: Vec = entries - .filter_map(|entry| entry.ok()) + .flatten() .flat_map(|entry| { let path = entry.path(); if path.is_dir() { @@ -115,17 +111,18 @@ impl Show { .ok() .into_iter() .flatten() - .filter_map(|e| e.ok()) - .filter_map(|e| { - e.path() + .filter_map(Result::ok) + .filter_map(|entry| { + entry + .path() .file_stem() - .and_then(|s| s.to_str()) + .and_then(std::ffi::OsStr::to_str) .map(str::to_owned) }) .collect::>() - } else if path.extension().and_then(|s| s.to_str()) == Some("json") { + } else if path.extension().and_then(std::ffi::OsStr::to_str) == Some("json") { path.file_stem() - .and_then(|s| s.to_str()) + .and_then(std::ffi::OsStr::to_str) .map(str::to_owned) .into_iter() .collect() @@ -134,21 +131,18 @@ impl Show { } }) .collect(); - if cached_targets.is_empty() { log::error!( "Cache directory exists but contains no valid SPIR-V target files (*.json): {}", cache_dir.display() ); } - let mut targets: Vec = legacy_targets .chain(cached_targets) - .filter(|t| t.contains("vulkan")) + .filter(|target| target.contains("vulkan")) .collect::>() .into_iter() .collect(); - targets.sort(); Ok(targets.into_iter()) } From b438d7d1ec41114c02595efbb9245956557ac2d0 Mon Sep 17 00:00:00 2001 From: jer Date: Wed, 4 Jun 2025 13:43:05 +1000 Subject: [PATCH 08/13] chore: silence clippy --- crates/cargo-gpu/src/show.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index a6866e7..ecdbddf 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -92,7 +92,7 @@ impl Show { fn available_spirv_targets_iter() -> anyhow::Result> { let legacy_targets = legacy_target_specs::TARGET_SPECS .iter() - .map(|(spec, _src)| (*spec).to_string()); + .map(|(spec, _src)| (*spec).to_owned()); let cache_dir = cache_dir()?; if !cache_dir.exists() { @@ -102,6 +102,11 @@ impl Show { ); } let entries = fs::read_dir(&cache_dir)?; + + #[expect( + clippy::shadow_unrelated, + reason = "coz we use 'entry' in repeated nestings" + )] let cached_targets: Vec = entries .flatten() .flat_map(|entry| { From 1b25b5adc29740c82886b6a04e9545fff7872348 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 4 Jun 2025 10:07:32 +0200 Subject: [PATCH 09/13] create `mod target_specs` and move all related code there --- crates/cargo-gpu/src/install.rs | 82 +----------------- crates/cargo-gpu/src/legacy_target_specs.rs | 18 ---- crates/cargo-gpu/src/lib.rs | 2 +- crates/cargo-gpu/src/target_specs.rs | 96 +++++++++++++++++++++ 4 files changed, 99 insertions(+), 99 deletions(-) delete mode 100644 crates/cargo-gpu/src/legacy_target_specs.rs create mode 100644 crates/cargo-gpu/src/target_specs.rs diff --git a/crates/cargo-gpu/src/install.rs b/crates/cargo-gpu/src/install.rs index 50a34e0..fa8bbb9 100644 --- a/crates/cargo-gpu/src/install.rs +++ b/crates/cargo-gpu/src/install.rs @@ -1,12 +1,11 @@ //! Install a dedicated per-shader crate that has the `rust-gpu` compiler in it. -use crate::legacy_target_specs::write_legacy_target_specs; use crate::spirv_source::{ get_channel_from_rustc_codegen_spirv_build_script, query_metadata, FindPackage as _, }; +use crate::target_specs::update_spec_files; use crate::{cache_dir, spirv_source::SpirvSource}; use anyhow::Context as _; -use cargo_metadata::Metadata; use spirv_builder::SpirvBuilder; use std::path::{Path, PathBuf}; @@ -196,83 +195,6 @@ package = "rustc_codegen_spirv" Ok(()) } - /// Copy spec files from one dir to another, assuming no subdirectories - fn copy_spec_files(src: &Path, dst: &Path) -> anyhow::Result<()> { - std::fs::create_dir_all(dst)?; - let dir = std::fs::read_dir(src)?; - for dir_entry in dir { - let file = dir_entry?; - let file_path = file.path(); - if file_path.is_file() { - std::fs::copy(file_path, dst.join(file.file_name()))?; - } - } - Ok(()) - } - - /// Add the target spec files to the crate. - fn update_spec_files( - source: &SpirvSource, - install_dir: &Path, - dummy_metadata: &Metadata, - skip_rebuild: bool, - ) -> anyhow::Result { - let mut target_specs_dst = install_dir.join("target-specs"); - if !skip_rebuild { - if let Ok(target_specs) = - dummy_metadata.find_package("rustc_codegen_spirv-target-specs") - { - log::info!( - "target-specs: found crate `rustc_codegen_spirv-target-specs` with manifest at `{}`", - target_specs.manifest_path - ); - - let target_specs_src = target_specs - .manifest_path - .as_std_path() - .parent() - .and_then(|root| { - let src = root.join("target-specs"); - src.is_dir().then_some(src) - }) - .context("Could not find `target-specs` directory within `rustc_codegen_spirv-target-specs` dependency")?; - if source.is_path() { - // skip copy - log::info!( - "target-specs: source is local path, use target-specs from `{}`", - target_specs_src.display() - ); - target_specs_dst = target_specs_src; - } else { - // copy over the target-specs - log::info!( - "target-specs: Copy target specs from `{}`", - target_specs_src.display() - ); - Self::copy_spec_files(&target_specs_src, &target_specs_dst) - .context("copying target-specs json files")?; - } - } else { - // use legacy target specs bundled with cargo gpu - if source.is_path() { - // This is a stupid situation: - // * We can't be certain that there are `target-specs` in the local checkout (there may be some in `spirv-builder`) - // * We can't dump our legacy ones into the `install_dir`, as that would modify the local rust-gpu checkout - // -> do what the old cargo gpu did, one global dir for all target specs - // and hope parallel runs don't shred each other - target_specs_dst = cache_dir()?.join("legacy-target-specs-for-local-checkout"); - } - log::info!( - "target-specs: Writing legacy target specs to `{}`", - target_specs_dst.display() - ); - write_legacy_target_specs(&target_specs_dst)?; - } - } - - Ok(target_specs_dst) - } - /// Install the binary pair and return the [`InstalledBackend`], from which you can create [`SpirvBuilder`] instances. /// /// # Errors @@ -339,7 +261,7 @@ package = "rustc_codegen_spirv" log::debug!("update_spec_files"); let target_spec_dir = - Self::update_spec_files(&source, &install_dir, &dummy_metadata, skip_rebuild) + update_spec_files(&source, &install_dir, &dummy_metadata, skip_rebuild) .context("writing target spec files")?; if !skip_rebuild { diff --git a/crates/cargo-gpu/src/legacy_target_specs.rs b/crates/cargo-gpu/src/legacy_target_specs.rs deleted file mode 100644 index 796b0e3..0000000 --- a/crates/cargo-gpu/src/legacy_target_specs.rs +++ /dev/null @@ -1,18 +0,0 @@ -//! Legacy target specs are spec jsons for versions before `rustc_codegen_spirv-target-specs` -//! came bundled with them. Instead, cargo gpu needs to bundle these legacy spec files. Luckily, -//! they are the same for all versions, as bundling target specs with the codegen backend was -//! introduced before the first target spec update. - -use anyhow::Context as _; -use std::path::Path; - -/// Extract legacy target specs from our executable into some directory -pub fn write_legacy_target_specs(target_spec_dir: &Path) -> anyhow::Result<()> { - std::fs::create_dir_all(target_spec_dir)?; - for (filename, contents) in legacy_target_specs::TARGET_SPECS { - let path = target_spec_dir.join(filename); - std::fs::write(&path, contents.as_bytes()) - .with_context(|| format!("writing legacy target spec file at [{}]", path.display()))?; - } - Ok(()) -} diff --git a/crates/cargo-gpu/src/lib.rs b/crates/cargo-gpu/src/lib.rs index b3adabf..323743b 100644 --- a/crates/cargo-gpu/src/lib.rs +++ b/crates/cargo-gpu/src/lib.rs @@ -61,12 +61,12 @@ mod config; mod dump_usage; mod install; mod install_toolchain; -mod legacy_target_specs; mod linkage; mod lockfile; mod metadata; mod show; mod spirv_source; +mod target_specs; mod test; pub use install::*; diff --git a/crates/cargo-gpu/src/target_specs.rs b/crates/cargo-gpu/src/target_specs.rs new file mode 100644 index 0000000..f12c064 --- /dev/null +++ b/crates/cargo-gpu/src/target_specs.rs @@ -0,0 +1,96 @@ +//! Legacy target specs are spec jsons for versions before `rustc_codegen_spirv-target-specs` +//! came bundled with them. Instead, cargo gpu needs to bundle these legacy spec files. Luckily, +//! they are the same for all versions, as bundling target specs with the codegen backend was +//! introduced before the first target spec update. + +use crate::cache_dir; +use crate::spirv_source::{FindPackage as _, SpirvSource}; +use anyhow::Context as _; +use cargo_metadata::Metadata; +use std::path::{Path, PathBuf}; + +/// Extract legacy target specs from our executable into some directory +pub fn write_legacy_target_specs(target_spec_dir: &Path) -> anyhow::Result<()> { + std::fs::create_dir_all(target_spec_dir)?; + for (filename, contents) in legacy_target_specs::TARGET_SPECS { + let path = target_spec_dir.join(filename); + std::fs::write(&path, contents.as_bytes()) + .with_context(|| format!("writing legacy target spec file at [{}]", path.display()))?; + } + Ok(()) +} + +/// Copy spec files from one dir to another, assuming no subdirectories +fn copy_spec_files(src: &Path, dst: &Path) -> anyhow::Result<()> { + std::fs::create_dir_all(dst)?; + let dir = std::fs::read_dir(src)?; + for dir_entry in dir { + let file = dir_entry?; + let file_path = file.path(); + if file_path.is_file() { + std::fs::copy(file_path, dst.join(file.file_name()))?; + } + } + Ok(()) +} + +/// Add the target spec files to the crate. +pub fn update_spec_files( + source: &SpirvSource, + install_dir: &Path, + dummy_metadata: &Metadata, + skip_rebuild: bool, +) -> anyhow::Result { + let mut target_specs_dst = install_dir.join("target-specs"); + if !skip_rebuild { + if let Ok(target_specs) = dummy_metadata.find_package("rustc_codegen_spirv-target-specs") { + log::info!( + "target-specs: found crate `rustc_codegen_spirv-target-specs` with manifest at `{}`", + target_specs.manifest_path + ); + + let target_specs_src = target_specs + .manifest_path + .as_std_path() + .parent() + .and_then(|root| { + let src = root.join("target-specs"); + src.is_dir().then_some(src) + }) + .context("Could not find `target-specs` directory within `rustc_codegen_spirv-target-specs` dependency")?; + if source.is_path() { + // skip copy + log::info!( + "target-specs: source is local path, use target-specs from `{}`", + target_specs_src.display() + ); + target_specs_dst = target_specs_src; + } else { + // copy over the target-specs + log::info!( + "target-specs: Copy target specs from `{}`", + target_specs_src.display() + ); + copy_spec_files(&target_specs_src, &target_specs_dst) + .context("copying target-specs json files")?; + } + } else { + // use legacy target specs bundled with cargo gpu + if source.is_path() { + // This is a stupid situation: + // * We can't be certain that there are `target-specs` in the local checkout (there may be some in `spirv-builder`) + // * We can't dump our legacy ones into the `install_dir`, as that would modify the local rust-gpu checkout + // -> do what the old cargo gpu did, one global dir for all target specs + // and hope parallel runs don't shred each other + target_specs_dst = cache_dir()?.join("legacy-target-specs-for-local-checkout"); + } + log::info!( + "target-specs: Writing legacy target specs to `{}`", + target_specs_dst.display() + ); + write_legacy_target_specs(&target_specs_dst)?; + } + } + + Ok(target_specs_dst) +} From ccd2e2554336fa2c2329d90d0121d8729295bcfa Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 4 Jun 2025 10:39:41 +0200 Subject: [PATCH 10/13] refactor `update_spec_files` to allow querying `target_specs` dir without updating --- crates/cargo-gpu/src/install.rs | 7 +-- crates/cargo-gpu/src/target_specs.rs | 92 +++++++++++++++++----------- 2 files changed, 59 insertions(+), 40 deletions(-) diff --git a/crates/cargo-gpu/src/install.rs b/crates/cargo-gpu/src/install.rs index fa8bbb9..d7a6894 100644 --- a/crates/cargo-gpu/src/install.rs +++ b/crates/cargo-gpu/src/install.rs @@ -3,7 +3,7 @@ use crate::spirv_source::{ get_channel_from_rustc_codegen_spirv_build_script, query_metadata, FindPackage as _, }; -use crate::target_specs::update_spec_files; +use crate::target_specs::update_target_specs_files; use crate::{cache_dir, spirv_source::SpirvSource}; use anyhow::Context as _; use spirv_builder::SpirvBuilder; @@ -260,9 +260,8 @@ package = "rustc_codegen_spirv" log::info!("selected toolchain channel `{toolchain_channel:?}`"); log::debug!("update_spec_files"); - let target_spec_dir = - update_spec_files(&source, &install_dir, &dummy_metadata, skip_rebuild) - .context("writing target spec files")?; + let target_spec_dir = update_target_specs_files(&source, &dummy_metadata, !skip_rebuild) + .context("writing target spec files")?; if !skip_rebuild { log::debug!("ensure_toolchain_and_components_exist"); diff --git a/crates/cargo-gpu/src/target_specs.rs b/crates/cargo-gpu/src/target_specs.rs index f12c064..21ed889 100644 --- a/crates/cargo-gpu/src/target_specs.rs +++ b/crates/cargo-gpu/src/target_specs.rs @@ -34,22 +34,29 @@ fn copy_spec_files(src: &Path, dst: &Path) -> anyhow::Result<()> { Ok(()) } -/// Add the target spec files to the crate. -pub fn update_spec_files( +/// Computes the `target-specs` directory to use and updates the target spec files, if enabled. +pub fn update_target_specs_files( source: &SpirvSource, - install_dir: &Path, dummy_metadata: &Metadata, - skip_rebuild: bool, + update_files: bool, ) -> anyhow::Result { - let mut target_specs_dst = install_dir.join("target-specs"); - if !skip_rebuild { - if let Ok(target_specs) = dummy_metadata.find_package("rustc_codegen_spirv-target-specs") { - log::info!( - "target-specs: found crate `rustc_codegen_spirv-target-specs` with manifest at `{}`", - target_specs.manifest_path - ); + log::info!( + "target-specs: Resolving target specs `{}`", + if update_files { + "and update them" + } else { + "without updating" + } + ); + + let mut target_specs_dst = source.install_dir()?.join("target-specs"); + if let Ok(target_specs) = dummy_metadata.find_package("rustc_codegen_spirv-target-specs") { + log::info!( + "target-specs: found crate `rustc_codegen_spirv-target-specs` with manifest at `{}`", + target_specs.manifest_path + ); - let target_specs_src = target_specs + let target_specs_src = target_specs .manifest_path .as_std_path() .parent() @@ -58,34 +65,47 @@ pub fn update_spec_files( src.is_dir().then_some(src) }) .context("Could not find `target-specs` directory within `rustc_codegen_spirv-target-specs` dependency")?; - if source.is_path() { - // skip copy - log::info!( - "target-specs: source is local path, use target-specs from `{}`", - target_specs_src.display() - ); - target_specs_dst = target_specs_src; - } else { - // copy over the target-specs - log::info!( - "target-specs: Copy target specs from `{}`", - target_specs_src.display() - ); + log::info!( + "target-specs: found `rustc_codegen_spirv-target-specs` with `target-specs` directory `{}`", + target_specs_dst.display() + ); + + if source.is_path() { + // skip copy + log::info!( + "target-specs resolution: source is local path, use target-specs directly from `{}`", + target_specs_dst.display() + ); + target_specs_dst = target_specs_src; + } else { + // copy over the target-specs + log::info!( + "target-specs resolution: coping target-specs from `{}`{}", + target_specs_dst.display(), + if update_files { "" } else { " was skipped" } + ); + if update_files { copy_spec_files(&target_specs_src, &target_specs_dst) .context("copying target-specs json files")?; } - } else { - // use legacy target specs bundled with cargo gpu - if source.is_path() { - // This is a stupid situation: - // * We can't be certain that there are `target-specs` in the local checkout (there may be some in `spirv-builder`) - // * We can't dump our legacy ones into the `install_dir`, as that would modify the local rust-gpu checkout - // -> do what the old cargo gpu did, one global dir for all target specs - // and hope parallel runs don't shred each other - target_specs_dst = cache_dir()?.join("legacy-target-specs-for-local-checkout"); - } + } + } else { + // use legacy target specs bundled with cargo gpu + if source.is_path() { + // This is a stupid situation: + // * We can't be certain that there are `target-specs` in the local checkout (there may be some in `spirv-builder`) + // * We can't dump our legacy ones into the `install_dir`, as that would modify the local rust-gpu checkout + // -> do what the old cargo gpu did, one global dir for all target specs + // and hope parallel runs don't shred each other + target_specs_dst = cache_dir()?.join("legacy-target-specs-for-local-checkout"); + } + log::info!( + "target-specs resolution: legacy target specs in directory `{}`", + target_specs_dst.display() + ); + if update_files { log::info!( - "target-specs: Writing legacy target specs to `{}`", + "target-specs: Writing legacy target specs into `{}`", target_specs_dst.display() ); write_legacy_target_specs(&target_specs_dst)?; From 76aa6af51b9a4c7018d8bac74aef65e33040eb74 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 4 Jun 2025 10:40:37 +0200 Subject: [PATCH 11/13] make `show targets` use new `target_specs` path querying --- crates/cargo-gpu/src/show.rs | 93 ++++++++++++------------------------ 1 file changed, 30 insertions(+), 63 deletions(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index ecdbddf..943f32d 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -1,8 +1,11 @@ //! Display various information about `cargo gpu`, eg its cache directory. -use std::fs; - use crate::cache_dir; +use crate::spirv_source::{query_metadata, SpirvSource}; +use crate::target_specs::update_target_specs_files; +use anyhow::bail; +use std::fs; +use std::path::Path; /// Show the computed source of the spirv-std dependency. #[derive(Clone, Debug, clap::Parser)] @@ -25,7 +28,7 @@ pub enum Info { Capabilities, /// All available SPIR-V targets - Targets, + Targets(SpirvSourceDep), } /// `cargo gpu show` @@ -51,8 +54,7 @@ impl Show { println!("{}\n", cache_dir()?.display()); } Info::SpirvSource(SpirvSourceDep { shader_crate }) => { - let rust_gpu_source = - crate::spirv_source::SpirvSource::get_rust_gpu_deps_from_shader(shader_crate)?; + let rust_gpu_source = SpirvSource::get_rust_gpu_deps_from_shader(shader_crate)?; println!("{rust_gpu_source}\n"); } Info::Commitsh => { @@ -68,8 +70,10 @@ impl Show { println!(" {capability:?}"); } } - Info::Targets => { - for target in Self::available_spirv_targets_iter()? { + Info::Targets(SpirvSourceDep { shader_crate }) => { + let (source, targets) = Self::available_spirv_targets_iter(shader_crate)?; + println!("All available targets for rust-gpu version '{}':", source); + for target in targets { println!("{target}"); } } @@ -89,66 +93,29 @@ impl Show { /// List all available spirv targets, note: the targets from compile time of cargo-gpu and those /// in the cache-directory will be picked up. - fn available_spirv_targets_iter() -> anyhow::Result> { - let legacy_targets = legacy_target_specs::TARGET_SPECS - .iter() - .map(|(spec, _src)| (*spec).to_owned()); - - let cache_dir = cache_dir()?; - if !cache_dir.exists() { - log::error!( - "SPIR-V cache directory does not exist: {}", - cache_dir.display() - ); + fn available_spirv_targets_iter( + shader_crate: &Path, + ) -> anyhow::Result<(SpirvSource, impl Iterator)> { + let source = SpirvSource::new(shader_crate, None, None)?; + let install_dir = source.install_dir()?; + if !install_dir.is_dir() { + bail!("rust-gpu version {} is not installed", source); } - let entries = fs::read_dir(&cache_dir)?; + let dummy_metadata = query_metadata(&install_dir)?; + let target_specs_dir = update_target_specs_files(&source, &dummy_metadata, false)?; - #[expect( - clippy::shadow_unrelated, - reason = "coz we use 'entry' in repeated nestings" - )] - let cached_targets: Vec = entries - .flatten() - .flat_map(|entry| { - let path = entry.path(); - if path.is_dir() { - fs::read_dir(path) - .ok() - .into_iter() - .flatten() - .filter_map(Result::ok) - .filter_map(|entry| { - entry - .path() - .file_stem() - .and_then(std::ffi::OsStr::to_str) - .map(str::to_owned) - }) - .collect::>() - } else if path.extension().and_then(std::ffi::OsStr::to_str) == Some("json") { - path.file_stem() - .and_then(std::ffi::OsStr::to_str) - .map(str::to_owned) - .into_iter() - .collect() - } else { - Vec::new() + let mut targets = fs::read_dir(target_specs_dir)? + .filter_map(|entry| { + let file = entry.ok()?; + if file.path().is_file() { + if let Some(target) = file.file_name().to_string_lossy().strip_suffix(".json") { + return Some(target.to_owned()); + } } + None }) - .collect(); - if cached_targets.is_empty() { - log::error!( - "Cache directory exists but contains no valid SPIR-V target files (*.json): {}", - cache_dir.display() - ); - } - let mut targets: Vec = legacy_targets - .chain(cached_targets) - .filter(|target| target.contains("vulkan")) - .collect::>() - .into_iter() - .collect(); + .collect::>(); targets.sort(); - Ok(targets.into_iter()) + Ok((source, targets.into_iter())) } } From 37e32c36ccb71a541c8da43ae2e4fc648629ec4f Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 4 Jun 2025 10:43:43 +0200 Subject: [PATCH 12/13] fix clippy --- crates/cargo-gpu/src/show.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index 943f32d..b7eb19b 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -72,7 +72,7 @@ impl Show { } Info::Targets(SpirvSourceDep { shader_crate }) => { let (source, targets) = Self::available_spirv_targets_iter(shader_crate)?; - println!("All available targets for rust-gpu version '{}':", source); + println!("All available targets for rust-gpu version '{source}':"); for target in targets { println!("{target}"); } From 042da7e7b15bc58d6d677a7bbfd66138a0ee57a1 Mon Sep 17 00:00:00 2001 From: Firestar99 Date: Wed, 4 Jun 2025 17:27:20 +0200 Subject: [PATCH 13/13] update docs on target specs --- crates/cargo-gpu/src/target_specs.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/cargo-gpu/src/target_specs.rs b/crates/cargo-gpu/src/target_specs.rs index 21ed889..00deeef 100644 --- a/crates/cargo-gpu/src/target_specs.rs +++ b/crates/cargo-gpu/src/target_specs.rs @@ -1,7 +1,26 @@ -//! Legacy target specs are spec jsons for versions before `rustc_codegen_spirv-target-specs` -//! came bundled with them. Instead, cargo gpu needs to bundle these legacy spec files. Luckily, -//! they are the same for all versions, as bundling target specs with the codegen backend was -//! introduced before the first target spec update. +//! This module deals with target specs, which are json metadata files that need to be passed to +//! rustc to add foreign targets such as `spirv_unknown_vulkan1.2`. +//! +//! There are 4 version ranges of `rustc_codegen_spirv` and they all need different handling of +//! their target specs: +//! * "ancient" versions such as 0.9.0 or earlier do not need target specs, just passing the target +//! string (`spirv-unknown-vulkan1.2`) directly is sufficient. We still prep target-specs for them +//! like the "legacy" variant below, spirv-builder +//! [will just ignore it](https://github.com/Rust-GPU/rust-gpu/blob/369122e1703c0c32d3d46f46fa11ccf12667af03/crates/spirv-builder/src/lib.rs#L987) +//! * "legacy" versions require target specs to compile, which is a requirement introduced by some +//! rustc version. Back then it was decided that cargo gpu would ship them, as they'd probably +//! never change, right? So now we're stuck with having to ship these "legacy" target specs with +//! cargo gpu *forever*. These are the symbol `legacy_target_specs::TARGET_SPECS`, with +//! `legacy_target_specs` being a **fixed** version of `rustc_codegen_spirv-target-specs`, +//! which must **never** update. +//! * As of [PR 256](https://github.com/Rust-GPU/rust-gpu/pull/256), `rustc_codegen_spirv` now has +//! a direct dependency on `rustc_codegen_spirv-target-specs`, allowing cargo gpu to pull the +//! required target specs directly from that dependency. At this point, the target specs are +//! still the same as the legacy target specs. +//! * The [edition 2024 PR](https://github.com/Rust-GPU/rust-gpu/pull/249) must update the +//! target specs to comply with newly added validation within rustc. This is why the new system +//! was implemented, so we can support both old and new target specs without having to worry +//! which version of cargo gpu you are using. It'll "just work". use crate::cache_dir; use crate::spirv_source::{FindPackage as _, SpirvSource};