Skip to content

Commit 924d0ab

Browse files
authored
Ran cargo fmt + added some .context() calls for non-obvious errors (#56)
1 parent ea3f5ed commit 924d0ab

File tree

5 files changed

+102
-53
lines changed

5 files changed

+102
-53
lines changed

crates/cargo-gpu/src/install.rs

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
//! Install a dedicated per-shader crate that has the `rust-gpu` compiler in it.
2+
23
use std::io::Write as _;
34

45
use anyhow::Context as _;
@@ -117,18 +118,24 @@ impl Install {
117118

118119
/// Create the `spirv-builder-cli` crate.
119120
fn write_source_files(&self) -> anyhow::Result<()> {
120-
let spirv_cli = self.spirv_cli(&self.spirv_install.shader_crate)?;
121-
let checkout = spirv_cli.cached_checkout_path()?;
122-
std::fs::create_dir_all(checkout.join("src"))?;
121+
let spirv_cli = self
122+
.spirv_cli(&self.spirv_install.shader_crate)
123+
.context("running spirv cli")?;
124+
let checkout = spirv_cli
125+
.cached_checkout_path()
126+
.context("getting cached checkout path")?;
127+
std::fs::create_dir_all(checkout.join("src")).context("creating directory for 'src'")?;
123128
for (filename, contents) in SPIRV_BUILDER_FILES {
124129
log::debug!("writing {filename}");
125130
let path = checkout.join(filename);
126-
let mut file = std::fs::File::create(&path)?;
131+
let mut file = std::fs::File::create(&path)
132+
.with_context(|| format!("creating a file at [{}]", path.display()))?;
127133
let mut replaced_contents = contents.replace("${CHANNEL}", &spirv_cli.channel);
128134
if filename == &"Cargo.toml" {
129135
replaced_contents = Self::update_cargo_toml(&replaced_contents, &spirv_cli.source);
130136
}
131-
file.write_all(replaced_contents.as_bytes())?;
137+
file.write_all(replaced_contents.as_bytes())
138+
.context("writing to file")?;
132139
}
133140
Ok(())
134141
}
@@ -165,10 +172,14 @@ impl Install {
165172
/// Add the target spec files to the crate.
166173
fn write_target_spec_files(&self) -> anyhow::Result<()> {
167174
for (filename, contents) in TARGET_SPECS {
168-
let path = target_spec_dir()?.join(filename);
175+
let path = target_spec_dir()
176+
.context("creating target spec dir")?
177+
.join(filename);
169178
if !path.is_file() || self.spirv_install.force_spirv_cli_rebuild {
170-
let mut file = std::fs::File::create(&path)?;
171-
file.write_all(contents.as_bytes())?;
179+
let mut file = std::fs::File::create(&path)
180+
.with_context(|| format!("creating file at [{}]", path.display()))?;
181+
file.write_all(contents.as_bytes())
182+
.context("writing to file")?;
172183
}
173184
}
174185
Ok(())
@@ -183,10 +194,16 @@ impl Install {
183194
format!("could not create cache directory '{}'", cache_dir.display())
184195
})?;
185196

186-
let spirv_version = self.spirv_cli(&self.spirv_install.shader_crate)?;
187-
spirv_version.ensure_toolchain_and_components_exist()?;
197+
let spirv_version = self
198+
.spirv_cli(&self.spirv_install.shader_crate)
199+
.context("running spirv cli")?;
200+
spirv_version
201+
.ensure_toolchain_and_components_exist()
202+
.context("ensuring toolchain and components exist")?;
188203

189-
let checkout = spirv_version.cached_checkout_path()?;
204+
let checkout = spirv_version
205+
.cached_checkout_path()
206+
.context("getting cached checkout path")?;
190207
let release = checkout.join("target").join("release");
191208

192209
let dylib_filename = format!(
@@ -214,8 +231,9 @@ impl Install {
214231
"writing spirv-builder-cli source files into '{}'",
215232
checkout.display()
216233
);
217-
self.write_source_files()?;
218-
self.write_target_spec_files()?;
234+
self.write_source_files().context("writing source files")?;
235+
self.write_target_spec_files()
236+
.context("writing target spec files")?;
219237

220238
crate::user_output!(
221239
"Compiling shader-specific `spirv-builder-cli` for {}\n",
@@ -231,20 +249,29 @@ impl Install {
231249

232250
build_command.args([
233251
"--features",
234-
&Self::get_required_spirv_builder_version(spirv_version.date)?,
252+
&Self::get_required_spirv_builder_version(spirv_version.date)
253+
.context("getting required spirv builder version")?,
235254
]);
236255

237256
log::debug!("building artifacts with `{:?}`", build_command);
238257

239-
let build_output = build_command
258+
build_command
240259
.stdout(std::process::Stdio::inherit())
241260
.stderr(std::process::Stdio::inherit())
242-
.output()?;
243-
anyhow::ensure!(build_output.status.success(), "...build error!");
261+
.output()
262+
.context("getting command output")
263+
.and_then(|output| {
264+
if output.status.success() {
265+
Ok(output)
266+
} else {
267+
Err(anyhow::anyhow!("bad status {:?}", output.status))
268+
}
269+
})
270+
.context("running build command")?;
244271

245272
if dylib_path.is_file() {
246273
log::info!("successfully built {}", dylib_path.display());
247-
std::fs::rename(&dylib_path, &dest_dylib_path)?;
274+
std::fs::rename(&dylib_path, &dest_dylib_path).context("renaming dylib path")?;
248275
} else {
249276
log::error!("could not find {}", dylib_path.display());
250277
anyhow::bail!("spirv-builder-cli build failed");
@@ -257,11 +284,11 @@ impl Install {
257284
};
258285
if cli_path.is_file() {
259286
log::info!("successfully built {}", cli_path.display());
260-
std::fs::rename(&cli_path, &dest_cli_path)?;
287+
std::fs::rename(&cli_path, &dest_cli_path).context("renaming cli path")?;
261288
} else {
262289
log::error!("could not find {}", cli_path.display());
263290
log::debug!("contents of '{}':", release.display());
264-
for maybe_entry in std::fs::read_dir(&release)? {
291+
for maybe_entry in std::fs::read_dir(&release).context("reading release dir")? {
265292
let entry = maybe_entry?;
266293
log::debug!("{}", entry.file_name().to_string_lossy());
267294
}

crates/cargo-gpu/src/main.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ fn main() {
110110
#[expect(clippy::restriction, reason = "Our central place for safely exiting")]
111111
std::process::exit(1);
112112
};
113-
};
113+
}
114114
}
115115

116116
/// Wrappable "main" to catch errors.
@@ -150,11 +150,11 @@ fn run() -> anyhow::Result<()> {
150150
command.run()?;
151151
} else {
152152
command.run()?;
153-
};
153+
}
154154
}
155155
Command::Show(show) => show.run()?,
156156
Command::DumpUsage => dump_full_usage_for_readme()?,
157-
};
157+
}
158158

159159
Ok(())
160160
}

crates/cargo-gpu/src/show.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use crate::cache_dir;
55
/// Show the computed source of the spirv-std dependency.
66
#[derive(Clone, Debug, clap::Parser)]
77
pub struct SpirvSourceDep {
8-
/// The location of the shader-crate to inspect to determine its spirv-std dependency.
8+
/// The location of the shader-crate to inspect to determine its spirv-std dependency.
99
#[clap(long, default_value = "./")]
1010
pub shader_crate: std::path::PathBuf,
1111
}

crates/cargo-gpu/src/spirv_cli.rs

Lines changed: 50 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -61,21 +61,24 @@ impl SpirvCli {
6161
Self::ensure_workspace_rust_version_doesnt_conflict_with_shader(
6262
shader_crate_path,
6363
is_force_overwrite_lockfiles_v4_to_v3,
64-
)?;
64+
)
65+
.context("ensure_workspace_rust_version_doesnt_conflict_with_shader")?;
6566

6667
if let Some(shader_crate_lock) = maybe_shader_crate_lock {
6768
cargo_lock_files_with_changed_manifest_versions.push(shader_crate_lock);
6869
}
6970

7071
let (default_rust_gpu_source, rust_gpu_date, default_rust_gpu_channel) =
71-
SpirvSource::get_rust_gpu_deps_from_shader(shader_crate_path)?;
72+
SpirvSource::get_rust_gpu_deps_from_shader(shader_crate_path)
73+
.context("get_rust_gpu_deps_from_shader")?;
7274

7375
let maybe_workspace_crate_lock =
7476
Self::ensure_shader_rust_version_doesnt_conflict_with_any_cargo_locks(
7577
shader_crate_path,
7678
default_rust_gpu_channel.clone(),
7779
is_force_overwrite_lockfiles_v4_to_v3,
78-
)?;
80+
)
81+
.context("ensure_shader_rust_version_doesnt_conflict_with_any_cargo_locks")?;
7982

