Skip to content

Commit

Permalink
remove missing from the package list error and fmt
Browse files Browse the repository at this point in the history
  • Loading branch information
Eh2406 committed Jul 31, 2018
1 parent 138b644 commit a418364
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 107 deletions.
79 changes: 22 additions & 57 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 = {
Expand All @@ -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() {
Expand All @@ -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.
Expand All @@ -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 = {
Expand All @@ -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);
}
}
Expand All @@ -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);
}
}
Expand Down Expand Up @@ -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,
};

Expand Down Expand Up @@ -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<_>>();

Expand Down Expand Up @@ -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) => {
Expand Down Expand Up @@ -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<_>>();

Expand All @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/core/resolver/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 3 additions & 4 deletions src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
};
Expand Down
5 changes: 2 additions & 3 deletions src/cargo/ops/cargo_pkgid.rs
Original file line number Diff line number Diff line change
@@ -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"),
};
Expand Down
14 changes: 6 additions & 8 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand All @@ -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)
}

Expand Down Expand Up @@ -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;
Expand Down
31 changes: 21 additions & 10 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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: &[],
Expand Down Expand Up @@ -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(()),
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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())
{
Expand Down
33 changes: 9 additions & 24 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
@@ -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() {
Expand Down Expand Up @@ -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
"),
",
),
);
}

Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -733,7 +717,8 @@ warning: unused manifest key: project.bulid
),
);

let p = project().at("bar")
let p = project()
.at("bar")
.file(
"Cargo.toml",
r#"
Expand Down

0 comments on commit a418364

Please sign in to comment.