diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index de8220a8a..0f1c2eacd 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -1,6 +1,7 @@ use std::cell::RefCell; use std::collections::hash_map::{Entry, HashMap}; use std::collections::BTreeMap; +use std::io; use std::path::{Path, PathBuf}; use std::slice; @@ -13,7 +14,7 @@ use core::{Dependency, PackageIdSpec}; use core::{EitherManifest, Package, SourceId, VirtualManifest}; use ops; use sources::PathSource; -use util::errors::{CargoResult, CargoResultExt}; +use util::errors::{CargoResult, CargoResultExt, ManifestError}; use util::paths; use util::toml::read_manifest; use util::{Config, Filesystem}; @@ -495,9 +496,21 @@ impl<'cfg> Workspace<'cfg> { self.members.push(manifest_path.clone()); let candidates = { - let pkg = match *self.packages.load(&manifest_path)? { - MaybePackage::Package(ref p) => p, - MaybePackage::Virtual(_) => return Ok(()), + let pkg = match self.packages.load(&manifest_path) { + Ok(MaybePackage::Package(ref p)) => p, + Ok(MaybePackage::Virtual(_)) => return Ok(()), + Err(err) => { + return Err(if err + .iter_chain() + .any(|e| e.downcast_ref::().is_some() ) + { + // don't wrap io errors to ensure ManifestErrors + // are for actual existing manifests + err + } else { + ManifestError::new(err, manifest_path).into() + }); + } }; pkg.dependencies() .iter() @@ -508,7 +521,8 @@ impl<'cfg> Workspace<'cfg> { .collect::>() }; for candidate in candidates { - self.find_path_deps(&candidate, root_manifest, true)?; + self.find_path_deps(&candidate, root_manifest, true) + .map_err(|err| ManifestError::new(err, manifest_path.clone()))?; } Ok(()) } diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 5c49665c9..4431a9116 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -3,6 +3,7 @@ use std::fmt; use std::process::{ExitStatus, Output}; use std::str; +use std::path::PathBuf; use core::{TargetKind, Workspace}; use failure::{Context, Error, Fail}; @@ -72,6 +73,40 @@ impl fmt::Display for Internal { } } +/// Error related to a particular manifest and providing it's path. +pub struct ManifestError { + cause: Error, + manifest: PathBuf, +} + +impl ManifestError { + pub fn new(cause: Error, manifest: PathBuf) -> Self { + Self { cause, manifest } + } + + pub fn manifest_path(&self) -> &PathBuf { + &self.manifest + } +} + +impl Fail for ManifestError { + fn cause(&self) -> Option<&Fail> { + self.cause.as_fail().into() + } +} + +impl fmt::Debug for ManifestError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.cause.fmt(f) + } +} + +impl fmt::Display for ManifestError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + self.cause.fmt(f) + } +} + // ============================================================================= // Process errors #[derive(Debug, Fail)] diff --git a/tests/testsuite/main.rs b/tests/testsuite/main.rs index 4131b1fb4..bd1dc3388 100644 --- a/tests/testsuite/main.rs +++ b/tests/testsuite/main.rs @@ -66,6 +66,7 @@ mod jobserver; mod local_registry; mod lockfile_compat; mod login; +mod member_errors; mod metabuild; mod metadata; mod net_config; diff --git a/tests/testsuite/member_errors.rs b/tests/testsuite/member_errors.rs new file mode 100644 index 000000000..cd27f1252 --- /dev/null +++ b/tests/testsuite/member_errors.rs @@ -0,0 +1,104 @@ +use cargo::core::Workspace; +use cargo::util::{config::Config, errors::ManifestError}; + +use support::project; + +/// Tests inclusion of a `ManifestError` pointing to a member manifest +/// when that manifest fails to deserialize. +#[test] +fn toml_deserialize_manifest_error() { + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.1.0" + authors = [] + + [dependencies] + bar = { path = "bar" } + + [workspace] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + + [dependencies] + foobar == "0.55" + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + let root_manifest_path = p.root().join("Cargo.toml"); + let member_manifest_path = p.root().join("bar").join("Cargo.toml"); + + let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err(); + eprintln!("{:?}", error); + + let manifest_err = error + .iter_chain() + .filter_map(|err| err.downcast_ref::()) + .last() + .expect("No ManifestError"); + + assert_eq!(manifest_err.manifest_path(), &member_manifest_path); +} + +/// Tests inclusion of a `ManifestError` pointing to a member manifest +/// when that manifest has an invalid dependency path. +#[test] +fn member_manifest_path_io_error() { + let p = project() + .file( + "Cargo.toml", + r#" + [project] + name = "foo" + version = "0.1.0" + authors = [] + + [dependencies] + bar = { path = "bar" } + + [workspace] + "#, + ) + .file("src/main.rs", "fn main() {}") + .file( + "bar/Cargo.toml", + r#" + [project] + name = "bar" + version = "0.1.0" + authors = [] + + [dependencies] + foobar = { path = "nosuch" } + "#, + ) + .file("bar/src/main.rs", "fn main() {}") + .build(); + + let root_manifest_path = p.root().join("Cargo.toml"); + let member_manifest_path = p.root().join("bar").join("Cargo.toml"); + + let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err(); + eprintln!("{:?}", error); + + let manifest_err = error + .iter_chain() + .filter_map(|err| err.downcast_ref::()) + .last() + .expect("No ManifestError"); + + assert_eq!(manifest_err.manifest_path(), &member_manifest_path); +}