Skip to content

Commit dba478b

Browse files
committed
Auto merge of #7579 - alexcrichton:lockfile-fmt, r=ehuss
Turn the new lock file format on by default This commit enables the support added in #7070 by default. This means that gradually over time all `Cargo.lock` files will be migrated to the new format. Cargo shipped with Rust 1.38.0 supports this new lock file format, so any project using Rust 1.38.0 or above will converge quickly onto the new lock file format and continue working. The main benefit of the new format is to be more friendly to git merge conflicts. Information is deduplicated throughout the lock file to avoid verbose `depedencies` lists and the `checksum` data is all listed inline with `[[package]]`. This has been deployed with rust-lang/rust for some time now and it subjectively at least seems to have greatly reduced the amount of bouncing that happens for touching `Cargo.lock`.
2 parents ad618cf + c37a46c commit dba478b

File tree

9 files changed

+156
-78
lines changed

9 files changed

+156
-78
lines changed

src/cargo/core/resolver/encode.rs

+25-1
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ impl EncodableResolve {
132132
/// `features`. Care should be taken when using this Resolve. One of the
133133
/// primary uses is to be used with `resolve_with_previous` to guide the
134134
/// resolver to create a complete Resolve.
135-
pub fn into_resolve(self, ws: &Workspace<'_>) -> CargoResult<Resolve> {
135+
pub fn into_resolve(self, original: &str, ws: &Workspace<'_>) -> CargoResult<Resolve> {
136136
let path_deps = build_path_deps(ws);
137137
let mut checksums = HashMap::new();
138138

@@ -333,6 +333,30 @@ impl EncodableResolve {
333333
unused_patches.push(id);
334334
}
335335

336+
// We have a curious issue where in the "v1 format" we buggily had a
337+
// trailing blank line at the end of lock files under some specific
338+
// conditions.
339+
//
340+
// Cargo is trying to write new lockfies in the "v2 format" but if you
341+
// have no dependencies, for example, then the lockfile encoded won't
342+
// really have any indicator that it's in the new format (no
343+
// dependencies or checksums listed). This means that if you type `cargo
344+
// new` followed by `cargo build` it will generate a "v2 format" lock
345+
// file since none previously existed. When reading this on the next
346+
// `cargo build`, however, it generates a new lock file because when
347+
// reading in that lockfile we think it's the v1 format.
348+
//
349+
// To help fix this issue we special case here. If our lockfile only has
350+
// one trailing newline, not two, *and* it only has one package, then
351+
// this is actually the v2 format.
352+
if original.ends_with("\n")
353+
&& !original.ends_with("\n\n")
354+
&& version == ResolveVersion::V1
355+
&& g.iter().count() == 1
356+
{
357+
version = ResolveVersion::V2;
358+
}
359+
336360
Ok(Resolve::new(
337361
g,
338362
replacements,

src/cargo/core/resolver/mod.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ pub fn resolve(
151151
cksums,
152152
BTreeMap::new(),
153153
Vec::new(),
154-
ResolveVersion::default(),
154+
ResolveVersion::default_for_new_lockfiles(),
155155
);
156156

157157
check_cycles(&resolve)?;

src/cargo/core/resolver/resolve.rs

+70-30
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::borrow::Borrow;
2+
use std::cmp;
23
use std::collections::{HashMap, HashSet};
34
use std::fmt;
45
use std::iter::FromIterator;
@@ -15,7 +16,6 @@ use super::encode::Metadata;
1516
///
1617
/// Each instance of `Resolve` also understands the full set of features used
1718
/// for each package.
18-
#[derive(PartialEq)]
1919
pub struct Resolve {
2020
/// A graph, whose vertices are packages and edges are dependency specifications
2121
/// from `Cargo.toml`. We need a `Vec` here because the same package
@@ -59,9 +59,12 @@ pub struct Resolve {
5959
///
6060
/// It's theorized that we can add more here over time to track larger changes
6161
/// to the `Cargo.lock` format, but we've yet to see how that strategy pans out.
62-
#[derive(PartialEq, Clone, Debug)]
62+
#[derive(PartialEq, Eq, Clone, Copy, Debug, PartialOrd, Ord)]
6363
pub enum ResolveVersion {
64+
// Historical baseline for when this abstraction was added.
6465
V1,
66+
// Update around 2019 where `dependencies` arrays got compressed and
67+
// checksums are listed inline.
6568
V2,
6669
}
6770

@@ -210,21 +213,35 @@ unable to verify that `{0}` is the same as when the lockfile was generated
210213
// Be sure to just copy over any unknown metadata.
211214
self.metadata = previous.metadata.clone();
212215

213-
// The goal of Cargo is largely to preserve the encoding of
214-
// `Cargo.lock` that it finds on the filesystem. Sometimes `Cargo.lock`
215-
// changes are in the works where they haven't been set as the default
216-
// yet but will become the default soon. We want to preserve those
217-
// features if we find them.
216+
// The goal of Cargo is largely to preserve the encoding of `Cargo.lock`
217+
// that it finds on the filesystem. Sometimes `Cargo.lock` changes are
218+
// in the works where they haven't been set as the default yet but will
219+
// become the default soon.
218220
//
219-
// For this reason if the previous `Cargo.lock` is from the future, or
220-
// otherwise it looks like it's produced with future features we
221-
// understand, then the new resolve will be encoded with the same
222-
// version. Note that new instances of `Resolve` always use the default
223-
// encoding, and this is where we switch it to a future encoding if the
224-
// future encoding isn't yet the default.
225-
if previous.version.from_the_future() {
226-
self.version = previous.version.clone();
227-
}
221+
// The scenarios we could be in are:
222+
//
223+
// * This is a brand new lock file with nothing previous. In that case
224+
// this method isn't actually called at all, but instead
225+
// `default_for_new_lockfiles` called below was encoded during the
226+
// resolution step, so that's what we're gonna use.
227+
//
228+
// * We have an old lock file. In this case we want to switch the
229+
// version to `default_for_old_lockfiles`. That puts us in one of
230+
// three cases:
231+
//
232+
// * Our version is older than the default. This means that we're
233+
// migrating someone forward, so we switch the encoding.
234+
// * Our version is equal to the default, nothing to do!
235+
// * Our version is *newer* than the default. This is where we
236+
// critically keep the new version of encoding.
237+
//
238+
// This strategy should get new lockfiles into the pipeline more quickly
239+
// while ensuring that any time an old cargo sees a future lock file it
240+
// keeps the future lockfile encoding.
241+
self.version = cmp::max(
242+
previous.version,
243+
ResolveVersion::default_for_old_lockfiles(),
244+
);
228245

229246
Ok(())
230247
}
@@ -358,6 +375,26 @@ unable to verify that `{0}` is the same as when the lockfile was generated
358375
}
359376
}
360377

378+
impl PartialEq for Resolve {
379+
fn eq(&self, other: &Resolve) -> bool {
380+
macro_rules! compare {
381+
($($fields:ident)* | $($ignored:ident)*) => {
382+
let Resolve { $($fields,)* $($ignored,)* } = self;
383+
$(drop($ignored);)*
384+
$($fields == &other.$fields)&&*
385+
}
386+
}
387+
compare! {
388+
// fields to compare
389+
graph replacements reverse_replacements empty_features features
390+
checksums metadata unused_patches public_dependencies
391+
|
392+
// fields to ignore
393+
version
394+
}
395+
}
396+
}
397+
361398
impl fmt::Debug for Resolve {
362399
fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
363400
writeln!(fmt, "graph: {:?}", self.graph)?;
@@ -370,23 +407,26 @@ impl fmt::Debug for Resolve {
370407
}
371408

372409
impl ResolveVersion {
373-
/// The default way to encode `Cargo.lock`.
410+
/// The default way to encode new `Cargo.lock` files.
374411
///
375-
/// This is used for new `Cargo.lock` files that are generated without a
376-
/// previous `Cargo.lock` files, and generally matches with what we want to
377-
/// encode.
378-
pub fn default() -> ResolveVersion {
379-
ResolveVersion::V1
412+
/// It's important that if a new version of `ResolveVersion` is added that
413+
/// this is not updated until *at least* the support for the version is in
414+
/// the stable release of Rust. It's ok for this to be newer than
415+
/// `default_for_old_lockfiles` below.
416+
pub fn default_for_new_lockfiles() -> ResolveVersion {
417+
ResolveVersion::V2
380418
}
381419

382-
/// Returns whether this encoding version is "from the future".
420+
/// The default way to encode old preexisting `Cargo.lock` files. This is
421+
/// often trailing the new lockfiles one above to give older projects a
422+
/// longer time to catch up.
383423
///
384-
/// This means that this encoding version is not currently the default but
385-
/// intended to become the default "soon".
386-
pub fn from_the_future(&self) -> bool {
387-
match self {
388-
ResolveVersion::V2 => true,
389-
ResolveVersion::V1 => false,
390-
}
424+
/// It's important that this trails behind `default_for_new_lockfiles` for
425+
/// quite some time. This gives projects a quite large window to update in
426+
/// where we don't force updates, so if projects span many versions of Cargo
427+
/// all those versions of Cargo will have support for a new version of the
428+
/// lock file.
429+
pub fn default_for_old_lockfiles() -> ResolveVersion {
430+
ResolveVersion::V1
391431
}
392432
}

src/cargo/ops/lockfile.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ pub fn load_pkg_lockfile(ws: &Workspace<'_>) -> CargoResult<Option<Resolve>> {
2222
let resolve = (|| -> CargoResult<Option<Resolve>> {
2323
let resolve: toml::Value = cargo_toml::parse(&s, f.path(), ws.config())?;
2424
let v: resolver::EncodableResolve = resolve.try_into()?;
25-
Ok(Some(v.into_resolve(ws)?))
25+
Ok(Some(v.into_resolve(&s, ws)?))
2626
})()
2727
.chain_err(|| format!("failed to parse lock file at: {}", f.path().display()))?;
2828
Ok(resolve)
@@ -172,7 +172,7 @@ fn are_equal_lockfiles(mut orig: String, current: &str, ws: &Workspace<'_>) -> b
172172
let res: CargoResult<bool> = (|| {
173173
let old: resolver::EncodableResolve = toml::from_str(&orig)?;
174174
let new: resolver::EncodableResolve = toml::from_str(current)?;
175-
Ok(old.into_resolve(ws)? == new.into_resolve(ws)?)
175+
Ok(old.into_resolve(&orig, ws)? == new.into_resolve(current, ws)?)
176176
})();
177177
if let Ok(true) = res {
178178
return true;

tests/testsuite/lockfile_compat.rs

+44-41
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,14 @@ fn oldest_lockfile_still_works() {
1010
}
1111
}
1212

13+
fn assert_lockfiles_eq(expected: &str, actual: &str) {
14+
for (l, r) in expected.lines().zip(actual.lines()) {
15+
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
16+
}
17+
18+
assert_eq!(expected.lines().count(), actual.lines().count());
19+
}
20+
1321
fn oldest_lockfile_still_works_with_command(cargo_command: &str) {
1422
Package::new("bar", "0.1.0").publish();
1523

@@ -24,11 +32,11 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
2432
name = "foo"
2533
version = "0.0.1"
2634
dependencies = [
27-
"bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
35+
"bar [..]",
2836
]
2937
3038
[metadata]
31-
"checksum bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "[..]"
39+
"[..]" = "[..]"
3240
"#;
3341

3442
let old_lockfile = r#"
@@ -65,11 +73,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
6573
p.cargo(cargo_command).run();
6674

6775
let lock = p.read_lockfile();
68-
for (l, r) in expected_lockfile.lines().zip(lock.lines()) {
69-
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
70-
}
71-
72-
assert_eq!(lock.lines().count(), expected_lockfile.lines().count());
76+
assert_lockfiles_eq(expected_lockfile, &lock);
7377
}
7478

7579
#[cargo_test]
@@ -115,11 +119,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
115119
p.cargo("build --locked").run();
116120

117121
let lock = p.read_lockfile();
118-
for (l, r) in old_lockfile.lines().zip(lock.lines()) {
119-
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
120-
}
121-
122-
assert_eq!(lock.lines().count(), old_lockfile.lines().count());
122+
assert_lockfiles_eq(&old_lockfile, &lock);
123123
}
124124

125125
#[cargo_test]
@@ -166,9 +166,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
166166
p.cargo("build").run();
167167

168168
let lock = p.read_lockfile();
169-
assert!(lock.starts_with(
170-
r#"
171-
# This file is automatically @generated by Cargo.
169+
assert_lockfiles_eq(
170+
r#"# This file is automatically @generated by Cargo.
172171
# It is not intended for manual editing.
173172
[[package]]
174173
name = "bar"
@@ -179,13 +178,14 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
179178
name = "foo"
180179
version = "0.0.1"
181180
dependencies = [
182-
"bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
181+
"bar [..]",
183182
]
184183
185184
[metadata]
186-
"#
187-
.trim()
188-
));
185+
"[..]" = "[..]"
186+
"#,
187+
&lock,
188+
);
189189
}
190190

191191
#[cargo_test]
@@ -413,22 +413,16 @@ fn current_lockfile_format() {
413413
name = \"bar\"
414414
version = \"0.1.0\"
415415
source = \"registry+https://github.com/rust-lang/crates.io-index\"
416+
checksum = \"[..]\"
416417
417418
[[package]]
418419
name = \"foo\"
419420
version = \"0.0.1\"
420421
dependencies = [
421-
\"bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)\",
422+
\"bar\",
422423
]
423-
424-
[metadata]
425-
\"checksum bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)\" = \"[..]\"";
426-
427-
for (l, r) in expected.lines().zip(actual.lines()) {
428-
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
429-
}
430-
431-
assert_eq!(actual.lines().count(), expected.lines().count());
424+
";
425+
assert_lockfiles_eq(expected, &actual);
432426
}
433427

434428
#[cargo_test]
@@ -447,7 +441,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
447441
name = "foo"
448442
version = "0.0.1"
449443
dependencies = [
450-
"bar 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
444+
"bar",
451445
]
452446
"#;
453447

@@ -472,7 +466,24 @@ dependencies = [
472466
p.cargo("build").run();
473467

474468
let lock = p.read_lockfile();
475-
assert!(lock.starts_with(lockfile.trim()));
469+
assert_lockfiles_eq(
470+
r#"# [..]
471+
# [..]
472+
[[package]]
473+
name = "bar"
474+
version = "0.1.0"
475+
source = "registry+https://github.com/rust-lang/crates.io-index"
476+
checksum = "[..]"
477+
478+
[[package]]
479+
name = "foo"
480+
version = "0.0.1"
481+
dependencies = [
482+
"bar",
483+
]
484+
"#,
485+
&lock,
486+
);
476487
}
477488

478489
#[cargo_test]
@@ -549,11 +560,7 @@ dependencies = [
549560
p.cargo("fetch").run();
550561

551562
let lock = p.read_lockfile();
552-
for (l, r) in lockfile.lines().zip(lock.lines()) {
553-
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
554-
}
555-
556-
assert_eq!(lock.lines().count(), lockfile.lines().count());
563+
assert_lockfiles_eq(&lockfile, &lock);
557564
}
558565

559566
#[cargo_test]
@@ -624,9 +631,5 @@ dependencies = [
624631
p.cargo("fetch").run();
625632

626633
let lock = p.read_lockfile();
627-
for (l, r) in lockfile.lines().zip(lock.lines()) {
628-
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
629-
}
630-
631-
assert_eq!(lock.lines().count(), lockfile.lines().count());
634+
assert_lockfiles_eq(&lockfile, &lock);
632635
}

tests/testsuite/new.rs

+11
Original file line numberDiff line numberDiff line change
@@ -527,3 +527,14 @@ fn new_with_reference_link() {
527527
let contents = fs::read_to_string(paths::root().join("foo/Cargo.toml")).unwrap();
528528
assert!(contents.contains("# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html"))
529529
}
530+
531+
#[cargo_test]
532+
fn lockfile_constant_during_new() {
533+
cargo_process("new foo").env("USER", "foo").run();
534+
535+
cargo_process("build").cwd(&paths::root().join("foo")).run();
536+
let before = fs::read_to_string(paths::root().join("foo/Cargo.lock")).unwrap();
537+
cargo_process("build").cwd(&paths::root().join("foo")).run();
538+
let after = fs::read_to_string(paths::root().join("foo/Cargo.lock")).unwrap();
539+
assert_eq!(before, after);
540+
}

0 commit comments

Comments
 (0)