Skip to content

Commit 63a08ee

Browse files
borsbors
committed
Auto merge of #5831 - Eh2406:i5684, r=alexcrichton
cargo can silently fix some bad lockfiles (use --locked to disable) Lock files often get corrupted by git merge. This makes all cargo commands silently fix that kind of corruption. If you want to be sure that your CI does not change the lock file you have commited --- Then make sure to use `--locked` in your CI Edit: original description below --------------- This is a continuation of @dwijnand work in #5809, and closes #5684 This adds a `ignore_errors` arg to reading a lock file which ignores sections it doesn't understand. Specifically things that depend on versions that don't exist in the lock file. Then all users pass false except for the two that relate to `update` command. I think the open questions for this pr relate to testing. - Now that we are passing false in all other commands, do they each need a test for a bad lockfile? - Do we need a test with a more subtly corrupted lock file, or is this always sufficient for `update` to clean up?
2 parents f14aea5 + a418364 commit 63a08ee

File tree

6 files changed

+53
-63
lines changed

6 files changed

+53
-63
lines changed

‎src/cargo/core/resolver/encode.rs

+20-31
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,12 @@ use std::collections::{BTreeMap, HashMap, HashSet};
22
use std::fmt;
33
use std::str::FromStr;
44

5-
use serde::ser;
65
use serde::de;
6+
use serde::ser;
77

88
use core::{Dependency, Package, PackageId, SourceId, Workspace};
9-
use util::{internal, Graph};
109
use util::errors::{CargoError, CargoResult, CargoResultExt};
10+
use util::{internal, Graph};
1111

1212
use super::Resolve;
1313

