Skip to content

Commit

Permalink
Start migration to the failure crate
Browse files Browse the repository at this point in the history
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
alexcrichton committed Dec 19, 2017
1 parent 4bbfc70 commit 37cffbe
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 37cffbe

Please sign in to comment.