Skip to content

Commit

Permalink
Auto merge of #4795 - alexcrichton:failure, r=withoutboats
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bors committed Dec 19, 2017
2 parents 77a773a + 37cffbe commit 868d373
Show file tree
Hide file tree
Showing 67 changed files with 511 additions and 589 deletions.
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
6 changes: 3 additions & 3 deletions src/bin/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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)
})
}
}
Expand Down
24 changes: 13 additions & 11 deletions src/bin/cargo.rs
Original file line number Diff line number Diff line change
@@ -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]
Expand All @@ -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 {
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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))
}
};

Expand All @@ -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::<ProcessError>() {
if let Some(code) = perr.exit.as_ref().and_then(|c| c.code()) {
return Err(CliError::code(code));
}
Expand Down
4 changes: 2 additions & 2 deletions src/bin/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
};
Expand Down
3 changes: 2 additions & 1 deletion src/bin/help.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
4 changes: 2 additions & 2 deletions src/bin/install.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down Expand Up @@ -154,7 +154,7 @@ pub fn execute(options: Options, config: &mut Config) -> CliResult {

let krates = options.arg_crate.iter().map(|s| &s[..]).collect::<Vec<_>>();
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,
};
Expand Down
6 changes: 3 additions & 3 deletions src/bin/locate_project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() };
Expand Down
10 changes: 6 additions & 4 deletions src/bin/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -48,15 +48,17 @@ 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 {
Some(token) => token,
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)?;
Expand All @@ -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()
}
};
Expand Down
5 changes: 3 additions & 2 deletions src/bin/owner.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::ops;
use cargo::util::{CargoError, CliResult, Config};
use cargo::util::{CliResult, Config};

#[derive(Deserialize)]
pub struct Options {
Expand Down Expand Up @@ -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)?;
Expand Down
5 changes: 3 additions & 2 deletions src/bin/publish.rs
Original file line number Diff line number Diff line change
@@ -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)]
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions src/bin/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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
Expand All @@ -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)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/bin/rustc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
};
Expand Down
5 changes: 3 additions & 2 deletions src/bin/search.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::ops;
use cargo::util::{CargoError, CliResult, Config};
use cargo::util::{CliResult, Config};

use std::cmp;

Expand Down Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/bin/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Expand Down Expand Up @@ -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),
})
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/bin/yank.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use cargo::ops;
use cargo::util::{CargoError, CliResult, Config};
use cargo::util::{CliResult, Config};

#[derive(Deserialize)]
pub struct Options {
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 4 additions & 3 deletions src/cargo/core/dependency.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,9 +350,10 @@ impl FromStr for Platform {
fn from_str(s: &str) -> CargoResult<Platform> {
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()))
}
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/core/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})?;
}

Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
12 changes: 6 additions & 6 deletions src/cargo/core/package_id_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -49,7 +49,7 @@ impl PackageIdSpec {
where I: IntoIterator<Item=&'a PackageId>
{
let spec = PackageIdSpec::parse(spec).chain_err(|| {
format!("invalid package id specification: `{}`", spec)
format_err!("invalid package id specification: `{}`", spec)
})?;
spec.query(i)
}
Expand All @@ -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) => {
Expand Down Expand Up @@ -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)
};
Expand Down
Loading

0 comments on commit 868d373

Please sign in to comment.