From: Tim Carey-Smith + Tom Dale Date: Sat, 21 Jun 2014 00:51:35 +0000 (-0700) Subject: Handle some errors better X-Git-Tag: archive/raspbian/0.35.0-2+rpi1~3^2^2^2^2^2^2^2~987 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=f137281b986d2897e771858bd0d66d51ca426dcb;p=cargo.git Handle some errors better Also, use cargo_try in more places --- diff --git a/src/cargo/core/registry.rs b/src/cargo/core/registry.rs index c3353bcaf..b43d91499 100644 --- a/src/cargo/core/registry.rs +++ b/src/cargo/core/registry.rs @@ -1,6 +1,6 @@ use std::vec::Vec; use core::{Source, SourceId, Summary, Dependency, PackageId, Package}; -use util::{CargoResult,Config}; +use util::{CargoResult, ChainError, Config, human}; pub trait Registry { fn query(&mut self, name: &Dependency) -> CargoResult>; @@ -77,32 +77,35 @@ impl PackageRegistry { fn load(&mut self, namespace: &SourceId, override: bool) -> CargoResult<()> { - let mut source = namespace.load(&try!(Config::new())); - let dst = if override {&mut self.overrides} else {&mut self.summaries}; - // Ensure the source has fetched all necessary remote data. - try!(source.update()); + (|| { + let mut source = namespace.load(&try!(Config::new())); + let dst = if override {&mut self.overrides} else {&mut self.summaries}; - // Get the summaries - for summary in (try!(source.list())).iter() { - assert!(!dst.contains(summary), "duplicate summaries"); - dst.push(summary.clone()); - // self.summaries.push(summary.clone()); - } + // Ensure the source has fetched all necessary remote data. + try!(source.update()); - // Save off the source - self.sources.push(source); + // Get the summaries + for summary in (try!(source.list())).iter() { + assert!(!dst.contains(summary), "duplicate summaries"); + dst.push(summary.clone()); + // self.summaries.push(summary.clone()); + } - // Track that the source has been searched - self.searched.push(namespace.clone()); + // Save off the source + self.sources.push(source); - Ok(()) + // Track that the source has been searched + self.searched.push(namespace.clone()); + + Ok(()) + }).chain_error(|| human(format!("Unable to update {}", namespace))) } } impl Registry for PackageRegistry { fn query(&mut self, dep: &Dependency) -> CargoResult> { - let overrides = try!(self.overrides.query(dep)); + let overrides = try!(self.overrides.query(dep)); // this can never fail in practice if overrides.is_empty() { // Ensure the requested namespace is loaded diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 3f082968d..ef209186c 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -1,5 +1,9 @@ +use std::fmt; +use std::fmt::{Show, Formatter}; + use url; use url::Url; + use core::{Summary,Package,PackageId}; use sources::{PathSource,GitSource}; use util::{Config,CargoResult}; @@ -47,12 +51,34 @@ pub enum SourceKind { RegistryKind } -#[deriving(Show,Clone,PartialEq)] +#[deriving(Clone,PartialEq)] pub struct SourceId { pub kind: SourceKind, pub url: Url } +impl Show for SourceId { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + match *self { + SourceId { kind: PathKind, ref url } => { + try!(write!(f, "{}", url)) + }, + SourceId { kind: GitKind(ref reference), ref url } => { + try!(write!(f, "{}", url)); + if reference.as_slice() != "master" { + try!(write!(f, " (ref={})", reference)); + } + }, + SourceId { kind: RegistryKind, .. } => { + // TODO: Central registry vs. alternates + try!(write!(f, "the package registry")); + } + } + + Ok(()) + } +} + impl SourceId { pub fn new(kind: SourceKind, url: Url) -> SourceId { SourceId { kind: kind, url: url } diff --git a/src/cargo/lib.rs b/src/cargo/lib.rs index aa166dfd6..ebdc4415c 100644 --- a/src/cargo/lib.rs +++ b/src/cargo/lib.rs @@ -35,7 +35,10 @@ macro_rules! some( macro_rules! cargo_try ( ($expr:expr) => ({ use util::CargoError; - try!($expr.map_err(|err| err.to_error())) + match $expr.map_err(|err| err.to_error()) { + Ok(val) => val, + Err(err) => return Err(err) + } }) ) diff --git a/src/cargo/util/config.rs b/src/cargo/util/config.rs index 0be433c1f..603f43c84 100644 --- a/src/cargo/util/config.rs +++ b/src/cargo/util/config.rs @@ -2,7 +2,7 @@ use std::{io,fmt,os}; use std::collections::HashMap; use serialize::{Encodable,Encoder}; use toml; -use util::{CargoResult, ChainError, Require, internal, human}; +use util::{CargoResult, CargoError, ChainError, Require, internal, human}; pub struct Config { home_path: Path diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 6342ffad7..37c7aede2 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -1,7 +1,7 @@ use std::io::process::{Command,ProcessOutput,ProcessExit,ExitStatus,ExitSignal}; use std::io::IoError; use std::fmt; -use std::fmt::{Show, Formatter}; +use std::fmt::{Show, Formatter, FormatError}; use std::str; use TomlError = toml::Error; @@ -64,7 +64,7 @@ macro_rules! from_error ( impl Show for Box { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - try!(write!(f, "{}", self.description())); + cargo_try!(write!(f, "{}", self.description())); Ok(()) } } @@ -101,6 +101,12 @@ pub trait ChainError { fn chain_error(self, callback: || -> E) -> CargoResult ; } +impl<'a, T> ChainError for ||:'a -> CargoResult { + fn chain_error(self, callback: || -> E) -> CargoResult { + self().map_err(|err| callback().with_cause(err)) + } +} + impl BoxError for Result { fn box_error(self) -> CargoResult { self.map_err(|err| err.box_error()) @@ -109,9 +115,7 @@ impl BoxError for Result { impl ChainError for Result { fn chain_error(self, callback: || -> E) -> CargoResult { - self.map_err(|err| { - callback().with_cause(err) - }) + self.map_err(|err| callback().with_cause(err)) } } @@ -127,6 +131,14 @@ impl CargoError for TomlError { from_error!(TomlError) +impl CargoError for FormatError { + fn description(&self) -> String { + "formatting failed".to_str() + } +} + +from_error!(FormatError) + pub struct ProcessError { pub msg: String, pub command: String, @@ -144,18 +156,18 @@ impl Show for ProcessError { Some(ExitStatus(i)) | Some(ExitSignal(i)) => i.to_str(), None => "never executed".to_str() }; - try!(write!(f, "{} (status={})", self.msg, exit)); + cargo_try!(write!(f, "{} (status={})", self.msg, exit)); match self.output { Some(ref out) => { match str::from_utf8(out.output.as_slice()) { Some(s) if s.trim().len() > 0 => { - try!(write!(f, "\n--- stdout\n{}", s)); + cargo_try!(write!(f, "\n--- stdout\n{}", s)); } Some(..) | None => {} } match str::from_utf8(out.error.as_slice()) { Some(s) if s.trim().len() > 0 => { - try!(write!(f, "\n--- stderr\n{}", s)); + cargo_try!(write!(f, "\n--- stderr\n{}", s)); } Some(..) | None => {} } @@ -232,6 +244,14 @@ pub struct CliError { pub exit_code: uint } +impl CargoError for CliError { + fn description(&self) -> String { + self.error.to_str() + } +} + +from_error!(CliError) + impl CliError { pub fn new(error: S, code: uint) -> CliError { let error = human(error.as_slice().to_str()); diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index d3c332430..a17294cfd 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -2,7 +2,7 @@ pub use self::config::Config; pub use self::process_builder::{process, ProcessBuilder}; pub use self::result::{Wrap, Require}; pub use self::errors::{CargoResult, CargoError, BoxError, ChainError, CliResult}; -pub use self::errors::{CliError, ProcessError}; +pub use self::errors::{CliError, FromError, ProcessError}; pub use self::errors::{process_error, internal_error, internal, human}; pub use self::paths::realpath; diff --git a/src/cargo/util/paths.rs b/src/cargo/util/paths.rs index bfdf0ca90..50fad8b78 100644 --- a/src/cargo/util/paths.rs +++ b/src/cargo/util/paths.rs @@ -27,7 +27,7 @@ pub fn realpath(original: &Path) -> io::IoResult { Ok(ref stat) if stat.kind != io::TypeSymlink => break, Ok(..) => { followed += 1; - let path = try!(fs::readlink(&result)); + let path = cargo_try!(fs::readlink(&result)); result.pop(); result.push(path); } diff --git a/src/cargo/util/process_builder.rs b/src/cargo/util/process_builder.rs index 49e07b9c2..c56eeabc4 100644 --- a/src/cargo/util/process_builder.rs +++ b/src/cargo/util/process_builder.rs @@ -17,10 +17,10 @@ pub struct ProcessBuilder { impl Show for ProcessBuilder { fn fmt(&self, f: &mut Formatter) -> fmt::Result { - try!(write!(f, "`{}", self.program)); + cargo_try!(write!(f, "`{}", self.program)); if self.args.len() > 0 { - try!(write!(f, " {}", self.args.connect(" "))); + cargo_try!(write!(f, " {}", self.args.connect(" "))); } write!(f, "`") @@ -80,7 +80,7 @@ impl ProcessBuilder { let msg = || format!("Could not execute process `{}`", self.debug_string()); - let exit = try!(command.status().map_err(|_| { + let exit = cargo_try!(command.status().map_err(|_| { process_error(msg(), &command, None, None) })); @@ -98,7 +98,7 @@ impl ProcessBuilder { let msg = || format!("Could not execute process `{}`", self.debug_string()); - let output = try!(command.output().map_err(|_| { + let output = cargo_try!(command.output().map_err(|_| { process_error(msg(), &command, None, None) })); diff --git a/src/cargo/util/toml.rs b/src/cargo/util/toml.rs index 6b082a46e..bfd7c0ffa 100644 --- a/src/cargo/util/toml.rs +++ b/src/cargo/util/toml.rs @@ -11,10 +11,10 @@ use util::{CargoResult, Require, human}; pub fn to_manifest(contents: &[u8], source_id: &SourceId) -> CargoResult<(Manifest, Vec)> { - let root = try!(toml::parse_from_bytes(contents).map_err(|_| { + let root = cargo_try!(toml::parse_from_bytes(contents).map_err(|_| { human("Cargo.toml is not valid Toml") })); - let toml = try!(toml_to_manifest(root).map_err(|_| { + let toml = cargo_try!(toml_to_manifest(root).map_err(|_| { human("Cargo.toml is not a valid manifest") })); @@ -41,7 +41,7 @@ fn toml_to_manifest(root: toml::Value) -> CargoResult { let deps = match deps { Some(deps) => { - let table = try!(deps.get_table().require(|| { + let table = cargo_try!(deps.get_table().require(|| { human("dependencies must be a table") })).clone(); @@ -56,14 +56,14 @@ fn toml_to_manifest(root: toml::Value) -> CargoResult { let mut details = HashMap::::new(); for (k, v) in table.iter() { - let v = try!(v.get_str().require(|| { + let v = cargo_try!(v.get_str().require(|| { human("dependency values must be string") })); details.insert(k.clone(), v.clone()); } - let version = try!(details.find_equiv(&"version") + let version = cargo_try!(details.find_equiv(&"version") .require(|| { human("dependencies must include a version") })).clone(); @@ -178,7 +178,7 @@ impl TomlManifest { } }; - deps.push(try!(Dependency::parse(n.as_slice(), + deps.push(cargo_try!(Dependency::parse(n.as_slice(), version.as_slice(), &source_id))) }