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 3 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
23 changes: 16 additions & 7 deletions src/cargo/core/resolver/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ struct Patch {
pub type Metadata = BTreeMap<String, String>;

impl EncodableResolve {
pub fn into_resolve(self, ws: &Workspace) -> CargoResult<Resolve> {
pub fn into_resolve(self, ws: &Workspace, ignore_errors: bool) -> CargoResult<Resolve> {
let path_deps = build_path_deps(ws);

let packages = {
Expand Down Expand Up @@ -74,18 +74,27 @@ impl EncodableResolve {
(live_pkgs, all_pkgs)
};

let lookup_id = |enc_id: &EncodablePackageId| -> CargoResult<Option<PackageId>> {
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 {
// 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
but is missing from the package list{}",
enc_id,
suggestion,
);
},
}
Expand All @@ -105,7 +114,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, Some(id))? {
g.link(id.clone(), to_depend_on);
}
}
Expand All @@ -118,7 +127,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, Some(id))? {
replacements.insert(id.clone(), replace_id);
}
}
Expand Down Expand Up @@ -151,7 +160,7 @@ impl EncodableResolve {
let k = &k[prefix.len()..];
let enc_id: EncodablePackageId = k.parse()
.chain_err(|| internal("invalid encoding of checksum in lockfile"))?;
let id = match lookup_id(&enc_id) {
let id = match lookup_id(&enc_id, None) {
Ok(Some(id)) => id,
_ => continue,
};
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/cargo_generate_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ pub fn update_lockfile(ws: &Workspace, opts: &UpdateOptions) -> CargoResult<()>
bail!("you can't update in the offline mode");
}

let previous_resolve = match ops::load_pkg_lockfile(ws)? {
// `ignore_errors` is set to true, because we are about to clean the errors up.
let previous_resolve = match ops::load_pkg_lockfile(ws, true)? {
Some(resolve) => resolve,
None => return generate_lockfile(ws),
};
Expand Down
2 changes: 1 addition & 1 deletion src/cargo/ops/cargo_pkgid.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use core::{PackageIdSpec, Workspace};
use util::CargoResult;

pub fn pkgid(ws: &Workspace, spec: Option<&str>) -> CargoResult<PackageIdSpec> {
let resolve = match ops::load_pkg_lockfile(ws)? {
let resolve = match ops::load_pkg_lockfile(ws, false)? {
Some(resolve) => resolve,
None => bail!("a Cargo.lock must exist for this command"),
};
Expand Down
7 changes: 4 additions & 3 deletions src/cargo/ops/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use util::Filesystem;
use util::errors::{CargoResult, CargoResultExt};
use util::toml as cargo_toml;

pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
pub fn load_pkg_lockfile(ws: &Workspace, ignore_errors: bool) -> CargoResult<Option<Resolve>> {
if !ws.root().join("Cargo.lock").exists() {
return Ok(None);
}
Expand All @@ -24,7 +24,7 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
(|| -> 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)?))
Ok(Some(v.into_resolve(ws, ignore_errors)?))
})()
.chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
Ok(resolve)
Expand Down Expand Up @@ -115,7 +115,8 @@ 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)?;
Ok(old.into_resolve(ws)? == new.into_resolve(ws)?)
// `ignore_errors` is set to true, because we may be about to clean the errors up.
Ok(old.into_resolve(ws, true)? == new.into_resolve(ws, true)?)
})();
if let Ok(true) = res {
return true;
Expand Down
4 changes: 2 additions & 2 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ pub fn resolve_ws_with_method<'a>(

Some(resolve)
} else {
ops::load_pkg_lockfile(ws)?
ops::load_pkg_lockfile(ws, false)?
};

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)?;
let prev = ops::load_pkg_lockfile(ws, false)?;
let resolve = resolve_with_previous(
registry,
ws,
Expand Down
6 changes: 6 additions & 0 deletions tests/testsuite/bad_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -379,9 +379,15 @@ fn bad_dependency_in_lockfile() {

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),
);
}

#[test]
Expand Down