Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cargo can silently fix some bad lockfiles (use --locked to disable) #5831

Merged
merged 5 commits into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 20 additions & 31 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 Down Expand Up @@ -43,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 @@ -54,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 @@ -71,24 +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| -> 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 {
bail!(
"package `{}` is specified as a dependency, \
but is missing from the package list",
enc_id
);
},
}
let lookup_id = |enc_id: &EncodablePackageId| -> Option<PackageId> {
live_pkgs.get(enc_id).map(|&(ref id, _)| id.clone())
};

let g = {
Expand All @@ -105,7 +89,7 @@ impl EncodableResolve {
};

for edge in deps.iter() {
if let Some(to_depend_on) = lookup_id(edge)? {
if let Some(to_depend_on) = lookup_id(edge) {
g.link(id.clone(), to_depend_on);
}
}
Expand All @@ -118,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)? {
if let Some(replace_id) = lookup_id(replace) {
replacements.insert(id.clone(), replace_id);
}
}
Expand Down Expand Up @@ -149,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) {
Ok(Some(id)) => id,
Some(id) => id,
_ => continue,
};

Expand Down Expand Up @@ -193,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 @@ -293,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 @@ -349,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 @@ -371,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
4 changes: 2 additions & 2 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::{Resolve, SourceId, Workspace};
use core::resolver::Method;
use core::PackageId;
use core::{Resolve, SourceId, Workspace};
use ops;
use util::config::Config;
use util::CargoResult;
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_pkgid.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use ops;
use core::{PackageIdSpec, Workspace};
use ops;
use util::CargoResult;

pub fn pkgid(ws: &Workspace, spec: Option<&str>) -> CargoResult<PackageIdSpec> {
Expand Down
7 changes: 3 additions & 4 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@ use std::io::prelude::*;

use toml;

use core::{resolver, Resolve, Workspace};
use core::resolver::WorkspaceResolve;
use util::Filesystem;
use core::{resolver, Resolve, Workspace};
use util::errors::{CargoResult, CargoResultExt};
use util::toml as cargo_toml;
use util::Filesystem;

pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
if !ws.root().join("Cargo.lock").exists() {
Expand All @@ -25,8 +25,7 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> 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)?))
})()
.chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
})().chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
Ok(resolve)
}

Expand Down
25 changes: 18 additions & 7 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};
use sources::PathSource;
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 @@ -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
27 changes: 9 additions & 18 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,17 +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
",
),
);
assert_that(p.cargo("build"), execs().with_status(0));
}

#[test]
Expand Down Expand Up @@ -727,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