Skip to content

Commit 0b3ad41

Browse files
authored
fix(fix): Migrate workspace dependencies (#14890)
### What does this PR try to resolve? Technically, the edition doesn't affect the workspace. We could adjust this as we're inheriting to not be a problem. But it feels weird to keep this around in newer editions. We could frame this as around the what the package is, including inheritance. The nice thing is that default-features works for all versions that inheritance works so we're not forcing the users hand with multiple editions in a workspace. Fixes #14886 ### How should we test and review this PR? ### Additional information
2 parents 519f069 + f8468ab commit 0b3ad41

File tree

2 files changed

+138
-10
lines changed

2 files changed

+138
-10
lines changed

src/cargo/ops/fix.rs

Lines changed: 63 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -252,6 +252,59 @@ fn check_version_control(gctx: &GlobalContext, opts: &FixOptions) -> CargoResult
252252
}
253253

254254
fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
255+
// HACK: Duplicate workspace migration logic between virtual manifests and real manifests to
256+
// reduce multiple Migrating messages being reported for the same file to the user
257+
if matches!(ws.root_maybe(), MaybePackage::Virtual(_)) {
258+
// Warning: workspaces do not have an edition so this should only include changes needed by
259+
// packages that preserve the behavior of the workspace on all editions
260+
let highest_edition = pkgs
261+
.iter()
262+
.map(|p| p.manifest().edition())
263+
.max()
264+
.unwrap_or_default();
265+
let prepare_for_edition = highest_edition.saturating_next();
266+
if highest_edition == prepare_for_edition
267+
|| (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
268+
{
269+
//
270+
} else {
271+
let mut manifest_mut = LocalManifest::try_new(ws.root_manifest())?;
272+
let document = &mut manifest_mut.data;
273+
let mut fixes = 0;
274+
275+
if Edition::Edition2024 <= prepare_for_edition {
276+
let root = document.as_table_mut();
277+
278+
if let Some(workspace) = root
279+
.get_mut("workspace")
280+
.and_then(|t| t.as_table_like_mut())
281+
{
282+
// strictly speaking, the edition doesn't apply to this table but it should be safe
283+
// enough
284+
fixes += rename_dep_fields_2024(workspace, "dependencies");
285+
}
286+
}
287+
288+
if 0 < fixes {
289+
// HACK: As workspace migration is a special case, only report it if something
290+
// happened
291+
let file = ws.root_manifest();
292+
let file = file.strip_prefix(ws.root()).unwrap_or(file);
293+
let file = file.display();
294+
ws.gctx().shell().status(
295+
"Migrating",
296+
format!("{file} from {highest_edition} edition to {prepare_for_edition}"),
297+
)?;
298+
299+
let verb = if fixes == 1 { "fix" } else { "fixes" };
300+
let msg = format!("{file} ({fixes} {verb})");
301+
ws.gctx().shell().status("Fixed", msg)?;
302+
303+
manifest_mut.write()?;
304+
}
305+
}
306+
}
307+
255308
for pkg in pkgs {
256309
let existing_edition = pkg.manifest().edition();
257310
let prepare_for_edition = existing_edition.saturating_next();
@@ -268,15 +321,15 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
268321
format!("{file} from {existing_edition} edition to {prepare_for_edition}"),
269322
)?;
270323

324+
let mut manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
325+
let document = &mut manifest_mut.data;
326+
let mut fixes = 0;
327+
271328
let ws_original_toml = match ws.root_maybe() {
272329
MaybePackage::Package(package) => package.manifest().original_toml(),
273330
MaybePackage::Virtual(manifest) => manifest.original_toml(),
274331
};
275332
if Edition::Edition2024 <= prepare_for_edition {
276-
let mut manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
277-
let document = &mut manifest_mut.data;
278-
let mut fixes = 0;
279-
280333
let root = document.as_table_mut();
281334

282335
if let Some(workspace) = root
@@ -331,14 +384,14 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
331384
ws_original_toml,
332385
);
333386
}
387+
}
334388

335-
if 0 < fixes {
336-
let verb = if fixes == 1 { "fix" } else { "fixes" };
337-
let msg = format!("{file} ({fixes} {verb})");
338-
ws.gctx().shell().status("Fixed", msg)?;
389+
if 0 < fixes {
390+
let verb = if fixes == 1 { "fix" } else { "fixes" };
391+
let msg = format!("{file} ({fixes} {verb})");
392+
ws.gctx().shell().status("Fixed", msg)?;
339393

340-
manifest_mut.write()?;
341-
}
394+
manifest_mut.write()?;
342395
}
343396
}
344397

tests/testsuite/fix.rs

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2643,6 +2643,81 @@ a = {path = "a", default-features = false}
26432643
);
26442644
}
26452645

2646+
#[cargo_test]
2647+
fn migrate_rename_underscore_fields_in_virtual_manifest() {
2648+
let p = project()
2649+
.file(
2650+
"Cargo.toml",
2651+
r#"
2652+
[workspace]
2653+
members = ["foo"]
2654+
resolver = "2"
2655+
2656+
[workspace.dependencies]
2657+
# Before default_features
2658+
a = {path = "a", default_features = false} # After default_features value
2659+
# After default_features line
2660+
"#,
2661+
)
2662+
.file(
2663+
"foo/Cargo.toml",
2664+
r#"
2665+
[package]
2666+
name = "foo"
2667+
edition = "2021"
2668+
"#,
2669+
)
2670+
.file("foo/src/lib.rs", "")
2671+
.file(
2672+
"a/Cargo.toml",
2673+
r#"
2674+
[package]
2675+
name = "a"
2676+
version = "0.0.1"
2677+
edition = "2015"
2678+
"#,
2679+
)
2680+
.file("a/src/lib.rs", "")
2681+
.build();
2682+
2683+
p.cargo("fix --edition --allow-no-vcs")
2684+
.with_stderr_data(str![[r#"
2685+
[MIGRATING] Cargo.toml from 2021 edition to 2024
2686+
[FIXED] Cargo.toml (1 fix)
2687+
[MIGRATING] foo/Cargo.toml from 2021 edition to 2024
2688+
[CHECKING] foo v0.0.0 ([ROOT]/foo/foo)
2689+
[MIGRATING] foo/src/lib.rs from 2021 edition to 2024
2690+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
2691+
2692+
"#]])
2693+
.run();
2694+
assert_e2e().eq(
2695+
p.read_file("Cargo.toml"),
2696+
str![[r#"
2697+
2698+
[workspace]
2699+
members = ["foo"]
2700+
resolver = "2"
2701+
2702+
[workspace.dependencies]
2703+
# Before default_features
2704+
a = {path = "a", default-features = false} # After default_features value
2705+
# After default_features line
2706+
2707+
"#]],
2708+
);
2709+
assert_e2e().eq(
2710+
p.read_file("foo/Cargo.toml"),
2711+
str![[r#"
2712+
2713+
[package]
2714+
name = "foo"
2715+
edition = "2021"
2716+
2717+
"#]],
2718+
);
2719+
}
2720+
26462721
#[cargo_test]
26472722
fn remove_ignored_default_features() {
26482723
Package::new("dep_simple", "0.1.0").publish();

0 commit comments

Comments
 (0)