More work on porting errors

src/bin/cargo-read-manifest.rs

@@ -6,7 +6,7 @@ extern crate serialize;
 extern crate hammer;
 
 use hammer::FlagConfig;
-use cargo::{execute_main_without_stdin,CLIResult};
+use cargo::{execute_main_without_stdin,CLIResult,CLIError};
 use cargo::core::Package;
 use cargo::ops::cargo_read_manifest::read_manifest;
 
@@ -23,4 +23,9 @@ fn main() {
 
 fn execute(options: Options) -> CLIResult<Option<Package>> {
     read_manifest(options.manifest_path.as_slice()).map(|m| Some(m))
+        .map_err(|err| CLIError {
+            msg: err.get_desc().to_owned(),
+            detail: err.get_detail().map(|s| s.to_owned()),
+            exit_code: 1
+        })
 }

src/cargo/core/resolver.rs

@@ -5,7 +5,7 @@ use core::{
     Summary,
     Registry
 };
-use core::errors::CargoResult;
+use util::result::CargoResult;
 
 /* TODO:
  * - The correct input here is not a registry. Resolves should be performable

src/cargo/core/source.rs

@@ -1,5 +1,5 @@
 use core::{Summary,NameVer,Package};
-use core::errors::CargoResult;
+use util::CargoResult;
 
 /**
  * A Source finds and downloads remote packages based on names and

src/cargo/core/summary.rs

@@ -36,10 +36,12 @@ pub trait SummaryVec {
 }
 
 impl SummaryVec for Vec<Summary> {
+    // TODO: Move to Registery
     fn names(&self) -> Vec<~str> {
         self.iter().map(|summary| summary.name_ver.get_name().to_owned()).collect()
     }
 
+    // TODO: Delete
     fn deps(&self) -> Vec<Dependency> {
         self.iter().map(|summary| Dependency::with_namever(summary.get_name_ver())).collect()
     }

src/cargo/ops/cargo_compile.rs

@@ -16,42 +16,37 @@
 
 use std::os;
 use util::config;
-use util::config::{all_configs,ConfigValue};
+use util::config::{ConfigValue};
 use core::{PackageSet,Source};
 use core::resolver::resolve;
 use sources::path::PathSource;
 use ops::cargo_rustc;
 use ops::cargo_read_manifest::read_manifest;
-use core::errors::{CargoError,CLIError,CLIResult,ToResult};
+// use core::errors::{CargoError,CLIError,CLIResult,ToResult};
 use core::summary::SummaryVec;
+use util::{other_error, CargoError, CargoResult, Wrap};
 
-pub fn compile(manifest_path: &str) -> CLIResult<()> {
+pub fn compile(manifest_path: &str) -> CargoResult<()> {
     let root_dep = try!(read_manifest(manifest_path)).to_dependency();
 
-    let configs = try!(all_configs(os::getcwd()).to_result(|err: CargoError|
+    let configs = try!(config::all_configs(os::getcwd()));
-        CLIError::new("Could not load configurations", Some(err.to_str()), 1)));
 
     let config_paths = configs.find(&("paths".to_owned())).map(|v| v.clone()).unwrap_or_else(|| ConfigValue::new());
 
     let mut paths: Vec<Path> = match config_paths.get_value() {
-        &config::String(_) => return Err(CLIError::new("The path was configured as a String instead of a List", None, 1)),
+        &config::String(_) => return Err(other_error("The path was configured as a String instead of a List")),
         &config::List(ref list) => list.iter().map(|path| Path::new(path.as_slice())).collect()
     };
 
     paths.push(Path::new(manifest_path).dir_path());
 
     let source = PathSource::new(paths);
-    let summaries = try!(source.list().to_result(|err|
+    let summaries = try!(source.list().wrap("unable to list packages from source"));
-        CLIError::new(format!("Unable to list packages from {}", source), Some(err.to_str()), 1)));
+    let resolved = try!(resolve([root_dep], &summaries).wrap("unable to resolve dependencies"));
 
-    let resolved = try!(resolve([root_dep], &summaries).to_result(|err: CargoError|
+    try!(source.download(resolved.as_slice()).wrap("unable to download packages"));
-        CLIError::new("Unable to resolve dependencies", Some(err.to_str()), 1)));
 
-    try!(source.download(resolved.as_slice()).to_result(|err|
+    let packages = try!(source.get(resolved.as_slice()).wrap("unable ot get packages from source"));
-        CLIError::new(format!("Unable to download packages from {}", source), Some(err.to_str()), 1)));
-
-    let packages = try!(source.get(resolved.as_slice()).to_result(|err|
-        CLIError::new(format!("Unable to get packages from {} for {}", source, summaries.names()), Some(err.to_str()), 1)));
 
     let package_set = PackageSet::new(packages.as_slice());
 

src/cargo/ops/cargo_read_manifest.rs

@@ -1,16 +1,24 @@
 use toml;
 use toml::from_toml;
-use core;
+use core::Package;
 use core::manifest::{TomlManifest};
-use core::errors::{CLIError,CLIResult,ToResult};
+use util::{other_error,CargoResult,CargoError};
 
-pub fn read_manifest(manifest_path: &str) -> CLIResult<core::Package> {
+pub fn read_manifest(path: &str) -> CargoResult<Package> {
-    let root = try!(toml::parse_from_file(manifest_path.clone()).to_result(|err|
+    let root = try!(parse_from_file(path));
-        CLIError::new(format!("Cargo.toml was not valid Toml: {}", manifest_path), Some(err.to_str()), 1)));
+    let toml = try!(load_toml(path, root));
-
+    toml.to_package(path)
-    let toml_manifest = try!(from_toml::<TomlManifest>(root.clone()).to_result(|err: toml::Error|
+}
-        CLIError::new(format!("Cargo.toml was not in the right format: {}", manifest_path), Some(err.to_str()), 1)));
+
-
+fn parse_from_file(path: &str) -> CargoResult<toml::Value> {
-    toml_manifest.to_package(manifest_path.as_slice()).to_result(|err|
+    toml::parse_from_file(path.clone()).map_err(|err| to_cargo_err(path, err))
-        CLIError::new(format!("Cargo.toml was not in the right format: {}", manifest_path), Some(err.to_str()), 1))
+}
+
+fn load_toml(path: &str, root: toml::Value) -> CargoResult<TomlManifest> {
+    from_toml::<TomlManifest>(root).map_err(|err| to_cargo_err(path, err))
+}
+
+fn to_cargo_err(path: &str, err: toml::Error) -> CargoError {
+    other_error("Cargo.toml is not valid Toml")
+        .with_detail(format!("path={}; err={}", path, err.to_str()))
 }

src/cargo/ops/cargo_rustc.rs

@@ -4,13 +4,14 @@ use std::path::Path;
 use core::errors::{CLIError,CLIResult,ToResult};
 use core;
 use util;
+use util::{other_error,CargoResult,CargoError};
 
 type Args = Vec<~str>;
 
-pub fn compile(pkgs: &core::PackageSet) -> CLIResult<()> {
+pub fn compile(pkgs: &core::PackageSet) -> CargoResult<()> {
     let sorted = match pkgs.sort() {
         Some(pkgs) => pkgs,
-        None => return Err(CLIError::new("Circular dependency detected", None, 1))
+        None => return Err(other_error("circular dependency detected"))
     };
 
     for pkg in sorted.iter() {
@@ -21,14 +22,13 @@ pub fn compile(pkgs: &core::PackageSet) -> CLIResult<()> {
     Ok(())
 }
 
-fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CLIResult<()> {
+fn compile_pkg(pkg: &core::Package, pkgs: &core::PackageSet) -> CargoResult<()> {
     // Build up the destination
     // let src = pkg.get_root().join(Path::new(pkg.get_source().path.as_slice()));
     let target_dir = pkg.get_absolute_target_dir();
 
     // First ensure that the directory exists
-    try!(mk_target(&target_dir).to_result(|err|
+    try!(mk_target(&target_dir).map_err(|err| other_error("could not create target directory")));
-        CLIError::new(format!("Could not create the target directory {}", target_dir.display()), Some(err.to_str()), 1)));
 
     // compile
     for target in pkg.get_targets().iter() {
@@ -42,7 +42,7 @@ fn mk_target(target: &Path) -> io::IoResult<()> {
     io::fs::mkdir_recursive(target, io::UserRWX)
 }
 
-fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> CLIResult<()> {
+fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]) -> CargoResult<()> {
     let mut args = Vec::new();
 
     build_base_args(&mut args, target, dest);
@@ -52,8 +52,7 @@ fn rustc(root: &Path, target: &core::Target, dest: &Path, deps: &[core::Package]
         .cwd(root.clone())
         .args(args.as_slice())
         .exec()
-        .to_result(|err|
+        .map_err(|err| rustc_to_cargo_err(&args, root, err)));
-            CLIError::new(format!("Couldn't execute `rustc {}` in `{}`", args.connect(" "), root.display()), Some(err.to_str()), 1)));
 
     Ok(())
 }
@@ -74,3 +73,8 @@ fn build_deps_args(dst: &mut Args, deps: &[core::Package]) {
         dst.push(dir.as_str().unwrap().to_owned());
     }
 }
+
+fn rustc_to_cargo_err(args: &Vec<~str>, cwd: &Path, err: io::IoError) -> CargoError {
+    other_error("failed to exec rustc")
+        .with_detail(format!("args={}; root={}; cause={}", args.connect(" "), cwd.display(), err.to_str()))
+}

src/cargo/sources/path.rs

@@ -2,8 +2,8 @@ use std::fmt;
 use std::fmt::{Show,Formatter};
 use core::{NameVer,Package,Summary};
 use core::source::Source;
-use core::errors::{CargoResult,CargoCLIError,ToResult};
 use cargo_read_manifest = ops::cargo_read_manifest::read_manifest;
+use util::{CargoResult};
 
 pub struct PathSource {
     paths: Vec<Path>
@@ -49,5 +49,5 @@ impl Source for PathSource {
 
 fn read_manifest(path: &Path) -> CargoResult<Package> {
     let joined = path.join("Cargo.toml");
-    cargo_read_manifest(joined.as_str().unwrap()).to_result(|err| CargoCLIError(err))
+    cargo_read_manifest(joined.as_str().unwrap())
 }

src/cargo/util/config.rs

@@ -1,10 +1,8 @@
-extern crate collections;
-extern crate serialize;
-extern crate toml;
-
-use core::errors::{CargoResult,CargoError,ToResult};
-use serialize::{Encodable,Encoder};
 use std::{io,fmt};
+use collections::HashMap;
+use serialize::{Encodable,Encoder};
+use toml;
+use util::{other_error,CargoResult,Require};
 
 #[deriving(Eq,TotalEq,Clone,Encodable,Decodable)]
 pub enum Location {
@@ -76,12 +74,12 @@ impl fmt::Show for ConfigValue {
 }
 
 pub fn get_config(pwd: Path, key: &str) -> CargoResult<ConfigValue> {
-    find_in_tree(&pwd, |file| extract_config(file, key)).to_result(|_|
+    find_in_tree(&pwd, |file| extract_config(file, key))
-        CargoError::described(format!("Config key not found: {}", key)))
+        .map_err(|_| other_error("config key not found").with_detail(format!("key={}", key)))
 }
 
-pub fn all_configs(pwd: Path) -> CargoResult<collections::HashMap<~str, ConfigValue>> {
+pub fn all_configs(pwd: Path) -> CargoResult<HashMap<~str, ConfigValue>> {
-    let mut map = collections::HashMap::new();
+    let mut map = HashMap::new();
 
     try!(walk_tree(&pwd, |file| {
         extract_all_configs(file, &mut map)
@@ -101,7 +99,7 @@ fn find_in_tree<T>(pwd: &Path, walk: |io::fs::File| -> CargoResult<T>) -> CargoR
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).to_result(|_| CargoError::other()));
+            let file = try!(io::fs::File::open(&possible).map_err(|_| other_error("could not open file")));
             match walk(file) {
                 Ok(res) => return Ok(res),
                 _ => ()
@@ -111,7 +109,7 @@ fn find_in_tree<T>(pwd: &Path, walk: |io::fs::File| -> CargoResult<T>) -> CargoR
         if !current.pop() { break; }
     }
 
-    Err(CargoError::other())
+    Err(other_error(""))
 }
 
 fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult<()> {
@@ -121,14 +119,14 @@ fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult
     loop {
         let possible = current.join(".cargo").join("config");
         if possible.exists() {
-            let file = try!(io::fs::File::open(&possible).to_result(|_| CargoError::other()));
+            let file = try!(io::fs::File::open(&possible).map_err(|_| other_error("could not open file")));
             match walk(file) {
                 Err(_) => err = false,
                 _ => ()
             }
         }
 
-        if err { return Err(CargoError::other()); }
+        if err { return Err(other_error("")); }
         if !current.pop() { break; }
     }
 
@@ -138,26 +136,26 @@ fn walk_tree(pwd: &Path, walk: |io::fs::File| -> CargoResult<()>) -> CargoResult
 fn extract_config(file: io::fs::File, key: &str) -> CargoResult<ConfigValue> {
     let path = file.path().clone();
     let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf).to_result(|_| CargoError::other()));
+    let root = try!(toml::parse_from_buffer(&mut buf).map_err(|_| other_error("")));
-    let val = try!(root.lookup(key).to_result(|_| CargoError::other()));
+    let val = try!(root.lookup(key).require(other_error("")));
 
     let v = match val {
         &toml::String(ref val) => String(val.to_owned()),
         &toml::Array(ref val) => List(val.iter().map(|s: &toml::Value| s.to_str()).collect()),
-        _ => return Err(CargoError::other())
+        _ => return Err(other_error(""))
     };
 
     Ok(ConfigValue{ value: v, path: vec!(path) })
 }
 
-fn extract_all_configs(file: io::fs::File, map: &mut collections::HashMap<~str, ConfigValue>) -> CargoResult<()> {
+fn extract_all_configs(file: io::fs::File, map: &mut HashMap<~str, ConfigValue>) -> CargoResult<()> {
     let path = file.path().clone();
     let mut buf = io::BufferedReader::new(file);
-    let root = try!(toml::parse_from_buffer(&mut buf).to_result(|err|
+    let root = try!(toml::parse_from_buffer(&mut buf).map_err(|err|
-        CargoError::described(format!("Couldn't parse Toml manifest `{}`: {}", path.display(), err))));
+        other_error("could not parse Toml manifest").with_detail(format!("path={}; err={}", path.display(), err.to_str()))));
 
-    let table = try!(root.get_table().to_result(|err|
+    let table = try!(root.get_table()
-        CargoError::described(format!("Couldn't parse Toml manifest `{}`: {}", path.display(), err))));
+        .require(other_error("could not parse Toml manifest").with_detail(format!("path={}", path.display()))));
 
     for (key, value) in table.iter() {
         match value {
@@ -167,8 +165,8 @@ fn extract_all_configs(file: io::fs::File, map: &mut collections::HashMap<~str,
                     ConfigValue { path: vec!(), value: List(vec!()) }
                 });
 
-                try!(merge_array(config, val.as_slice(), &path).to_result(|err|
+                try!(merge_array(config, val.as_slice(), &path).map_err(|err|
-                    CargoError::described(format!("The `{}` key in your config {}", key, err))));
+                    other_error("missing").with_detail(format!("The `{}` key in your config {}", key, err))));
             },
             _ => ()
         }
@@ -179,11 +177,11 @@ fn extract_all_configs(file: io::fs::File, map: &mut collections::HashMap<~str,
 
 fn merge_array(existing: &mut ConfigValue, val: &[toml::Value], path: &Path) -> CargoResult<()> {
     match existing.value {
-        String(_) => return Err(CargoError::described("should be an Array, but it was a String")),
+        String(_) => return Err(other_error("should be an Array, but it was a String")),
         List(ref mut list) => {
             let new_list: Vec<CargoResult<~str>> = val.iter().map(|s: &toml::Value| toml_string(s)).collect();
             if new_list.iter().any(|v| v.is_err()) {
-                return Err(CargoError::described("should be an Array of Strings, but was an Array of other values"));
+                return Err(other_error("should be an Array of Strings, but was an Array of other values"));
             } else {
                 let new_list: Vec<~str> = new_list.move_iter().map(|v| v.unwrap()).collect();
                 list.push_all(new_list.as_slice());
@@ -197,6 +195,6 @@ fn merge_array(existing: &mut ConfigValue, val: &[toml::Value], path: &Path) ->
 fn toml_string(val: &toml::Value) -> CargoResult<~str> {
     match val {
         &toml::String(ref str) => Ok(str.to_owned()),
-        _ => Err(CargoError::other())
+        _ => Err(other_error(""))
     }
 }

src/cargo/util/mod.rs

@@ -1,4 +1,6 @@
 pub use self::process_builder::{process,ProcessBuilder};
+pub use self::result::{CargoError,CargoResult,Wrap,Require,other_error};
+
 pub mod graph;
 pub mod process_builder;
 pub mod config;

src/cargo/util/result.rs

@@ -2,14 +2,39 @@ use std::io;
 
 pub type CargoResult<T> = Result<T, CargoError>;
 
-#[deriving(Show)]
+pub fn other_error(desc: &'static str) -> CargoError {
+    CargoError {
+        kind: OtherCargoError,
+        desc: desc,
+        detail: None,
+        cause: None
+    }
+}
+
+#[deriving(Show,Clone)]
 pub struct CargoError {
     kind: CargoErrorKind,
     desc: &'static str,
-    detail: Option<~str>
+    detail: Option<~str>,
+    cause: Option<Box<CargoError>>
 }
 
-#[deriving(Show)]
+impl CargoError {
+    pub fn get_desc(&self) -> &'static str {
+        self.desc
+    }
+
+    pub fn get_detail<'a>(&'a self) -> Option<&'a str> {
+        self.detail.as_ref().map(|s| s.as_slice())
+    }
+
+    pub fn with_detail(mut self, detail: ~str) -> CargoError {
+        self.detail = Some(detail);
+        self
+    }
+}
+
+#[deriving(Show,Clone)]
 pub enum CargoErrorKind {
     InternalError,
     IoError(io::IoError),
@@ -18,7 +43,7 @@ pub enum CargoErrorKind {
 
 type CargoCliResult<T> = Result<T, CargoCliError>;
 
-#[deriving(Show)]
+#[deriving(Show,Clone)]
 pub struct CargoCliError {
     kind: CargoCliErrorKind,
     exit_status: uint,
@@ -27,7 +52,30 @@ pub struct CargoCliError {
     cause: Option<CargoError>
 }
 
-#[deriving(Show)]
+#[deriving(Show,Clone)]
 pub enum CargoCliErrorKind {
     OtherCargoCliError
 }
+
+pub trait Wrap {
+    fn wrap(self, desc: &'static str) -> Self;
+}
+
+impl<T> Wrap for Result<T, CargoError> {
+    fn wrap(self, desc: &'static str) -> Result<T, CargoError> {
+        self
+    }
+}
+
+pub trait Require<T> {
+    fn require(self, err: CargoError) -> CargoResult<T>;
+}
+
+impl<T> Require<T> for Option<T> {
+    fn require(self, err: CargoError) -> CargoResult<T> {
+        match self {
+            Some(x) => Ok(x),
+            None => Err(err)
+        }
+    }
+}