fix(toml): Error on [project] in Edition 2024#13747
Conversation
|
r? @weihanglo rustbot has assigned @weihanglo. Use |
|
This only errors out on edition 2024, though for hard errors it's better to get a consensus from the team. @rfcbot fcp merge |
|
Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members: No concerns currently listed. Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up! See this document for info about what commands tagged team members can give me. |
|
🔔 This is now entering its final comment period, as per the review above. 🔔 |
|
☔ The latest upstream changes (presumably #13754) made this pull request unmergeable. Please resolve the merge conflicts. |
This misses out on features that shouldn't be relevant to fix, like avoid-dev-deps. However, this prepares the way for workspace re-loading.
This opens the door for fixing the workspace
|
☀️ Test successful - checks-actions |
Update cargo 11 commits in 48eca1b164695022295ce466b64b44e4e0228b08..6f06fe908a5ee0f415c187f868ea627e82efe07d 2024-04-12 21:16:36 +0000 to 2024-04-16 18:47:44 +0000 - fix(toml): Error on `[project]` in Edition 2024 (rust-lang/cargo#13747) - feat(update): Include a Locking message (rust-lang/cargo#13759) - chore(deps): update rust crate gix to 0.62.0 [security] (rust-lang/cargo#13760) - test(schemas): Ensure tests cover the correct case (rust-lang/cargo#13761) - feat(resolve): Tell the user the style of resovle done (rust-lang/cargo#13754) - Make sure to also wrap the initial `-vV` invocation (rust-lang/cargo#13659) - docs: update `checkout` GitHub action version (rust-lang/cargo#13757) - Recategorize cargo test's `--doc` flag under "Target Selection" (rust-lang/cargo#13756) - Reword sentence describing workspace toml for clarity (rust-lang/cargo#13753) - docs(ref): Update unstable docs for msrv-policy (rust-lang/cargo#13751) - refactor(config): Consistently use kebab-case (rust-lang/cargo#13748) r? ghost
Update cargo 11 commits in 48eca1b164695022295ce466b64b44e4e0228b08..6f06fe908a5ee0f415c187f868ea627e82efe07d 2024-04-12 21:16:36 +0000 to 2024-04-16 18:47:44 +0000 - fix(toml): Error on `[project]` in Edition 2024 (rust-lang/cargo#13747) - feat(update): Include a Locking message (rust-lang/cargo#13759) - chore(deps): update rust crate gix to 0.62.0 [security] (rust-lang/cargo#13760) - test(schemas): Ensure tests cover the correct case (rust-lang/cargo#13761) - feat(resolve): Tell the user the style of resovle done (rust-lang/cargo#13754) - Make sure to also wrap the initial `-vV` invocation (rust-lang/cargo#13659) - docs: update `checkout` GitHub action version (rust-lang/cargo#13757) - Recategorize cargo test's `--doc` flag under "Target Selection" (rust-lang/cargo#13756) - Reword sentence describing workspace toml for clarity (rust-lang/cargo#13753) - docs(ref): Update unstable docs for msrv-policy (rust-lang/cargo#13751) - refactor(config): Consistently use kebab-case (rust-lang/cargo#13748) r? ghost
| if opts.edition { | ||
| check_resolver_change(ws, opts)?; | ||
| let specs = opts.compile_opts.spec.to_package_id_specs(&original_ws)?; | ||
| let members: Vec<&Package> = original_ws |
There was a problem hiding this comment.
Sorry, there seems to be some problem here, it doesn't fetch all members of the workspace. If the members contain [project], then cargo fix --edition won't complete the migration either.
Is this intentional, or am I misinterpreting?
There was a problem hiding this comment.
We do filter by the --package flag which is us just doing what the user told us. I'm not seeing how presence of [proiject] causes a problem. We also have a test showing that it works.
There was a problem hiding this comment.
Sorry, I don't seem to have been clear.
What I meant was that if the members in the workspace use [project] instead of [package], then executing the command cargo fix --edition in the workspace does not modify the members.
I'm wondering if the changes should be done on all members at once.
There was a problem hiding this comment.
I think I'm still missing the problem case.
Could you create a test case to demonstrate it?
There was a problem hiding this comment.
If there is a workspace ws containing the member hello.
which:
# [ws]/Cargo.toml
[workspace]
members = ["hello"]
[project] # Pay attention here
name = "ws"
version = "0.1.0"
edition = "2021"
# [ws]/hello/Cargo.toml
[project] # Pay attention here
name = "hello"
version = "0.1.0"
edition = "2021"
After running cargo fix --edition --allow-no-vcs in the workspace. The cargo.toml of ws completes the modification, but the cargo.toml of member hello does not.
I think both should be modified to [package] here.
There was a problem hiding this comment.
I've written that up as a test case which is passing.
Yes, we aren't migrating hello's Cargo.toml but we also aren't migrating hellos src/lib.rs. Manifest and rust migration should be done hand-in-hand. The user has to opt-in to migrating hello with the --workspace flag. cargo fix --edition --workspace will migrate both packages, manifest and rust.
Details
#[cargo_test]
fn migrate_project_to_package_ws() {
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["edition2024"]
[workspace]
members = ["hello"]
# Before project
[ project ] # After project header
# After project header line
name = "foo"
edition = "2021"
# After project table
"#,
)
.file("src/lib.rs", "")
.file(
"hello/Cargo.toml",
r#"
cargo-features = ["edition2024"]
# Before project
[ project ] # After project header
# After project header line
name = "hello"
edition = "2021"
# After project table
"#,
)
.file("hello/src/lib.rs", "")
.build();
p.cargo("fix --edition --allow-no-vcs")
.masquerade_as_nightly_cargo(&["edition2024"])
.with_stderr(
"\
[MIGRATING] Cargo.toml from 2021 edition to 2024
[FIXED] Cargo.toml (1 fix)
[WARNING] [CWD]/hello/Cargo.toml: `[project]` is deprecated in favor of `[package]`
[LOCKING] 2 packages to latest compatible versions
[CHECKING] foo v0.0.0 ([CWD])
[MIGRATING] src/lib.rs from 2021 edition to 2024
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]s
",
)
.run();
assert_eq!(
p.read_file("Cargo.toml"),
r#"
cargo-features = ["edition2024"]
[workspace]
members = ["hello"]
# Before project
[ package ] # After project header
# After project header line
name = "foo"
edition = "2021"
# After project table
"#
);
assert_eq!(
p.read_file("hello/Cargo.toml"),
r#"
cargo-features = ["edition2024"]
# Before project
[ project ] # After project header
# After project header line
name = "hello"
edition = "2021"
# After project table
"#
);
}
What does this PR try to resolve?
[package]was added in 86b2a2a in the pre-1.0 days but[project]was never removed nor did we warn on its use until recently in #11135. So likely we can't remove it completely but we can remove it in Edition 2024+.This includes
cargo fixsupport which is hard coded directly into thecargo fixcommand.This is part of
While we haven't signed off on everything in #13629, I figured this (and a couple other changes) are not very controversial.
How should we test and review this PR?
Per commit. Tests are added to show the behavior changes, including in
cargo fix.Additional information