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

Remove root from cargo lock #4571

Merged
merged 7 commits into from
Oct 4, 2017

Conversation

raytung
Copy link
Contributor

@raytung raytung commented Oct 3, 2017

Removing [root] section from Cargo.lock for #4563

Tests of interest has been updated accordingly, especially tests/lockfile-compat.rs#oldest_lockfile_still_works

@rust-highfive
Copy link

r? @matklad

(rust_highfive has picked a reviewer for you, use r? to override)

let toml = toml::Value::try_from(WorkspaceResolve {
ws: ws,
resolve: resolve,
use_root_key: use_root_key,
use_root_key: false,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this field can be removed now as well?

@@ -971,7 +971,7 @@ fn stale_cached_version() {
let rev = repo.revparse_single("HEAD").unwrap().id();

File::create(&foo.root().join("Cargo.lock")).unwrap().write_all(format!(r#"
[root]
[[package]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it true that changes in tests are mostly cosmetics? That is, the tests pass with [root] as well as with [[package]]?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it. I made the same change against master branch and the test passes without a hitch

@@ -328,7 +328,7 @@ Cargo will take the latest commit and write that information out into our
`Cargo.lock` when we build for the first time. That file will look like this:

```toml
[root]
[[package]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent catch! I haven't thought of the docs at all!

@lukaslueg
Copy link
Contributor

A workspace with an old-style lock file should get transformed by cargo update, shouldn't it? Can we have a test (extended) to see to that effect?

@matklad
Copy link
Member

matklad commented Oct 3, 2017

@lukaslueg yeah, I think it makes sense to add this test as well. tests/lockfile-compat.rs sounds like a proper place for it!

for (l, r) in expected_lock_file.lines().zip(lock.lines()) {
assert!(lines_match(l, r), "Lines differ:\n{}\n\n{}", l, r);
}
for cargo_command in cargo_commands {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I think this won't work as expected, because the second time around the lockfile will already be updated.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah! of course. I'll probably create a helper method and loop through the vector.

@matklad
Copy link
Member

matklad commented Oct 4, 2017

LGTM!

I'll tag this relnotes: it's probably worth mentioning that lockfiles might loose [root] section on the next update.

@bors r+

@bors
Copy link
Contributor

bors commented Oct 4, 2017

📌 Commit cab464e has been approved by matklad

@bors
Copy link
Contributor

bors commented Oct 4, 2017

⌛ Testing commit cab464e with merge f0b06ca...

bors added a commit that referenced this pull request Oct 4, 2017
Remove root from cargo lock

Removing [root] section from Cargo.lock for #4563

Tests of interest has been updated accordingly, especially [tests/lockfile-compat.rs#oldest_lockfile_still_works](https://github.com/rust-lang/cargo/compare/master...raytung:remove-root-from-cargo-lock?expand=1#diff-48866d1891f97cb799dd0ca7148d1eefR10)
@matklad matklad added the relnotes Release-note worthy label Oct 4, 2017
@bors
Copy link
Contributor

bors commented Oct 4, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: matklad
Pushing f0b06ca to master...

@bors bors merged commit cab464e into rust-lang:master Oct 4, 2017
@raytung
Copy link
Contributor Author

raytung commented Oct 5, 2017

Cheers @matklad and @lukaslueg! 😄

@budziq
Copy link

budziq commented Oct 17, 2017

Hi I was recently hit by this budziq/rust-skeptic#65. I'm parsing cargo.lock [root] section to get version numbers of resolved dependencies (strangely this still works in nightly).

What would be the correct way to get versions of all deps in the workspace?

@matklad
Copy link
Member

matklad commented Oct 17, 2017

@budziq
Copy link

budziq commented Oct 17, 2017

Thanks @matklad! Is it's output stable?

@matklad
Copy link
Member

matklad commented Oct 17, 2017

@alexcrichton sometimes I think we should add a

# The format of this file is unspecified and may change between Cargo releases.
# Use `cargo metadata` command to get the information about resolved dependencies in a stable way.

comment to the top of our lockfiles ... :)

@matklad
Copy link
Member

matklad commented Oct 17, 2017

@budziq yep! Moreover, it'll yell at you if you don't pass the --format-version 1 argument explicitly :)

@budziq
Copy link

budziq commented Oct 17, 2017

Thanks!

illicitonion added a commit to illicitonion/pants that referenced this pull request Jan 5, 2018
1.22 removed [root]: rust-lang/cargo#4571

The old file is compatible, cargo updates it when it runs, which means I can't
release because the workspace isn't clean.
illicitonion added a commit to pantsbuild/pants that referenced this pull request Jan 5, 2018
1.22 removed [root]: rust-lang/cargo#4571

The old file is compatible, cargo updates it when it runs, which means I can't
release because the workspace isn't clean.
kamalmarhubi added a commit to kamalmarhubi/rls-span that referenced this pull request Feb 23, 2018
Support for reading rootless lock files was added in cargo PR 3031,
which landed in cargo 0.14, released in November of 2016.  Cargo stopped
generating the root section in cargo PR 4571, which landed in cargo
0.23.

refs rust-lang/cargo#3031
refs rust-lang/cargo#4563
refs rust-lang/cargo#4571
ithinuel referenced this pull request in edef1c/grit Jun 10, 2018
@ehuss ehuss added this to the 1.22.0 milestone Feb 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
relnotes Release-note worthy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants