Skip to content

Commit

Permalink
make more anyhow::Result usages explicit
Browse files Browse the repository at this point in the history
  • Loading branch information
syphar committed Jun 24, 2024
1 parent 79ebd4d commit 296c02f
Show file tree
Hide file tree
Showing 17 changed files with 54 additions and 59 deletions.
8 changes: 4 additions & 4 deletions src/crates/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::CrateTrait;
use crate::cmd::{Command, ProcessLinesActions};
use crate::prepare::PrepareError;
use crate::Workspace;
use anyhow::{Context as _, Result};
use anyhow::Context as _;
use log::{info, warn};
use std::path::{Path, PathBuf};

Expand Down Expand Up @@ -63,7 +63,7 @@ impl GitRepo {
}

impl CrateTrait for GitRepo {
fn fetch(&self, workspace: &Workspace) -> Result<()> {
fn fetch(&self, workspace: &Workspace) -> anyhow::Result<()> {
// The credential helper that suppresses the password prompt shows this message when a
// repository requires authentication:
//
Expand Down Expand Up @@ -105,15 +105,15 @@ impl CrateTrait for GitRepo {
}
}

fn purge_from_cache(&self, workspace: &Workspace) -> Result<()> {
fn purge_from_cache(&self, workspace: &Workspace) -> anyhow::Result<()> {
let path = self.cached_path(workspace);
if path.exists() {
crate::utils::remove_dir_all(&path)?;
}
Ok(())
}

fn copy_source_to(&self, workspace: &Workspace, dest: &Path) -> Result<()> {
fn copy_source_to(&self, workspace: &Workspace, dest: &Path) -> anyhow::Result<()> {
Command::new(workspace, "git")
.args(&["clone"])
.args(&[self.cached_path(workspace).as_path(), dest])
Expand Down
2 changes: 0 additions & 2 deletions src/crates/local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,6 @@ fn copy_dir(src: &Path, dest: &Path) -> anyhow::Result<()> {

#[cfg(test)]
mod tests {
use anyhow::Result;

#[test]
fn test_copy_dir() -> anyhow::Result<()> {
let tmp_src = tempfile::tempdir()?;
Expand Down
12 changes: 6 additions & 6 deletions src/crates/registry.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::CrateTrait;
use crate::Workspace;
use anyhow::{Context as _, Result};
use anyhow::Context as _;
use flate2::read::GzDecoder;
use log::info;
use std::fs::File;
Expand Down Expand Up @@ -88,7 +88,7 @@ impl RegistryCrate {
.join(format!("{}-{}.crate", self.name, self.version))
}

fn fetch_url(&self, workspace: &Workspace) -> Result<String> {
fn fetch_url(&self, workspace: &Workspace) -> anyhow::Result<String> {
match &self.registry {
Registry::CratesIo => Ok(format!(
"{0}/{1}/{1}-{2}.crate",
Expand Down Expand Up @@ -151,7 +151,7 @@ impl RegistryCrate {
}

impl CrateTrait for RegistryCrate {
fn fetch(&self, workspace: &Workspace) -> Result<()> {
fn fetch(&self, workspace: &Workspace) -> anyhow::Result<()> {
let local = self.cache_path(workspace);
if local.exists() {
info!("crate {} {} is already in cache", self.name, self.version);
Expand All @@ -173,15 +173,15 @@ impl CrateTrait for RegistryCrate {
Ok(())
}

fn purge_from_cache(&self, workspace: &Workspace) -> Result<()> {
fn purge_from_cache(&self, workspace: &Workspace) -> anyhow::Result<()> {
let path = self.cache_path(workspace);
if path.exists() {
crate::utils::remove_file(&path)?;
}
Ok(())
}

fn copy_source_to(&self, workspace: &Workspace, dest: &Path) -> Result<()> {
fn copy_source_to(&self, workspace: &Workspace, dest: &Path) -> anyhow::Result<()> {
let cached = self.cache_path(workspace);
let mut file = File::open(cached)?;
let mut tar = Archive::new(GzDecoder::new(BufReader::new(&mut file)));
Expand Down Expand Up @@ -216,7 +216,7 @@ impl std::fmt::Display for RegistryCrate {
}
}

fn unpack_without_first_dir<R: Read>(archive: &mut Archive<R>, path: &Path) -> Result<()> {
fn unpack_without_first_dir<R: Read>(archive: &mut Archive<R>, path: &Path) -> anyhow::Result<()> {
let entries = archive.entries()?;
for entry in entries {
let mut entry = entry?;
Expand Down
2 changes: 1 addition & 1 deletion src/native/windows.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::CurrentUser;
use crate::cmd::KillFailedError;
use anyhow::{anyhow, Result};
use anyhow::anyhow;
use std::fs::File;
use std::path::Path;
use windows_sys::Win32::Foundation::CloseHandle;
Expand Down
24 changes: 14 additions & 10 deletions src/prepare.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::cmd::Command;
use crate::{build::CratePatch, Crate, Toolchain, Workspace};
use anyhow::{Context as _, Result};
use anyhow::Context as _;
use log::info;
use std::path::Path;
use toml::{
Expand Down Expand Up @@ -33,7 +33,7 @@ impl<'a> Prepare<'a> {
}
}

pub(crate) fn prepare(&mut self) -> Result<()> {
pub(crate) fn prepare(&mut self) -> anyhow::Result<()> {
self.krate.copy_source_to(self.workspace, self.source_dir)?;
self.validate_manifest()?;
self.remove_override_files()?;
Expand All @@ -44,7 +44,7 @@ impl<'a> Prepare<'a> {
Ok(())
}

fn validate_manifest(&self) -> Result<()> {
fn validate_manifest(&self) -> anyhow::Result<()> {
info!(
"validating manifest of {} on toolchain {}",
self.krate, self.toolchain
Expand All @@ -67,7 +67,7 @@ impl<'a> Prepare<'a> {
Ok(())
}

fn remove_override_files(&self) -> Result<()> {
fn remove_override_files(&self) -> anyhow::Result<()> {
let paths = [
&Path::new(".cargo").join("config"),
&Path::new(".cargo").join("config.toml"),
Expand All @@ -84,15 +84,15 @@ impl<'a> Prepare<'a> {
Ok(())
}

fn tweak_toml(&self) -> Result<()> {
fn tweak_toml(&self) -> anyhow::Result<()> {
let path = self.source_dir.join("Cargo.toml");
let mut tweaker = TomlTweaker::new(self.krate, &path, &self.patches)?;
tweaker.tweak();
tweaker.save(&path)?;
Ok(())
}

fn capture_lockfile(&mut self) -> Result<()> {
fn capture_lockfile(&mut self) -> anyhow::Result<()> {
if self.source_dir.join("Cargo.lock").exists() {
info!(
"crate {} already has a lockfile, it will not be regenerated",
Expand Down Expand Up @@ -137,7 +137,7 @@ impl<'a> Prepare<'a> {
Ok(())
}

fn fetch_deps(&mut self) -> Result<()> {
fn fetch_deps(&mut self) -> anyhow::Result<()> {
fetch_deps(self.workspace, self.toolchain, self.source_dir, &[])
}
}
Expand All @@ -147,7 +147,7 @@ pub(crate) fn fetch_deps(
toolchain: &Toolchain,
source_dir: &Path,
fetch_build_std_targets: &[&str],
) -> Result<()> {
) -> anyhow::Result<()> {
let mut missing_deps = false;
let mut cmd = Command::new(workspace, toolchain.cargo())
.args(&["fetch", "--manifest-path", "Cargo.toml"])
Expand Down Expand Up @@ -183,7 +183,11 @@ struct TomlTweaker<'a> {
}

impl<'a> TomlTweaker<'a> {
pub fn new(krate: &'a Crate, cargo_toml: &'a Path, patches: &[CratePatch]) -> Result<Self> {
pub fn new(
krate: &'a Crate,
cargo_toml: &'a Path,
patches: &[CratePatch],
) -> anyhow::Result<Self> {
let toml_content =
::std::fs::read_to_string(cargo_toml).context(PrepareError::MissingCargoToml)?;
let table: Table =
Expand Down Expand Up @@ -349,7 +353,7 @@ impl<'a> TomlTweaker<'a> {
}
}

pub fn save(self, output_file: &Path) -> Result<()> {
pub fn save(self, output_file: &Path) -> anyhow::Result<()> {
let crate_name = self.krate.to_string();
::std::fs::write(output_file, toml::to_string(&self.table)?.as_bytes())?;
info!(
Expand Down
1 change: 0 additions & 1 deletion src/toolchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,6 @@ pub(crate) fn list_installed_toolchains(rustup_home: &Path) -> anyhow::Result<Ve
#[cfg(test)]
mod tests {
use super::Toolchain;
use anyhow::Result;

#[test]
fn test_dist_serde_repr() -> anyhow::Result<()> {
Expand Down
10 changes: 5 additions & 5 deletions src/tools/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ mod binary_crates;
mod rustup;

use crate::workspace::Workspace;
use anyhow::{bail, Result};
use anyhow::bail;
use binary_crates::BinaryCrate;
use log::info;
use rustup::Rustup;
Expand Down Expand Up @@ -33,9 +33,9 @@ static INSTALLABLE_TOOLS: &[&dyn Tool] = &[

trait Tool: Send + Sync {
fn name(&self) -> &'static str;
fn is_installed(&self, workspace: &Workspace) -> Result<bool>;
fn install(&self, workspace: &Workspace, fast_install: bool) -> Result<()>;
fn update(&self, workspace: &Workspace, fast_install: bool) -> Result<()>;
fn is_installed(&self, workspace: &Workspace) -> anyhow::Result<bool>;
fn install(&self, workspace: &Workspace, fast_install: bool) -> anyhow::Result<()>;
fn update(&self, workspace: &Workspace, fast_install: bool) -> anyhow::Result<()>;

fn binary_path(&self, workspace: &Workspace) -> PathBuf {
crate::utils::normalize_path(&workspace.cargo_home().join("bin").join(format!(
Expand All @@ -46,7 +46,7 @@ trait Tool: Send + Sync {
}
}

pub(crate) fn install(workspace: &Workspace, fast_install: bool) -> Result<()> {
pub(crate) fn install(workspace: &Workspace, fast_install: bool) -> anyhow::Result<()> {
for tool in INSTALLABLE_TOOLS {
if tool.is_installed(workspace)? {
info!("tool {} is installed, trying to update it", tool.name());
Expand Down
8 changes: 4 additions & 4 deletions src/tools/rustup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::cmd::{Binary, Command, Runnable};
use crate::toolchain::MAIN_TOOLCHAIN_NAME;
use crate::tools::{Tool, RUSTUP};
use crate::workspace::Workspace;
use anyhow::{Context as _, Result};
use anyhow::Context as _;
use std::env::consts::EXE_SUFFIX;
use std::fs::{self, File};
use std::io;
Expand All @@ -23,7 +23,7 @@ impl Tool for Rustup {
"rustup"
}

fn is_installed(&self, workspace: &Workspace) -> Result<bool> {
fn is_installed(&self, workspace: &Workspace) -> anyhow::Result<bool> {
let path = self.binary_path(workspace);
if !path.is_file() {
return Ok(false);
Expand All @@ -32,7 +32,7 @@ impl Tool for Rustup {
crate::native::is_executable(path)
}

fn install(&self, workspace: &Workspace, _fast_install: bool) -> Result<()> {
fn install(&self, workspace: &Workspace, _fast_install: bool) -> anyhow::Result<()> {
fs::create_dir_all(workspace.cargo_home())?;
fs::create_dir_all(workspace.rustup_home())?;

Expand Down Expand Up @@ -73,7 +73,7 @@ impl Tool for Rustup {
Ok(())
}

fn update(&self, workspace: &Workspace, _fast_install: bool) -> Result<()> {
fn update(&self, workspace: &Workspace, _fast_install: bool) -> anyhow::Result<()> {
Command::new(workspace, &RUSTUP)
.args(&["self", "update"])
.run()
Expand Down
14 changes: 7 additions & 7 deletions src/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::build::BuildDirectory;
use crate::cmd::{Command, SandboxImage};
use crate::inside_docker::CurrentContainer;
use crate::Toolchain;
use anyhow::{Context as _, Result};
use anyhow::Context as _;
use log::info;
use std::fs;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -135,7 +135,7 @@ impl WorkspaceBuilder {

/// Initialize the workspace. This will create all the necessary local files and fetch the rest from the network. It's
/// not unexpected for this method to take minutes to run on slower network connections.
pub fn init(self) -> Result<Workspace> {
pub fn init(self) -> anyhow::Result<Workspace> {
std::fs::create_dir_all(&self.path).with_context(|| {
format!(
"failed to create workspace directory: {}",
Expand Down Expand Up @@ -207,7 +207,7 @@ impl Workspace {
}

/// Remove all the contents of all the build directories, freeing disk space.
pub fn purge_all_build_dirs(&self) -> Result<()> {
pub fn purge_all_build_dirs(&self) -> anyhow::Result<()> {
let dir = self.builds_dir();
if dir.exists() {
crate::utils::remove_dir_all(&dir)?;
Expand All @@ -216,7 +216,7 @@ impl Workspace {
}

/// Remove all the contents of the caches in the workspace, freeing disk space.
pub fn purge_all_caches(&self) -> Result<()> {
pub fn purge_all_caches(&self) -> anyhow::Result<()> {
let mut paths = vec![
self.cache_dir(),
self.cargo_home().join("git"),
Expand Down Expand Up @@ -265,7 +265,7 @@ impl Workspace {
/// # Ok(())
/// # }
/// ```
pub fn installed_toolchains(&self) -> Result<Vec<Toolchain>> {
pub fn installed_toolchains(&self) -> anyhow::Result<Vec<Toolchain>> {
crate::toolchain::list_installed_toolchains(&self.rustup_home())
}

Expand Down Expand Up @@ -313,7 +313,7 @@ impl Workspace {
&self.inner.rustup_profile
}

fn init(&self, fast_init: bool) -> Result<()> {
fn init(&self, fast_init: bool) -> anyhow::Result<()> {
info!("installing tools required by rustwide");
crate::tools::install(self, fast_init)?;
if !self.fetch_registry_index_during_builds() {
Expand All @@ -324,7 +324,7 @@ impl Workspace {
}

#[allow(clippy::unnecessary_wraps)] // hopefully we could actually catch the error here at some point
fn update_cratesio_registry(&self) -> Result<()> {
fn update_cratesio_registry(&self) -> anyhow::Result<()> {
// This nop cargo command is to update the registry so we don't have to do it for each
// crate. using `install` is a temporary solution until
// https://github.com/rust-lang/cargo/pull/5961 is ready
Expand Down
12 changes: 6 additions & 6 deletions tests/buildtest/inside_docker.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg_attr(windows, allow(unused))]

use anyhow::{Context, Result};
use anyhow::Context;
use std::io::Write;
use std::path::Path;
use std::process::Command;
Expand All @@ -22,7 +22,7 @@ fn test_path_based_patch() {
execute("buildtest::path_based_patch").unwrap();
}

fn execute(test: &str) -> Result<()> {
fn execute(test: &str) -> anyhow::Result<()> {
// The current working directory is mounted in the container to /outside.
// The binary to execute is remapped to be prefixed by /outside instead of the current
// directory.
Expand Down Expand Up @@ -69,13 +69,13 @@ fn execute(test: &str) -> Result<()> {
}

trait CommandExt {
fn map_user_group(&mut self) -> Result<&mut Self>;
fn assert(&mut self) -> Result<()>;
fn map_user_group(&mut self) -> anyhow::Result<&mut Self>;
fn assert(&mut self) -> anyhow::Result<()>;
}

impl CommandExt for Command {
#[cfg(unix)]
fn map_user_group(&mut self) -> Result<&mut Self> {
fn map_user_group(&mut self) -> anyhow::Result<&mut Self> {
use std::os::unix::fs::MetadataExt;
let gid = std::fs::metadata(DOCKER_SOCKET)?.gid();
let uid = nix::unistd::Uid::effective();
Expand All @@ -89,7 +89,7 @@ impl CommandExt for Command {
Ok(self)
}

fn assert(&mut self) -> Result<()> {
fn assert(&mut self) -> anyhow::Result<()> {
let out = self.output()?;
if !out.status.success() {
eprintln!("failed to execute command {:?}", self);
Expand Down
1 change: 0 additions & 1 deletion tests/buildtest/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use log::LevelFilter;
use rustwide::cmd::{ProcessLinesActions, SandboxBuilder};

Expand Down
1 change: 0 additions & 1 deletion tests/buildtest/runner.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use rand::{distributions::Alphanumeric, Rng};
use rustwide::{cmd::SandboxBuilder, Build, BuildBuilder, Crate, Toolchain, Workspace};
use std::path::Path;
Expand Down
1 change: 0 additions & 1 deletion tests/integration/crates_alt.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use anyhow::Result;
use rustwide::{AlternativeRegistry, Crate};

const INDEX_URL: &str = "https://github.com/rust-lang/staging.crates.io-index";
Expand Down
Loading

0 comments on commit 296c02f

Please sign in to comment.