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 c3353bcaf479b70f84beea8b1016f31dd66efe9d..b43d91499a3337baef26ebcd28b70fa8540274d1 100644 (file)
--- 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<Vec<Summary>>;
@@ -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<Vec<Summary>> {
-        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 3f082968d51a361d48115c692a779b4a209cedac..ef209186c07876c0674bb2749ff0b9ddd9258fba 100644 (file)
--- 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 aa166dfd603d04d6c0810f7248b1776f8f2bb984..ebdc4415c184fb269a15dd2eddf925158bed08b0 100644 (file)
--- 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 0be433c1f15f01501d0dad6c216f10bec5941541..603f43c84ef02f3dd39287e4411b0c9b1ff34335 100644 (file)
--- 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 6342ffad7f31c5bd0ebb0daa6e41561e1c16f606..37c7aede256e5189ca67a6ef968c1bb92e699a3a 100644 (file)
--- 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<CargoError> {
     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<T> {
     fn chain_error<E: CargoError>(self, callback: || -> E) -> CargoResult<T> ;
 }
 
+impl<'a, T> ChainError<T> for ||:'a -> CargoResult<T> {
+    fn chain_error<E: CargoError>(self, callback: || -> E) -> CargoResult<T> {
+        self().map_err(|err| callback().with_cause(err))
+    }
+}
+
 impl<T, E: CargoError> BoxError<T> for Result<T, E> {
     fn box_error(self) -> CargoResult<T> {
         self.map_err(|err| err.box_error())
@@ -109,9 +115,7 @@ impl<T, E: CargoError> BoxError<T> for Result<T, E> {
 
 impl<T, E: CargoError> ChainError<T> for Result<T, E> {
     fn chain_error<E: CargoError>(self, callback: || -> E) -> CargoResult<T>  {
-        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<S: Str>(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 d3c332430ec7a5898725189f0c83101ccb5e9372..a17294cfdfce6d5eb1ee5ef39e8c3cbb0bfe0a43 100644 (file)
--- 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 bfdf0ca90465604d1bcd2577a3d2761f463644de..50fad8b7834f261cfc0d70600b339cdf501c279c 100644 (file)
--- a/src/cargo/util/paths.rs
+++ b/src/cargo/util/paths.rs
@@ -27,7 +27,7 @@ pub fn realpath(original: &Path) -> io::IoResult<Path> {
                 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 49e07b9c277c5bda0a6501ba1e23cc077f7a1379..c56eeabc458284281b6e3de6cca48626e5f3b1f8 100644 (file)
--- 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 6b082a46e2450a76913e5c6bbf93c42448df68a8..bfd7c0ffa6ba30d5bb9d004229479b0983b4a51c 100644 (file)
--- 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<Path>)> {
-    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<TomlManifest> {
 
     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<TomlManifest> {
                         let mut details = HashMap::<String, String>::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)))
                 }