8083
if let Some(workspace_crate_lock) = maybe_workspace_crate_lock {
8184
cargo_lock_files_with_changed_manifest_versions.push(workspace_crate_lock);
@@ -104,12 +107,13 @@ impl SpirvCli {
104107

105108
/// Create and/or return the cache directory
106109
pub fn cached_checkout_path(&self) -> anyhow::Result<std::path::PathBuf> {
107-
let checkout_dir = crate::cache_dir()?
110+
let checkout_dir = crate::cache_dir()
111+
.context("reading cache dir")?
108112
.join("spirv-builder-cli")
109113
.join(crate::to_dirname(self.to_string().as_ref()));
110-
std::fs::create_dir_all(&checkout_dir).with_context(|| {
111-
format!("could not create checkout dir '{}'", checkout_dir.display())
112-
})?;
114+
std::fs::create_dir_all(&checkout_dir)
115+
.with_context(|| format!("could not create checkout dir '{}'", checkout_dir.display()))
116+
.context("crating directory in cahce dir")?;
113117

114118
Ok(checkout_dir)
115119
}
@@ -124,7 +128,8 @@ impl SpirvCli {
124128
// Check for the required toolchain
125129
let output_toolchain_list = std::process::Command::new("rustup")
126130
.args(["toolchain", "list"])
127-
.output()?;
131+
.output()
132+
.context("running rustup command")?;
128133
anyhow::ensure!(
129134
output_toolchain_list.status.success(),
130135
"could not list installed toolchains"
@@ -145,7 +150,8 @@ impl SpirvCli {
145150
.arg(&self.channel)
146151
.stdout(std::process::Stdio::inherit())
147152
.stderr(std::process::Stdio::inherit())
148-
.output()?;
153+
.output()
154+
.context("adding toolchain")?;
149155
anyhow::ensure!(
150156
output_toolchain_add.status.success(),
151157
"could not install required toolchain"
@@ -156,7 +162,8 @@ impl SpirvCli {
156162
let output_component_list = std::process::Command::new("rustup")
157163
.args(["component", "list", "--toolchain"])
158164
.arg(&self.channel)
159-
.output()?;
165+
.output()
166+
.context("getting toolchain list")?;
160167
anyhow::ensure!(
161168
output_component_list.status.success(),
162169
"could not list installed components"
@@ -184,7 +191,8 @@ impl SpirvCli {
184191
.args(["rust-src", "rustc-dev", "llvm-tools"])
185192
.stdout(std::process::Stdio::inherit())
186193
.stderr(std::process::Stdio::inherit())
187-
.output()?;
194+
.output()
195+
.context("adding rustup component")?;
188196
anyhow::ensure!(
189197
output_component_add.status.success(),
190198
"could not install required components"
@@ -200,10 +208,10 @@ impl SpirvCli {
200208
return Ok(());
201209
}
202210
log::debug!("asking for consent to install the required toolchain");
203-
crossterm::terminal::enable_raw_mode()?;
211+
crossterm::terminal::enable_raw_mode().context("enabling raw mode")?;
204212
crate::user_output!("{prompt} [y/n]: ");
205-
let input = crossterm::event::read()?;
206-
crossterm::terminal::disable_raw_mode()?;
213+
let input = crossterm::event::read().context("reading crossterm event")?;
214+
crossterm::terminal::disable_raw_mode().context("disabling raw mode")?;
207215

208216
if let crossterm::event::Event::Key(crossterm::event::KeyEvent {
209217
code: crossterm::event::KeyCode::Char('y'),
@@ -223,7 +231,8 @@ impl SpirvCli {
223231
is_force_overwrite_lockfiles_v4_to_v3: bool,
224232
) -> anyhow::Result<Option<std::path::PathBuf>> {
225233
log::debug!("Ensuring no v3/v4 `Cargo.lock` conflicts from workspace Rust...");
226-
let workspace_rust_version = Self::get_rustc_version(None)?;
234+
let workspace_rust_version =
235+
Self::get_rustc_version(None).context("reading rustc version")?;
227236
if version_check::Version::at_least(
228237
&workspace_rust_version,
229238
RUST_VERSION_THAT_USES_V4_CARGO_LOCKS,
@@ -238,7 +247,8 @@ impl SpirvCli {
238247
Self::handle_conflicting_cargo_lock_v4(
239248
shader_crate_path,
240249
is_force_overwrite_lockfiles_v4_to_v3,
241-
)?;
250+
)
251+
.context("handling v4/v3 conflict")?;
242252

243253
if is_force_overwrite_lockfiles_v4_to_v3 {
244254
Ok(Some(shader_crate_path.join("Cargo.lock")))
@@ -254,7 +264,8 @@ impl SpirvCli {
254264
is_force_overwrite_lockfiles_v4_to_v3: bool,
255265
) -> anyhow::Result<Option<std::path::PathBuf>> {
256266
log::debug!("Ensuring no v3/v4 `Cargo.lock` conflicts from shader's Rust...");
257-
let shader_rust_version = Self::get_rustc_version(Some(channel))?;
267+
let shader_rust_version =
268+
Self::get_rustc_version(Some(channel)).context("getting rustc version")?;
258269
if version_check::Version::at_least(
259270
&shader_rust_version,
260271
RUST_VERSION_THAT_USES_V4_CARGO_LOCKS,
@@ -279,14 +290,18 @@ impl SpirvCli {
279290
Self::handle_conflicting_cargo_lock_v4(
280291
shader_crate_path,
281292
is_force_overwrite_lockfiles_v4_to_v3,
282-
)?;
293+
)
294+
.context("handling v4/v3 conflict")?;
283295
}
284296

285-
if let Some(workspace_root) = Self::get_workspace_root(shader_crate_path)? {
297+
if let Some(workspace_root) =
298+
Self::get_workspace_root(shader_crate_path).context("reading workspace root")?
299+
{
286300
Self::handle_conflicting_cargo_lock_v4(
287301
workspace_root,
288302
is_force_overwrite_lockfiles_v4_to_v3,
289-
)?;
303+
)
304+
.context("handling conflicting cargo v4")?;
290305
return Ok(Some(workspace_root.join("Cargo.lock")));
291306
}
292307

@@ -299,7 +314,8 @@ impl SpirvCli {
299314
fn get_workspace_root(
300315
shader_crate_path: &std::path::Path,
301316
) -> anyhow::Result<Option<&std::path::Path>> {
302-
let shader_cargo_toml = std::fs::read_to_string(shader_crate_path.join("Cargo.toml"))?;
317+
let shader_cargo_toml = std::fs::read_to_string(shader_crate_path.join("Cargo.toml"))
318+
.with_context(|| format!("reading Cargo.toml at {}", shader_crate_path.display()))?;
303319
if !shader_cargo_toml.contains("workspace = true") {
304320
return Ok(None);
305321
}
@@ -327,13 +343,15 @@ impl SpirvCli {
327343
is_force_overwrite_lockfiles_v4_to_v3: bool,
328344
) -> anyhow::Result<()> {
329345
let shader_cargo_lock_path = folder.join("Cargo.lock");
330-
let shader_cargo_lock = std::fs::read_to_string(shader_cargo_lock_path.clone())?;
331-
let third_line = shader_cargo_lock.lines().nth(2).context("")?;
346+
let shader_cargo_lock = std::fs::read_to_string(shader_cargo_lock_path.clone())
347+
.context("reading shader cargo lock")?;
348+
let third_line = shader_cargo_lock.lines().nth(2).context("no third line")?;
332349
if third_line.contains("version = 4") {
333350
Self::handle_v3v4_conflict(
334351
&shader_cargo_lock_path,
335352
is_force_overwrite_lockfiles_v4_to_v3,
336-
)?;
353+
)
354+
.context("handling v4/v3 conflict")?;
337355
return Ok(());
338356
}
339357
if third_line.contains("version = 3") {
@@ -355,7 +373,8 @@ impl SpirvCli {
355373
Self::exit_with_v3v4_hack_suggestion();
356374
}
357375

358-
Self::replace_cargo_lock_manifest_version(offending_cargo_lock, "4", "3")?;
376+
Self::replace_cargo_lock_manifest_version(offending_cargo_lock, "4", "3")
377+
.context("replacing version 4 -> 3")?;
359378

360379
Ok(())
361380
}
@@ -365,7 +384,8 @@ impl SpirvCli {
365384
pub fn revert_cargo_lock_manifest_versions(&self) -> anyhow::Result<()> {
366385
for offending_cargo_lock in &self.cargo_lock_files_with_changed_manifest_versions {
367386
log::debug!("Reverting: {}", offending_cargo_lock.display());
368-
Self::replace_cargo_lock_manifest_version(offending_cargo_lock, "3", "4")?;
387+
Self::replace_cargo_lock_manifest_version(offending_cargo_lock, "3", "4")
388+
.context("replacing version 3 -> 4")?;
369389
}
370390

371391
Ok(())
@@ -383,7 +403,8 @@ impl SpirvCli {
383403
to_version,
384404
offending_cargo_lock.display()
385405
);
386-
let old_contents = std::fs::read_to_string(offending_cargo_lock)?;
406+
let old_contents = std::fs::read_to_string(offending_cargo_lock)
407+
.context("reading offending Cargo.lock")?;
387408
let new_contents = old_contents.replace(
388409
&format!("\nversion = {from_version}\n"),
389410
&format!("\nversion = {to_version}\n"),
@@ -392,7 +413,8 @@ impl SpirvCli {
392413
let mut file = std::fs::OpenOptions::new()
393414
.write(true)
394415
.truncate(true)
395-
.open(offending_cargo_lock)?;
416+
.open(offending_cargo_lock)
417+
.context("opening offending Cargo.lock")?;
396418
file.write_all(new_contents.as_bytes())?;
397419

398420
Ok(())

crates/cargo-gpu/src/spirv_source.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ impl SpirvSource {
110110
if !canonical_path.is_dir() {
111111
log::error!("{shader_crate_path:?} is not a directory, aborting");
112112
anyhow::bail!("{shader_crate_path:?} is not a directory");
113-
};
113+
}
114114

115115
*shader_crate_path = canonical_path;
116116

0 commit comments

Comments
 (0)