diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs index 17fe4d47eef..e6fc45d091d 100644 --- a/src/cargo/core/workspace.rs +++ b/src/cargo/core/workspace.rs @@ -495,11 +495,7 @@ impl<'cfg> Workspace<'cfg> { self.members.push(manifest_path.clone()); let candidates = { - let pkg = match *self - .packages - .load(&manifest_path) - .map_err(|err| ManifestError::new(err, manifest_path.clone()))? - { + let pkg = match *self.packages.load(&manifest_path)? { MaybePackage::Package(ref p) => p, MaybePackage::Virtual(_) => return Ok(()), }; diff --git a/src/cargo/ops/cargo_read_manifest.rs b/src/cargo/ops/cargo_read_manifest.rs index aefe5b0eec5..d3f9d3e4e15 100644 --- a/src/cargo/ops/cargo_read_manifest.rs +++ b/src/cargo/ops/cargo_read_manifest.rs @@ -153,7 +153,7 @@ fn read_nested_packages( "skipping malformed package found at `{}`", path.to_string_lossy() ); - errors.push(err); + errors.push(err.into()); return Ok(()); } Ok(tuple) => tuple, diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 4431a9116da..6fc28981ddc 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -73,25 +73,37 @@ impl fmt::Display for Internal { } } -/// Error related to a particular manifest and providing it's path. +/// Error wrapper related to a particular manifest and providing it's path. +/// +/// This error adds no displayable info of it's own. pub struct ManifestError { cause: Error, manifest: PathBuf, } impl ManifestError { - pub fn new(cause: Error, manifest: PathBuf) -> Self { - Self { cause, manifest } + pub fn new>(cause: E, manifest: PathBuf) -> Self { + Self { + cause: cause.into(), + manifest, + } } pub fn manifest_path(&self) -> &PathBuf { &self.manifest } + + /// Returns an iterator over the `ManifestError` chain of causes. + /// + /// So if this error was not caused by another `ManifestError` this will be empty. + pub fn manifest_causes(&self) -> ManifestCauses { + ManifestCauses { current: self } + } } impl Fail for ManifestError { fn cause(&self) -> Option<&Fail> { - self.cause.as_fail().into() + self.cause.as_fail().cause() } } @@ -107,6 +119,22 @@ impl fmt::Display for ManifestError { } } +/// An iterator over the `ManifestError` chain of causes. +pub struct ManifestCauses<'a> { + current: &'a ManifestError, +} + +impl<'a> Iterator for ManifestCauses<'a> { + type Item = &'a ManifestError; + + fn next(&mut self) -> Option { + self.current = self.current.cause.downcast_ref()?; + Some(self.current) + } +} + +impl<'a> ::std::iter::FusedIterator for ManifestCauses<'a> {} + // ============================================================================= // Process errors #[derive(Debug, Fail)] diff --git a/src/cargo/util/toml/mod.rs b/src/cargo/util/toml/mod.rs index 4f42a7335c4..367ef8b395f 100644 --- a/src/cargo/util/toml/mod.rs +++ b/src/cargo/util/toml/mod.rs @@ -19,7 +19,7 @@ use core::{Dependency, Manifest, PackageId, Summary, Target}; use core::{Edition, EitherManifest, Feature, Features, VirtualManifest}; use core::{GitReference, PackageIdSpec, SourceId, WorkspaceConfig, WorkspaceRootConfig}; use sources::{CRATES_IO_INDEX, CRATES_IO_REGISTRY}; -use util::errors::{CargoError, CargoResult, CargoResultExt}; +use util::errors::{CargoError, CargoResult, CargoResultExt, ManifestError}; use util::paths; use util::{self, Config, ToUrl}; @@ -30,17 +30,17 @@ pub fn read_manifest( path: &Path, source_id: &SourceId, config: &Config, -) -> CargoResult<(EitherManifest, Vec)> { +) -> Result<(EitherManifest, Vec), ManifestError> { trace!( "read_manifest; path={}; source-id={}", path.display(), source_id ); - let contents = paths::read(path)?; + let contents = paths::read(path).map_err(|err| ManifestError::new(err, path.into()))?; - let ret = do_read_manifest(&contents, path, source_id, config) - .chain_err(|| format!("failed to parse manifest at `{}`", path.display()))?; - Ok(ret) + do_read_manifest(&contents, path, source_id, config) + .chain_err(|| format!("failed to parse manifest at `{}`", path.display())) + .map_err(|err| ManifestError::new(err, path.into())) } fn do_read_manifest( diff --git a/tests/testsuite/member_errors.rs b/tests/testsuite/member_errors.rs index 34ff21db3be..d8458501f45 100644 --- a/tests/testsuite/member_errors.rs +++ b/tests/testsuite/member_errors.rs @@ -44,15 +44,12 @@ fn toml_deserialize_manifest_error() { let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err(); eprintln!("{:?}", error); - let manifest_errs: Vec<_> = error - .iter_chain() - .filter_map(|err| err.downcast_ref::()) - .map(|err| err.manifest_path()) - .collect(); - - assert_eq!(manifest_errs.len(), 2, "{:?}", manifest_errs); - assert_eq!(manifest_errs[0], &root_manifest_path); - assert_eq!(manifest_errs[1], &member_manifest_path); + let manifest_err: &ManifestError = error.downcast_ref().expect("Not a ManifestError"); + assert_eq!(manifest_err.manifest_path(), &root_manifest_path); + + let causes: Vec<_> = manifest_err.manifest_causes().collect(); + assert_eq!(causes.len(), 1, "{:?}", causes); + assert_eq!(causes[0].manifest_path(), &member_manifest_path); } /// Tests inclusion of a `ManifestError` pointing to a member manifest @@ -97,14 +94,11 @@ fn member_manifest_path_io_error() { let error = Workspace::new(&root_manifest_path, &Config::default().unwrap()).unwrap_err(); eprintln!("{:?}", error); - let manifest_errs: Vec<_> = error - .iter_chain() - .filter_map(|err| err.downcast_ref::()) - .map(|err| err.manifest_path()) - .collect(); + let manifest_err: &ManifestError = error.downcast_ref().expect("Not a ManifestError"); + assert_eq!(manifest_err.manifest_path(), &root_manifest_path); - assert_eq!(manifest_errs.len(), 3, "{:?}", manifest_errs); - assert_eq!(manifest_errs[0], &root_manifest_path); - assert_eq!(manifest_errs[1], &member_manifest_path); - assert_eq!(manifest_errs[2], &missing_manifest_path); + let causes: Vec<_> = manifest_err.manifest_causes().collect(); + assert_eq!(causes.len(), 2, "{:?}", causes); + assert_eq!(causes[0].manifest_path(), &member_manifest_path); + assert_eq!(causes[1].manifest_path(), &missing_manifest_path); }