From 5c544f87bccb850211f01c5f410d85fac9a9ad77 Mon Sep 17 00:00:00 2001 From: Thomas Buckley-Houston Date: Sat, 21 Dec 2024 17:09:22 +0100 Subject: [PATCH 1/2] Refactor out all panics. Use `anyhow` instead. --- Cargo.lock | 7 ++ Cargo.toml | 8 +- clippy.toml | 2 + crates/cargo-gpu/Cargo.toml | 1 + crates/cargo-gpu/src/build.rs | 80 +++++++------ crates/cargo-gpu/src/install.rs | 79 ++++++------ crates/cargo-gpu/src/main.rs | 68 ++++++----- crates/cargo-gpu/src/show.rs | 8 +- crates/cargo-gpu/src/spirv_cli.rs | 54 ++++----- crates/cargo-gpu/src/spirv_source.rs | 172 ++++++++++++++++----------- crates/cargo-gpu/src/toml.rs | 79 ++++++------ crates/spirv-builder-cli/src/main.rs | 2 +- 12 files changed, 308 insertions(+), 252 deletions(-) create mode 100644 clippy.toml diff --git a/Cargo.lock b/Cargo.lock index 88ba813..ec3345b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -60,6 +60,12 @@ dependencies = [ "windows-sys 0.59.0", ] +[[package]] +name = "anyhow" +version = "1.0.94" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1fd03a028ef38ba2276dce7e33fcd6369c158a1bca17946c4b1b701891c1ff7" + [[package]] name = "autocfg" version = "1.4.0" @@ -82,6 +88,7 @@ checksum = "325918d6fe32f23b19878fe4b34794ae41fc19ddbe53b10571a4874d44ffd39b" name = "cargo-gpu" version = "0.1.0" dependencies = [ + "anyhow", "chrono", "clap", "directories", diff --git a/Cargo.toml b/Cargo.toml index f8122de..a97e989 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,8 +14,9 @@ exclude = [ resolver = "2" [workspace.dependencies] +anyhow = "1.0.94" clap = { version = "4.4.8", features = ["derive"] } -chrono = { version = "0.4.38", default-features = false } +chrono = { version = "0.4.38", default-features = false, features = ["std"] } directories = "5.0.1" env_home = "0.1.0" env_logger = "0.10" @@ -51,9 +52,4 @@ pattern_type_mismatch = { level = "allow", priority = 1 } print_stdout = { level = "allow", priority = 1 } std_instead_of_alloc = { level = "allow", priority = 1 } -# TODO: Try to not depend on allowing these lints -unwrap_used = { level = "allow", priority = 1 } -get_unwrap = { level = "allow", priority = 1 } -expect_used = { level = "allow", priority = 1 } -panic = { level = "allow", priority = 1 } diff --git a/clippy.toml b/clippy.toml new file mode 100644 index 0000000..f69b4a6 --- /dev/null +++ b/clippy.toml @@ -0,0 +1,2 @@ +allow-unwrap-in-tests = true +allow-panic-in-tests = true diff --git a/crates/cargo-gpu/Cargo.toml b/crates/cargo-gpu/Cargo.toml index e6043d1..e7ad33c 100644 --- a/crates/cargo-gpu/Cargo.toml +++ b/crates/cargo-gpu/Cargo.toml @@ -9,6 +9,7 @@ keywords = ["gpu", "compiler"] # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html [dependencies] +anyhow.workspace = true spirv-builder-cli = { path = "../spirv-builder-cli", default-features = false } clap.workspace = true directories.workspace = true diff --git a/crates/cargo-gpu/src/build.rs b/crates/cargo-gpu/src/build.rs index d26718a..b6f958a 100644 --- a/crates/cargo-gpu/src/build.rs +++ b/crates/cargo-gpu/src/build.rs @@ -2,6 +2,7 @@ use std::io::Write as _; +use anyhow::Context as _; use clap::Parser; use spirv_builder_cli::{Linkage, ShaderModule}; @@ -33,35 +34,34 @@ pub struct Build { impl Build { /// Entrypoint - pub fn run(&mut self) { - let (dylib_path, spirv_builder_cli_path) = self.install.run(); + pub fn run(&mut self) -> anyhow::Result<()> { + let (dylib_path, spirv_builder_cli_path) = self.install.run()?; // Ensure the shader output dir exists log::debug!("ensuring output-dir '{}' exists", self.output_dir.display()); - std::fs::create_dir_all(&self.output_dir).unwrap(); - self.output_dir = self.output_dir.canonicalize().unwrap(); + std::fs::create_dir_all(&self.output_dir)?; + self.output_dir = self.output_dir.canonicalize()?; // Ensure the shader crate exists - self.install.shader_crate = self.install.shader_crate.canonicalize().unwrap(); - assert!( + self.install.shader_crate = self.install.shader_crate.canonicalize()?; + anyhow::ensure!( self.install.shader_crate.exists(), "shader crate '{}' does not exist. (Current dir is '{}')", self.install.shader_crate.display(), - std::env::current_dir().unwrap().display() + std::env::current_dir()?.display() ); let spirv_builder_args = spirv_builder_cli::Args { dylib_path, shader_crate: self.install.shader_crate.clone(), shader_target: self.shader_target.clone(), - path_to_target_spec: target_spec_dir().join(format!("{}.json", self.shader_target)), + path_to_target_spec: target_spec_dir()?.join(format!("{}.json", self.shader_target)), no_default_features: self.no_default_features, features: self.features.clone(), output_dir: self.output_dir.clone(), }; - // UNWRAP: safe because we know this always serializes - let arg = serde_json::to_string_pretty(&spirv_builder_args).unwrap(); + let arg = serde_json::to_string_pretty(&spirv_builder_args)?; log::info!("using spirv-builder-cli arg: {arg}"); // Call spirv-builder-cli to compile the shaders. @@ -69,9 +69,8 @@ impl Build { .arg(arg) .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) - .output() - .unwrap(); - assert!(output.status.success(), "build failed"); + .output()?; + anyhow::ensure!(output.status.success(), "build failed"); let spirv_manifest = self.output_dir.join("spirv-manifest.json"); if spirv_manifest.is_file() { @@ -81,51 +80,52 @@ impl Build { ); } else { log::error!("missing raw manifest '{}'", spirv_manifest.display()); - panic!("missing raw manifest"); + anyhow::bail!("missing raw manifest"); } let shaders: Vec = - serde_json::from_reader(std::fs::File::open(&spirv_manifest).unwrap()).unwrap(); + serde_json::from_reader(std::fs::File::open(&spirv_manifest)?)?; - let mut linkage: Vec<_> = shaders + let mut linkage: Vec = shaders .into_iter() .map( |ShaderModule { entry, path: filepath, - }| { + }| + -> anyhow::Result { use relative_path::PathExt as _; - let path = self.output_dir.join(filepath.file_name().unwrap()); - std::fs::copy(&filepath, &path).unwrap(); - let path_relative_to_shader_crate = path - .relative_to(&self.install.shader_crate) - .unwrap() - .to_path(""); - Linkage::new(entry, path_relative_to_shader_crate) + let path = self.output_dir.join( + filepath + .file_name() + .context("Couldn't parse file name from shader module path")?, + ); + std::fs::copy(&filepath, &path)?; + let path_relative_to_shader_crate = + path.relative_to(&self.install.shader_crate)?.to_path(""); + Ok(Linkage::new(entry, path_relative_to_shader_crate)) }, ) - .collect(); + .collect::>>()?; // Write the shader manifest json file let manifest_path = self.output_dir.join("manifest.json"); // Sort the contents so the output is deterministic linkage.sort(); // UNWRAP: safe because we know this always serializes - let json = serde_json::to_string_pretty(&linkage).unwrap(); - let mut file = std::fs::File::create(&manifest_path).unwrap_or_else(|error| { - log::error!( - "could not create shader manifest file '{}': {error}", + let json = serde_json::to_string_pretty(&linkage)?; + let mut file = std::fs::File::create(&manifest_path).with_context(|| { + format!( + "could not create shader manifest file '{}'", manifest_path.display(), - ); - panic!("{error}") - }); - file.write_all(json.as_bytes()).unwrap_or_else(|error| { - log::error!( - "could not write shader manifest file '{}': {error}", + ) + })?; + file.write_all(json.as_bytes()).with_context(|| { + format!( + "could not write shader manifest file '{}'", manifest_path.display(), - ); - panic!("{error}") - }); + ) + })?; log::info!("wrote manifest to '{}'", manifest_path.display()); @@ -134,8 +134,10 @@ impl Build { "removing spirv-manifest.json file '{}'", spirv_manifest.display() ); - std::fs::remove_file(spirv_manifest).unwrap(); + std::fs::remove_file(spirv_manifest)?; } + + Ok(()) } } diff --git a/crates/cargo-gpu/src/install.rs b/crates/cargo-gpu/src/install.rs index ca73728..ce02d73 100644 --- a/crates/cargo-gpu/src/install.rs +++ b/crates/cargo-gpu/src/install.rs @@ -1,6 +1,8 @@ //! Install a dedicated per-shader crate that has the `rust-gpu` compiler in it. use std::io::Write as _; +use anyhow::Context as _; + use crate::{cache_dir, spirv_cli::SpirvCli, spirv_source::SpirvSource, target_spec_dir}; /// These are the files needed to create the dedicated, per-shader `rust-gpu` builder create. @@ -120,7 +122,7 @@ pub struct Install { impl Install { /// Returns a [`SpirvCLI`] instance, responsible for ensuring the right version of the `spirv-builder-cli` crate. - fn spirv_cli(&self, shader_crate_path: &std::path::PathBuf) -> SpirvCli { + fn spirv_cli(&self, shader_crate_path: &std::path::PathBuf) -> anyhow::Result { SpirvCli::new( shader_crate_path, self.spirv_builder_source.clone(), @@ -130,20 +132,21 @@ impl Install { } /// Create the `spirv-builder-cli` crate. - fn write_source_files(&self) { - let spirv_cli = self.spirv_cli(&self.shader_crate); - let checkout = spirv_cli.cached_checkout_path(); - std::fs::create_dir_all(checkout.join("src")).unwrap(); + fn write_source_files(&self) -> anyhow::Result<()> { + let spirv_cli = self.spirv_cli(&self.shader_crate)?; + let checkout = spirv_cli.cached_checkout_path()?; + std::fs::create_dir_all(checkout.join("src"))?; for (filename, contents) in SPIRV_BUILDER_FILES { log::debug!("writing {filename}"); let path = checkout.join(filename); - let mut file = std::fs::File::create(&path).unwrap(); + let mut file = std::fs::File::create(&path)?; let mut replaced_contents = contents.replace("${CHANNEL}", &spirv_cli.channel); if filename == &"Cargo.toml" { replaced_contents = Self::update_cargo_toml(&replaced_contents, &spirv_cli.source); } - file.write_all(replaced_contents.as_bytes()).unwrap(); + file.write_all(replaced_contents.as_bytes())?; } + Ok(()) } /// Update the `Cargo.toml` file in the `spirv-builder-cli` crate so that it contains @@ -176,33 +179,30 @@ impl Install { } /// Add the target spec files to the crate. - fn write_target_spec_files(&self) { + fn write_target_spec_files(&self) -> anyhow::Result<()> { for (filename, contents) in TARGET_SPECS { - let path = target_spec_dir().join(filename); + let path = target_spec_dir()?.join(filename); if !path.is_file() || self.force_spirv_cli_rebuild { - let mut file = std::fs::File::create(&path).unwrap(); - file.write_all(contents.as_bytes()).unwrap(); + let mut file = std::fs::File::create(&path)?; + file.write_all(contents.as_bytes())?; } } + Ok(()) } /// Install the binary pair and return the paths, (dylib, cli). - pub fn run(&self) -> (std::path::PathBuf, std::path::PathBuf) { + pub fn run(&self) -> anyhow::Result<(std::path::PathBuf, std::path::PathBuf)> { // Ensure the cache dir exists - let cache_dir = cache_dir(); + let cache_dir = cache_dir()?; log::info!("cache directory is '{}'", cache_dir.display()); - std::fs::create_dir_all(&cache_dir).unwrap_or_else(|error| { - log::error!( - "could not create cache directory '{}': {error}", - cache_dir.display() - ); - panic!("could not create cache dir"); - }); + std::fs::create_dir_all(&cache_dir).with_context(|| { + format!("could not create cache directory '{}'", cache_dir.display()) + })?; - let spirv_version = self.spirv_cli(&self.shader_crate); - spirv_version.ensure_toolchain_and_components_exist(); + let spirv_version = self.spirv_cli(&self.shader_crate)?; + spirv_version.ensure_toolchain_and_components_exist()?; - let checkout = spirv_version.cached_checkout_path(); + let checkout = spirv_version.cached_checkout_path()?; let release = checkout.join("target").join("release"); let dylib_filename = format!( @@ -227,8 +227,8 @@ impl Install { "writing spirv-builder-cli source files into '{}'", checkout.display() ); - self.write_source_files(); - self.write_target_spec_files(); + self.write_source_files()?; + self.write_target_spec_files()?; let mut command = std::process::Command::new("cargo"); command @@ -239,7 +239,7 @@ impl Install { command.args([ "--features", - &Self::get_required_spirv_builder_version(spirv_version.date), + &Self::get_required_spirv_builder_version(spirv_version.date)?, ]); log::debug!("building artifacts with `{:?}`", command); @@ -247,16 +247,15 @@ impl Install { let output = command .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) - .output() - .unwrap(); - assert!(output.status.success(), "...build error!"); + .output()?; + anyhow::ensure!(output.status.success(), "...build error!"); if dylib_path.is_file() { log::info!("successfully built {}", dylib_path.display()); - std::fs::rename(&dylib_path, &dest_dylib_path).unwrap(); + std::fs::rename(&dylib_path, &dest_dylib_path)?; } else { log::error!("could not find {}", dylib_path.display()); - panic!("spirv-builder-cli build failed"); + anyhow::bail!("spirv-builder-cli build failed"); } let cli_path = if cfg!(target_os = "windows") { @@ -266,18 +265,18 @@ impl Install { }; if cli_path.is_file() { log::info!("successfully built {}", cli_path.display()); - std::fs::rename(&cli_path, &dest_cli_path).unwrap(); + std::fs::rename(&cli_path, &dest_cli_path)?; } else { log::error!("could not find {}", cli_path.display()); log::debug!("contents of '{}':", release.display()); - for maybe_entry in std::fs::read_dir(&release).unwrap() { - let entry = maybe_entry.unwrap(); + for maybe_entry in std::fs::read_dir(&release)? { + let entry = maybe_entry?; log::debug!("{}", entry.file_name().to_string_lossy()); } - panic!("spirv-builder-cli build failed"); + anyhow::bail!("spirv-builder-cli build failed"); } } - (dest_dylib_path, dest_cli_path) + Ok((dest_dylib_path, dest_cli_path)) } /// The `spirv-builder` crate from the main `rust-gpu` repo hasn't always been setup to @@ -287,15 +286,15 @@ impl Install { /// TODO: /// * Warn the user that certain `cargo-gpu` features aren't available when building with /// older versions of `spirv-builder`, eg setting the target spec. - fn get_required_spirv_builder_version(date: chrono::NaiveDate) -> String { + fn get_required_spirv_builder_version(date: chrono::NaiveDate) -> anyhow::Result { let parse_date = chrono::NaiveDate::parse_from_str; - let pre_cli_date = parse_date("2024-04-24", "%Y-%m-%d").unwrap(); + let pre_cli_date = parse_date("2024-04-24", "%Y-%m-%d")?; - if date < pre_cli_date { + Ok(if date < pre_cli_date { "spirv-builder-pre-cli" } else { "spirv-builder-0_10" } - .into() + .into()) } } diff --git a/crates/cargo-gpu/src/main.rs b/crates/cargo-gpu/src/main.rs index 364b3e1..86167c0 100644 --- a/crates/cargo-gpu/src/main.rs +++ b/crates/cargo-gpu/src/main.rs @@ -50,6 +50,8 @@ //! conduct other post-processing, like converting the `spv` files into `wgsl` files, //! for example. +use anyhow::Context as _; + use build::Build; use clap::Parser as _; use install::Install; @@ -63,7 +65,7 @@ mod spirv_cli; mod spirv_source; mod toml; -fn main() { +fn main() -> anyhow::Result<()> { env_logger::builder().init(); let args = std::env::args() @@ -79,19 +81,21 @@ fn main() { match cli.command { Command::Install(install) => { log::debug!("installing with arguments: {install:#?}"); - let (_, _) = install.run(); + let (_, _) = install.run()?; } Command::Build(mut build) => { log::debug!("building with arguments: {build:#?}"); - build.run(); + build.run()?; } Command::Toml(toml) => { log::debug!("building by toml file with arguments: {toml:#?}"); - toml.run(); + toml.run()?; } - Command::Show(show) => show.run(), - Command::DumpUsage => dump_full_usage_for_readme(), - } + Command::Show(show) => show.run()?, + Command::DumpUsage => dump_full_usage_for_readme()?, + }; + + Ok(()) } /// All of the available subcommands for `cargo gpu` @@ -125,48 +129,51 @@ pub(crate) struct Cli { command: Command, } -fn cache_dir() -> std::path::PathBuf { +fn cache_dir() -> anyhow::Result { let dir = directories::BaseDirs::new() - .unwrap_or_else(|| { - log::error!("could not find the user home directory"); - panic!("cache_dir failed"); - }) + .with_context(|| "could not find the user home directory")? .cache_dir() .join("rust-gpu"); - if cfg!(test) { + Ok(if cfg!(test) { let thread_id = std::thread::current().id(); let id = format!("{thread_id:?}").replace('(', "-").replace(')', ""); dir.join("tests").join(id) } else { dir - } + }) } /// Location of the target spec metadata files -fn target_spec_dir() -> std::path::PathBuf { - let dir = cache_dir().join("target-specs"); - std::fs::create_dir_all(&dir).unwrap(); - dir +fn target_spec_dir() -> anyhow::Result { + let dir = cache_dir()?.join("target-specs"); + std::fs::create_dir_all(&dir)?; + Ok(dir) } /// Convenience function for internal use. Dumps all the CLI usage instructions. Useful for /// updating the README. -fn dump_full_usage_for_readme() { +fn dump_full_usage_for_readme() -> anyhow::Result<()> { use clap::CommandFactory as _; let mut command = Cli::command(); let mut buffer: Vec = Vec::default(); command.build(); - write_help(&mut buffer, &mut command, 0); - println!("{}", String::from_utf8(buffer).unwrap()); + write_help(&mut buffer, &mut command, 0)?; + println!("{}", String::from_utf8(buffer)?); + + Ok(()) } /// Recursive function to print the usage instructions for each subcommand. -fn write_help(buffer: &mut impl std::io::Write, cmd: &mut clap::Command, _depth: usize) { +fn write_help( + buffer: &mut impl std::io::Write, + cmd: &mut clap::Command, + _depth: usize, +) -> anyhow::Result<()> { if cmd.get_name() == "help" { - return; + return Ok(()); } let mut command = cmd.get_name().to_owned(); @@ -175,16 +182,17 @@ fn write_help(buffer: &mut impl std::io::Write, cmd: &mut clap::Command, _depth: "\n* {}{}", command.remove(0).to_uppercase(), command - ) - .unwrap(); - writeln!(buffer).unwrap(); - cmd.write_long_help(buffer).unwrap(); + )?; + writeln!(buffer)?; + cmd.write_long_help(buffer)?; for sub in cmd.get_subcommands_mut() { - writeln!(buffer).unwrap(); + writeln!(buffer)?; #[expect(clippy::used_underscore_binding, reason = "Used in recursion only")] - write_help(buffer, sub, _depth + 1); + write_help(buffer, sub, _depth + 1)?; } + + Ok(()) } /// Returns a string suitable to use as a directory. @@ -210,7 +218,7 @@ mod test { } pub fn tests_teardown() { - let cache_dir = cache_dir(); + let cache_dir = cache_dir().unwrap(); if !cache_dir.exists() { return; } diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index cc1e9d8..c1668e2 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -29,17 +29,19 @@ pub struct Show { impl Show { /// Entrypoint - pub fn run(self) { + pub fn run(self) -> anyhow::Result<()> { log::info!("{:?}: ", self.command); match self.command { Info::CacheDirectory => { - println!("{}", cache_dir().display()); + println!("{}", cache_dir()?.display()); } Info::SpirvSource(SpirvSourceDep { shader_crate }) => { let rust_gpu_source = - crate::spirv_source::SpirvSource::get_spirv_std_dep_definition(&shader_crate); + crate::spirv_source::SpirvSource::get_spirv_std_dep_definition(&shader_crate)?; println!("{rust_gpu_source}"); } } + + Ok(()) } } diff --git a/crates/cargo-gpu/src/spirv_cli.rs b/crates/cargo-gpu/src/spirv_cli.rs index 52c4c0e..bc26ed9 100644 --- a/crates/cargo-gpu/src/spirv_cli.rs +++ b/crates/cargo-gpu/src/spirv_cli.rs @@ -1,6 +1,8 @@ //! Query the shader crate to find what version of `rust-gpu` it depends on. //! Then ensure that the relevant Rust toolchain and components are installed. +use anyhow::Context as _; + use crate::spirv_source::SpirvSource; /// Cargo dependency for `spirv-builder` and the rust toolchain channel. @@ -40,9 +42,9 @@ impl SpirvCli { maybe_rust_gpu_source: Option, maybe_rust_gpu_version: Option, maybe_rust_gpu_channel: Option, - ) -> Self { + ) -> anyhow::Result { let (default_rust_gpu_source, rust_gpu_date, default_rust_gpu_channel) = - SpirvSource::get_rust_gpu_deps_from_shader(shader_crate_path); + SpirvSource::get_rust_gpu_deps_from_shader(shader_crate_path)?; let mut maybe_spirv_source: Option = None; if let Some(rust_gpu_version) = maybe_rust_gpu_version { @@ -56,27 +58,23 @@ impl SpirvCli { maybe_spirv_source = Some(source); } - Self { + Ok(Self { source: maybe_spirv_source.unwrap_or(default_rust_gpu_source), channel: maybe_rust_gpu_channel.unwrap_or(default_rust_gpu_channel), date: rust_gpu_date, - } + }) } /// Create and/or return the cache directory - pub fn cached_checkout_path(&self) -> std::path::PathBuf { - let checkout_dir = crate::cache_dir() + pub fn cached_checkout_path(&self) -> anyhow::Result { + let checkout_dir = crate::cache_dir()? .join("spirv-builder-cli") .join(crate::to_dirname(self.to_string().as_ref())); - std::fs::create_dir_all(&checkout_dir).unwrap_or_else(|error| { - log::error!( - "could not create checkout dir '{}': {error}", - checkout_dir.display() - ); - panic!("could not create checkout dir"); - }); + std::fs::create_dir_all(&checkout_dir).with_context(|| { + format!("could not create checkout dir '{}'", checkout_dir.display()) + })?; - checkout_dir + Ok(checkout_dir) } /// Use `rustup` to install the toolchain and components, if not already installed. @@ -85,13 +83,12 @@ impl SpirvCli { /// /// * rustup toolchain add nightly-2024-04-24 /// * rustup component add --toolchain nightly-2024-04-24 rust-src rustc-dev llvm-tools - pub fn ensure_toolchain_and_components_exist(&self) { + pub fn ensure_toolchain_and_components_exist(&self) -> anyhow::Result<()> { // Check for the required toolchain let output_toolchain_list = std::process::Command::new("rustup") .args(["toolchain", "list"]) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_toolchain_list.status.success(), "could not list installed toolchains" ); @@ -107,9 +104,8 @@ impl SpirvCli { .arg(&self.channel) .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_toolchain_add.status.success(), "could not install required toolchain" ); @@ -119,9 +115,8 @@ impl SpirvCli { let output_component_list = std::process::Command::new("rustup") .args(["component", "list", "--toolchain"]) .arg(&self.channel) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_component_list.status.success(), "could not list installed components" ); @@ -144,13 +139,14 @@ impl SpirvCli { .args(["rust-src", "rustc-dev", "llvm-tools"]) .stdout(std::process::Stdio::inherit()) .stderr(std::process::Stdio::inherit()) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_component_add.status.success(), "could not install required components" ); } + + Ok(()) } } @@ -161,8 +157,8 @@ mod test { #[test_log::test] fn cached_checkout_dir_sanity() { let shader_template_path = crate::test::shader_crate_template_path(); - let spirv = SpirvCli::new(&shader_template_path, None, None, None); - let dir = spirv.cached_checkout_path(); + let spirv = SpirvCli::new(&shader_template_path, None, None, None).unwrap(); + let dir = spirv.cached_checkout_path().unwrap(); let name = dir .file_name() .unwrap() diff --git a/crates/cargo-gpu/src/spirv_source.rs b/crates/cargo-gpu/src/spirv_source.rs index bd2f200..f6e0ac3 100644 --- a/crates/cargo-gpu/src/spirv_source.rs +++ b/crates/cargo-gpu/src/spirv_source.rs @@ -4,6 +4,8 @@ //! version. Then with that we `git checkout` the `rust-gpu` repo that corresponds to that version. //! From there we can look at the source code to get the required Rust toolchain. +use anyhow::Context as _; + /// The canonical `rust-gpu` URI const RUST_GPU_REPO: &str = "https://github.com/Rust-GPU/rust-gpu"; @@ -50,18 +52,18 @@ impl SpirvSource { /// Look into the shader crate to get the version of `rust-gpu` it's using. pub fn get_rust_gpu_deps_from_shader( shader_crate_path: &std::path::PathBuf, - ) -> (Self, chrono::NaiveDate, String) { - let rust_gpu_source = Self::get_spirv_std_dep_definition(shader_crate_path); + ) -> anyhow::Result<(Self, chrono::NaiveDate, String)> { + let rust_gpu_source = Self::get_spirv_std_dep_definition(shader_crate_path)?; - rust_gpu_source.ensure_repo_is_installed(); - rust_gpu_source.checkout(); + rust_gpu_source.ensure_repo_is_installed()?; + rust_gpu_source.checkout()?; - let date = rust_gpu_source.get_version_date(); - let channel = Self::get_channel_from_toolchain_toml(&rust_gpu_source.to_dirname()); + let date = rust_gpu_source.get_version_date()?; + let channel = Self::get_channel_from_toolchain_toml(&rust_gpu_source.to_dirname()?)?; log::debug!("Parsed version, date and toolchain channel from shader-defined `rust-gpu`: {rust_gpu_source:?}, {date}, {channel}"); - (rust_gpu_source, date, channel) + Ok((rust_gpu_source, date, channel)) } /// Convert the source to just its version. @@ -84,42 +86,43 @@ impl SpirvSource { /// Convert the `rust-gpu` source into a string that can be used as a directory. /// It needs to be dynamically created because an end-user might want to swap out the source, /// maybe using their own fork for example. - fn to_dirname(&self) -> std::path::PathBuf { + fn to_dirname(&self) -> anyhow::Result { let dir = crate::to_dirname(self.to_string().as_ref()); - crate::cache_dir().join("rust-gpu-repo").join(dir) + Ok(crate::cache_dir()?.join("rust-gpu-repo").join(dir)) } /// Checkout the `rust-gpu` repo to the requested version. - fn checkout(&self) { + fn checkout(&self) -> anyhow::Result<()> { log::debug!( "Checking out `rust-gpu` repo at {} to {}", - self.to_dirname().display(), + self.to_dirname()?.display(), self.to_version() ); let output_checkout = std::process::Command::new("git") - .current_dir(self.to_dirname()) + .current_dir(self.to_dirname()?) .args(["checkout", self.to_version().as_ref()]) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_checkout.status.success(), "couldn't checkout revision '{}' of `rust-gpu` at {}", self.to_version(), - self.to_dirname().to_string_lossy() + self.to_dirname()?.to_string_lossy() ); + + Ok(()) } /// Get the date of the version of `rust-gpu` used by the shader. This allows us to know what /// features we can use in the `spirv-builder` crate. - fn get_version_date(&self) -> chrono::NaiveDate { + fn get_version_date(&self) -> anyhow::Result { let date_format = "%Y-%m-%d"; log::debug!( "Getting `rust-gpu` version date from {}", - self.to_dirname().display(), + self.to_dirname()?.display(), ); let output_date = std::process::Command::new("git") - .current_dir(self.to_dirname()) + .current_dir(self.to_dirname()?) .args([ "show", "--no-patch", @@ -127,13 +130,12 @@ impl SpirvSource { format!("--date=format:'{date_format}'").as_ref(), self.to_version().as_ref(), ]) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_date.status.success(), "couldn't get `rust-gpu` version date at for {} at {}", self.to_version(), - self.to_dirname().to_string_lossy() + self.to_dirname()?.to_string_lossy() ); let date_string = String::from_utf8_lossy(&output_date.stdout) .to_string() @@ -145,47 +147,53 @@ impl SpirvSource { self.to_version() ); - chrono::NaiveDate::parse_from_str(&date_string, date_format).unwrap() + Ok(chrono::NaiveDate::parse_from_str( + &date_string, + date_format, + )?) } /// Parse the `rust-toolchain.toml` in the working tree of the checked-out version of the `rust-gpu` repo. - fn get_channel_from_toolchain_toml(path: &std::path::PathBuf) -> String { + fn get_channel_from_toolchain_toml(path: &std::path::PathBuf) -> anyhow::Result { log::debug!("Parsing `rust-toolchain.toml` at {path:?} for the used toolchain"); - let contents = std::fs::read_to_string(path.join("rust-toolchain.toml")).unwrap(); - let toml: toml::Table = toml::from_str(&contents).unwrap(); + let contents = std::fs::read_to_string(path.join("rust-toolchain.toml"))?; + let toml: toml::Table = toml::from_str(&contents)?; let Some(toolchain) = toml.get("toolchain") else { - panic!("Couldn't find `[toolchain]` section in `rust-toolchain.toml` at {path:?}"); + anyhow::bail!( + "Couldn't find `[toolchain]` section in `rust-toolchain.toml` at {path:?}" + ); }; let Some(channel) = toolchain.get("channel") else { - panic!("Couldn't find `channel` field in `rust-toolchain.toml` at {path:?}"); + anyhow::bail!("Couldn't find `channel` field in `rust-toolchain.toml` at {path:?}"); }; - channel.to_string().replace('"', "") + Ok(channel.to_string().replace('"', "")) } /// Get the shader crate's `spirv_std = ...` definition in its `Cargo.toml` - pub fn get_spirv_std_dep_definition(shader_crate_path: &std::path::PathBuf) -> Self { - let cwd = std::env::current_dir().expect("no cwd"); + pub fn get_spirv_std_dep_definition( + shader_crate_path: &std::path::PathBuf, + ) -> anyhow::Result { + let cwd = std::env::current_dir().context("no cwd")?; let exec_path = if shader_crate_path.is_absolute() { shader_crate_path.clone() } else { cwd.join(shader_crate_path) } .canonicalize() - .expect("could not get absolute path to shader crate"); + .context("could not get absolute path to shader crate")?; if !exec_path.is_dir() { log::error!("{exec_path:?} is not a directory, aborting"); - panic!("{exec_path:?} is not a directory"); + anyhow::bail!("{exec_path:?} is not a directory"); } log::debug!("Running `cargo tree` on {}", exec_path.display()); let output_cargo_tree = std::process::Command::new("cargo") .current_dir(&exec_path) .args(["tree", "--workspace", "--prefix", "none"]) - .output() - .unwrap(); - assert!( + .output()?; + anyhow::ensure!( output_cargo_tree.status.success(), "could not query shader's `Cargo.toml` for `spirv-std` dependency" ); @@ -197,7 +205,7 @@ impl SpirvSource { log::trace!(" found {maybe_spirv_std_def:?}"); let Some(spirv_std_def) = maybe_spirv_std_def else { - panic!("`spirv-std` not found in shader's `Cargo.toml` at {exec_path:?}:\n{cargo_tree_string}"); + anyhow::bail!("`spirv-std` not found in shader's `Cargo.toml` at {exec_path:?}:\n{cargo_tree_string}"); }; Self::parse_spirv_std_source_and_version(spirv_std_def) @@ -207,17 +215,20 @@ impl SpirvSource { /// `spirv-std v0.9.0 (https://github.com/Rust-GPU/rust-gpu?rev=54f6978c#54f6978c) (*)` /// Which would return: /// `SpirvSource::Git("https://github.com/Rust-GPU/rust-gpu", "54f6978c")` - fn parse_spirv_std_source_and_version(spirv_std_def: &str) -> Self { + fn parse_spirv_std_source_and_version(spirv_std_def: &str) -> anyhow::Result { log::trace!("parsing spirv-std source and version from def: '{spirv_std_def}'"); let parts: Vec = spirv_std_def.split_whitespace().map(String::from).collect(); let version = parts .get(1) - .expect("Couldn't find `spirv_std` version in shader crate") + .context("Couldn't find `spirv_std` version in shader crate")? .to_owned(); let mut source = Self::CratesIO(version.clone()); if parts.len() > 2 { - let mut source_string = parts.get(2).unwrap().to_owned(); + let mut source_string = parts + .get(2) + .context("Couldn't get Uri from dependency string")? + .to_owned(); source_string = source_string.replace(['(', ')'], ""); // Unfortunately Uri ignores the fragment/hash portion of the Uri. @@ -226,7 +237,7 @@ impl SpirvSource { // // // So here we'll parse the fragment out of the source string by hand - let uri = source_string.parse::().unwrap(); + let uri = source_string.parse::()?; let maybe_hash = if source_string.contains('#') { let splits = source_string.split('#'); splits.last().map(std::borrow::ToOwned::to_owned) @@ -234,7 +245,7 @@ impl SpirvSource { None }; if uri.scheme().is_some() { - source = Self::parse_git_source(version, &uri, maybe_hash); + source = Self::parse_git_source(version, &uri, maybe_hash)?; } else { source = Self::Path((source_string, version)); } @@ -242,69 +253,90 @@ impl SpirvSource { log::debug!("Parsed `rust-gpu` source and version: {source:?}"); - source + Ok(source) } /// Parse a Git source like: `https://github.com/Rust-GPU/rust-gpu?rev=54f6978c#54f6978c` - fn parse_git_source(version: String, uri: &http::Uri, fragment: Option) -> Self { + fn parse_git_source( + version: String, + uri: &http::Uri, + fragment: Option, + ) -> anyhow::Result { log::trace!( "parsing git source from version: '{version}' and uri: '{uri}' and fragment: {}", fragment.as_deref().unwrap_or("?") ); let repo = format!( "{}://{}{}", - uri.scheme().unwrap(), - uri.host().unwrap(), + uri.scheme().context("Couldn't parse scheme from Uri")?, + uri.host().context("Couldn't parse host from Uri")?, uri.path() ); - let rev = uri.query().map_or_else( - || fragment.unwrap_or(version), - |query| { - let marker = "rev="; - let sanity_check = query.contains(marker) && query.split('=').count() == 2; - assert!(sanity_check, "revision not found in Git URI: {query}"); - query.replace(marker, "") - }, - ); + let rev = Self::parse_git_revision(uri.query(), fragment, version); + + Ok(Self::Git { url: repo, rev }) + } + + /// Decide the Git revision to use. + fn parse_git_revision( + maybe_query: Option<&str>, + maybe_fragment: Option, + version: String, + ) -> String { + let marker = "rev="; + let maybe_sane_query = maybe_query.and_then(|query| { + // TODO: This might seem a little crude, but it saves adding a whole query parsing dependency. + let sanity_check = query.contains(marker) && query.split('=').count() == 2; + sanity_check.then_some(query) + }); + + if let Some(query) = maybe_sane_query { + return query.replace(marker, ""); + } - Self::Git { url: repo, rev } + if let Some(fragment) = maybe_fragment { + return fragment; + } + + version } /// `git clone` the `rust-gpu` repo. We use it to get the required Rust toolchain to compile /// the shader. - fn ensure_repo_is_installed(&self) { - if self.to_dirname().exists() { + fn ensure_repo_is_installed(&self) -> anyhow::Result<()> { + if self.to_dirname()?.exists() { log::debug!( "Not cloning `rust-gpu` repo ({}) as it already exists at {}", self.to_repo(), - self.to_dirname().to_string_lossy().as_ref(), + self.to_dirname()?.to_string_lossy().as_ref(), ); - return; + return Ok(()); } log::debug!( "Cloning `rust-gpu` repo {} to {}", self.to_repo(), - self.to_dirname().to_string_lossy().as_ref(), + self.to_dirname()?.to_string_lossy().as_ref(), ); let output_clone = std::process::Command::new("git") .args([ "clone", self.to_repo().as_ref(), - self.to_dirname().to_string_lossy().as_ref(), + self.to_dirname()?.to_string_lossy().as_ref(), ]) - .output() - .unwrap(); + .output()?; - assert!( + anyhow::ensure!( output_clone.status.success(), "couldn't clone `rust-gpu` {} to {}\n{}", self.to_repo(), - self.to_dirname().to_string_lossy(), + self.to_dirname()?.to_string_lossy(), String::from_utf8_lossy(&output_clone.stderr) ); + + Ok(()) } } @@ -315,7 +347,7 @@ mod test { #[test_log::test] fn parsing_spirv_std_dep_for_shader_template() { let shader_template_path = crate::test::shader_crate_template_path(); - let source = SpirvSource::get_spirv_std_dep_definition(&shader_template_path); + let source = SpirvSource::get_spirv_std_dep_definition(&shader_template_path).unwrap(); assert_eq!( source, SpirvSource::Git { @@ -329,7 +361,7 @@ mod test { fn parsing_spirv_std_dep_for_git_source() { let definition = "spirv-std v9.9.9 (https://github.com/Rust-GPU/rust-gpu?rev=82a0f69#82a0f69) (*)"; - let source = SpirvSource::parse_spirv_std_source_and_version(definition); + let source = SpirvSource::parse_spirv_std_source_and_version(definition).unwrap(); assert_eq!( source, SpirvSource::Git { @@ -342,7 +374,7 @@ mod test { #[test_log::test] fn parsing_spirv_std_dep_for_git_source_hash() { let definition = "spirv-std v9.9.9 (https://github.com/Rust-GPU/rust-gpu#82a0f69) (*)"; - let source = SpirvSource::parse_spirv_std_source_and_version(definition); + let source = SpirvSource::parse_spirv_std_source_and_version(definition).unwrap(); assert_eq!( source, SpirvSource::Git { diff --git a/crates/cargo-gpu/src/toml.rs b/crates/cargo-gpu/src/toml.rs index f067f5a..69a5f53 100644 --- a/crates/cargo-gpu/src/toml.rs +++ b/crates/cargo-gpu/src/toml.rs @@ -1,5 +1,7 @@ //! Build a shader based on the data in the `[package.metadata.rust-gpu.build.spirv-builder]` section of //! a shader's `Cargo.toml`. + +use anyhow::Context as _; use clap::Parser; use crate::{Cli, Command}; @@ -36,39 +38,42 @@ pub struct Toml { impl Toml { /// Entrypoint - pub fn run(&self) { - let (path, toml) = Self::parse_cargo_toml(self.path.clone()); + pub fn run(&self) -> anyhow::Result<()> { + let (path, toml) = Self::parse_cargo_toml(self.path.clone())?; + let working_directory = path + .parent() + .context("Couldn't find parent for shader's `Cargo.toml`")?; // Determine if this is a workspace's Cargo.toml or a crate's Cargo.toml let (toml_type, table) = if toml.contains_key("workspace") { let table = Self::get_metadata_rustgpu_table(&toml, "workspace") - .unwrap_or_else(|| { - panic!( + .with_context(|| { + format!( "toml file '{}' is missing a [workspace.metadata.rust-gpu] table", path.display() - ); - }) + ) + })? .clone(); ("workspace", table) } else if toml.contains_key("package") { let mut table = Self::get_metadata_rustgpu_table(&toml, "package") - .unwrap_or_else(|| { - panic!( + .with_context(|| { + format!( "toml file '{}' is missing a [package.metadata.rust-gpu] table", path.display() - ); - }) + ) + })? .clone(); // Ensure the package name is included as the shader-crate parameter if !table.contains_key("shader-crate") { table.insert( "shader-crate".to_owned(), - format!("{}", path.parent().unwrap().display()).into(), + format!("{}", working_directory.display()).into(), ); } ("package", table) } else { - panic!("toml file '{}' must describe a workspace containing [workspace.metadata.rust-gpu.build] or a describe a crate with [package.metadata.rust-gpu.build]", path.display()); + anyhow::bail!("toml file '{}' must describe a workspace containing [workspace.metadata.rust-gpu.build] or a describe a crate with [package.metadata.rust-gpu.build]", path.display()); }; log::info!( "building with [{toml_type}.metadata.rust-gpu.build] section of the toml file at '{}'", @@ -76,34 +81,36 @@ impl Toml { ); log::debug!("table: {table:#?}"); - let mut parameters = table + let mut parameters: Vec = table .get("build") - .unwrap_or_else(|| panic!("toml is missing the 'build' table")) + .with_context(|| "toml is missing the 'build' table")? .as_table() - .unwrap_or_else(|| { - panic!("toml file's '{toml_type}.metadata.rust-gpu.build' property is not a table") - }) + .with_context(|| { + format!("toml file's '{toml_type}.metadata.rust-gpu.build' property is not a table") + })? .into_iter() - .flat_map(|(key, val)| { - if let toml::Value::String(string) = val { - [format!("--{key}"), string.clone()] + .map(|(key, val)| -> anyhow::Result> { + Ok(if let toml::Value::String(string) = val { + [format!("--{key}"), string.clone()].into() } else { let mut value = String::new(); let ser = toml::ser::ValueSerializer::new(&mut value); - serde::Serialize::serialize(val, ser).unwrap(); - [format!("--{key}"), value] - } + serde::Serialize::serialize(val, ser)?; + [format!("--{key}"), value].into() + }) }) - .collect::>(); + .collect::>>>()? + .into_iter() + .flatten() + .collect(); parameters.insert(0, "cargo-gpu".to_owned()); parameters.insert(1, "build".to_owned()); - let working_directory = path.parent().unwrap(); log::info!( "issuing cargo commands from the working directory '{}'", working_directory.display() ); - std::env::set_current_dir(working_directory).unwrap(); + std::env::set_current_dir(working_directory)?; log::debug!("build parameters: {parameters:#?}"); if let Cli { @@ -112,20 +119,24 @@ impl Toml { { // Ensure that the output directory is relative to the toml file if build.output_dir.is_relative() { - let dir = path.parent().expect("no path parent"); + let dir = path.parent().context("no path parent")?; build.output_dir = dir.join(build.output_dir); } log::debug!("build: {build:?}"); - build.run(); + build.run()?; } else { log::error!("parameters found in [{toml_type}.metadata.rust-gpu.build] were not parameters to `cargo gpu build`"); - panic!("could not determin build command"); + anyhow::bail!("could not determin build command"); } + + Ok(()) } /// Parse the contents of the shader's `Cargo.toml` - pub fn parse_cargo_toml(mut path: std::path::PathBuf) -> (std::path::PathBuf, toml::Table) { + pub fn parse_cargo_toml( + mut path: std::path::PathBuf, + ) -> anyhow::Result<(std::path::PathBuf, toml::Table)> { // Find the path to the toml file to use let parsed_path = if path.is_file() && path.ends_with(".toml") { path @@ -135,16 +146,16 @@ impl Toml { path } else { log::error!("toml file '{}' is not a file", path.display()); - panic!("toml file '{}' is not a file", path.display()); + anyhow::bail!("toml file '{}' is not a file", path.display()); } }; log::info!("using toml file '{}'", parsed_path.display()); - let contents = std::fs::read_to_string(&parsed_path).unwrap(); - let toml: toml::Table = toml::from_str(&contents).unwrap(); + let contents = std::fs::read_to_string(&parsed_path)?; + let toml: toml::Table = toml::from_str(&contents)?; - (parsed_path, toml) + Ok((parsed_path, toml)) } /// Parse the `[package.metadata.rust-gpu]` section. diff --git a/crates/spirv-builder-cli/src/main.rs b/crates/spirv-builder-cli/src/main.rs index 496c2cc..09b2780 100644 --- a/crates/spirv-builder-cli/src/main.rs +++ b/crates/spirv-builder-cli/src/main.rs @@ -99,7 +99,7 @@ fn main() { let mut shaders = vec![]; match module { ModuleResult::MultiModule(modules) => { - assert!(!modules.is_empty(), "No shader modules to compile"); + anyhow::ensure!(!modules.is_empty(), "No shader modules to compile"); for (entry, filepath) in modules.into_iter() { log::debug!("compiled {entry} {}", filepath.display()); shaders.push(ShaderModule::new(entry, filepath)); From 9dc0c0337d4bbc6c786f83587abac482332537f1 Mon Sep 17 00:00:00 2001 From: Thomas Buckley-Houston Date: Sat, 21 Dec 2024 20:49:02 +0100 Subject: [PATCH 2/2] Ask user if they want to install new toolchains Also add some user output for the major steps of installing toolchains, compiling `spirv-builder-cli` and compiling the actual shader. Surprisingly there's no way in the standard library to get a single keypress from the user! There's ways to get a line, therefore a keypress then a newline, but that's not so conventional for responding to a "y/n" prompt. Fixes #14 --- Cargo.lock | 151 +++++++++++++++++++++++++++ Cargo.toml | 2 +- README.md | 45 +++++++- crates/cargo-gpu/Cargo.toml | 1 + crates/cargo-gpu/src/build.rs | 8 +- crates/cargo-gpu/src/install.rs | 10 ++ crates/cargo-gpu/src/main.rs | 46 +++++++- crates/cargo-gpu/src/show.rs | 4 +- crates/cargo-gpu/src/spirv_cli.rs | 37 ++++++- crates/cargo-gpu/src/spirv_source.rs | 2 + crates/spirv-builder-cli/Cargo.toml | 5 + crates/spirv-builder-cli/src/main.rs | 2 +- justfile | 2 +- 13 files changed, 300 insertions(+), 15 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ec3345b..607acfe 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -91,6 +91,7 @@ dependencies = [ "anyhow", "chrono", "clap", + "crossterm", "directories", "env_logger 0.10.2", "http", @@ -164,6 +165,31 @@ version = "1.0.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5b63caa9aa9397e2d9480a9b13673856c78d8ac123288526c37d7839f2a86990" +[[package]] +name = "crossterm" +version = "0.28.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "829d955a0bb380ef178a640b91779e3987da38c9aea133b20614cfed8cdea9c6" +dependencies = [ + "bitflags", + "crossterm_winapi", + "mio", + "parking_lot", + "rustix", + "signal-hook", + "signal-hook-mio", + "winapi", +] + +[[package]] +name = "crossterm_winapi" +version = "0.9.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "acdd7c62a3665c7f6830a51635d9ac9b23ed385797f70a83bb8bafe9c572ab2b" +dependencies = [ + "winapi", +] + [[package]] name = "directories" version = "5.0.1" @@ -231,6 +257,16 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5" +[[package]] +name = "errno" +version = "0.3.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "33d852cb9b869c2a9b3df2f71a3074817f01e1844f839a144f5fcef059a4eb5d" +dependencies = [ + "libc", + "windows-sys 0.59.0", +] + [[package]] name = "fnv" version = "1.0.7" @@ -338,6 +374,22 @@ dependencies = [ "libc", ] +[[package]] +name = "linux-raw-sys" +version = "0.4.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "78b3ae25bc7c8c38cec158d1f2757ee79e9b3740fbc7ccf0e59e4b08d793fa89" + +[[package]] +name = "lock_api" +version = "0.4.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "07af8b9cdd281b7915f413fa73f29ebd5d55d0d3f0155584dade1ff18cea1b17" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.22" @@ -359,6 +411,18 @@ version = "2.7.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "78ca9ab1a0babb1e7d5695e3530886289c18cf2f87ec19a575a0abdce112e3a3" +[[package]] +name = "mio" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "2886843bf800fba2e3377cff24abf6379b4c4d5c6681eaf9ea5b0d15090450bd" +dependencies = [ + "libc", + "log", + "wasi", + "windows-sys 0.52.0", +] + [[package]] name = "nu-ansi-term" version = "0.46.0" @@ -396,6 +460,29 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "parking_lot" +version = "0.12.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f1bf18183cf54e8d6059647fc3063646a1801cf30896933ec2311622cc4b9a27" +dependencies = [ + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.9.10" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "1e401f977ab385c9e4e3ab30627d6f26d00e2c73eef317493c4ec6d468726cf8" +dependencies = [ + "cfg-if", + "libc", + "redox_syscall", + "smallvec", + "windows-targets 0.52.6", +] + [[package]] name = "pin-project-lite" version = "0.2.15" @@ -420,6 +507,15 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "redox_syscall" +version = "0.5.8" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "03a862b389f93e68874fbf580b9de08dd02facb9a788ebadaf4a3fd33cf58834" +dependencies = [ + "bitflags", +] + [[package]] name = "redox_users" version = "0.4.6" @@ -481,12 +577,31 @@ version = "1.9.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2" +[[package]] +name = "rustix" +version = "0.38.42" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f93dc38ecbab2eb790ff964bb77fa94faf256fd3e73285fd7ba0903b76bedb85" +dependencies = [ + "bitflags", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.59.0", +] + [[package]] name = "ryu" version = "1.0.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "f3cb5ba0dc43242ce17de99c180e96db90b235b8a9fdc9543c96d2209116bd9f" +[[package]] +name = "scopeguard" +version = "1.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" + [[package]] name = "serde" version = "1.0.214" @@ -537,6 +652,42 @@ dependencies = [ "lazy_static", ] +[[package]] +name = "signal-hook" +version = "0.3.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8621587d4798caf8eb44879d42e56b9a93ea5dcd315a6487c357130095b62801" +dependencies = [ + "libc", + "signal-hook-registry", +] + +[[package]] +name = "signal-hook-mio" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34db1a06d485c9142248b7a054f034b349b212551f3dfd19c94d45a754a217cd" +dependencies = [ + "libc", + "mio", + "signal-hook", +] + +[[package]] +name = "signal-hook-registry" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a9e9e0b4211b72e7b8b6e85c807d36c212bdb33ea8587f7569562a84df5465b1" +dependencies = [ + "libc", +] + +[[package]] +name = "smallvec" +version = "1.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" + [[package]] name = "spirv-builder-cli" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index a97e989..a23359e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -17,6 +17,7 @@ resolver = "2" anyhow = "1.0.94" clap = { version = "4.4.8", features = ["derive"] } chrono = { version = "0.4.38", default-features = false, features = ["std"] } +crossterm = "0.28.1" directories = "5.0.1" env_home = "0.1.0" env_logger = "0.10" @@ -49,7 +50,6 @@ multiple_crate_versions = { level = "allow", priority = 1 } pub_with_shorthand = { level = "allow", priority = 1 } partial_pub_fields = { level = "allow", priority = 1 } pattern_type_mismatch = { level = "allow", priority = 1 } -print_stdout = { level = "allow", priority = 1 } std_instead_of_alloc = { level = "allow", priority = 1 } diff --git a/README.md b/README.md index e0a2b1f..ace69f4 100644 --- a/README.md +++ b/README.md @@ -100,6 +100,9 @@ Options: --force-spirv-cli-rebuild Force `spirv-builder-cli` and `rustc_codegen_spirv` to be rebuilt + --auto-install-rust-toolchain + Assume "yes" to "Install Rust toolchain: [y/n]" prompt + -h, --help Print help (see a summary with '-h') @@ -134,6 +137,9 @@ Options: --force-spirv-cli-rebuild Force `spirv-builder-cli` and `rustc_codegen_spirv` to be rebuilt + --auto-install-rust-toolchain + Assume "yes" to "Install Rust toolchain: [y/n]" prompt + --shader-target Shader target @@ -197,13 +203,44 @@ Options: Show some useful values -Usage: cargo-gpu show [OPTIONS] +Usage: cargo-gpu show -Options: - --cache-directory - Displays the location of the cache directory +Commands: + cache-directory Displays the location of the cache directory + spirv-source The source location of spirv-std + help Print this message or the help of the given subcommand(s) +Options: -h, --help Print help + + * Cache-directory + + Displays the location of the cache directory + + Usage: cargo-gpu show cache-directory + + Options: + -h, --help + Print help + + + * Spirv-source + + The source location of spirv-std + + Usage: cargo-gpu show spirv-source [OPTIONS] + + Options: + --shader-crate + The location of the shader-crate to inspect to determine its spirv-std dependency + + [default: ./] + + -h, --help + Print help + + + ```` diff --git a/crates/cargo-gpu/Cargo.toml b/crates/cargo-gpu/Cargo.toml index e7ad33c..b05cd68 100644 --- a/crates/cargo-gpu/Cargo.toml +++ b/crates/cargo-gpu/Cargo.toml @@ -21,6 +21,7 @@ serde_json.workspace = true toml.workspace = true chrono.workspace = true http.workspace = true +crossterm.workspace = true [dev-dependencies] test-log.workspace = true diff --git a/crates/cargo-gpu/src/build.rs b/crates/cargo-gpu/src/build.rs index b6f958a..0f7bf68 100644 --- a/crates/cargo-gpu/src/build.rs +++ b/crates/cargo-gpu/src/build.rs @@ -1,7 +1,5 @@ //! `cargo gpu build`, analogous to `cargo build` -use std::io::Write as _; - use anyhow::Context as _; use clap::Parser; use spirv_builder_cli::{Linkage, ShaderModule}; @@ -64,6 +62,11 @@ impl Build { let arg = serde_json::to_string_pretty(&spirv_builder_args)?; log::info!("using spirv-builder-cli arg: {arg}"); + crate::user_output!( + "Running `spirv-builder-cli` to compile shader at {}...\n", + self.install.shader_crate.display() + ); + // Call spirv-builder-cli to compile the shaders. let output = std::process::Command::new(spirv_builder_cli_path) .arg(arg) @@ -112,7 +115,6 @@ impl Build { let manifest_path = self.output_dir.join("manifest.json"); // Sort the contents so the output is deterministic linkage.sort(); - // UNWRAP: safe because we know this always serializes let json = serde_json::to_string_pretty(&linkage)?; let mut file = std::fs::File::create(&manifest_path).with_context(|| { format!( diff --git a/crates/cargo-gpu/src/install.rs b/crates/cargo-gpu/src/install.rs index ce02d73..c6e3eef 100644 --- a/crates/cargo-gpu/src/install.rs +++ b/crates/cargo-gpu/src/install.rs @@ -118,6 +118,10 @@ pub struct Install { /// Force `spirv-builder-cli` and `rustc_codegen_spirv` to be rebuilt. #[clap(long)] force_spirv_cli_rebuild: bool, + + /// Assume "yes" to "Install Rust toolchain: [y/n]" prompt. + #[clap(long, action)] + auto_install_rust_toolchain: bool, } impl Install { @@ -128,6 +132,7 @@ impl Install { self.spirv_builder_source.clone(), self.spirv_builder_version.clone(), self.rust_toolchain.clone(), + self.auto_install_rust_toolchain, ) } @@ -230,6 +235,11 @@ impl Install { self.write_source_files()?; self.write_target_spec_files()?; + crate::user_output!( + "Compiling shader-specific `spirv-builder-cli` for {}\n", + self.shader_crate.display() + ); + let mut command = std::process::Command::new("cargo"); command .current_dir(&checkout) diff --git a/crates/cargo-gpu/src/main.rs b/crates/cargo-gpu/src/main.rs index 86167c0..1c11f4f 100644 --- a/crates/cargo-gpu/src/main.rs +++ b/crates/cargo-gpu/src/main.rs @@ -65,9 +65,51 @@ mod spirv_cli; mod spirv_source; mod toml; -fn main() -> anyhow::Result<()> { +/// Central function to write to the user. +#[macro_export] +macro_rules! user_output { + ($($args: tt)*) => { + #[allow( + clippy::allow_attributes, + clippy::useless_attribute, + unused_imports, + reason = "`std::io::Write` is only sometimes called??" + )] + use std::io::Write as _; + + #[expect( + clippy::non_ascii_literal, + reason = "CRAB GOOD. CRAB IMPORTANT." + )] + { + print!("🦀 "); + } + print!($($args)*); + std::io::stdout().flush().unwrap(); + } +} + +fn main() { + #[cfg(debug_assertions)] + std::env::set_var("RUST_BACKTRACE", "1"); + env_logger::builder().init(); + if let Err(error) = run() { + log::error!("{error:?}"); + + #[expect( + clippy::print_stderr, + reason = "Our central place for outputting error messages" + )] + { + eprintln!("Error: {error}"); + } + }; +} + +/// Wrappable "main" to catch errors. +fn run() -> anyhow::Result<()> { let args = std::env::args() .filter(|arg| { // Calling cargo-gpu as the cargo subcommand "cargo gpu" passes "gpu" @@ -161,7 +203,7 @@ fn dump_full_usage_for_readme() -> anyhow::Result<()> { command.build(); write_help(&mut buffer, &mut command, 0)?; - println!("{}", String::from_utf8(buffer)?); + user_output!("{}", String::from_utf8(buffer)?); Ok(()) } diff --git a/crates/cargo-gpu/src/show.rs b/crates/cargo-gpu/src/show.rs index c1668e2..cc9c623 100644 --- a/crates/cargo-gpu/src/show.rs +++ b/crates/cargo-gpu/src/show.rs @@ -33,12 +33,12 @@ impl Show { log::info!("{:?}: ", self.command); match self.command { Info::CacheDirectory => { - println!("{}", cache_dir()?.display()); + crate::user_output!("{}\n", cache_dir()?.display()); } Info::SpirvSource(SpirvSourceDep { shader_crate }) => { let rust_gpu_source = crate::spirv_source::SpirvSource::get_spirv_std_dep_definition(&shader_crate)?; - println!("{rust_gpu_source}"); + crate::user_output!("{rust_gpu_source}\n"); } } diff --git a/crates/cargo-gpu/src/spirv_cli.rs b/crates/cargo-gpu/src/spirv_cli.rs index bc26ed9..13813f9 100644 --- a/crates/cargo-gpu/src/spirv_cli.rs +++ b/crates/cargo-gpu/src/spirv_cli.rs @@ -23,6 +23,8 @@ pub struct SpirvCli { pub channel: String, /// The date of the pinned version of `rust-gpu` pub date: chrono::NaiveDate, + /// Has the user overridden the toolchain consent prompt + is_toolchain_install_consent: bool, } impl core::fmt::Display for SpirvCli { @@ -42,6 +44,7 @@ impl SpirvCli { maybe_rust_gpu_source: Option, maybe_rust_gpu_version: Option, maybe_rust_gpu_channel: Option, + is_toolchain_install_consent: bool, ) -> anyhow::Result { let (default_rust_gpu_source, rust_gpu_date, default_rust_gpu_channel) = SpirvSource::get_rust_gpu_deps_from_shader(shader_crate_path)?; @@ -62,6 +65,7 @@ impl SpirvCli { source: maybe_spirv_source.unwrap_or(default_rust_gpu_source), channel: maybe_rust_gpu_channel.unwrap_or(default_rust_gpu_channel), date: rust_gpu_date, + is_toolchain_install_consent, }) } @@ -99,6 +103,10 @@ impl SpirvCli { { log::debug!("toolchain {} is already installed", self.channel); } else { + self.get_consent_for_toolchain_install( + format!("Install Rust {} with `rustup`", self.channel).as_ref(), + )?; + let output_toolchain_add = std::process::Command::new("rustup") .args(["toolchain", "add"]) .arg(&self.channel) @@ -133,6 +141,10 @@ impl SpirvCli { if all_components_installed { log::debug!("all required components are installed"); } else { + self.get_consent_for_toolchain_install( + "Install toolchain components (rust-src, rustc-dev, llvm-tools) with `rustup`", + )?; + let output_component_add = std::process::Command::new("rustup") .args(["component", "add", "--toolchain"]) .arg(&self.channel) @@ -148,6 +160,29 @@ impl SpirvCli { Ok(()) } + + /// Prompt user if they want to install a new Rust toolchain. + fn get_consent_for_toolchain_install(&self, prompt: &str) -> anyhow::Result<()> { + if self.is_toolchain_install_consent { + return Ok(()); + } + crossterm::terminal::enable_raw_mode()?; + crate::user_output!("{prompt} [y/n]: "); + let input = crossterm::event::read()?; + crossterm::terminal::disable_raw_mode()?; + crate::user_output!("{:?}\n", input); + + if let crossterm::event::Event::Key(crossterm::event::KeyEvent { + code: crossterm::event::KeyCode::Char('y'), + .. + }) = input + { + Ok(()) + } else { + crate::user_output!("Exiting...\n"); + std::process::exit(0); + } + } } #[cfg(test)] @@ -157,7 +192,7 @@ mod test { #[test_log::test] fn cached_checkout_dir_sanity() { let shader_template_path = crate::test::shader_crate_template_path(); - let spirv = SpirvCli::new(&shader_template_path, None, None, None).unwrap(); + let spirv = SpirvCli::new(&shader_template_path, None, None, None, true).unwrap(); let dir = spirv.cached_checkout_path().unwrap(); let name = dir .file_name() diff --git a/crates/cargo-gpu/src/spirv_source.rs b/crates/cargo-gpu/src/spirv_source.rs index f6e0ac3..d98fde9 100644 --- a/crates/cargo-gpu/src/spirv_source.rs +++ b/crates/cargo-gpu/src/spirv_source.rs @@ -320,6 +320,8 @@ impl SpirvSource { self.to_dirname()?.to_string_lossy().as_ref(), ); + crate::user_output!("Cloning `rust-gpu` repo..."); + let output_clone = std::process::Command::new("git") .args([ "clone", diff --git a/crates/spirv-builder-cli/Cargo.toml b/crates/spirv-builder-cli/Cargo.toml index c89b824..6723c5b 100644 --- a/crates/spirv-builder-cli/Cargo.toml +++ b/crates/spirv-builder-cli/Cargo.toml @@ -41,3 +41,8 @@ package = "spirv-builder" optional = true git = "https://github.com/Rust-GPU/rust-gpu" # ${AUTO-REPLACE-SOURCE} rev = "60dcb82" # ${AUTO-REPLACE-VERSION} + +[lints.rust] +# This crate is most often run by end users compiling their shaders so it's not so relevant +# for them to see warnings. +warnings = "allow" diff --git a/crates/spirv-builder-cli/src/main.rs b/crates/spirv-builder-cli/src/main.rs index 09b2780..496c2cc 100644 --- a/crates/spirv-builder-cli/src/main.rs +++ b/crates/spirv-builder-cli/src/main.rs @@ -99,7 +99,7 @@ fn main() { let mut shaders = vec![]; match module { ModuleResult::MultiModule(modules) => { - anyhow::ensure!(!modules.is_empty(), "No shader modules to compile"); + assert!(!modules.is_empty(), "No shader modules to compile"); for (entry, filepath) in modules.into_iter() { log::debug!("compiled {entry} {}", filepath.display()); shaders.push(ShaderModule::new(entry, filepath)); diff --git a/justfile b/justfile index 1f53c29..ff42105 100644 --- a/justfile +++ b/justfile @@ -1,7 +1,7 @@ [group: 'ci'] build-shader-template: cargo install --path crates/cargo-gpu - cargo gpu install --shader-crate crates/shader-crate-template + cargo gpu install --shader-crate crates/shader-crate-template --auto-install-rust-toolchain cargo gpu build --shader-crate crates/shader-crate-template --output-dir test-shaders ls -lah test-shaders cat test-shaders/manifest.json