Cargo: Remove `[root]` section from `Cargo.lock` file

[matklad]

Cargo.lock includes [root] section which is no longer used for anything, because it became obsolete when Cargo workspaces were implemented.

About a year ago, support for rootless lockfiles was implemented: https://github.com/rust-lang/cargo/commit/5e644df1fffc5726b2bf4ce8e29990602208ac51.

However, by default Cargo still adds an arbitrary root to be compatible with older Cargos. Given that we have a year's worth of new Cargos, I think we can safely switch to rootless lockfiles.

This code should be removed: https://github.com/rust-lang/cargo/blob/d7e3b7f24265016c9ce4455504b914dc92c92b07/src/cargo/ops/lockfile.rs#L45-L49

However, we should make sure that Cargo is still able to read a lockfile with a root section (there's a test for this).

[matklad]

@alexcrichton do you also think that a year is a long enough grace period? To be clear, new Cargos will always read the old lockfiles, but a Cargo from a year ago will fail to read the new lockfile.

[alexcrichton]

Whoa I had no idea that we had support for this! In that case I think we're probably in a pretty good situation. Although I think we may be missing one piece to flip the switch.

Right now while an older Cargo will parse a root-missing lockfile, will it also generate one? The problem we just want to avoid is that if you build with different verisons of Cargo the lock file oscillates between two different formats. If older Cargo though generates a lock file without [root] when reading a lock file without [root], sounds great!

[matklad]

If older Cargo though generates a lock file without [root] when reading a lock file without [root], sounds great!

This is exactly what was implemented back then, yeah!

[raytung]

@matklad Mind if I give this a go? :)

[matklad]

@raytung sure, it's yours :)

[tamird]

Fixed in #4571?

[matklad]

Indeed!

[DanielKeep]

This is a problem for me. This means that the current nightly version of Cargo actively breaks compatibility with older versions of Rust. Every time I run cargo build, even if it doesn't need to change the lock file, it removes the [root] section. This means that if I want to test against older versions of Rust, I have to manually go and reset the lock file from source control.

Additionally, it means I can't work on changes in the new version of Rust, then back-port those changes; I have to work exclusively in an older version. So far as I'm aware, there's no way to prevent Cargo from messing with the lock file. If I make the file read-only, cargo build just fails outright (again, even if it doesn't need to modify it).

The question on this issue was "if we aren't using this, why not remove it?" My question is: "if it doesn't hurt anything, why remove it?"

[matklad]

Every time I run cargo build, even if it doesn't need to change the lock file, it removes the [root] section.

So even with --frozen the lockfile is modified? If this is the case, I think we should fix this.

Additionally, it means I can't work on changes in the new version of Rust, then back-port those changes; I have to work exclusively in an older version.

This is true: this change makes lockfiles generated by newer Cargos incompatible with older Cargos.
Please note that "older" here means "one year old or more": For about one year Cargo is able to read lockfiles without a [root] section, as a preparation for their eventual removal.

Could you elaborate what is the backwards compatibility story for your use case? Why do you need to publish updates,which are to be build with a year's old toolchain?

if it doesn't hurt anything, why remove it?

The main (not very strong) motivation here is to prevent the user from interpreting the [root] section of Cargo.lock as something meaningful. Just listing some package (what we do now) does cause confusion: https://github.com/rust-lang/cargo/issues/3704. So yeah, it kinda hurts a little now :)

[raytung]

At the moment we can't build old projects with --frozen. This is what I got:

$ cd old_project
$ rustup run nightly cargo build --frozen
error: the lock file needs to be updated but --frozen was passed to prevent this

[DanielKeep]

As noted, --frozen and --locked don't help; they just prevent building entirely. Cargo appears determined to remove the [root] section come hell or high water.

Could you elaborate what is the backwards compatibility story for your use case? Why do you need to publish updates,which are to be build with a year's old toolchain?

I said I'd support 1.11. I have no technical reason to break that promise: there is nothing my code is doing that can't be done in 1.11. (Even then, it's 1.11 due to dependencies.) I personally loathe it when people break back compat without a damn good reason for doing so, so I take my promises in this regard very seriously. It's not about needing to build on a year-old toolchain, it's about there being no reason to not build on a year-old toolchain.

So yeah, it kinda hurts a little now :)

This sounds more like "workspaces and [root] don't get along" combined with "that's not how you're supposed to interpret that", not that retaining [root] is itself a problem.

The problem here isn't that Cargo isn't writing [root] when it updates the lockfile any more, it's that I can't get it to stop updating the lockfile, even when it has no reason to do so. Incidentally, I'd be fine with maintaining two different, incompatible lock files if they were under different filenames (i.e. Cargo.lock, Cargo.lockv2, etc.).

[isislovecruft]

Hi!

We (The Tor Project) appear to be having the same bug as @DanielKeep (and reasoning for backwards compatibility with the lockfiles which are now in our older, maintenance-only branches). However, instead of with --locked, we're specifying --frozen, and newer cargo is desperately trying to update the lockfile, then bailing because it can't. Our bug for tracking this is https://bugs.torproject.org/24608, the description of which includes the logs of a TravisCI build which is failing (we suspect) for this reason.

Should I make a new ticket for this issue?

[carols10cents]

Reopening this seems like a good start :)

[carols10cents]

My first thought was that we patched --locked but not --frozen (because PR #4684 only mentioned --locked in the subject), but I have now learned that --frozen implies --locked. I have learned that from adding a test to cargo that tests --frozen with an old lockfile in addition to the existing test that tests --locked with an old lockfile and the test i added passed.

Given that there was an additional fix for this issue for cases in which the [root] section was not the first crate in the sorted crate array, I'm now suspecting there's a similar edge case tor is hitting, something involving workspaces and the [root] section pointing to a non-arbitrary existing crate.

My next step is going to be attempting to reproduce and reduce the circumstances in which the failure tor is seeing in travisci occurs.

[matklad]

@isislovecruft , on Traivis I see

$ if [[ "$RUST_OPTIONS" != "" ]]; then rustc --version; fi

rustc 1.24.0-nightly (9fe7aa353 2017-12-11)

install.12

0.02s$ if [[ "$RUST_OPTIONS" != "" ]]; then cargo --version; fi

cargo 0.25.0-nightly (930f9d949 2017-12-05)

So this means that the test is using the fresh nightly version of Cargo. Does it mean that this is a fresh regression?

[matklad]

Hm, I am now remembering a PR which removes some checksum checking from the lockfile, I wonder if it is the culprit...

[carols10cents]

@matklad I noticed cargo was updated in rust-lang/rust 8 days ago, which matches when tor started seeing travis fail ("This is ​causing build errors on Travis, which picked up the newest cargo nightly a week ago.")

[matklad]

Yep, the commit I am talking about is https://github.com/rust-lang/cargo/commit/a0dfbb7a967f465ed034334a0cd2bd2e4cb60511

[carols10cents]

Aaaahhh ok, that sounds like a new issue then?

[matklad]

@carols10cents seems so! If tor faced this issue and they are testing on nightly/beta, then they would have caught it ages ago.

[carols10cents]

Ok I've opened a new issue: https://github.com/rust-lang/cargo/issues/4815

and I'm reclosing this one :)