From 5075c77957e127be51e1b5271567f906abeb50c6 Mon Sep 17 00:00:00 2001 From: Pascal Hertleif Date: Wed, 13 Nov 2019 20:51:57 +0100 Subject: Use anyhow::Result in xtask, add contexts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This builds on #2231 but was actually done before that. You see, the cause for #2231 was that I got this error message: Error: Error { kind: Io(Os { code: 2, kind: NotFound, message: "No such file or directory" }) } Just switching to `anyhow::Result` got me stack traces (when setting `RUST_LIB_BACKTRACE=1`) that at least showed stack backtrace: 0: std::backtrace::Backtrace::create 1: std::backtrace::Backtrace::capture 2: anyhow::error:: for anyhow::Error>::from 3: xtask::install_server 4: xtask::install 5: xtask::main 6: std::rt::lang_start::{{closure}} 7: std::panicking::try::do_call 8: __rust_maybe_catch_panic 9: std::rt::lang_start_internal 10: std::rt::lang_start 11: main With the added contexts (not at all exhaustive), the error became Error: install server Caused by: 0: build AutoCfg with target directory 1: No such file or directory (os error 2) Since anyhow is such a small thing (no new transitive dependencies!), and in general gives you `Result>` on steroids, I think this a nice small change. The only slightly annoying thing was to replace all the `Err(format!(…))?` calls (haven't even looked at whether we can make it support wrapping strings though), but the `bail!` macro is shorter anyway :) --- Cargo.lock | 7 +++++++ xtask/Cargo.toml | 1 + xtask/src/bin/pre-commit.rs | 4 ++-- xtask/src/codegen.rs | 8 +++----- xtask/src/codegen/gen_parser_tests.rs | 4 ++-- xtask/src/lib.rs | 16 +++++++--------- xtask/src/main.rs | 25 ++++++++++++++----------- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 97452951c..c9ab3ef10 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8,6 +8,11 @@ dependencies = [ "memchr 2.2.1 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "anyhow" +version = "1.0.19" +source = "registry+https://github.com/rust-lang/crates.io-index" + [[package]] name = "arrayvec" version = "0.5.1" @@ -1823,6 +1828,7 @@ dependencies = [ name = "xtask" version = "0.1.0" dependencies = [ + "anyhow 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)", "autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)", "pico-args 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)", "proc-macro2 1.0.6 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1847,6 +1853,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" [metadata] "checksum aho-corasick 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)" = "58fb5e95d83b38284460a5fda7d6470aa0b8844d283a0b614b8535e880800d2d" +"checksum anyhow 1.0.19 (registry+https://github.com/rust-lang/crates.io-index)" = "57114fc2a6cc374bce195d3482057c846e706d252ff3604363449695684d7a0d" "checksum arrayvec 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "cff77d8686867eceff3105329d4698d96c2391c176d5d03adc90c7389162b5b8" "checksum atty 0.2.13 (registry+https://github.com/rust-lang/crates.io-index)" = "1803c647a3ec87095e7ae7acfca019e98de5ec9a7d01343f611cf3152ed71a90" "checksum autocfg 0.1.7 (registry+https://github.com/rust-lang/crates.io-index)" = "1d49d90015b3c36167a20fe2810c5cd875ad504b39cff3d4eae7977e6b7c1cb2" diff --git a/xtask/Cargo.toml b/xtask/Cargo.toml index 72b39c3b4..bba4be192 100644 --- a/xtask/Cargo.toml +++ b/xtask/Cargo.toml @@ -13,3 +13,4 @@ quote = "1.0.2" proc-macro2 = "1.0.1" ron = "0.5.1" serde = { version = "1.0.0", features = ["derive"] } +anyhow = "1.0.19" diff --git a/xtask/src/bin/pre-commit.rs b/xtask/src/bin/pre-commit.rs index cc6ccb25e..44507fb74 100644 --- a/xtask/src/bin/pre-commit.rs +++ b/xtask/src/bin/pre-commit.rs @@ -19,10 +19,10 @@ fn update_staged() -> Result<()> { .current_dir(&root) .output()?; if !output.status.success() { - Err(format!( + anyhow::bail!( "`git diff --diff-filter=MAR --name-only --cached` exited with {}", output.status - ))?; + ); } for line in String::from_utf8(output.stdout)?.lines() { run(&format!("git update-index --add {}", root.join(line).to_string_lossy()), ".")?; diff --git a/xtask/src/codegen.rs b/xtask/src/codegen.rs index 770b55a9a..53f524f42 100644 --- a/xtask/src/codegen.rs +++ b/xtask/src/codegen.rs @@ -52,7 +52,7 @@ fn update(path: &Path, contents: &str, mode: Mode) -> Result<()> { _ => (), } if mode == Mode::Verify { - Err(format!("`{}` is not up-to-date", path.display()))?; + anyhow::bail!("`{}` is not up-to-date", path.display()); } eprintln!("updating {}", path.display()); fs::write(path, contents)?; @@ -101,10 +101,8 @@ fn do_extract_comment_blocks(text: &str, allow_blocks_with_empty_lins: bool) -> let is_comment = line.starts_with(prefix); if is_comment { block.push(line[prefix.len()..].to_string()); - } else { - if !block.is_empty() { - res.push(mem::replace(&mut block, Vec::new())) - } + } else if !block.is_empty() { + res.push(mem::replace(&mut block, Vec::new())); } } if !block.is_empty() { diff --git a/xtask/src/codegen/gen_parser_tests.rs b/xtask/src/codegen/gen_parser_tests.rs index d0f0f683b..2977da2fa 100644 --- a/xtask/src/codegen/gen_parser_tests.rs +++ b/xtask/src/codegen/gen_parser_tests.rs @@ -102,10 +102,10 @@ fn tests_from_dir(dir: &Path) -> Result { for test in collect_tests(&text) { if test.ok { if let Some(old_test) = res.ok.insert(test.name.clone(), test) { - return Err(format!("Duplicate test: {}", old_test.name).into()); + anyhow::bail!("Duplicate test: {}", old_test.name); } } else if let Some(old_test) = res.err.insert(test.name.clone(), test) { - return Err(format!("Duplicate test: {}", old_test.name).into()); + anyhow::bail!("Duplicate test: {}", old_test.name); } } Ok(()) diff --git a/xtask/src/lib.rs b/xtask/src/lib.rs index bae4c4650..bfee2f9c8 100644 --- a/xtask/src/lib.rs +++ b/xtask/src/lib.rs @@ -2,10 +2,10 @@ pub mod codegen; +use anyhow::Context; +pub use anyhow::Result; use std::{ - env, - error::Error, - fs, + env, fs, io::{Error as IoError, ErrorKind}, path::{Path, PathBuf}, process::{Command, Output, Stdio}, @@ -13,8 +13,6 @@ use std::{ use crate::codegen::Mode; -pub type Result = std::result::Result>; - const TOOLCHAIN: &str = "stable"; pub fn project_root() -> PathBuf { @@ -69,7 +67,7 @@ pub fn run_rustfmt(mode: Mode) -> Result<()> { .status() { Ok(status) if status.success() => (), - _ => install_rustfmt()?, + _ => install_rustfmt().context("install rustfmt")?, }; if mode == Mode::Verify { @@ -112,7 +110,7 @@ pub fn run_clippy() -> Result<()> { .status() { Ok(status) if status.success() => (), - _ => install_clippy()?, + _ => install_clippy().context("install clippy")?, }; let allowed_lints = [ @@ -162,9 +160,9 @@ where let exec = args.next().unwrap(); let mut cmd = Command::new(exec); f(cmd.args(args).current_dir(proj_dir).stderr(Stdio::inherit())); - let output = cmd.output()?; + let output = cmd.output().with_context(|| format!("running `{}`", cmdline))?; if !output.status.success() { - Err(format!("`{}` exited with {}", cmdline, output.status))?; + anyhow::bail!("`{}` exited with {}", cmdline, output.status); } Ok(output) } diff --git a/xtask/src/main.rs b/xtask/src/main.rs index 84842b428..7eab1c949 100644 --- a/xtask/src/main.rs +++ b/xtask/src/main.rs @@ -9,6 +9,7 @@ //! `.cargo/config`. mod help; +use anyhow::Context; use autocfg; use core::fmt::Write; use core::str; @@ -114,21 +115,21 @@ fn handle_extra_flags(e: pico_args::Error) -> Result<()> { write!(&mut invalid_flags, "{}, ", flag)?; } let (invalid_flags, _) = invalid_flags.split_at(invalid_flags.len() - 2); - Err(format!("Invalid flags: {}", invalid_flags).into()) + anyhow::bail!("Invalid flags: {}", invalid_flags) } else { - Err(e.to_string().into()) + anyhow::bail!(e.to_string()) } } fn install(opts: InstallOpt) -> Result<()> { if cfg!(target_os = "macos") { - fix_path_for_mac()? + fix_path_for_mac().context("Fix path for mac")? } if let Some(server) = opts.server { - install_server(server)?; + install_server(server).context("install server")?; } if let Some(client) = opts.client { - install_client(client)?; + install_client(client).context("install client")?; } Ok(()) } @@ -140,7 +141,7 @@ fn fix_path_for_mac() -> Result<()> { const ROOT_DIR: &str = ""; let home_dir = match env::var("HOME") { Ok(home) => home, - Err(e) => Err(format!("Failed getting HOME from environment with error: {}.", e))?, + Err(e) => anyhow::bail!("Failed getting HOME from environment with error: {}.", e), }; [ROOT_DIR, &home_dir] @@ -154,12 +155,12 @@ fn fix_path_for_mac() -> Result<()> { if !vscode_path.is_empty() { let vars = match env::var_os("PATH") { Some(path) => path, - None => Err("Could not get PATH variable from env.")?, + None => anyhow::bail!("Could not get PATH variable from env."), }; let mut paths = env::split_paths(&vars).collect::>(); paths.append(&mut vscode_path); - let new_paths = env::join_paths(paths)?; + let new_paths = env::join_paths(paths).context("build env PATH")?; env::set_var("PATH", &new_paths); } @@ -198,7 +199,7 @@ fn install_client(ClientOpt::VsCode: ClientOpt) -> Result<()> { let code_binary = match code_binary { Some(it) => it, - None => Err("Can't execute `code --version`. Perhaps it is not in $PATH?")?, + None => anyhow::bail!("Can't execute `code --version`. Perhaps it is not in $PATH?"), }; Cmd { @@ -219,8 +220,10 @@ fn install_client(ClientOpt::VsCode: ClientOpt) -> Result<()> { .run_with_output()?; if !str::from_utf8(&output.stdout)?.contains("ra-lsp") { - Err("Could not install the Visual Studio Code extension. \ - Please make sure you have at least NodeJS 10.x installed and try again.")?; + anyhow::bail!( + "Could not install the Visual Studio Code extension. \ + Please make sure you have at least NodeJS 10.x installed and try again." + ); } Ok(()) -- cgit v1.2.3