From 37cffbe0a3e921a7873cdab563331b33a4450220 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 8 Dec 2017 11:31:17 -0800 Subject: [PATCH] Start migration to the `failure` crate This commit is the initial steps to migrate Cargo's error handling from the `error-chain` crate to the `failure` crate. This is intended to be a low-cost (in terms of diff) transition where possible so it's note "purely idiomatic `failure` crate" just yet. The `error-chain` dependency is dropped in this commit and Cargo now canonically uses the `Error` type from the `failure` crate. The main last remnant of `error-chain` is a custom local extension trait to use `chain_err` instead of `with_context`. I'll try to follow up with a commit that renames this later but I wanted to make sure everything worked first! (and `chain_err` is used practically everywhere). Some minor tweaks happened in the tests as I touched up a few error messages here and there but overall the UI of Cargo should be exactly the same before and after this commit. --- Cargo.toml | 2 +- src/bin/bench.rs | 6 +- src/bin/cargo.rs | 24 ++-- src/bin/check.rs | 4 +- src/bin/help.rs | 3 +- src/bin/install.rs | 4 +- src/bin/locate_project.rs | 6 +- src/bin/login.rs | 10 +- src/bin/owner.rs | 5 +- src/bin/publish.rs | 5 +- src/bin/run.rs | 7 +- src/bin/rustc.rs | 4 +- src/bin/search.rs | 5 +- src/bin/test.rs | 6 +- src/bin/yank.rs | 5 +- src/cargo/core/dependency.rs | 7 +- src/cargo/core/manifest.rs | 4 +- src/cargo/core/package.rs | 2 +- src/cargo/core/package_id_spec.rs | 12 +- src/cargo/core/registry.rs | 9 +- src/cargo/core/resolver/mod.rs | 12 +- src/cargo/core/source/source_id.rs | 4 +- src/cargo/core/workspace.rs | 20 +-- src/cargo/lib.rs | 52 +++----- src/cargo/ops/cargo_clean.rs | 4 +- src/cargo/ops/cargo_compile.rs | 16 +-- src/cargo/ops/cargo_doc.rs | 5 +- src/cargo/ops/cargo_install.rs | 94 ++++++------- src/cargo/ops/cargo_new.rs | 28 ++-- src/cargo/ops/cargo_package.rs | 6 +- src/cargo/ops/cargo_read_manifest.rs | 10 +- src/cargo/ops/cargo_run.rs | 12 +- src/cargo/ops/cargo_rustc/custom_build.rs | 7 +- src/cargo/ops/cargo_rustc/fingerprint.rs | 2 +- src/cargo/ops/cargo_rustc/job_queue.rs | 5 +- src/cargo/ops/cargo_rustc/layout.rs | 4 +- src/cargo/ops/cargo_rustc/mod.rs | 13 +- src/cargo/ops/cargo_test.rs | 12 +- src/cargo/ops/lockfile.rs | 8 +- src/cargo/ops/registry.rs | 24 ++-- src/cargo/sources/config.rs | 16 +-- src/cargo/sources/directory.rs | 2 +- src/cargo/sources/git/source.rs | 8 +- src/cargo/sources/git/utils.rs | 11 +- src/cargo/sources/path.rs | 14 +- src/cargo/sources/registry/index.rs | 4 +- src/cargo/sources/registry/mod.rs | 6 +- src/cargo/sources/registry/remote.rs | 4 +- src/cargo/sources/replaced.rs | 6 +- src/cargo/util/cfg.rs | 6 +- src/cargo/util/config.rs | 60 ++++----- src/cargo/util/errors.rs | 153 ++++++++-------------- src/cargo/util/flock.rs | 15 ++- src/cargo/util/important_paths.rs | 5 +- src/cargo/util/mod.rs | 2 +- src/cargo/util/network.rs | 80 +++++------ src/cargo/util/paths.rs | 39 +++--- src/cargo/util/process_builder.rs | 55 ++++---- src/cargo/util/to_semver.rs | 13 +- src/cargo/util/to_url.rs | 4 +- src/cargo/util/toml/mod.rs | 20 +-- src/crates-io/Cargo.toml | 2 +- src/crates-io/lib.rs | 68 +++------- tests/build.rs | 2 +- tests/cargotest/support/mod.rs | 22 ++-- tests/doc.rs | 4 +- tests/install.rs | 11 +- 67 files changed, 511 insertions(+), 589 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 76ce4d803ee..fe2658b3767 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ crypto-hash = "0.3" curl = "0.4.6" docopt = "0.8.1" env_logger = "0.4" -error-chain = "0.11.0" +failure = "0.1.1" filetime = "0.1" flate2 = "0.2" fs2 = "0.4" diff --git a/src/bin/bench.rs b/src/bin/bench.rs index f871932d9ec..eaf4c0d6b86 100644 --- a/src/bin/bench.rs +++ b/src/bin/bench.rs @@ -2,7 +2,7 @@ use std::env; use cargo::core::Workspace; use cargo::ops::{self, MessageFormat, Packages}; -use cargo::util::{CliResult, CliError, Config, CargoErrorKind}; +use cargo::util::{CliResult, CliError, Config}; use cargo::util::important_paths::{find_root_manifest_for_wd}; #[derive(Deserialize)] @@ -144,8 +144,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { None => Ok(()), Some(err) => { Err(match err.exit.as_ref().and_then(|e| e.code()) { - Some(i) => CliError::new("bench failed".into(), i), - None => CliError::new(CargoErrorKind::CargoTestErrorKind(err).into(), 101) + Some(i) => CliError::new(format_err!("bench failed"), i), + None => CliError::new(err.into(), 101) }) } } diff --git a/src/bin/cargo.rs b/src/bin/cargo.rs index 53025a23583..b54fe28d97e 100644 --- a/src/bin/cargo.rs +++ b/src/bin/cargo.rs @@ -1,5 +1,7 @@ extern crate cargo; extern crate env_logger; +#[macro_use] +extern crate failure; extern crate git2_curl; extern crate toml; #[macro_use] @@ -15,8 +17,8 @@ use std::fs; use std::path::{Path, PathBuf}; use cargo::core::shell::{Shell, Verbosity}; -use cargo::util::{self, CliResult, lev_distance, Config, CargoResult, CargoError, CargoErrorKind}; -use cargo::util::CliError; +use cargo::util::{self, CliResult, lev_distance, Config, CargoResult}; +use cargo::util::{CliError, ProcessError}; #[derive(Deserialize)] pub struct Flags { @@ -87,7 +89,7 @@ fn main() { let args: Vec<_> = try!(env::args_os() .map(|s| { s.into_string().map_err(|s| { - CargoError::from(format!("invalid unicode in argument: {:?}", s)) + format_err!("invalid unicode in argument: {:?}", s) }) }) .collect()); @@ -315,15 +317,15 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[String]) -> C let command = match path { Some(command) => command, None => { - return Err(CargoError::from(match find_closest(config, cmd) { - Some(closest) => { - format!("no such subcommand: `{}`\n\n\tDid you mean `{}`?\n", + let err = match find_closest(config, cmd) { + Some(closest) => { + format_err!("no such subcommand: `{}`\n\n\tDid you mean `{}`?\n", cmd, closest) - } - None => format!("no such subcommand: `{}`", cmd), - }) - .into()) + } + None => format_err!("no such subcommand: `{}`", cmd), + }; + return Err(CliError::new(err, 101)) } }; @@ -336,7 +338,7 @@ fn execute_external_subcommand(config: &Config, cmd: &str, args: &[String]) -> C Err(e) => e, }; - if let &CargoErrorKind::ProcessErrorKind(ref perr) = err.kind() { + if let Some(perr) = err.downcast_ref::() { if let Some(code) = perr.exit.as_ref().and_then(|c| c.code()) { return Err(CliError::code(code)); } diff --git a/src/bin/check.rs b/src/bin/check.rs index d6d17950136..98172a28c1b 100644 --- a/src/bin/check.rs +++ b/src/bin/check.rs @@ -114,8 +114,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { Some("test") => true, None => false, Some(profile) => { - let err = format!("unknown profile: `{}`, only `test` is currently supported", - profile).into(); + let err = format_err!("unknown profile: `{}`, only `test` is \ + currently supported", profile); return Err(CliError::new(err, 101)) } }; diff --git a/src/bin/help.rs b/src/bin/help.rs index f7f564ee7b9..409247c614e 100644 --- a/src/bin/help.rs +++ b/src/bin/help.rs @@ -18,5 +18,6 @@ pub fn execute(_: Options, _: &mut Config) -> CliResult { // This is a dummy command just so that `cargo help help` works. // The actual delegation of help flag to subcommands is handled by the // cargo command. - Err(CliError::new("help command should not be executed directly".into(), 101)) + Err(CliError::new(format_err!("help command should not be executed directly"), + 101)) } diff --git a/src/bin/install.rs b/src/bin/install.rs index 2d3a135a4db..2366f788a45 100644 --- a/src/bin/install.rs +++ b/src/bin/install.rs @@ -1,6 +1,6 @@ use cargo::ops; use cargo::core::{SourceId, GitReference}; -use cargo::util::{CargoError, CliResult, Config, ToUrl}; +use cargo::util::{CliResult, Config, ToUrl}; #[derive(Deserialize)] pub struct Options { @@ -154,7 +154,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { let krates = options.arg_crate.iter().map(|s| &s[..]).collect::>(); let vers = match (&options.flag_vers, &options.flag_version) { - (&Some(_), &Some(_)) => return Err(CargoError::from("Invalid arguments.").into()), + (&Some(_), &Some(_)) => return Err(format_err!("invalid arguments").into()), (&Some(ref v), _) | (_, &Some(ref v)) => Some(v.as_ref()), _ => None, }; diff --git a/src/bin/locate_project.rs b/src/bin/locate_project.rs index 6e16cca2d37..7065938c0c9 100644 --- a/src/bin/locate_project.rs +++ b/src/bin/locate_project.rs @@ -27,9 +27,9 @@ pub fn execute(flags: LocateProjectFlags, config: &mut Config) -> CliResult { let root = find_root_manifest_for_wd(flags.flag_manifest_path, config.cwd())?; let string = root.to_str() - .ok_or_else(|| "Your project path contains \ - characters not representable in \ - Unicode".into()) + .ok_or_else(|| format_err!("your project path contains \ + characters not representable in \ + Unicode")) .map_err(|e| CliError::new(e, 1))?; let location = ProjectLocation { root: string.to_string() }; diff --git a/src/bin/login.rs b/src/bin/login.rs index d8a0ad47109..e363507ba25 100644 --- a/src/bin/login.rs +++ b/src/bin/login.rs @@ -4,7 +4,7 @@ use std::io; use cargo::ops; use cargo::core::{SourceId, Source}; use cargo::sources::RegistrySource; -use cargo::util::{CargoError, CliResult, CargoResultExt, Config}; +use cargo::util::{CliResult, CargoResultExt, Config, CargoError}; #[derive(Deserialize)] pub struct Options { @@ -48,7 +48,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { &options.flag_z)?; if options.flag_registry.is_some() && !config.cli_unstable().unstable_options { - return Err(CargoError::from("registry option is an unstable feature and requires -Zunstable-options to use.").into()); + return Err(format_err!("registry option is an unstable feature and \ + requires -Zunstable-options to use.").into()); } let token = match options.arg_token { @@ -56,7 +57,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { None => { let host = match options.flag_registry { Some(ref _registry) => { - return Err(CargoError::from("token must be provided when --registry is provided.").into()); + return Err(format_err!("token must be provided when \ + --registry is provided.").into()) } None => { let src = SourceId::crates_io(config)?; @@ -71,7 +73,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { let input = io::stdin(); input.lock().read_line(&mut line).chain_err(|| { "failed to read stdin" - })?; + }).map_err(CargoError::from)?; line.trim().to_string() } }; diff --git a/src/bin/owner.rs b/src/bin/owner.rs index 466d565975e..5708985ea82 100644 --- a/src/bin/owner.rs +++ b/src/bin/owner.rs @@ -1,5 +1,5 @@ use cargo::ops; -use cargo::util::{CargoError, CliResult, Config}; +use cargo::util::{CliResult, Config}; #[derive(Deserialize)] pub struct Options { @@ -67,7 +67,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { }; if opts.registry.is_some() && !config.cli_unstable().unstable_options { - return Err(CargoError::from("registry option is an unstable feature and requires -Zunstable-options to use.").into()); + return Err(format_err!("registry option is an unstable feature and \ + requires -Zunstable-options to use.").into()) } ops::modify_owners(config, &opts)?; diff --git a/src/bin/publish.rs b/src/bin/publish.rs index 6b574cb7f5c..7294ea70ee6 100644 --- a/src/bin/publish.rs +++ b/src/bin/publish.rs @@ -1,6 +1,6 @@ use cargo::core::Workspace; use cargo::ops; -use cargo::util::{CargoError, CliResult, Config}; +use cargo::util::{CliResult, Config}; use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(Deserialize)] @@ -74,7 +74,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { } = options; if registry.is_some() && !config.cli_unstable().unstable_options { - return Err(CargoError::from("registry option is an unstable feature and requires -Zunstable-options to use.").into()); + return Err(format_err!("registry option is an unstable feature and \ + requires -Zunstable-options to use.").into()) } // TODO: Deprecated diff --git a/src/bin/run.rs b/src/bin/run.rs index 21486baf6ea..48939d6547e 100644 --- a/src/bin/run.rs +++ b/src/bin/run.rs @@ -2,7 +2,7 @@ use std::iter::FromIterator; use cargo::core::Workspace; use cargo::ops::{self, MessageFormat, Packages}; -use cargo::util::{CliResult, CliError, Config, CargoErrorKind}; +use cargo::util::{CliResult, CliError, Config}; use cargo::util::important_paths::{find_root_manifest_for_wd}; #[derive(Deserialize)] @@ -118,8 +118,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { // bad and we always want to forward that up. let exit = match err.exit { Some(exit) => exit, - None => return Err( - CliError::new(CargoErrorKind::ProcessErrorKind(err).into(), 101)), + None => return Err(CliError::new(err.into(), 101)), }; // If `-q` was passed then we suppress extra error information about @@ -129,7 +128,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { Err(if options.flag_quiet == Some(true) { CliError::code(exit_code) } else { - CliError::new(CargoErrorKind::ProcessErrorKind(err).into(), exit_code) + CliError::new(err.into(), exit_code) }) } } diff --git a/src/bin/rustc.rs b/src/bin/rustc.rs index 747f2ead958..98d7c410de4 100644 --- a/src/bin/rustc.rs +++ b/src/bin/rustc.rs @@ -104,8 +104,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { Some("bench") => CompileMode::Bench, Some("check") => CompileMode::Check {test: false}, Some(mode) => { - let err = format!("unknown profile: `{}`, use dev, - test, or bench", mode).into(); + let err = format_err!("unknown profile: `{}`, use dev, + test, or bench", mode); return Err(CliError::new(err, 101)) } }; diff --git a/src/bin/search.rs b/src/bin/search.rs index 70b38ecfbfd..23a656c0653 100644 --- a/src/bin/search.rs +++ b/src/bin/search.rs @@ -1,5 +1,5 @@ use cargo::ops; -use cargo::util::{CargoError, CliResult, Config}; +use cargo::util::{CliResult, Config}; use std::cmp; @@ -57,7 +57,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { } = options; if registry.is_some() && !config.cli_unstable().unstable_options { - return Err(CargoError::from("registry option is an unstable feature and requires -Zunstable-options to use.").into()); + return Err(format_err!("registry option is an unstable feature and \ + requires -Zunstable-options to use.").into()) } // TODO: Depricated diff --git a/src/bin/test.rs b/src/bin/test.rs index 58a616ad92b..c4c2bac3daa 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -2,7 +2,7 @@ use std::env; use cargo::core::Workspace; use cargo::ops::{self, MessageFormat, Packages}; -use cargo::util::{CliResult, CliError, Config, CargoErrorKind}; +use cargo::util::{CliResult, CliError, Config}; use cargo::util::important_paths::find_root_manifest_for_wd; #[derive(Deserialize)] @@ -177,8 +177,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { None => Ok(()), Some(err) => { Err(match err.exit.as_ref().and_then(|e| e.code()) { - Some(i) => CliError::new(err.hint().into(), i), - None => CliError::new(CargoErrorKind::CargoTestErrorKind(err).into(), 101), + Some(i) => CliError::new(format_err!("{}", err.hint()), i), + None => CliError::new(err.into(), 101), }) } } diff --git a/src/bin/yank.rs b/src/bin/yank.rs index ce5dfb7ea5f..293beec21e1 100644 --- a/src/bin/yank.rs +++ b/src/bin/yank.rs @@ -1,5 +1,5 @@ use cargo::ops; -use cargo::util::{CargoError, CliResult, Config}; +use cargo::util::{CliResult, Config}; #[derive(Deserialize)] pub struct Options { @@ -56,7 +56,8 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult { &options.flag_z)?; if options.flag_registry.is_some() && !config.cli_unstable().unstable_options { - return Err(CargoError::from("registry option is an unstable feature and requires -Zunstable-options to use.").into()); + return Err(format_err!("registry option is an unstable feature and \ + requires -Zunstable-options to use.").into()) } ops::yank(config, diff --git a/src/cargo/core/dependency.rs b/src/cargo/core/dependency.rs index 24d6c9f35f8..f34ef2f141e 100644 --- a/src/cargo/core/dependency.rs +++ b/src/cargo/core/dependency.rs @@ -350,9 +350,10 @@ impl FromStr for Platform { fn from_str(s: &str) -> CargoResult { if s.starts_with("cfg(") && s.ends_with(')') { let s = &s[4..s.len()-1]; - s.parse().map(Platform::Cfg).chain_err(|| { - format!("failed to parse `{}` as a cfg expression", s) - }) + let p = s.parse().map(Platform::Cfg).chain_err(|| { + format_err!("failed to parse `{}` as a cfg expression", s) + })?; + Ok(p) } else { Ok(Platform::Name(s.to_string())) } diff --git a/src/cargo/core/manifest.rs b/src/cargo/core/manifest.rs index 51ed364ee25..be54d62bf00 100644 --- a/src/cargo/core/manifest.rs +++ b/src/cargo/core/manifest.rs @@ -332,8 +332,8 @@ impl Manifest { pub fn feature_gate(&self) -> CargoResult<()> { if self.im_a_teapot.is_some() { self.features.require(Feature::test_dummy_unstable()).chain_err(|| { - "the `im-a-teapot` manifest key is unstable and may not work \ - properly in England" + format_err!("the `im-a-teapot` manifest key is unstable and may \ + not work properly in England") })?; } diff --git a/src/cargo/core/package.rs b/src/cargo/core/package.rs index 1edc51b03ea..7042dd5d0b0 100644 --- a/src/cargo/core/package.rs +++ b/src/cargo/core/package.rs @@ -207,7 +207,7 @@ impl<'cfg> PackageSet<'cfg> { internal(format!("couldn't find source for `{}`", id)) })?; let pkg = source.download(id).chain_err(|| { - "unable to get packages from source" + format_err!("unable to get packages from source") })?; assert!(slot.fill(pkg).is_ok()); Ok(slot.borrow().unwrap()) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index d271f2f6660..318fc0b3ed8 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -6,7 +6,7 @@ use url::Url; use core::PackageId; use util::{ToUrl, ToSemver}; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt}; #[derive(Clone, PartialEq, Eq, Debug)] pub struct PackageIdSpec { @@ -49,7 +49,7 @@ impl PackageIdSpec { where I: IntoIterator { let spec = PackageIdSpec::parse(spec).chain_err(|| { - format!("invalid package id specification: `{}`", spec) + format_err!("invalid package id specification: `{}`", spec) })?; spec.query(i) } @@ -70,11 +70,11 @@ impl PackageIdSpec { url.set_fragment(None); let (name, version) = { let mut path = url.path_segments().ok_or_else(|| { - CargoError::from(format!("pkgid urls must have a path: {}", url)) + format_err!("pkgid urls must have a path: {}", url) })?; let path_name = path.next_back().ok_or_else(|| { - CargoError::from(format!("pkgid urls must have at least one path \ - component: {}", url)) + format_err!("pkgid urls must have at least one path \ + component: {}", url) })?; match frag { Some(fragment) => { @@ -150,7 +150,7 @@ impl PackageIdSpec { let mut vec = vec![ret, other]; vec.extend(ids); minimize(&mut msg, &vec, self); - Err(msg.into()) + Err(format_err!("{}", msg)) } None => Ok(ret) }; diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index c1200fd36a2..9257530cadc 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -211,7 +211,7 @@ impl<'cfg> PackageRegistry<'cfg> { } Ok(summary) }).collect::>>().chain_err(|| { - format!("failed to resolve patches for `{}`", url) + format_err!("failed to resolve patches for `{}`", url) })?; self.patches.insert(url.clone(), deps); @@ -236,7 +236,8 @@ impl<'cfg> PackageRegistry<'cfg> { // Ensure the source has fetched all necessary remote data. let _p = profile::start(format!("updating: {}", source_id)); self.sources.get_mut(source_id).unwrap().update() - })().chain_err(|| format!("Unable to update {}", source_id)) + })().chain_err(|| format_err!("Unable to update {}", source_id))?; + Ok(()) } fn query_overrides(&mut self, dep: &Dependency) @@ -362,8 +363,8 @@ impl<'cfg> Registry for PackageRegistry<'cfg> { // Ensure the requested source_id is loaded self.ensure_loaded(dep.source_id(), Kind::Normal).chain_err(|| { - format!("failed to load source for a dependency \ - on `{}`", dep.name()) + format_err!("failed to load source for a dependency \ + on `{}`", dep.name()) })?; let source = self.sources.get_mut(dep.source_id()); diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs index 712808391dc..cfefbf48992 100644 --- a/src/cargo/core/resolver/mod.rs +++ b/src/cargo/core/resolver/mod.rs @@ -822,7 +822,7 @@ fn activation_error(cx: &Context, .collect::>() .join(", "))); - return msg.into() + return format_err!("{}", msg) } // Once we're all the way down here, we're definitely lost in the @@ -886,7 +886,7 @@ fn activation_error(cx: &Context, dep.version_req()) }; - msg.into() + format_err!("{}", msg) } // Returns if `a` and `b` are compatible in the semver sense. This is a @@ -1116,10 +1116,10 @@ impl<'a> Context<'a> { let mut summaries = registry.query_vec(dep)?.into_iter(); let s = summaries.next().ok_or_else(|| { - format!("no matching package for override `{}` found\n\ - location searched: {}\n\ - version required: {}", - spec, dep.source_id(), dep.version_req()) + format_err!("no matching package for override `{}` found\n\ + location searched: {}\n\ + version required: {}", + spec, dep.source_id(), dep.version_req()) })?; let summaries = summaries.collect::>(); if !summaries.is_empty() { diff --git a/src/cargo/core/source/source_id.rs b/src/cargo/core/source/source_id.rs index 350437055e5..1651f530689 100644 --- a/src/cargo/core/source/source_id.rs +++ b/src/cargo/core/source/source_id.rs @@ -93,7 +93,7 @@ impl SourceId { pub fn from_url(string: &str) -> CargoResult { let mut parts = string.splitn(2, '+'); let kind = parts.next().unwrap(); - let url = parts.next().ok_or_else(|| format!("invalid source `{}`", string))?; + let url = parts.next().ok_or_else(|| format_err!("invalid source `{}`", string))?; match kind { "git" => { @@ -124,7 +124,7 @@ impl SourceId { let url = url.to_url()?; SourceId::new(Kind::Path, url) } - kind => Err(format!("unsupported source protocol: {}", kind).into()) + kind => Err(format_err!("unsupported source protocol: {}", kind)) } } diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index c83dc63426b..5d21dd8d41a 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -196,11 +196,12 @@ impl<'cfg> Workspace<'cfg> { /// actually a "virtual Cargo.toml", in which case an error is returned /// indicating that something else should be passed. pub fn current(&self) -> CargoResult<&Package> { - self.current_opt().ok_or_else(|| - format!("manifest path `{}` is a virtual manifest, but this \ - command requires running against an actual package in \ - this workspace", self.current_manifest.display()).into() - ) + let pkg = self.current_opt().ok_or_else(|| { + format_err!("manifest path `{}` is a virtual manifest, but this \ + command requires running against an actual package in \ + this workspace", self.current_manifest.display()) + })?; + Ok(pkg) } pub fn current_opt(&self) -> Option<&Package> { @@ -753,12 +754,13 @@ impl WorkspaceRootConfig { None => return Ok(Vec::new()), }; let res = glob(path).chain_err(|| { - format!("could not parse pattern `{}`", &path) + format_err!("could not parse pattern `{}`", &path) })?; - res.map(|p| { + let res = res.map(|p| { p.chain_err(|| { - format!("unable to match path to pattern `{}`", &path) + format_err!("unable to match path to pattern `{}`", &path) }) - }).collect() + }).collect::, _>>()?; + Ok(res) } } diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index f20118b8004..d68d8864a90 100755 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -2,7 +2,7 @@ #![cfg_attr(test, deny(warnings))] #![recursion_limit="128"] -#[macro_use] extern crate error_chain; +#[macro_use] extern crate failure; #[macro_use] extern crate log; #[macro_use] extern crate scoped_tls; #[macro_use] extern crate serde_derive; @@ -38,26 +38,20 @@ extern crate url; extern crate core_foundation; use std::fmt; -use std::error::Error; -use error_chain::ChainedError; use serde::Deserialize; use serde::ser; use docopt::Docopt; +use failure::Error; use core::Shell; use core::shell::Verbosity::Verbose; -pub use util::{CargoError, CargoErrorKind, CargoResult, CliError, CliResult, Config}; +pub use util::{CargoError, CargoResult, CliError, CliResult, Config}; +pub use util::errors::Internal; pub const CARGO_ENV: &'static str = "CARGO"; -macro_rules! bail { - ($($fmt:tt)*) => ( - return Err(::util::errors::CargoError::from(format_args!($($fmt)*).to_string())) - ) -} - pub mod core; pub mod ops; pub mod sources; @@ -122,7 +116,7 @@ pub fn call_main_without_stdin<'de, Flags: Deserialize<'de>>( let flags = docopt.deserialize().map_err(|e| { let code = if e.fatal() {1} else {0}; - CliError::new(e.to_string().into(), code) + CliError::new(e.into(), code) })?; exec(flags, config) @@ -151,7 +145,7 @@ pub fn exit_with_error(err: CliError, shell: &mut Shell) -> ! { drop(writeln!(shell.err(), "{}", error)) } - if !handle_cause(error, shell) || hide { + if !handle_cause(&error, shell) || hide { drop(writeln!(shell.err(), "\nTo learn more, run the command again \ with --verbose.")); } @@ -164,44 +158,28 @@ pub fn handle_error(err: CargoError, shell: &mut Shell) { debug!("handle_error; err={:?}", &err); let _ignored_result = shell.error(&err); - handle_cause(err, shell); + handle_cause(&err, shell); } -fn handle_cause(cargo_err: E, shell: &mut Shell) -> bool - where E: ChainedError + 'static -{ +fn handle_cause(cargo_err: &Error, shell: &mut Shell) -> bool { fn print(error: String, shell: &mut Shell) { drop(writeln!(shell.err(), "\nCaused by:")); drop(writeln!(shell.err(), " {}", error)); } - //Error inspection in non-verbose mode requires inspecting the - //error kind to avoid printing Internal errors. The downcasting - //machinery requires &(Error + 'static), but the iterator (and - //underlying `cause`) return &Error. Because the borrows are - //constrained to this handling method, and because the original - //error object is constrained to be 'static, we're casting away - //the borrow's actual lifetime for purposes of downcasting and - //inspecting the error chain - unsafe fn extend_lifetime(r: &Error) -> &(Error + 'static) { - std::mem::transmute::<&Error, &Error>(r) - } - let verbose = shell.verbosity(); if verbose == Verbose { - //The first error has already been printed to the shell - //Print all remaining errors - for err in cargo_err.iter().skip(1) { + // The first error has already been printed to the shell + // Print all remaining errors + for err in cargo_err.causes().skip(1) { print(err.to_string(), shell); } } else { - //The first error has already been printed to the shell - //Print remaining errors until one marked as Internal appears - for err in cargo_err.iter().skip(1) { - let err = unsafe { extend_lifetime(err) }; - if let Some(&CargoError(CargoErrorKind::Internal(..), ..)) = - err.downcast_ref::() { + // The first error has already been printed to the shell + // Print remaining errors until one marked as Internal appears + for err in cargo_err.causes().skip(1) { + if err.downcast_ref::().is_some() { return false; } diff --git a/src/cargo/ops/cargo_clean.rs b/src/cargo/ops/cargo_clean.rs index bc25a888ef2..53a79fa0be4 100644 --- a/src/cargo/ops/cargo_clean.rs +++ b/src/cargo/ops/cargo_clean.rs @@ -98,11 +98,11 @@ fn rm_rf(path: &Path) -> CargoResult<()> { let m = fs::metadata(path); if m.as_ref().map(|s| s.is_dir()).unwrap_or(false) { fs::remove_dir_all(path).chain_err(|| { - "could not remove build directory" + format_err!("could not remove build directory") })?; } else if m.is_ok() { fs::remove_file(path).chain_err(|| { - "failed to remove build artifact" + format_err!("failed to remove build artifact") })?; } Ok(()) diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs index 53c5b99ebad..cc3400268f9 100644 --- a/src/cargo/ops/cargo_compile.rs +++ b/src/cargo/ops/cargo_compile.rs @@ -33,7 +33,6 @@ use core::resolver::Resolve; use ops::{self, BuildOutput, Executor, DefaultExecutor}; use util::config::Config; use util::{CargoResult, profile}; -use util::errors::{CargoResultExt, CargoError}; /// Contains information about how a package should be compiled. #[derive(Debug)] @@ -194,10 +193,10 @@ pub fn compile_with_exec<'a>(ws: &Workspace<'a>, for member in ws.members() { for warning in member.manifest().warnings().iter() { if warning.is_critical { - let err: CargoResult<_> = Err(CargoError::from(warning.message.to_owned())); - return err.chain_err(|| { - format!("failed to parse manifest at `{}`", member.manifest_path().display()) - }) + let err = format_err!("{}", warning.message); + let cx = format_err!("failed to parse manifest at `{}`", + member.manifest_path().display()); + return Err(err.context(cx).into()) } else { options.config.shell().warn(&warning.message)? } @@ -236,9 +235,10 @@ pub fn compile_ws<'a>(ws: &Workspace<'a>, let (packages, resolve_with_overrides) = resolve; if specs.is_empty() { - return Err(format!("manifest path `{}` contains no package: The manifest is virtual, \ - and the workspace has no members.", ws.current_manifest().display()).into()); - }; + bail!("manifest path `{}` contains no package: The manifest is virtual, \ + and the workspace has no members.", + ws.current_manifest().display()) + } let to_builds = specs.iter().map(|p| { let pkgid = p.query(resolve_with_overrides.iter())?; diff --git a/src/cargo/ops/cargo_doc.rs b/src/cargo/ops/cargo_doc.rs index 150a3a99608..9527f64aefb 100644 --- a/src/cargo/ops/cargo_doc.rs +++ b/src/cargo/ops/cargo_doc.rs @@ -23,8 +23,9 @@ pub fn doc(ws: &Workspace, options: &DocOptions) -> CargoResult<()> { let (packages, resolve_with_overrides) = resolve; if specs.is_empty() { - return Err(format!("manifest path `{}` contains no package: The manifest is virtual, \ - and the workspace has no members.", ws.current_manifest().display()).into()); + bail!("manifest path `{}` contains no package: The manifest is virtual, \ + and the workspace has no members.", + ws.current_manifest().display()) }; let pkgs = specs.iter().map(|p| { diff --git a/src/cargo/ops/cargo_install.rs b/src/cargo/ops/cargo_install.rs index addfa83af13..2c8c405fe64 100644 --- a/src/cargo/ops/cargo_install.rs +++ b/src/cargo/ops/cargo_install.rs @@ -16,7 +16,7 @@ use ops::{self, CompileFilter, DefaultExecutor}; use sources::{GitSource, PathSource, SourceConfigMap}; use util::{Config, internal}; use util::{Filesystem, FileLock}; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt}; #[derive(Deserialize, Serialize)] #[serde(untagged)] @@ -91,7 +91,7 @@ pub fn install(root: Option<&str>, summary.push(format!("Failed to install {} (see error(s) above).", failed.join(", "))); } if !succeeded.is_empty() || !failed.is_empty() { - opts.config.shell().status("\nSummary:", summary.join(" "))?; + opts.config.shell().status("Summary", summary.join(" "))?; } (!succeeded.is_empty(), !failed.is_empty()) @@ -136,13 +136,14 @@ fn install_one(root: &Filesystem, krate, vers, config, is_first_install, &mut |git| git.read_packages())? } else if source_id.is_path() { - let path = source_id.url().to_file_path() - .map_err(|()| CargoError::from("path sources must have a valid path"))?; + let path = source_id.url().to_file_path().map_err(|()| { + format_err!("path sources must have a valid path") + })?; let mut src = PathSource::new(&path, source_id, config); src.update().chain_err(|| { - format!("`{}` is not a crate root; specify a crate to \ - install from crates.io, or use --path or --git to \ - specify an alternate source", path.display()) + format_err!("`{}` is not a crate root; specify a crate to \ + install from crates.io, or use --path or --git to \ + specify an alternate source", path.display()) })?; select_pkg(PathSource::new(&path, source_id, config), krate, vers, config, is_first_install, @@ -150,9 +151,11 @@ fn install_one(root: &Filesystem, } else { select_pkg(map.load(source_id)?, krate, vers, config, is_first_install, - &mut |_| Err("must specify a crate to install from \ - crates.io, or use --path or --git to \ - specify alternate source".into()))? + &mut |_| { + bail!("must specify a crate to install from \ + crates.io, or use --path or --git to \ + specify alternate source") + })? }; let mut td_opt = None; @@ -193,8 +196,8 @@ fn install_one(root: &Filesystem, td.into_path(); } - CargoError::from(format!("failed to compile `{}`, intermediate artifacts can be \ - found at `{}`", pkg, ws.target_dir().display())) + format_err!("failed to compile `{}`, intermediate artifacts can be \ + found at `{}`", pkg, ws.target_dir().display()) })?; let binaries: Vec<(&str, &Path)> = compile.binaries.iter().map(|bin| { let name = bin.file_name().unwrap(); @@ -228,8 +231,8 @@ fn install_one(root: &Filesystem, continue } fs::copy(src, &dst).chain_err(|| { - format!("failed to copy `{}` to `{}`", src.display(), - dst.display()) + format_err!("failed to copy `{}` to `{}`", src.display(), + dst.display()) })?; } @@ -245,8 +248,8 @@ fn install_one(root: &Filesystem, let dst = dst.join(bin); config.shell().status("Installing", dst.display())?; fs::rename(&src, &dst).chain_err(|| { - format!("failed to move `{}` to `{}`", src.display(), - dst.display()) + format_err!("failed to move `{}` to `{}`", src.display(), + dst.display()) })?; installed.bins.push(dst); } @@ -261,8 +264,8 @@ fn install_one(root: &Filesystem, let dst = dst.join(bin); config.shell().status("Replacing", dst.display())?; fs::rename(&src, &dst).chain_err(|| { - format!("failed to move `{}` to `{}`", src.display(), - dst.display()) + format_err!("failed to move `{}` to `{}`", src.display(), + dst.display()) })?; replaced_names.push(bin); } @@ -340,30 +343,31 @@ fn select_pkg<'a, T>(mut source: T, // version range, otherwise parse it as a specific version let first = v.chars() .nth(0) - .ok_or("no version provided for the `--vers` flag")?; + .ok_or(format_err!("no version provided for the `--vers` flag"))?; match first { '<' | '>' | '=' | '^' | '~' => match v.parse::() { Ok(v) => Some(v.to_string()), Err(_) => { - let msg = format!("the `--vers` provided, `{}`, is \ - not a valid semver version requirement\n\n - Please have a look at \ - http://doc.crates.io/specifying-dependencies.html \ - for the correct format", v); - return Err(msg.into()); + bail!("the `--vers` provided, `{}`, is \ + not a valid semver version requirement\n\n + Please have a look at \ + http://doc.crates.io/specifying-dependencies.html \ + for the correct format", v) } }, _ => match v.parse::() { Ok(v) => Some(format!("={}", v)), Err(_) => { - let mut msg = format!("the `--vers` provided, `{}`, is \ - not a valid semver version\n\n\ - historically Cargo treated this \ - as a semver version requirement \ - accidentally\nand will continue \ - to do so, but this behavior \ - will be removed eventually", v); + let mut msg = format!("\ + the `--vers` provided, `{}`, is \ + not a valid semver version\n\n\ + historically Cargo treated this \ + as a semver version requirement \ + accidentally\nand will continue \ + to do so, but this behavior \ + will be removed eventually", v + ); // If it is not a valid version but it is a valid version // requirement, add a note to the warning @@ -390,8 +394,8 @@ fn select_pkg<'a, T>(mut source: T, None => { let vers_info = vers.map(|v| format!(" with version `{}`", v)) .unwrap_or_default(); - Err(format!("could not find `{}` in {}{}", name, - source.source_id(), vers_info).into()) + Err(format_err!("could not find `{}` in {}{}", name, + source.source_id(), vers_info)) } } } @@ -433,7 +437,7 @@ fn one(mut i: I, f: F) -> CargoResult> (Some(i1), Some(i2)) => { let mut v = vec![i1, i2]; v.extend(i); - Err(f(v).into()) + Err(format_err!("{}", f(v))) } (Some(i), None) => Ok(Some(i)), (None, _) => Ok(None) @@ -466,7 +470,7 @@ fn check_overwrites(dst: &Path, } } msg.push_str("Add --force to overwrite"); - Err(msg.into()) + Err(format_err!("{}", msg)) } fn find_duplicates(dst: &Path, @@ -511,7 +515,7 @@ fn find_duplicates(dst: &Path, } fn read_crate_list(file: &FileLock) -> CargoResult { - (|| -> CargoResult<_> { + let listing = (|| -> CargoResult<_> { let mut contents = String::new(); file.file().read_to_string(&mut contents)?; let listing = toml::from_str(&contents).chain_err(|| { @@ -524,9 +528,10 @@ fn read_crate_list(file: &FileLock) -> CargoResult { } } })().chain_err(|| { - format!("failed to parse crate metadata at `{}`", - file.path().to_string_lossy()) - }) + format_err!("failed to parse crate metadata at `{}`", + file.path().to_string_lossy()) + })?; + Ok(listing) } fn write_crate_list(file: &FileLock, listing: CrateListingV1) -> CargoResult<()> { @@ -538,9 +543,10 @@ fn write_crate_list(file: &FileLock, listing: CrateListingV1) -> CargoResult<()> file.write_all(data.as_bytes())?; Ok(()) })().chain_err(|| { - format!("failed to write crate metadata at `{}`", - file.path().to_string_lossy()) - }) + format_err!("failed to write crate metadata at `{}`", + file.path().to_string_lossy()) + })?; + Ok(()) } pub fn install_list(dst: Option<&str>, config: &Config) -> CargoResult<()> { @@ -591,7 +597,7 @@ pub fn uninstall(root: Option<&str>, } if !succeeded.is_empty() || !failed.is_empty() { - config.shell().status("\nSummary:", summary.join(" "))?; + config.shell().status("Summary", summary.join(" "))?; } !failed.is_empty() diff --git a/src/cargo/ops/cargo_new.rs b/src/cargo/ops/cargo_new.rs index c5d67dfea53..9e309700a79 100644 --- a/src/cargo/ops/cargo_new.rs +++ b/src/cargo/ops/cargo_new.rs @@ -13,7 +13,7 @@ use core::Workspace; use ops::is_bad_artifact_name; use util::{GitRepo, HgRepo, PijulRepo, FossilRepo, internal}; use util::{Config, paths}; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt}; use toml; @@ -101,8 +101,8 @@ fn get_name<'a>(path: &'a Path, opts: &'a NewOptions, config: &Config) -> CargoR } let dir_name = path.file_name().and_then(|s| s.to_str()).ok_or_else(|| { - CargoError::from(format!("cannot create a project with a non-unicode name: {:?}", - path.file_name().unwrap())) + format_err!("cannot create a project with a non-unicode name: {:?}", + path.file_name().unwrap()) })?; if opts.bin { @@ -237,10 +237,10 @@ cannot automatically generate Cargo.toml as the main target would be ambiguous", duplicates_checker.insert(i.target_name.as_ref(), i); } else { if let Some(plp) = previous_lib_relpath { - return Err(format!("cannot have a project with \ - multiple libraries, \ - found both `{}` and `{}`", - plp, i.relative_path).into()); + bail!("cannot have a project with \ + multiple libraries, \ + found both `{}` and `{}`", + plp, i.relative_path) } previous_lib_relpath = Some(&i.relative_path); } @@ -290,9 +290,10 @@ pub fn new(opts: &NewOptions, config: &Config) -> CargoResult<()> { }; mk(config, &mkopts).chain_err(|| { - format!("Failed to create project `{}` at `{}`", - name, path.display()) - }) + format_err!("Failed to create project `{}` at `{}`", + name, path.display()) + })?; + Ok(()) } pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> { @@ -365,9 +366,10 @@ pub fn init(opts: &NewOptions, config: &Config) -> CargoResult<()> { }; mk(config, &mkopts).chain_err(|| { - format!("Failed to create project `{}` at `{}`", - name, path.display()) - }) + format_err!("Failed to create project `{}` at `{}`", + name, path.display()) + })?; + Ok(()) } fn strip_rust_affixes(name: &str) -> &str { diff --git a/src/cargo/ops/cargo_package.rs b/src/cargo/ops/cargo_package.rs index 04c3f6d971c..c09d786ff7a 100644 --- a/src/cargo/ops/cargo_package.rs +++ b/src/cargo/ops/cargo_package.rs @@ -78,7 +78,7 @@ pub fn package(ws: &Workspace, config.shell().status("Packaging", pkg.package_id().to_string())?; dst.file().set_len(0)?; tar(ws, &src, dst.file(), &filename).chain_err(|| { - "failed to prepare local package for uploading" + format_err!("failed to prepare local package for uploading") })?; if opts.verify { dst.seek(SeekFrom::Start(0))?; @@ -207,8 +207,8 @@ fn tar(ws: &Workspace, let relative = util::without_prefix(file, root).unwrap(); check_filename(relative)?; let relative = relative.to_str().ok_or_else(|| { - format!("non-utf8 path in source directory: {}", - relative.display()) + format_err!("non-utf8 path in source directory: {}", + relative.display()) })?; config.shell().verbose(|shell| { shell.status("Archiving", &relative) diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index 19d9f6aef49..48c2d9dabe1 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -5,7 +5,7 @@ use std::path::{Path, PathBuf}; use core::{Package, SourceId, PackageId, EitherManifest}; use util::{self, Config}; -use util::errors::{CargoResult, CargoResultExt, CargoError}; +use util::errors::{CargoResult, CargoError}; use util::important_paths::find_project_manifest_exact; use util::toml::read_manifest; @@ -64,7 +64,7 @@ pub fn read_packages(path: &Path, source_id: &SourceId, config: &Config) if all_packages.is_empty() { match errors.pop() { Some(err) => Err(err), - None => Err(format!("Could not find Cargo.toml in `{}`", path.display()).into()), + None => Err(format_err!("Could not find Cargo.toml in `{}`", path.display())), } } else { Ok(all_packages.into_iter().map(|(_, v)| v).collect()) @@ -86,9 +86,9 @@ fn walk(path: &Path, callback: &mut FnMut(&Path) -> CargoResult) return Ok(()) } Err(e) => { - return Err(e).chain_err(|| { - format!("failed to read directory `{}`", path.display()) - }) + let cx = format!("failed to read directory `{}`", path.display()); + let e = CargoError::from(e); + return Err(e.context(cx).into()) } }; for dir in dirs { diff --git a/src/cargo/ops/cargo_run.rs b/src/cargo/ops/cargo_run.rs index 71d90470fa6..ff3c46b5669 100644 --- a/src/cargo/ops/cargo_run.rs +++ b/src/cargo/ops/cargo_run.rs @@ -1,8 +1,7 @@ use std::path::Path; use ops::{self, Packages}; -use util::{self, CargoResult, CargoError, ProcessError}; -use util::errors::CargoErrorKind; +use util::{self, CargoResult, ProcessError}; use core::Workspace; pub fn run(ws: &Workspace, @@ -19,8 +18,7 @@ pub fn run(ws: &Workspace, 1 => ws.members() .find(|pkg| pkg.name() == xs[0]) .ok_or_else(|| - CargoError::from( - format!("package `{}` is not a member of the workspace", xs[0])) + format_err!("package `{}` is not a member of the workspace", xs[0]) )?, _ => unreachable!("cargo run supports single package only"), } @@ -72,7 +70,9 @@ pub fn run(ws: &Workspace, match result { Ok(()) => Ok(None), - Err(CargoError(CargoErrorKind::ProcessErrorKind(e), ..)) => Ok(Some(e)), - Err(e) => Err(e) + Err(e) => { + let err = e.downcast::()?; + Ok(Some(err)) + } } } diff --git a/src/cargo/ops/cargo_rustc/custom_build.rs b/src/cargo/ops/cargo_rustc/custom_build.rs index 0ef0fa21882..95940bfcb13 100644 --- a/src/cargo/ops/cargo_rustc/custom_build.rs +++ b/src/cargo/ops/cargo_rustc/custom_build.rs @@ -6,7 +6,7 @@ use std::sync::{Mutex, Arc}; use core::PackageId; use util::{Freshness, Cfg}; -use util::errors::{CargoResult, CargoResultExt, CargoError}; +use util::errors::{CargoResult, CargoResultExt}; use util::{self, internal, profile, paths}; use util::machine_message; @@ -258,9 +258,8 @@ fn build_work<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, unit: &Unit<'a>) &mut |err_line| { state.stderr(err_line); Ok(()) }, true, ).map_err(|e| { - CargoError::from( - format!("failed to run custom build command for `{}`\n{}", - pkg_name, e.description())) + format_err!("failed to run custom build command for `{}`\n{}", + pkg_name, e) })?; diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 5c3284bab32..8325cf558cc 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -607,7 +607,7 @@ fn log_compare(unit: &Unit, compare: &CargoResult<()>) { }; info!("fingerprint error for {}: {}", unit.pkg, ce); - for cause in ce.iter().skip(1) { + for cause in ce.causes().skip(1) { info!(" cause: {}", cause); } } diff --git a/src/cargo/ops/cargo_rustc/job_queue.rs b/src/cargo/ops/cargo_rustc/job_queue.rs index 5bfc5d45850..281f057aba1 100644 --- a/src/cargo/ops/cargo_rustc/job_queue.rs +++ b/src/cargo/ops/cargo_rustc/job_queue.rs @@ -233,13 +233,12 @@ impl<'a> JobQueue<'a> { self.emit_warnings(Some(msg), key, cx)?; if self.active > 0 { - error = Some("build failed".into()); + error = Some(format_err!("build failed")); handle_error(e, &mut *cx.config.shell()); cx.config.shell().warn( "build failed, waiting for other \ jobs to finish...")?; - } - else { + } else { error = Some(e); } } diff --git a/src/cargo/ops/cargo_rustc/layout.rs b/src/cargo/ops/cargo_rustc/layout.rs index 464a68945bd..c5a6b875daa 100644 --- a/src/cargo/ops/cargo_rustc/layout.rs +++ b/src/cargo/ops/cargo_rustc/layout.rs @@ -92,7 +92,9 @@ impl Layout { // the target triple as a Path and then just use the file stem as the // component for the directory name. if let Some(triple) = triple { - path.push(Path::new(triple).file_stem().ok_or_else(|| "target was empty")?); + path.push(Path::new(triple).file_stem().ok_or_else(|| { + format_err!("target was empty") + })?); } path.push(dest); Layout::at(ws.config(), path) diff --git a/src/cargo/ops/cargo_rustc/mod.rs b/src/cargo/ops/cargo_rustc/mod.rs index 6664ab6bcdf..d301b47e2ae 100644 --- a/src/cargo/ops/cargo_rustc/mod.rs +++ b/src/cargo/ops/cargo_rustc/mod.rs @@ -14,7 +14,7 @@ use core::{Profile, Profiles, Workspace}; use core::shell::ColorChoice; use util::{self, ProcessBuilder, machine_message}; use util::{Config, internal, profile, join_paths}; -use util::errors::{CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt, Internal}; use util::Freshness; use self::job::{Job, Work}; @@ -422,9 +422,11 @@ fn rustc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, format!("Could not compile `{}`.", name) })?; } else { - exec.exec(rustc, &package_id, &target).map_err(|e| e.into_internal()).chain_err(|| { - format!("Could not compile `{}`.", name) - })?; + exec.exec(rustc, &package_id, &target) + .map_err(Internal::new) + .chain_err(|| { + format!("Could not compile `{}`.", name) + })?; } if do_rename && real_name != crate_name { @@ -702,7 +704,8 @@ fn rustdoc<'a, 'cfg>(cx: &mut Context<'a, 'cfg>, } } state.running(&rustdoc); - rustdoc.exec().chain_err(|| format!("Could not document `{}`.", name)) + rustdoc.exec().chain_err(|| format!("Could not document `{}`.", name))?; + Ok(()) })) } diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index ad9ea32e327..7fac6785525 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -2,7 +2,7 @@ use std::ffi::{OsString, OsStr}; use ops::{self, Compilation}; use util::{self, CargoTestError, Test, ProcessError}; -use util::errors::{CargoResult, CargoErrorKind, CargoError}; +use util::errors::CargoResult; use core::Workspace; pub struct TestOptions<'a> { @@ -105,16 +105,13 @@ fn run_unit_tests(options: &TestOptions, let result = cmd.exec(); match result { - Err(CargoError(CargoErrorKind::ProcessErrorKind(e), .. )) => { + Err(e) => { + let e = e.downcast::()?; errors.push((kind.clone(), test.clone(), e)); if !options.no_fail_fast { break; } } - Err(e) => { - //This is an unexpected Cargo error rather than a test failure - return Err(e) - } Ok(()) => {} } } @@ -206,7 +203,8 @@ fn run_doc_tests(options: &TestOptions, config.shell().verbose(|shell| { shell.status("Running", p.to_string()) })?; - if let Err(CargoError(CargoErrorKind::ProcessErrorKind(e), .. )) = p.exec() { + if let Err(e) = p.exec() { + let e = e.downcast::()?; errors.push(e); if !options.no_fail_fast { return Ok((Test::Doc, errors)); diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs index 7368bbf8ade..73961fde006 100644 --- a/src/cargo/ops/lockfile.rs +++ b/src/cargo/ops/lockfile.rs @@ -21,13 +21,14 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult> { format!("failed to read file: {}", f.path().display()) })?; - (|| -> CargoResult> { + let resolve = (|| -> CargoResult> { let resolve : toml::Value = cargo_toml::parse(&s, f.path(), ws.config())?; let v: resolver::EncodableResolve = resolve.try_into()?; Ok(Some(v.into_resolve(ws)?)) })().chain_err(|| { format!("failed to parse lock file at: {}", f.path().display()) - }) + })?; + Ok(resolve) } pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> { @@ -88,7 +89,8 @@ pub fn write_pkg_lockfile(ws: &Workspace, resolve: &Resolve) -> CargoResult<()> }).chain_err(|| { format!("failed to write {}", ws.root().join("Cargo.lock").display()) - }) + })?; + Ok(()) } fn are_equal_lockfiles(mut orig: String, current: &str, ws: &Workspace) -> bool { diff --git a/src/cargo/ops/registry.rs b/src/cargo/ops/registry.rs index bf6ba8135b7..3d53c59507a 100644 --- a/src/cargo/ops/registry.rs +++ b/src/cargo/ops/registry.rs @@ -19,7 +19,7 @@ use sources::{RegistrySource}; use util::config::{self, Config}; use util::paths; use util::ToUrl; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt}; use util::important_paths::find_root_manifest_for_wd; pub struct RegistryConfig { @@ -399,7 +399,7 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { if let Some(ref v) = opts.to_add { let v = v.iter().map(|s| &s[..]).collect::>(); let msg = registry.add_owners(&name, &v).map_err(|e| { - CargoError::from(format!("failed to invite owners to crate {}: {}", name, e)) + format_err!("failed to invite owners to crate {}: {}", name, e) })?; config.shell().status("Owner", msg)?; @@ -409,14 +409,14 @@ pub fn modify_owners(config: &Config, opts: &OwnersOptions) -> CargoResult<()> { let v = v.iter().map(|s| &s[..]).collect::>(); config.shell().status("Owner", format!("removing {:?} from crate {}", v, name))?; - registry.remove_owners(&name, &v).map_err(|e| { - CargoError::from(format!("failed to remove owners from crate {}: {}", name, e)) + registry.remove_owners(&name, &v).chain_err(|| { + format!("failed to remove owners from crate {}", name) })?; } if opts.list { - let owners = registry.list_owners(&name).map_err(|e| { - CargoError::from(format!("failed to list owners of crate {}: {}", name, e)) + let owners = registry.list_owners(&name).chain_err(|| { + format!("failed to list owners of crate {}", name) })?; for owner in owners.iter() { print!("{}", owner.login); @@ -456,13 +456,13 @@ pub fn yank(config: &Config, if undo { config.shell().status("Unyank", format!("{}:{}", name, version))?; - registry.unyank(&name, &version).map_err(|e| { - CargoError::from(format!("failed to undo a yank: {}", e)) + registry.unyank(&name, &version).chain_err(|| { + "failed to undo a yank" })?; } else { config.shell().status("Yank", format!("{}:{}", name, version))?; - registry.yank(&name, &version).map_err(|e| { - CargoError::from(format!("failed to yank: {}", e)) + registry.yank(&name, &version).chain_err(|| { + "failed to yank" })?; } @@ -487,8 +487,8 @@ pub fn search(query: &str, } let (mut registry, _) = registry(config, None, index, reg)?; - let (crates, total_crates) = registry.search(query, limit).map_err(|e| { - CargoError::from(format!("failed to retrieve search results from the registry: {}", e)) + let (crates, total_crates) = registry.search(query, limit).chain_err(|| { + "failed to retrieve search results from the registry" })?; let names = crates.iter() diff --git a/src/cargo/sources/config.rs b/src/cargo/sources/config.rs index 5aa44110a85..98361ddf03a 100644 --- a/src/cargo/sources/config.rs +++ b/src/cargo/sources/config.rs @@ -13,7 +13,7 @@ use core::{Source, SourceId, GitReference}; use sources::ReplacedSource; use util::{Config, ToUrl}; use util::config::ConfigValue; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt}; #[derive(Clone)] pub struct SourceConfigMap<'cfg> { @@ -191,13 +191,12 @@ restore the source replacement configuration to continue the build let mut srcs = srcs.into_iter(); let src = srcs.next().ok_or_else(|| { - CargoError::from(format!("no source URL specified for `source.{}`, need \ - either `registry` or `local-registry` defined", - name)) + format_err!("no source URL specified for `source.{}`, need \ + either `registry` or `local-registry` defined", + name) })?; if srcs.next().is_some() { - return Err(format!("more than one source URL specified for \ - `source.{}`", name).into()) + bail!("more than one source URL specified for `source.{}`", name) } let mut replace_with = None; @@ -216,11 +215,12 @@ restore the source replacement configuration to continue the build fn url(cfg: &ConfigValue, key: &str) -> CargoResult { let (url, path) = cfg.string(key)?; - url.to_url().chain_err(|| { + let url = url.to_url().chain_err(|| { format!("configuration key `{}` specified an invalid \ URL (in {})", key, path.display()) - }) + })?; + Ok(url) } } } diff --git a/src/cargo/sources/directory.rs b/src/cargo/sources/directory.rs index 57424c276c5..d2db5b08e3d 100644 --- a/src/cargo/sources/directory.rs +++ b/src/cargo/sources/directory.rs @@ -151,7 +151,7 @@ impl<'cfg> Source for DirectorySource<'cfg> { fn download(&mut self, id: &PackageId) -> CargoResult { self.packages.get(id).map(|p| &p.0).cloned().ok_or_else(|| { - format!("failed to find package with id: {}", id).into() + format_err!("failed to find package with id: {}", id) }) } diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index 13e266b042c..a07782b4fdf 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -6,7 +6,7 @@ use core::source::{Source, SourceId}; use core::GitReference; use core::{Package, PackageId, Summary, Registry, Dependency}; use util::Config; -use util::errors::{CargoError, CargoResult}; +use util::errors::{CargoResult, Internal}; use util::hex::short_hash; use sources::PathSource; use sources::git::utils::{GitRemote, GitRevision}; @@ -76,10 +76,10 @@ fn ident(url: &Url) -> CargoResult { pub fn canonicalize_url(url: &Url) -> CargoResult { let mut url = url.clone(); - // cannot-be-a-base-urls are not supported + // cannot-be-a-base-urls are not supported // eg. github.com:rust-lang-nursery/rustfmt.git if url.cannot_be_a_base() { - return Err(format!("invalid url `{}`: cannot-be-a-base-URLs are not supported", url).into()); + bail!("invalid url `{}`: cannot-be-a-base-URLs are not supported", url) } // Strip a trailing slash @@ -166,7 +166,7 @@ impl<'cfg> Source for GitSource<'cfg> { trace!("updating git source `{:?}`", self.remote); let repo = self.remote.checkout(&db_path, self.config)?; - let rev = repo.rev_for(&self.reference).map_err(CargoError::into_internal)?; + let rev = repo.rev_for(&self.reference).map_err(Internal::new)?; (repo, rev) } else { (self.remote.db_at(&db_path)?, actual_rev.unwrap()) diff --git a/src/cargo/sources/git/utils.rs b/src/cargo/sources/git/utils.rs index ed2bc281c55..4fac25bb2c2 100644 --- a/src/cargo/sources/git/utils.rs +++ b/src/cargo/sources/git/utils.rs @@ -12,7 +12,7 @@ use url::Url; use core::GitReference; use util::{ToUrl, internal, Config, network, Progress}; -use util::errors::{CargoResult, CargoResultExt, CargoError}; +use util::errors::{CargoResult, CargoResultExt, CargoError, Internal}; #[derive(PartialEq, Clone, Debug)] pub struct GitRevision(git2::Oid); @@ -175,7 +175,7 @@ impl GitDatabase { (|| { let b = self.repo.find_branch(s, git2::BranchType::Local)?; b.get().target().ok_or_else(|| { - CargoError::from(format!("branch `{}` did not have a target", s)) + format_err!("branch `{}` did not have a target", s) }) })().chain_err(|| { format!("failed to find branch `{}`", s) @@ -282,7 +282,7 @@ impl<'a> GitCheckout<'a> { for mut child in repo.submodules()? { update_submodule(repo, &mut child, cargo_config) - .map_err(CargoError::into_internal) + .map_err(Internal::new) .chain_err(|| { format!("failed to update submodule `{}`", child.name().unwrap_or("")) @@ -518,7 +518,7 @@ fn with_authentication(url: &str, cfg: &git2::Config, mut f: F) // In the case of an authentication failure (where we tried something) then // we try to give a more helpful error message about precisely what we // tried. - res.map_err(CargoError::from).map_err(|e| e.into_internal()).chain_err(|| { + let res = res.map_err(Internal::new).chain_err(|| { let mut msg = "failed to authenticate when downloading \ repository".to_string(); if !ssh_agent_attempts.is_empty() { @@ -540,7 +540,8 @@ fn with_authentication(url: &str, cfg: &git2::Config, mut f: F) } } msg - }) + })?; + Ok(res) } fn reset(repo: &git2::Repository, diff --git a/src/cargo/sources/path.rs b/src/cargo/sources/path.rs index 11760c29a63..18bfbf519a5 100644 --- a/src/cargo/sources/path.rs +++ b/src/cargo/sources/path.rs @@ -10,7 +10,7 @@ use ignore::gitignore::GitignoreBuilder; use core::{Package, PackageId, Summary, SourceId, Source, Dependency, Registry}; use ops; -use util::{self, CargoError, CargoResult, internal}; +use util::{self, CargoResult, internal}; use util::Config; pub struct PathSource<'cfg> { @@ -116,7 +116,7 @@ impl<'cfg> PathSource<'cfg> { p }; Pattern::new(pattern).map_err(|e| { - CargoError::from(format!("could not parse glob pattern `{}`: {}", p, e)) + format_err!("could not parse glob pattern `{}`: {}", p, e) }) }; @@ -168,10 +168,10 @@ impl<'cfg> PathSource<'cfg> { ) { Match::None => Ok(true), Match::Ignore(_) => Ok(false), - Match::Whitelist(pattern) => Err(CargoError::from(format!( + Match::Whitelist(pattern) => Err(format_err!( "exclude rules cannot start with `!`: {}", pattern.original() - ))), + )), } } else { match ignore_include.matched_path_or_any_parents( @@ -180,10 +180,10 @@ impl<'cfg> PathSource<'cfg> { ) { Match::None => Ok(false), Match::Ignore(_) => Ok(true), - Match::Whitelist(pattern) => Err(CargoError::from(format!( + Match::Whitelist(pattern) => Err(format_err!( "include rules cannot start with `!`: {}", pattern.original() - ))), + )), } } }; @@ -377,7 +377,7 @@ impl<'cfg> PathSource<'cfg> { warn!(" found submodule {}", file_path.display()); let rel = util::without_prefix(&file_path, root).unwrap(); let rel = rel.to_str().ok_or_else(|| { - CargoError::from(format!("invalid utf-8 filename: {}", rel.display())) + format_err!("invalid utf-8 filename: {}", rel.display()) })?; // Git submodules are currently only named through `/` path // separators, explicitly not `\` which windows uses. Who knew? diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs index 14c8ab7d6eb..4f92238c17f 100644 --- a/src/cargo/sources/registry/index.rs +++ b/src/cargo/sources/registry/index.rs @@ -9,7 +9,7 @@ use core::dependency::Dependency; use core::{SourceId, Summary, PackageId}; use sources::registry::{RegistryPackage, INDEX_LOCK}; use sources::registry::RegistryData; -use util::{CargoError, CargoResult, internal, Filesystem, Config}; +use util::{CargoResult, internal, Filesystem, Config}; pub struct RegistryIndex<'cfg> { source_id: SourceId, @@ -103,7 +103,7 @@ impl<'cfg> RegistryIndex<'cfg> { let err = load.load(&root, Path::new(&path), &mut |contents| { hit_closure = true; let contents = str::from_utf8(contents).map_err(|_| { - CargoError::from("registry index file was not valid utf-8") + format_err!("registry index file was not valid utf-8") })?; ret.reserve(contents.lines().count()); let lines = contents.lines() diff --git a/src/cargo/sources/registry/mod.rs b/src/cargo/sources/registry/mod.rs index 2d1e4a00e42..0759b94319d 100644 --- a/src/cargo/sources/registry/mod.rs +++ b/src/cargo/sources/registry/mod.rs @@ -330,9 +330,9 @@ impl<'cfg> RegistrySource<'cfg> { // crates.io should also block uploads with these sorts of tarballs, // but be extra sure by adding a check here as well. if !entry_path.starts_with(prefix) { - return Err(format!("invalid tarball downloaded, contains \ - a file at {:?} which isn't under {:?}", - entry_path, prefix).into()) + bail!("invalid tarball downloaded, contains \ + a file at {:?} which isn't under {:?}", + entry_path, prefix) } // Once that's verified, unpack the entry as usual. diff --git a/src/cargo/sources/registry/remote.rs b/src/cargo/sources/registry/remote.rs index 9546bdb58a5..a731c2cdd25 100644 --- a/src/cargo/sources/registry/remote.rs +++ b/src/cargo/sources/registry/remote.rs @@ -15,7 +15,7 @@ use sources::registry::{RegistryData, RegistryConfig, INDEX_LOCK}; use util::network; use util::{FileLock, Filesystem, LazyCell}; use util::{Config, Sha256, ToUrl, Progress}; -use util::errors::{CargoErrorKind, CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt, HttpNot200}; pub struct RemoteRegistry<'cfg> { index_path: Filesystem, @@ -239,7 +239,7 @@ impl<'cfg> RegistryData for RemoteRegistry<'cfg> { let code = handle.response_code()?; if code != 200 && code != 0 { let url = handle.effective_url()?.unwrap_or(&url); - Err(CargoErrorKind::HttpNot200(code, url.to_string()).into()) + Err(HttpNot200 { code, url: url.to_string() }.into()) } else { Ok(()) } diff --git a/src/cargo/sources/replaced.rs b/src/cargo/sources/replaced.rs index 88390614608..81980210f69 100644 --- a/src/cargo/sources/replaced.rs +++ b/src/cargo/sources/replaced.rs @@ -31,7 +31,8 @@ impl<'cfg> Registry for ReplacedSource<'cfg> { }).chain_err(|| { format!("failed to query replaced source {}", self.to_replace) - }) + })?; + Ok(()) } fn supports_checksums(&self) -> bool { @@ -52,7 +53,8 @@ impl<'cfg> Source for ReplacedSource<'cfg> { self.inner.update().chain_err(|| { format!("failed to update replaced source {}", self.to_replace) - }) + })?; + Ok(()) } fn download(&mut self, id: &PackageId) -> CargoResult { diff --git a/src/cargo/util/cfg.rs b/src/cargo/util/cfg.rs index 341b24d6d93..8307ef75ccd 100644 --- a/src/cargo/util/cfg.rs +++ b/src/cargo/util/cfg.rs @@ -215,7 +215,7 @@ impl<'a> Iterator for Tokenizer<'a> { return Some(Ok(Token::String(&self.orig[start+1..end]))) } } - return Some(Err("unterminated string in cfg".into())) + return Some(Err(format_err!("unterminated string in cfg"))) } Some((start, ch)) if is_ident_start(ch) => { while let Some(&(end, ch)) = self.s.peek() { @@ -228,10 +228,10 @@ impl<'a> Iterator for Tokenizer<'a> { return Some(Ok(Token::Ident(&self.orig[start..]))) } Some((_, ch)) => { - return Some(Err(format!("unexpected character in \ + return Some(Err(format_err!("unexpected character in \ cfg `{}`, expected parens, \ a comma, an identifier, or \ - a string", ch).into())) + a string", ch))) } None => return None } diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index fb2e4cb050d..77577ab3077 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -108,8 +108,8 @@ impl Config { "couldn't get the current directory of the process" })?; let homedir = homedir(&cwd).ok_or_else(|| { - "Cargo couldn't find your home directory. \ - This probably means that $HOME was not set." + format_err!("Cargo couldn't find your home directory. \ + This probably means that $HOME was not set.") })?; Ok(Config::new(shell, cwd, homedir)) } @@ -161,9 +161,8 @@ impl Config { // The method varies per operating system and might fail; in particular, // it depends on /proc being mounted on Linux, and some environments // (like containers or chroots) may not have that available. - env::current_exe() - .and_then(|path| path.canonicalize()) - .map_err(CargoError::from) + let exe = env::current_exe()?.canonicalize()?; + Ok(exe) } fn from_argv() -> CargoResult { @@ -175,36 +174,35 @@ impl Config { // - an absolute path (e.g. `/usr/local/bin/cargo`). // In either case, Path::canonicalize will return the full absolute path // to the target if it exists - env::args_os() - .next() - .ok_or(CargoError::from("no argv[0]")) + let argv0 = env::args_os() .map(PathBuf::from) - .and_then(|argv0| { - if argv0.components().count() == 1 { - probe_path(argv0) - } else { - argv0.canonicalize().map_err(CargoError::from) - } - }) + .next() + .ok_or(format_err!("no argv[0]"))?; + if argv0.components().count() == 1 { + probe_path(argv0) + } else { + Ok(argv0.canonicalize()?) + } } fn probe_path(argv0: PathBuf) -> CargoResult { - let paths = env::var_os("PATH").ok_or(CargoError::from("no PATH"))?; + let paths = env::var_os("PATH").ok_or(format_err!("no PATH"))?; for path in env::split_paths(&paths) { let candidate = PathBuf::from(path).join(&argv0); if candidate.is_file() { // PATH may have a component like "." in it, so we still need to // canonicalize. - return candidate.canonicalize().map_err(CargoError::from); + return Ok(candidate.canonicalize()?) } } - Err(CargoError::from("no cargo executable candidate found in PATH")) + bail!("no cargo executable candidate found in PATH") } - from_current_exe() + let exe = from_current_exe() .or_else(|_| from_argv()) - .chain_err(|| "couldn't get the path to cargo executable") + .chain_err(|| "couldn't get the path to cargo executable")?; + Ok(exe) }).map(AsRef::as_ref) } @@ -214,11 +212,11 @@ impl Config { pub fn set_values(&self, values: HashMap) -> CargoResult<()> { if self.values.borrow().is_some() { - return Err("Config values already found".into()); + bail!("config values already found") } match self.values.fill(values) { Ok(()) => Ok(()), - Err(_) => Err("Could not fill values".into()), + Err(_) => bail!("could not fill values"), } } @@ -443,7 +441,7 @@ impl Config { pub fn expected(&self, ty: &str, key: &str, val: CV) -> CargoResult { val.expected(ty, key).map_err(|e| { - format!("invalid configuration for key `{}`\n{}", key, e).into() + format_err!("invalid configuration for key `{}`\n{}", key, e) }) } @@ -551,7 +549,7 @@ impl Config { pub fn get_registry_index(&self, registry: &str) -> CargoResult { Ok(match self.get_string(&format!("registries.{}.index", registry))? { Some(index) => index.val.to_url()?, - None => return Err(CargoError::from(format!("No index found for registry: `{}`", registry)).into()), + None => bail!("No index found for registry: `{}`", registry), }) } @@ -714,8 +712,8 @@ impl ConfigValue { Ok(CV::List(val.into_iter().map(|toml| { match toml { toml::Value::String(val) => Ok((val, path.to_path_buf())), - v => Err(format!("expected string but found {} \ - in list", v.type_str()).into()), + v => bail!("expected string but found {} in list", + v.type_str()), } }).collect::>()?, path.to_path_buf())) } @@ -844,9 +842,9 @@ impl ConfigValue { } pub fn expected(&self, wanted: &str, key: &str) -> CargoResult { - Err(format!("expected a {}, but found a {} for `{}` in {}", - wanted, self.desc(), key, - self.definition_path().display()).into()) + bail!("expected a {}, but found a {} for `{}` in {}", + wanted, self.desc(), key, + self.definition_path().display()) } } @@ -889,8 +887,8 @@ fn walk_tree(pwd: &Path, mut walk: F) -> CargoResult<()> // in our history to be sure we pick up that standard location for // information. let home = homedir(pwd).ok_or_else(|| { - CargoError::from("Cargo couldn't find your home directory. \ - This probably means that $HOME was not set.") + format_err!("Cargo couldn't find your home directory. \ + This probably means that $HOME was not set.") })?; let config = home.join("config"); if !stash.contains(&config) && fs::metadata(&config).is_ok() { diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 70c50171951..de2047ce89c 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -1,101 +1,74 @@ #![allow(unknown_lints)] -use std::error::Error; use std::fmt; -use std::io; -use std::num; use std::process::{Output, ExitStatus}; use std::str; -use std::string; use core::TargetKind; +use failure::{Context, Error, Fail}; -use curl; -use git2; -use semver; -use serde_json; -use toml; -use registry; -use ignore; - -error_chain! { - types { - CargoError, CargoErrorKind, CargoResultExt, CargoResult; - } +pub use failure::Error as CargoError; +pub type CargoResult = Result; - links { - CrateRegistry(registry::Error, registry::ErrorKind); - } +pub trait CargoResultExt { + fn chain_err(self, f: F) -> Result> + where F: FnOnce() -> D, + D: fmt::Display + Send + Sync + 'static; +} - foreign_links { - ParseSemver(semver::ReqParseError); - Semver(semver::SemVerError); - Ignore(ignore::Error); - Io(io::Error); - SerdeJson(serde_json::Error); - TomlSer(toml::ser::Error); - TomlDe(toml::de::Error); - ParseInt(num::ParseIntError); - ParseBool(str::ParseBoolError); - Parse(string::ParseError); - Git(git2::Error); - Curl(curl::Error); +impl CargoResultExt for Result + where E: Into, +{ + fn chain_err(self, f: F) -> Result> + where F: FnOnce() -> D, + D: fmt::Display + Send + Sync + 'static, + { + self.map_err(|failure| { + let context = f(); + failure.into().context(context) + }) } +} - errors { - Internal(err: Box) { - description(err.description()) - display("{}", *err) - } - ProcessErrorKind(proc_err: ProcessError) { - description(&proc_err.desc) - display("{}", &proc_err.desc) - } - CargoTestErrorKind(test_err: CargoTestError) { - description(&test_err.desc) - display("{}", &test_err.desc) - } - HttpNot200(code: u32, url: String) { - description("failed to get a 200 response") - display("failed to get 200 response from `{}`, got {}", url, code) - } +#[derive(Debug, Fail)] +#[fail(display = "failed to get 200 response from `{}`, got {}", url, code)] +pub struct HttpNot200 { + pub code: u32, + pub url: String, +} + +pub struct Internal { + inner: Error, +} + +impl Internal { + pub fn new(inner: Error) -> Internal { + Internal { inner } } } -impl CargoError { - pub fn into_internal(self) -> Self { - CargoError(CargoErrorKind::Internal(Box::new(self.0)), self.1) +impl Fail for Internal { + fn cause(&self) -> Option<&Fail> { + self.inner.cause().cause() } +} - fn is_human(&self) -> bool { - match self.0 { - CargoErrorKind::Msg(_) | - CargoErrorKind::TomlSer(_) | - CargoErrorKind::TomlDe(_) | - CargoErrorKind::Curl(_) | - CargoErrorKind::HttpNot200(..) | - CargoErrorKind::ProcessErrorKind(_) | - CargoErrorKind::CrateRegistry(_) => true, - CargoErrorKind::ParseSemver(_) | - CargoErrorKind::Semver(_) | - CargoErrorKind::Ignore(_) | - CargoErrorKind::Io(_) | - CargoErrorKind::SerdeJson(_) | - CargoErrorKind::ParseInt(_) | - CargoErrorKind::ParseBool(_) | - CargoErrorKind::Parse(_) | - CargoErrorKind::Git(_) | - CargoErrorKind::Internal(_) | - CargoErrorKind::CargoTestErrorKind(_) | - CargoErrorKind::__Nonexhaustive { .. } => false - } +impl fmt::Debug for Internal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.inner.fmt(f) } } +impl fmt::Display for Internal { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.inner.fmt(f) + } +} // ============================================================================= // Process errors -#[derive(Debug)] +#[derive(Debug, Fail)] +#[fail(display = "{}", desc)] pub struct ProcessError { pub desc: String, pub exit: Option, @@ -106,7 +79,8 @@ pub struct ProcessError { // Cargo test errors. /// Error when testcases fail -#[derive(Debug)] +#[derive(Debug, Fail)] +#[fail(display = "{}", desc)] pub struct CargoTestError { pub test: Test, pub desc: String, @@ -168,31 +142,10 @@ pub struct CliError { pub exit_code: i32 } -impl Error for CliError { - fn description(&self) -> &str { - self.error.as_ref().map(|e| e.description()) - .unwrap_or("unknown cli error") - } - - fn cause(&self) -> Option<&Error> { - self.error.as_ref().and_then(|e| e.cause()) - } -} - -impl fmt::Display for CliError { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - if let Some(ref error) = self.error { - error.fmt(f) - } else { - self.description().fmt(f) - } - } -} - impl CliError { pub fn new(error: CargoError, code: i32) -> CliError { - let human = &error.is_human(); - CliError { error: Some(error), exit_code: code, unknown: !human } + let unknown = error.downcast_ref::().is_some(); + CliError { error: Some(error), exit_code: code, unknown } } pub fn code(code: i32) -> CliError { @@ -284,5 +237,5 @@ pub fn internal(error: S) -> CargoError { } fn _internal(error: &fmt::Display) -> CargoError { - CargoError::from_kind(error.to_string().into()).into_internal() + Internal::new(format_err!("{}", error)).into() } diff --git a/src/cargo/util/flock.rs b/src/cargo/util/flock.rs index 0a8887d8b03..2913d436310 100644 --- a/src/cargo/util/flock.rs +++ b/src/cargo/util/flock.rs @@ -1,5 +1,5 @@ use std::fs::{self, File, OpenOptions}; -use std::io::*; +use std::io::{Seek, Read, Write, SeekFrom}; use std::io; use std::path::{Path, PathBuf, Display}; @@ -9,7 +9,7 @@ use fs2::{FileExt, lock_contended_error}; use libc; use util::Config; -use util::errors::{CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt, CargoError}; pub struct FileLock { f: Option, @@ -295,18 +295,19 @@ fn acquire(config: &Config, Err(e) => { if e.raw_os_error() != lock_contended_error().raw_os_error() { - return Err(e).chain_err(|| { - format!("failed to lock file: {}", path.display()) - }) + let e = CargoError::from(e); + let cx = format!("failed to lock file: {}", path.display()); + return Err(e.context(cx).into()) } } } let msg = format!("waiting for file lock on {}", msg); config.shell().status_with_color("Blocking", &msg, Cyan)?; - return block().chain_err(|| { + block().chain_err(|| { format!("failed to lock file: {}", path.display()) - }); + })?; + return Ok(()); #[cfg(all(target_os = "linux", not(target_env = "musl")))] fn is_on_nfs_mount(path: &Path) -> bool { diff --git a/src/cargo/util/important_paths.rs b/src/cargo/util/important_paths.rs index 069979ea97c..faa5dfe9b0c 100644 --- a/src/cargo/util/important_paths.rs +++ b/src/cargo/util/important_paths.rs @@ -56,10 +56,9 @@ pub fn find_root_manifest_for_wd(manifest_path: Option, cwd: &Path) pub fn find_project_manifest_exact(pwd: &Path, file: &str) -> CargoResult { let manifest = pwd.join(file); - if fs::metadata(&manifest).is_ok() { + if manifest.exists() { Ok(manifest) } else { - Err(format!("Could not find `{}` in `{}`", - file, pwd.display()).into()) + bail!("Could not find `{}` in `{}`", file, pwd.display()) } } diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index 601933136bc..1e1b55e3615 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -1,7 +1,7 @@ pub use self::cfg::{Cfg, CfgExpr}; pub use self::config::{Config, ConfigValue, homedir}; pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness}; -pub use self::errors::{CargoResult, CargoResultExt, CargoError, CargoErrorKind, Test, CliResult}; +pub use self::errors::{CargoResult, CargoResultExt, CargoError, Test, CliResult}; pub use self::errors::{CliError, ProcessError, CargoTestError}; pub use self::errors::{process_error, internal}; pub use self::flock::{FileLock, Filesystem}; diff --git a/src/cargo/util/network.rs b/src/cargo/util/network.rs index 4c7c4dcb5cc..170805695b3 100644 --- a/src/cargo/util/network.rs +++ b/src/cargo/util/network.rs @@ -1,50 +1,34 @@ -use std; -use std::error::Error; - -use error_chain::ChainedError; -use util::Config; -use util::errors::{CargoError, CargoErrorKind, CargoResult}; +use curl; use git2; -fn maybe_spurious(err: &E) -> bool - where E: ChainedError + 'static { - //Error inspection in non-verbose mode requires inspecting the - //error kind to avoid printing Internal errors. The downcasting - //machinery requires &(Error + 'static), but the iterator (and - //underlying `cause`) return &Error. Because the borrows are - //constrained to this handling method, and because the original - //error object is constrained to be 'static, we're casting away - //the borrow's actual lifetime for purposes of downcasting and - //inspecting the error chain - unsafe fn extend_lifetime(r: &Error) -> &(Error + 'static) { - std::mem::transmute::<&Error, &Error>(r) - } +use failure::Error; + +use util::Config; +use util::errors::{CargoResult, HttpNot200}; - for e in err.iter() { - let e = unsafe { extend_lifetime(e) }; - if let Some(cargo_err) = e.downcast_ref::() { - match cargo_err.kind() { - &CargoErrorKind::Git(ref git_err) => { - match git_err.class() { - git2::ErrorClass::Net | - git2::ErrorClass::Os => return true, - _ => () - } - } - &CargoErrorKind::Curl(ref curl_err) - if curl_err.is_couldnt_connect() || - curl_err.is_couldnt_resolve_proxy() || - curl_err.is_couldnt_resolve_host() || - curl_err.is_operation_timedout() || - curl_err.is_recv_error() => { - return true - } - &CargoErrorKind::HttpNot200(code, ref _url) if 500 <= code && code < 600 => { - return true - } +fn maybe_spurious(err: &Error) -> bool { + for e in err.causes() { + if let Some(git_err) = e.downcast_ref::() { + match git_err.class() { + git2::ErrorClass::Net | + git2::ErrorClass::Os => return true, _ => () } } + if let Some(curl_err) = e.downcast_ref::() { + if curl_err.is_couldnt_connect() || + curl_err.is_couldnt_resolve_proxy() || + curl_err.is_couldnt_resolve_host() || + curl_err.is_operation_timedout() || + curl_err.is_recv_error() { + return true + } + } + if let Some(not_200) = e.downcast_ref::() { + if 500 <= not_200.code && not_200.code < 600 { + return true + } + } } false } @@ -83,8 +67,8 @@ pub fn with_retry(config: &Config, mut callback: F) -> CargoResult #[test] fn with_retry_repeats_the_call_then_works() { //Error HTTP codes (5xx) are considered maybe_spurious and will prompt retry - let error1 = CargoErrorKind::HttpNot200(501, "Uri".to_string()).into(); - let error2 = CargoErrorKind::HttpNot200(502, "Uri".to_string()).into(); + let error1 = HttpNot200 { code: 501, url: "Uri".to_string() }.into(); + let error2 = HttpNot200 { code: 502, url: "Uri".to_string() }.into(); let mut results: Vec> = vec![Ok(()), Err(error1), Err(error2)]; let config = Config::default().unwrap(); let result = with_retry(&config, || results.pop().unwrap()); @@ -93,12 +77,14 @@ fn with_retry_repeats_the_call_then_works() { #[test] fn with_retry_finds_nested_spurious_errors() { + use util::CargoError; + //Error HTTP codes (5xx) are considered maybe_spurious and will prompt retry //String error messages are not considered spurious - let error1 : CargoError = CargoErrorKind::HttpNot200(501, "Uri".to_string()).into(); - let error1 = CargoError::with_chain(error1, "A non-spurious wrapping err"); - let error2 = CargoError::from_kind(CargoErrorKind::HttpNot200(502, "Uri".to_string())); - let error2 = CargoError::with_chain(error2, "A second chained error"); + let error1 = CargoError::from(HttpNot200 { code: 501, url: "Uri".to_string() }); + let error1 = CargoError::from(error1.context("A non-spurious wrapping err")); + let error2 = CargoError::from(HttpNot200 { code: 502, url: "Uri".to_string() }); + let error2 = CargoError::from(error2.context("A second chained error")); let mut results: Vec> = vec![Ok(()), Err(error1), Err(error2)]; let config = Config::default().unwrap(); let result = with_retry(&config, || results.pop().unwrap()); diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index ea6a66958f9..90fdb978a68 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -6,17 +6,21 @@ use std::io::prelude::*; use std::path::{Path, PathBuf, Component}; use util::{internal, CargoResult}; -use util::errors::CargoResultExt; +use util::errors::{CargoResultExt, Internal, CargoError}; pub fn join_paths>(paths: &[T], env: &str) -> CargoResult { - env::join_paths(paths.iter()).or_else(|e| { - let paths = paths.iter().map(Path::new).collect::>(); - Err(internal(format!("failed to join path array: {:?}", paths))).chain_err(|| { - format!("failed to join search paths together: {}\n\ - Does ${} have an unterminated quote character?", - e, env) - }) - }) + let err = match env::join_paths(paths.iter()) { + Ok(paths) => return Ok(paths), + Err(e) => e, + }; + let paths = paths.iter().map(Path::new).collect::>(); + let err = CargoError::from(err); + let explain = Internal::new(format_err!("failed to join path array: {:?}", paths)); + let err = CargoError::from(err.context(explain)); + let more_explain = format!("failed to join search paths together\n\ + Does ${} have an unterminated quote character?", + env); + return Err(err.context(more_explain).into()) } pub fn dylib_path_envvar() -> &'static str { @@ -76,7 +80,7 @@ pub fn read(path: &Path) -> CargoResult { } pub fn read_bytes(path: &Path) -> CargoResult> { - (|| -> CargoResult<_> { + let res = (|| -> CargoResult<_> { let mut ret = Vec::new(); let mut f = File::open(path)?; if let Ok(m) = f.metadata() { @@ -86,7 +90,8 @@ pub fn read_bytes(path: &Path) -> CargoResult> { Ok(ret) })().chain_err(|| { format!("failed to read `{}`", path.display()) - }) + })?; + Ok(res) } pub fn write(path: &Path, contents: &[u8]) -> CargoResult<()> { @@ -96,7 +101,8 @@ pub fn write(path: &Path, contents: &[u8]) -> CargoResult<()> { Ok(()) })().chain_err(|| { format!("failed to write `{}`", path.display()) - }) + })?; + Ok(()) } pub fn append(path: &Path, contents: &[u8]) -> CargoResult<()> { @@ -111,7 +117,8 @@ pub fn append(path: &Path, contents: &[u8]) -> CargoResult<()> { Ok(()) })().chain_err(|| { internal(format!("failed to write `{}`", path.display())) - }) + })?; + Ok(()) } #[cfg(unix)] @@ -123,8 +130,8 @@ pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> { pub fn path2bytes(path: &Path) -> CargoResult<&[u8]> { match path.as_os_str().to_str() { Some(s) => Ok(s.as_bytes()), - None => Err(format!("invalid non-unicode path: {}", - path.display()).into()) + None => Err(format_err!("invalid non-unicode path: {}", + path.display())), } } @@ -139,7 +146,7 @@ pub fn bytes2path(bytes: &[u8]) -> CargoResult { use std::str; match str::from_utf8(bytes) { Ok(s) => Ok(PathBuf::from(s)), - Err(..) => Err("invalid non-unicode path".into()), + Err(..) => Err(format_err!("invalid non-unicode path")), } } diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index ab5de7f08aa..8082369d9d5 100644 --- a/src/cargo/util/process_builder.rs +++ b/src/cargo/util/process_builder.rs @@ -9,7 +9,6 @@ use jobserver::Client; use shell_escape::escape; use util::{CargoResult, CargoResultExt, CargoError, process_error, read2}; -use util::errors::CargoErrorKind; /// A builder object for an external process, similar to `std::process::Command`. #[derive(Clone, Debug)] @@ -128,17 +127,16 @@ impl ProcessBuilder { pub fn exec(&self) -> CargoResult<()> { let mut command = self.build_command(); let exit = command.status().chain_err(|| { - CargoErrorKind::ProcessErrorKind( - process_error(&format!("could not execute process `{}`", - self.debug_string()), None, None)) + process_error(&format!("could not execute process `{}`", + self.debug_string()), None, None) })?; if exit.success() { Ok(()) } else { - Err(CargoErrorKind::ProcessErrorKind(process_error( + Err(process_error( &format!("process didn't exit successfully: `{}`", self.debug_string()), - Some(&exit), None)).into()) + Some(&exit), None).into()) } } @@ -151,9 +149,13 @@ impl ProcessBuilder { let mut command = self.build_command(); let error = command.exec(); - Err(CargoError::with_chain(error, - CargoErrorKind::ProcessErrorKind(process_error( - &format!("could not execute process `{}`", self.debug_string()), None, None)))) + Err(CargoError::from(error).context( + process_error( + &format!("could not execute process `{}`", self.debug_string()), + None, + None, + ), + ).into()) } /// On unix, executes the process using the unix syscall `execvp`, which will block this @@ -169,18 +171,18 @@ impl ProcessBuilder { let mut command = self.build_command(); let output = command.output().chain_err(|| { - CargoErrorKind::ProcessErrorKind( - process_error( - &format!("could not execute process `{}`", self.debug_string()), - None, None)) + process_error( + &format!("could not execute process `{}`", self.debug_string()), + None, + None) })?; if output.status.success() { Ok(output) } else { - Err(CargoErrorKind::ProcessErrorKind(process_error( + Err(process_error( &format!("process didn't exit successfully: `{}`", self.debug_string()), - Some(&output.status), Some(&output))).into()) + Some(&output.status), Some(&output)).into()) } } @@ -235,10 +237,11 @@ impl ProcessBuilder { })?; child.wait() })().chain_err(|| { - CargoErrorKind::ProcessErrorKind( - process_error(&format!("could not execute process `{}`", - self.debug_string()), - None, None)) + process_error( + &format!("could not execute process `{}`", + self.debug_string()), + None, + None) })?; let output = Output { stdout: stdout, @@ -253,14 +256,16 @@ impl ProcessBuilder { None }; if !output.status.success() { - return Err(CargoErrorKind::ProcessErrorKind(process_error( + return Err(process_error( &format!("process didn't exit successfully: `{}`", self.debug_string()), - Some(&output.status), to_print)).into()) + Some(&output.status), to_print).into()) } else if let Some(e) = callback_error { - return Err(CargoError::with_chain(e, - CargoErrorKind::ProcessErrorKind(process_error( - &format!("failed to parse process output: `{}`", self.debug_string()), - Some(&output.status), to_print)))) + let cx = process_error( + &format!("failed to parse process output: `{}`", self.debug_string()), + Some(&output.status), + to_print, + ); + return Err(CargoError::from(e).context(cx).into()) } } diff --git a/src/cargo/util/to_semver.rs b/src/cargo/util/to_semver.rs index ad6aff16e3e..081a3720b49 100644 --- a/src/cargo/util/to_semver.rs +++ b/src/cargo/util/to_semver.rs @@ -1,30 +1,31 @@ use semver::Version; +use util::errors::CargoResult; pub trait ToSemver { - fn to_semver(self) -> Result; + fn to_semver(self) -> CargoResult; } impl ToSemver for Version { - fn to_semver(self) -> Result { Ok(self) } + fn to_semver(self) -> CargoResult { Ok(self) } } impl<'a> ToSemver for &'a str { - fn to_semver(self) -> Result { + fn to_semver(self) -> CargoResult { match Version::parse(self) { Ok(v) => Ok(v), - Err(..) => Err(format!("cannot parse '{}' as a semver", self)), + Err(..) => Err(format_err!("cannot parse '{}' as a semver", self)), } } } impl<'a> ToSemver for &'a String { - fn to_semver(self) -> Result { + fn to_semver(self) -> CargoResult { (**self).to_semver() } } impl<'a> ToSemver for &'a Version { - fn to_semver(self) -> Result { + fn to_semver(self) -> CargoResult { Ok(self.clone()) } } diff --git a/src/cargo/util/to_url.rs b/src/cargo/util/to_url.rs index f6a4d23a5ec..7a40ccd75f7 100644 --- a/src/cargo/util/to_url.rs +++ b/src/cargo/util/to_url.rs @@ -13,7 +13,7 @@ pub trait ToUrl { impl<'a> ToUrl for &'a str { fn to_url(self) -> CargoResult { Url::parse(self).map_err(|s| { - format!("invalid url `{}`: {}", self, s).into() + format_err!("invalid url `{}`: {}", self, s) }) } } @@ -21,7 +21,7 @@ impl<'a> ToUrl for &'a str { impl<'a> ToUrl for &'a Path { fn to_url(self) -> CargoResult { Url::from_file_path(self).map_err(|()| { - format!("invalid path url `{}`", self.display()).into() + format_err!("invalid path url `{}`", self.display()) }) } } diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 58445a0c2c5..be86d5e86c9 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -30,9 +30,10 @@ pub fn read_manifest(path: &Path, source_id: &SourceId, config: &Config) trace!("read_manifest; path={}; source-id={}", path.display(), source_id); let contents = paths::read(path)?; - do_read_manifest(&contents, path, source_id, config).chain_err(|| { + let ret = do_read_manifest(&contents, path, source_id, config).chain_err(|| { format!("failed to parse manifest at `{}`", path.display()) - }) + })?; + Ok(ret) } fn do_read_manifest(contents: &str, @@ -127,9 +128,8 @@ in the future.", file.display()); return Ok(ret) } - Err(first_error).chain_err(|| { - "could not parse input as TOML" - }) + let first_error = CargoError::from(first_error); + Err(first_error.context("could not parse input as TOML").into()) } type TomlLibTarget = TomlTarget; @@ -556,12 +556,12 @@ impl TomlManifest { let project = me.project.as_ref().or_else(|| me.package.as_ref()); let project = project.ok_or_else(|| { - CargoError::from("no `package` section found.") + format_err!("no `package` section found") })?; let package_name = project.name.trim(); if package_name.is_empty() { - bail!("package name cannot be an empty string.") + bail!("package name cannot be an empty string") } let pkgid = project.to_package_id(source_id)?; @@ -825,9 +825,9 @@ impl TomlManifest { let mut dep = replacement.to_dependency(spec.name(), cx, None)?; { let version = spec.version().ok_or_else(|| { - CargoError::from(format!("replacements must specify a version \ - to replace, but `{}` does not", - spec)) + format_err!("replacements must specify a version \ + to replace, but `{}` does not", + spec) })?; dep.set_version_req(VersionReq::exact(version)); } diff --git a/src/crates-io/Cargo.toml b/src/crates-io/Cargo.toml index 26b5c991d25..8dc57558722 100644 --- a/src/crates-io/Cargo.toml +++ b/src/crates-io/Cargo.toml @@ -14,7 +14,7 @@ path = "lib.rs" [dependencies] curl = "0.4" -error-chain = "0.11.0" +failure = "0.1.1" serde = "1.0" serde_derive = "1.0" serde_json = "1.0" diff --git a/src/crates-io/lib.rs b/src/crates-io/lib.rs index 9645be81a6f..be284984779 100644 --- a/src/crates-io/lib.rs +++ b/src/crates-io/lib.rs @@ -3,7 +3,7 @@ extern crate curl; extern crate url; #[macro_use] -extern crate error_chain; +extern crate failure; extern crate serde_json; #[macro_use] extern crate serde_derive; @@ -11,46 +11,12 @@ extern crate serde_derive; use std::collections::BTreeMap; use std::fs::File; use std::io::prelude::*; -use std::io::{self, Cursor}; +use std::io::Cursor; use curl::easy::{Easy, List}; - use url::percent_encoding::{percent_encode, QUERY_ENCODE_SET}; -error_chain! { - foreign_links { - Curl(curl::Error); - Io(io::Error); - Json(serde_json::Error); - } - - errors { - NotOkResponse(code: u32, headers: Vec, body: Vec){ - description("failed to get a 200 OK response") - display("failed to get a 200 OK response, got {} -headers: - {} -body: -{}", code, headers.join("\n ", ), String::from_utf8_lossy(body)) - } - NonUtf8Body { - description("response body was not utf-8") - display("response body was not utf-8") - } - Api(errs: Vec) { - display("api errors: {}", errs.join(", ")) - } - Unauthorized { - display("unauthorized API access") - } - TokenMissing{ - display("no upload token found, please run `cargo login`") - } - NotFound { - display("cannot find crate") - } - } - } +pub type Result = std::result::Result; pub struct Registry { host: String, @@ -196,7 +162,7 @@ impl Registry { let token = match self.token.as_ref() { Some(s) => s, - None => return Err(Error::from_kind(ErrorKind::TokenMissing)), + None => bail!("no upload token found, please run `cargo login`"), }; self.handle.put(true)?; self.handle.url(&url)?; @@ -286,7 +252,7 @@ impl Registry { if authorized == Auth::Authorized { let token = match self.token.as_ref() { Some(s) => s, - None => return Err(Error::from_kind(ErrorKind::TokenMissing)), + None => bail!("no upload token found, please run `cargo login`"), }; headers.append(&format!("Authorization: {}", token))?; } @@ -323,22 +289,28 @@ fn handle(handle: &mut Easy, match handle.response_code()? { 0 => {} // file upload url sometimes 200 => {} - 403 => return Err(Error::from_kind(ErrorKind::Unauthorized)), - 404 => return Err(Error::from_kind(ErrorKind::NotFound)), - code => return Err(Error::from_kind(ErrorKind::NotOkResponse(code, headers, body))), + 403 => bail!("received 403 unauthorized response code"), + 404 => bail!("received 404 not found response code"), + code => { + bail!("failed to get a 200 OK response, got {}\n\ + headers:\n\ + \t{}\n\ + body:\n\ + {}", + code, + headers.join("\n\t"), + String::from_utf8_lossy(&body)) + } } let body = match String::from_utf8(body) { Ok(body) => body, - Err(..) => return Err(Error::from_kind(ErrorKind::NonUtf8Body)), + Err(..) => bail!("response body was not valid utf-8"), }; match serde_json::from_str::(&body) { Ok(errors) => { - return Err(Error::from_kind(ErrorKind::Api(errors - .errors - .into_iter() - .map(|s| s.detail) - .collect()))) + let errors = errors.errors.into_iter().map(|s| s.detail); + bail!("api errors: {}", errors.collect::>().join(", ")) } Err(..) => {} } diff --git a/tests/build.rs b/tests/build.rs index 101adc5fceb..c4707b51939 100644 --- a/tests/build.rs +++ b/tests/build.rs @@ -222,7 +222,7 @@ fn cargo_compile_with_invalid_package_name() { [ERROR] failed to parse manifest at `[..]` Caused by: - package name cannot be an empty string. + package name cannot be an empty string ")) } diff --git a/tests/cargotest/support/mod.rs b/tests/cargotest/support/mod.rs index 4de27a48bc1..e5de1d3caaf 100644 --- a/tests/cargotest/support/mod.rs +++ b/tests/cargotest/support/mod.rs @@ -1,5 +1,4 @@ use std::env; -use std::error::Error; use std::ffi::OsStr; use std::fmt; use std::fs; @@ -14,7 +13,7 @@ use serde_json::{self, Value}; use url::Url; use hamcrest as ham; use cargo::util::ProcessBuilder; -use cargo::util::{CargoError, CargoErrorKind, ProcessError}; +use cargo::util::{ProcessError}; use support::paths::CargoPathExt; @@ -524,13 +523,13 @@ impl Execs { let mut matches = 0; - while let Some(..) = { + while let Some(..) = { if self.diff_lines(a.clone(), e.clone(), true).is_empty() { matches += 1; } a.next() } {} - + ham::expect(matches == number, format!("expected to find {} occurences:\n\ {}\n\n\ @@ -734,16 +733,14 @@ impl<'a> ham::Matcher<&'a mut ProcessBuilder> for Execs { match res { Ok(out) => self.match_output(&out), - Err(CargoError(CargoErrorKind::ProcessErrorKind( - ProcessError { output: Some(ref out), .. }), ..)) => { - self.match_output(out) - } Err(e) => { + let err = e.downcast_ref::(); + if let Some(&ProcessError { output: Some(ref out), .. }) = err { + return self.match_output(out) + } let mut s = format!("could not exec process {}: {}", process, e); - match e.cause() { - Some(cause) => s.push_str(&format!("\ncaused by: {}", - cause.description())), - None => {} + for cause in e.causes() { + s.push_str(&format!("\ncaused by: {}", cause)); } Err(s) } @@ -862,6 +859,7 @@ fn substitute_macros(input: &str) -> String { ("[INSTALLING]", " Installing"), ("[REPLACING]", " Replacing"), ("[UNPACKING]", " Unpacking"), + ("[SUMMARY]", " Summary"), ("[EXE]", if cfg!(windows) {".exe"} else {""}), ("[/]", if cfg!(windows) {"\\"} else {"/"}), ]; diff --git a/tests/doc.rs b/tests/doc.rs index 32379c37bc8..25ac13048c9 100644 --- a/tests/doc.rs +++ b/tests/doc.rs @@ -10,7 +10,7 @@ use cargotest::rustc_host; use cargotest::support::{project, execs, path2url}; use cargotest::support::registry::Package; use hamcrest::{assert_that, existing_file, existing_dir, is_not}; -use cargo::util::{CargoError, CargoErrorKind}; +use cargo::util::ProcessError; #[test] fn simple() { @@ -633,7 +633,7 @@ fn output_not_captured() { .build(); let error = p.cargo("doc").exec_with_output().err().unwrap(); - if let CargoError(CargoErrorKind::ProcessErrorKind(perr), ..) = error { + if let Ok(perr) = error.downcast::() { let output = perr.output.unwrap(); let stderr = str::from_utf8(&output.stderr).unwrap(); diff --git a/tests/install.rs b/tests/install.rs index 0b43acc44b7..0da65d5159e 100644 --- a/tests/install.rs +++ b/tests/install.rs @@ -73,8 +73,7 @@ fn multiple_pkgs() { [FINISHED] release [optimized] target(s) in [..] [INSTALLING] {home}[..]bin[..]bar[..] error: could not find `baz` in registry `[..]` - -Summary: Successfully installed foo, bar! Failed to install baz (see error(s) above). +[SUMMARY] Successfully installed foo, bar! Failed to install baz (see error(s) above). warning: be sure to add `[..]` to your PATH to be able to run the installed binaries error: some crates failed to install ", @@ -86,8 +85,7 @@ error: some crates failed to install execs().with_status(0).with_stderr(&format!("\ [REMOVING] {home}[..]bin[..]foo[..] [REMOVING] {home}[..]bin[..]bar[..] - -Summary: Successfully uninstalled foo, bar! +[SUMMARY] Successfully uninstalled foo, bar! ", home = cargo_home().display()))); @@ -938,7 +936,7 @@ fn not_both_vers_and_version() { assert_that(cargo_process("install").arg("foo").arg("--version").arg("0.1.1").arg("--vers").arg("0.1.2"), execs().with_status(101).with_stderr_contains("\ -error: Invalid arguments. +error: invalid arguments ")); } @@ -982,8 +980,7 @@ fn uninstall_multiple_and_some_pkg_does_not_exist() { execs().with_status(101).with_stderr(&format!("\ [REMOVING] {home}[..]bin[..]foo[..] error: package id specification `bar` matched no packages - -Summary: Successfully uninstalled foo! Failed to uninstall bar (see error(s) above). +[SUMMARY] Successfully uninstalled foo! Failed to uninstall bar (see error(s) above). error: some packages failed to uninstall ", home = cargo_home().display())));