remove missing from the package list error and fmt

Eh2406 · Eh2406 · commit a418364ddab8 · 2018-07-31T15:04:12.000-04:00
diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
@@ -2,12 +2,12 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 use std::fmt;
 use std::str::FromStr;
 
-use serde::ser;
 use serde::de;
+use serde::ser;
 
 use core::{Dependency, Package, PackageId, SourceId, Workspace};
-use util::{internal, Graph};
 use util::errors::{CargoError, CargoResult, CargoResultExt};
+use util::{internal, Graph};
 
 use super::Resolve;
 
@@ -29,23 +29,8 @@ struct Patch {
 
 pub type Metadata = BTreeMap<String, String>;
 
-pub enum ErrorHandle{
-    Ignore,
-    Raise,
-}
-
-impl ErrorHandle {
-    fn is_ignore(&self) -> bool {
-        use self::ErrorHandle::*;
-        match self {
-            Ignore => true,
-            Raise => false,
-        }
-    }
-}
-
 impl EncodableResolve {
-    pub fn into_resolve(self, ws: &Workspace, ignore_errors: ErrorHandle) -> CargoResult<Resolve> {
+    pub fn into_resolve(self, ws: &Workspace) -> CargoResult<Resolve> {
         let path_deps = build_path_deps(ws);
 
         let packages = {
@@ -58,7 +43,7 @@ impl EncodableResolve {
 
         // `PackageId`s in the lock file don't include the `source` part
         // for workspace members, so we reconstruct proper ids.
-        let (live_pkgs, all_pkgs) = {
+        let live_pkgs = {
             let mut live_pkgs = HashMap::new();
             let mut all_pkgs = HashSet::new();
             for pkg in packages.iter() {
@@ -69,10 +54,7 @@ impl EncodableResolve {
                 };
 
                 if !all_pkgs.insert(enc_id.clone()) {
-                    bail!(
-                        "package `{}` is specified twice in the lockfile",
-                        pkg.name
-                    );
+                    bail!("package `{}` is specified twice in the lockfile", pkg.name);
                 }
                 let id = match pkg.source.as_ref().or_else(|| path_deps.get(&pkg.name)) {
                     // We failed to find a local package in the workspace.
@@ -86,33 +68,11 @@ impl EncodableResolve {
 
                 assert!(live_pkgs.insert(enc_id, (id, pkg)).is_none())
             }
-            (live_pkgs, all_pkgs)
+            live_pkgs
         };
 
-        let lookup_id = |enc_id: &EncodablePackageId,
-                         dependent_pkg: Option<&PackageId>|
-         -> CargoResult<Option<PackageId>> {
-            match live_pkgs.get(enc_id) {
-                Some(&(ref id, _)) => Ok(Some(id.clone())),
-                None => if all_pkgs.contains(enc_id) {
-                    // Package is found in the lockfile, but it is
-                    // no longer a member of the workspace.
-                    Ok(None)
-                } else if ignore_errors.is_ignore() {
-                    // We are asked to ignore errors
-                    Ok(None)
-                } else {
-                    let suggestion = dependent_pkg
-                        .map(|p| format!("\n  consider running 'cargo update -p {}'", p.name()))
-                        .unwrap_or_default();
-                    bail!(
-                        "package `{}` is specified as a dependency, \
-                         but is missing from the package list{}",
-                        enc_id,
-                        suggestion,
-                    );
-                },
-            }
+        let lookup_id = |enc_id: &EncodablePackageId| -> Option<PackageId> {
+            live_pkgs.get(enc_id).map(|&(ref id, _)| id.clone())
         };
 
         let g = {
@@ -129,7 +89,7 @@ impl EncodableResolve {
                 };
 
                 for edge in deps.iter() {
-                    if let Some(to_depend_on) = lookup_id(edge, Some(id))? {
+                    if let Some(to_depend_on) = lookup_id(edge) {
                         g.link(id.clone(), to_depend_on);
                     }
                 }
@@ -142,7 +102,7 @@ impl EncodableResolve {
             for &(ref id, pkg) in live_pkgs.values() {
                 if let Some(ref replace) = pkg.replace {
                     assert!(pkg.dependencies.is_none());
-                    if let Some(replace_id) = lookup_id(replace, Some(id))? {
+                    if let Some(replace_id) = lookup_id(replace) {
                         replacements.insert(id.clone(), replace_id);
                     }
                 }
@@ -173,10 +133,11 @@ impl EncodableResolve {
         for (k, v) in metadata.iter().filter(|p| p.0.starts_with(prefix)) {
             to_remove.push(k.to_string());
             let k = &k[prefix.len()..];
-            let enc_id: EncodablePackageId = k.parse()
+            let enc_id: EncodablePackageId = k
+                .parse()
                 .chain_err(|| internal("invalid encoding of checksum in lockfile"))?;
-            let id = match lookup_id(&enc_id, None) {
-                Ok(Some(id)) => id,
+            let id = match lookup_id(&enc_id) {
+                Some(id) => id,
                 _ => continue,
             };
 
@@ -217,7 +178,8 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
     // such as `cargo install` with a lock file from a remote dependency. In
     // that case we don't need to fixup any path dependencies (as they're not
     // actually path dependencies any more), so we ignore them.
-    let members = ws.members()
+    let members = ws
+        .members()
         .filter(|p| p.package_id().source_id().is_path())
         .collect::<Vec<_>>();
 
@@ -317,7 +279,8 @@ impl FromStr for EncodablePackageId {
     fn from_str(s: &str) -> CargoResult<EncodablePackageId> {
         let mut s = s.splitn(3, ' ');
         let name = s.next().unwrap();
-        let version = s.next()
+        let version = s
+            .next()
             .ok_or_else(|| internal("invalid serialized PackageId"))?;
         let source_id = match s.next() {
             Some(s) => {
@@ -373,7 +336,8 @@ impl<'a, 'cfg> ser::Serialize for WorkspaceResolve<'a, 'cfg> {
         let mut ids: Vec<_> = self.resolve.iter().collect();
         ids.sort();
 
-        let encodable = ids.iter()
+        let encodable = ids
+            .iter()
             .filter_map(|&id| Some(encodable_resolve_node(id, self.resolve)))
             .collect::<Vec<_>>();
 
@@ -395,7 +359,8 @@ impl<'a, 'cfg> ser::Serialize for WorkspaceResolve<'a, 'cfg> {
         };
 
         let patch = Patch {
-            unused: self.resolve
+            unused: self
+                .resolve
                 .unused_patches()
                 .iter()
                 .map(|id| EncodableDependency {
diff --git a/src/cargo/core/resolver/mod.rs b/src/cargo/core/resolver/mod.rs
@@ -66,7 +66,7 @@ use self::context::{Activations, Context};
 use self::types::{ActivateError, ActivateResult, Candidate, ConflictReason, DepsFrame, GraphNode};
 use self::types::{RcVecIter, RegistryQueryer};
 
-pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve, ErrorHandle};
+pub use self::encode::{EncodableDependency, EncodablePackageId, EncodableResolve};
 pub use self::encode::{Metadata, WorkspaceResolve};
 pub use self::resolve::{Deps, DepsNotReplaced, Resolve};
 pub use self::types::Method;
diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -2,10 +2,10 @@ use std::collections::{BTreeMap, HashSet};
 
 use termcolor::Color::{self, Cyan, Green, Red};
 
-use core::PackageId;
 use core::registry::PackageRegistry;
+use core::resolver::Method;
+use core::PackageId;
 use core::{Resolve, SourceId, Workspace};
-use core::resolver::{Method, ErrorHandle};
 use ops;
 use util::config::Config;
 use util::CargoResult;
@@ -46,8 +46,7 @@ pub fn update_lockfile(ws: &Workspace, opts: &UpdateOptions) -> CargoResult<()>
         bail!("you can't update in the offline mode");
     }
 
-    // ignore errors, because we are about to clean them up.
-    let previous_resolve = match ops::load_pkg_lockfile(ws, ErrorHandle::Ignore)? {
+    let previous_resolve = match ops::load_pkg_lockfile(ws)? {
         Some(resolve) => resolve,
         None => return generate_lockfile(ws),
     };
diff --git a/src/cargo/ops/cargo_pkgid.rs b/src/cargo/ops/cargo_pkgid.rs
@@ -1,10 +1,9 @@
-use ops;
 use core::{PackageIdSpec, Workspace};
-use core::resolver::ErrorHandle;
+use ops;
 use util::CargoResult;
 
 pub fn pkgid(ws: &Workspace, spec: Option<&str>) -> CargoResult<PackageIdSpec> {
-    let resolve = match ops::load_pkg_lockfile(ws, ErrorHandle::Raise)? {
+    let resolve = match ops::load_pkg_lockfile(ws)? {
         Some(resolve) => resolve,
         None => bail!("a Cargo.lock must exist for this command"),
     };
diff --git a/src/cargo/ops/lockfile.rs b/src/cargo/ops/lockfile.rs
@@ -2,13 +2,13 @@ use std::io::prelude::*;
 
 use toml;
 
+use core::resolver::WorkspaceResolve;
 use core::{resolver, Resolve, Workspace};
-use core::resolver::{WorkspaceResolve, ErrorHandle};
-use util::Filesystem;
 use util::errors::{CargoResult, CargoResultExt};
 use util::toml as cargo_toml;
+use util::Filesystem;
 
-pub fn load_pkg_lockfile(ws: &Workspace, ignore_errors: ErrorHandle) -> CargoResult<Option<Resolve>> {
+pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
     if !ws.root().join("Cargo.lock").exists() {
         return Ok(None);
     }
@@ -24,9 +24,8 @@ pub fn load_pkg_lockfile(ws: &Workspace, ignore_errors: ErrorHandle) -> CargoRes
         (|| -> CargoResult<Option<Resolve>> {
             let resolve: toml::Value = cargo_toml::parse(&s, f.path(), ws.config())?;
             let v: resolver::EncodableResolve = resolve.try_into()?;
-            Ok(Some(v.into_resolve(ws, ignore_errors)?))
-        })()
-            .chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
+            Ok(Some(v.into_resolve(ws)?))
+        })().chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
     Ok(resolve)
 }
 
@@ -115,8 +114,7 @@ fn are_equal_lockfiles(mut orig: String, current: &str, ws: &Workspace) -> bool
         let res: CargoResult<bool> = (|| {
             let old: resolver::EncodableResolve = toml::from_str(&orig)?;
             let new: resolver::EncodableResolve = toml::from_str(current)?;
-            // ignore errors, because we may be about to clean them up.
-            Ok(old.into_resolve(ws, ErrorHandle::Ignore)? == new.into_resolve(ws, ErrorHandle::Ignore)?)
+            Ok(old.into_resolve(ws)? == new.into_resolve(ws)?)
         })();
         if let Ok(true) = res {
             return true;
diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
@@ -1,12 +1,12 @@
 use std::collections::HashSet;
 
-use core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace};
 use core::registry::PackageRegistry;
-use core::resolver::{self, Method, Resolve, ErrorHandle};
-use sources::PathSource;
+use core::resolver::{self, Method, Resolve};
+use core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace};
 use ops;
-use util::profile;
+use sources::PathSource;
 use util::errors::{CargoResult, CargoResultExt};
+use util::profile;
 
 /// Resolve all dependencies for the workspace using the previous
 /// lockfile as a guide if present.
@@ -82,7 +82,7 @@ pub fn resolve_ws_with_method<'a>(
 
         Some(resolve)
     } else {
-        ops::load_pkg_lockfile(ws, ErrorHandle::Raise)?
+        ops::load_pkg_lockfile(ws)?
     };
 
     let resolved_with_overrides = ops::resolve_with_previous(
@@ -106,7 +106,7 @@ fn resolve_with_registry<'cfg>(
     registry: &mut PackageRegistry<'cfg>,
     warn: bool,
 ) -> CargoResult<Resolve> {
-    let prev = ops::load_pkg_lockfile(ws, ErrorHandle::Raise)?;
+    let prev = ops::load_pkg_lockfile(ws)?;
     let resolve = resolve_with_previous(
         registry,
         ws,
@@ -274,7 +274,11 @@ pub fn resolve_with_previous<'a, 'cfg>(
                 // workspace, then we use `method` specified. Otherwise we use a
                 // base method with no features specified but using default features
                 // for any other packages specified with `-p`.
-                Method::Required { dev_deps, all_features, .. } => {
+                Method::Required {
+                    dev_deps,
+                    all_features,
+                    ..
+                } => {
                     let base = Method::Required {
                         dev_deps,
                         features: &[],
@@ -337,7 +341,10 @@ pub fn resolve_with_previous<'a, 'cfg>(
 
 /// Read the `paths` configuration variable to discover all path overrides that
 /// have been configured.
-pub fn add_overrides<'a>(registry: &mut PackageRegistry<'a>, ws: &Workspace<'a>) -> CargoResult<()> {
+pub fn add_overrides<'a>(
+    registry: &mut PackageRegistry<'a>,
+    ws: &Workspace<'a>,
+) -> CargoResult<()> {
     let paths = match ws.config().get_list("paths")? {
         Some(list) => list,
         None => return Ok(()),
@@ -366,7 +373,10 @@ pub fn add_overrides<'a>(registry: &mut PackageRegistry<'a>, ws: &Workspace<'a>)
     Ok(())
 }
 
-pub fn get_resolved_packages<'a>(resolve: &Resolve, registry: PackageRegistry<'a>) -> PackageSet<'a> {
+pub fn get_resolved_packages<'a>(
+    resolve: &Resolve,
+    registry: PackageRegistry<'a>,
+) -> PackageSet<'a> {
     let ids: Vec<PackageId> = resolve.iter().cloned().collect();
     registry.get(&ids)
 }
@@ -496,7 +506,8 @@ fn register_previous_locks<'a>(
             //       dependency on that crate to enable the feature. For now
             //       this bug is better than the always updating registry
             //       though...
-            if !ws.members()
+            if !ws
+                .members()
                 .any(|pkg| pkg.package_id() == member.package_id())
                 && (dep.is_optional() || !dep.is_transitive())
             {
diff --git a/tests/testsuite/bad_config.rs b/tests/testsuite/bad_config.rs
@@ -1,6 +1,6 @@
-use support::{basic_manifest, execs, project};
-use support::registry::Package;
 use support::hamcrest::assert_that;
+use support::registry::Package;
+use support::{basic_manifest, execs, project};
 
 #[test]
 fn bad1() {
@@ -153,13 +153,13 @@ fn bad_cargo_config_jobs() {
         .build();
     assert_that(
         p.cargo("build").arg("-v"),
-        execs()
-            .with_status(101)
-            .with_stderr("\
+        execs().with_status(101).with_stderr(
+            "\
 [ERROR] error in [..].cargo[/]config: \
 could not load config key `build.jobs`: \
 invalid value: integer `-1`, expected u32
-"),
+",
+        ),
     );
 }
 
@@ -371,23 +371,7 @@ fn bad_dependency_in_lockfile() {
         )
         .build();
 
-    assert_that(
-        p.cargo("build"),
-        execs().with_status(101).with_stderr(
-            "\
-[ERROR] failed to parse lock file at: [..]
-
-Caused by:
-  package `bar 0.1.0 ([..])` is specified as a dependency, but is missing from the package list
-  consider running 'cargo update -p foo'
-",
-        ),
-    );
-
-    assert_that(
-        p.cargo("update -p foo"),
-        execs().with_status(0),
-    );
+    assert_that(p.cargo("build"), execs().with_status(0));
 }
 
 #[test]
@@ -733,7 +717,8 @@ warning: unused manifest key: project.bulid
         ),
     );
 
-    let p = project().at("bar")
+    let p = project()
+        .at("bar")
         .file(
             "Cargo.toml",
             r#"