@@ -43,7 +43,7 @@ impl EncodableResolve {
4343

4444
// `PackageId`s in the lock file don't include the `source` part
4545
// for workspace members, so we reconstruct proper ids.
46-
let (live_pkgs, all_pkgs) = {
46+
let live_pkgs = {
4747
let mut live_pkgs = HashMap::new();
4848
let mut all_pkgs = HashSet::new();
4949
for pkg in packages.iter() {
@@ -54,10 +54,7 @@ impl EncodableResolve {
5454
};
5555

5656
if !all_pkgs.insert(enc_id.clone()) {
57-
bail!(
58-
"package `{}` is specified twice in the lockfile",
59-
pkg.name
60-
);
57+
bail!("package `{}` is specified twice in the lockfile", pkg.name);
6158
}
6259
let id = match pkg.source.as_ref().or_else(|| path_deps.get(&pkg.name)) {
6360
// We failed to find a local package in the workspace.
@@ -71,24 +68,11 @@ impl EncodableResolve {
7168

7269
assert!(live_pkgs.insert(enc_id, (id, pkg)).is_none())
7370
}
74-
(live_pkgs, all_pkgs)
71+
live_pkgs
7572
};
7673

77-
let lookup_id = |enc_id: &EncodablePackageId| -> CargoResult<Option<PackageId>> {
78-
match live_pkgs.get(enc_id) {
79-
Some(&(ref id, _)) => Ok(Some(id.clone())),
80-
None => if all_pkgs.contains(enc_id) {
81-
// Package is found in the lockfile, but it is
82-
// no longer a member of the workspace.
83-
Ok(None)
84-
} else {
85-
bail!(
86-
"package `{}` is specified as a dependency, \
87-
but is missing from the package list",
88-
enc_id
89-
);
90-
},
91-
}
74+
let lookup_id = |enc_id: &EncodablePackageId| -> Option<PackageId> {
75+
live_pkgs.get(enc_id).map(|&(ref id, _)| id.clone())
9276
};
9377

9478
let g = {
@@ -105,7 +89,7 @@ impl EncodableResolve {
10589
};
10690

10791
for edge in deps.iter() {
108-
if let Some(to_depend_on) = lookup_id(edge)? {
92+
if let Some(to_depend_on) = lookup_id(edge) {
10993
g.link(id.clone(), to_depend_on);
11094
}
11195
}
@@ -118,7 +102,7 @@ impl EncodableResolve {
118102
for &(ref id, pkg) in live_pkgs.values() {
119103
if let Some(ref replace) = pkg.replace {
120104
assert!(pkg.dependencies.is_none());
121-
if let Some(replace_id) = lookup_id(replace)? {
105+
if let Some(replace_id) = lookup_id(replace) {
122106
replacements.insert(id.clone(), replace_id);
123107
}
124108
}
@@ -149,10 +133,11 @@ impl EncodableResolve {
149133
for (k, v) in metadata.iter().filter(|p| p.0.starts_with(prefix)) {
150134
to_remove.push(k.to_string());
151135
let k = &k[prefix.len()..];
152-
let enc_id: EncodablePackageId = k.parse()
136+
let enc_id: EncodablePackageId = k
137+
.parse()
153138
.chain_err(|| internal("invalid encoding of checksum in lockfile"))?;
154139
let id = match lookup_id(&enc_id) {
155-
Ok(Some(id)) => id,
140+
Some(id) => id,
156141
_ => continue,
157142
};
158143

@@ -193,7 +178,8 @@ fn build_path_deps(ws: &Workspace) -> HashMap<String, SourceId> {
193178
// such as `cargo install` with a lock file from a remote dependency. In
194179
// that case we don't need to fixup any path dependencies (as they're not
195180
// actually path dependencies any more), so we ignore them.
196-
let members = ws.members()
181+
let members = ws
182+
.members()
197183
.filter(|p| p.package_id().source_id().is_path())
198184
.collect::<Vec<_>>();
199185

@@ -293,7 +279,8 @@ impl FromStr for EncodablePackageId {
293279
fn from_str(s: &str) -> CargoResult<EncodablePackageId> {
294280
let mut s = s.splitn(3, ' ');
295281
let name = s.next().unwrap();
296-
let version = s.next()
282+
let version = s
283+
.next()
297284
.ok_or_else(|| internal("invalid serialized PackageId"))?;
298285
let source_id = match s.next() {
299286
Some(s) => {
@@ -349,7 +336,8 @@ impl<'a, 'cfg> ser::Serialize for WorkspaceResolve<'a, 'cfg> {
349336
let mut ids: Vec<_> = self.resolve.iter().collect();
350337
ids.sort();
351338

352-
let encodable = ids.iter()
339+
let encodable = ids
340+
.iter()
353341
.filter_map(|&id| Some(encodable_resolve_node(id, self.resolve)))
354342
.collect::<Vec<_>>();
355343

@@ -371,7 +359,8 @@ impl<'a, 'cfg> ser::Serialize for WorkspaceResolve<'a, 'cfg> {
371359
};
372360

373361
let patch = Patch {
374-
unused: self.resolve
362+
unused: self
363+
.resolve
375364
.unused_patches()
376365
.iter()
377366
.map(|id| EncodableDependency {

‎src/cargo/ops/cargo_generate_lockfile.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ use std::collections::{BTreeMap, HashSet};
22

33
use termcolor::Color::{self, Cyan, Green, Red};
44

5-
use core::PackageId;
65
use core::registry::PackageRegistry;
7-
use core::{Resolve, SourceId, Workspace};
86
use core::resolver::Method;
7+
use core::PackageId;
8+
use core::{Resolve, SourceId, Workspace};
99
use ops;
1010
use util::config::Config;
1111
use util::CargoResult;

‎src/cargo/ops/cargo_pkgid.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
use ops;
21
use core::{PackageIdSpec, Workspace};
2+
use ops;
33
use util::CargoResult;
44

55
pub fn pkgid(ws: &Workspace, spec: Option<&str>) -> CargoResult<PackageIdSpec> {

‎src/cargo/ops/lockfile.rs

+3-4
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,11 @@ use std::io::prelude::*;
22

33
use toml;
44

5-
use core::{resolver, Resolve, Workspace};
65
use core::resolver::WorkspaceResolve;
7-
use util::Filesystem;
6+
use core::{resolver, Resolve, Workspace};
87
use util::errors::{CargoResult, CargoResultExt};
98
use util::toml as cargo_toml;
9+
use util::Filesystem;
1010

1111
pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
1212
if !ws.root().join("Cargo.lock").exists() {
@@ -25,8 +25,7 @@ pub fn load_pkg_lockfile(ws: &Workspace) -> CargoResult<Option<Resolve>> {
2525
let resolve: toml::Value = cargo_toml::parse(&s, f.path(), ws.config())?;
2626
let v: resolver::EncodableResolve = resolve.try_into()?;
2727
Ok(Some(v.into_resolve(ws)?))
28-
})()
29-
.chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
28+
})().chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
3029
Ok(resolve)
3130
}
3231

‎src/cargo/ops/resolve.rs

+18-7
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
11
use std::collections::HashSet;
22

3-
use core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace};
43
use core::registry::PackageRegistry;
54
use core::resolver::{self, Method, Resolve};
6-
use sources::PathSource;
5+
use core::{PackageId, PackageIdSpec, PackageSet, Source, SourceId, Workspace};
76
use ops;
8-
use util::profile;
7+
use sources::PathSource;
98
use util::errors::{CargoResult, CargoResultExt};
9+
use util::profile;
1010

1111
/// Resolve all dependencies for the workspace using the previous
1212
/// lockfile as a guide if present.
@@ -272,7 +272,11 @@ pub fn resolve_with_previous<'a, 'cfg>(
272272
// workspace, then we use `method` specified. Otherwise we use a
273273
// base method with no features specified but using default features
274274
// for any other packages specified with `-p`.
275-
Method::Required { dev_deps, all_features, .. } => {
275+
Method::Required {
276+
dev_deps,
277+
all_features,
278+
..
279+
} => {
276280
let base = Method::Required {
277281
dev_deps,
278282
features: &[],
@@ -335,7 +339,10 @@ pub fn resolve_with_previous<'a, 'cfg>(
335339

336340
/// Read the `paths` configuration variable to discover all path overrides that
337341
/// have been configured.
338-
pub fn add_overrides<'a>(registry: &mut PackageRegistry<'a>, ws: &Workspace<'a>) -> CargoResult<()> {
342+
pub fn add_overrides<'a>(
343+
registry: &mut PackageRegistry<'a>,
344+
ws: &Workspace<'a>,
345+
) -> CargoResult<()> {
339346
let paths = match ws.config().get_list("paths")? {
340347
Some(list) => list,
341348
None => return Ok(()),
@@ -364,7 +371,10 @@ pub fn add_overrides<'a>(registry: &mut PackageRegistry<'a>, ws: &Workspace<'a>)
364371
Ok(())
365372
}
366373

367-
pub fn get_resolved_packages<'a>(resolve: &Resolve, registry: PackageRegistry<'a>) -> PackageSet<'a> {
374+
pub fn get_resolved_packages<'a>(
375+
resolve: &Resolve,
376+
registry: PackageRegistry<'a>,
377+
) -> PackageSet<'a> {
368378
let ids: Vec<PackageId> = resolve.iter().cloned().collect();
369379
registry.get(&ids)
370380
}
@@ -494,7 +504,8 @@ fn register_previous_locks<'a>(
494504
// dependency on that crate to enable the feature. For now
495505
// this bug is better than the always updating registry
496506
// though...
497-
if !ws.members()
507+
if !ws
508+
.members()
498509
.any(|pkg| pkg.package_id() == member.package_id())
499510
&& (dep.is_optional() || !dep.is_transitive())
500511
{

‎tests/testsuite/bad_config.rs

+9-18
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
use support::{basic_manifest, execs, project};
2-
use support::registry::Package;
31
use support::hamcrest::assert_that;
2+
use support::registry::Package;
3+
use support::{basic_manifest, execs, project};
44

55
#[test]
66
fn bad1() {
@@ -153,13 +153,13 @@ fn bad_cargo_config_jobs() {
153153
.build();
154154
assert_that(
155155
p.cargo("build").arg("-v"),
156-
execs()
157-
.with_status(101)
158-
.with_stderr("\
156+
execs().with_status(101).with_stderr(
157+
"\
159158
[ERROR] error in [..].cargo[/]config: \
160159
could not load config key `build.jobs`: \
161160
invalid value: integer `-1`, expected u32
162-
"),
161+
",
162+
),
163163
);
164164
}
165165

@@ -371,17 +371,7 @@ fn bad_dependency_in_lockfile() {
371371
)
372372
.build();
373373

374-
assert_that(
375-
p.cargo("build"),
376-
execs().with_status(101).with_stderr(
377-
"\
378-
[ERROR] failed to parse lock file at: [..]
379-
380-
Caused by:
381-
package `bar 0.1.0 ([..])` is specified as a dependency, but is missing from the package list
382-
",
383-
),
384-
);
374+
assert_that(p.cargo("build"), execs().with_status(0));
385375
}
386376

387377
#[test]
@@ -754,7 +744,8 @@ warning: unused manifest key: project.bulid
754744
),
755745
);
756746

757-
let p = project().at("bar")
747+
let p = project()
748+
.at("bar")
758749
.file(
759750
"Cargo.toml",
760751
r#"

0 commit comments

Comments
 (0)