Skip to content

Commit

Permalink
fix(fix): Migrate workspace dependencies (#14890)
Browse files Browse the repository at this point in the history
### 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
  • Loading branch information
weihanglo authored Dec 5, 2024
2 parents 519f069 + f8468ab commit 0b3ad41
Show file tree
Hide file tree
Showing 2 changed files with 138 additions and 10 deletions.
73 changes: 63 additions & 10 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,59 @@ fn check_version_control(gctx: &GlobalContext, opts: &FixOptions) -> CargoResult
}

fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
// HACK: Duplicate workspace migration logic between virtual manifests and real manifests to
// reduce multiple Migrating messages being reported for the same file to the user
if matches!(ws.root_maybe(), MaybePackage::Virtual(_)) {
// Warning: workspaces do not have an edition so this should only include changes needed by
// packages that preserve the behavior of the workspace on all editions
let highest_edition = pkgs
.iter()
.map(|p| p.manifest().edition())
.max()
.unwrap_or_default();
let prepare_for_edition = highest_edition.saturating_next();
if highest_edition == prepare_for_edition
|| (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
{
//
} else {
let mut manifest_mut = LocalManifest::try_new(ws.root_manifest())?;
let document = &mut manifest_mut.data;
let mut fixes = 0;

if Edition::Edition2024 <= prepare_for_edition {
let root = document.as_table_mut();

if let Some(workspace) = root
.get_mut("workspace")
.and_then(|t| t.as_table_like_mut())
{
// strictly speaking, the edition doesn't apply to this table but it should be safe
// enough
fixes += rename_dep_fields_2024(workspace, "dependencies");
}
}

if 0 < fixes {
// HACK: As workspace migration is a special case, only report it if something
// happened
let file = ws.root_manifest();
let file = file.strip_prefix(ws.root()).unwrap_or(file);
let file = file.display();
ws.gctx().shell().status(
"Migrating",
format!("{file} from {highest_edition} edition to {prepare_for_edition}"),
)?;

let verb = if fixes == 1 { "fix" } else { "fixes" };
let msg = format!("{file} ({fixes} {verb})");
ws.gctx().shell().status("Fixed", msg)?;

manifest_mut.write()?;
}
}
}

for pkg in pkgs {
let existing_edition = pkg.manifest().edition();
let prepare_for_edition = existing_edition.saturating_next();
Expand All @@ -268,15 +321,15 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
format!("{file} from {existing_edition} edition to {prepare_for_edition}"),
)?;

let mut manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
let document = &mut manifest_mut.data;
let mut fixes = 0;

let ws_original_toml = match ws.root_maybe() {
MaybePackage::Package(package) => package.manifest().original_toml(),
MaybePackage::Virtual(manifest) => manifest.original_toml(),
};
if Edition::Edition2024 <= prepare_for_edition {
let mut manifest_mut = LocalManifest::try_new(pkg.manifest_path())?;
let document = &mut manifest_mut.data;
let mut fixes = 0;

let root = document.as_table_mut();

if let Some(workspace) = root
Expand Down Expand Up @@ -331,14 +384,14 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
ws_original_toml,
);
}
}

if 0 < fixes {
let verb = if fixes == 1 { "fix" } else { "fixes" };
let msg = format!("{file} ({fixes} {verb})");
ws.gctx().shell().status("Fixed", msg)?;
if 0 < fixes {
let verb = if fixes == 1 { "fix" } else { "fixes" };
let msg = format!("{file} ({fixes} {verb})");
ws.gctx().shell().status("Fixed", msg)?;

manifest_mut.write()?;
}
manifest_mut.write()?;
}
}

Expand Down
75 changes: 75 additions & 0 deletions tests/testsuite/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2643,6 +2643,81 @@ a = {path = "a", default-features = false}
);
}

#[cargo_test]
fn migrate_rename_underscore_fields_in_virtual_manifest() {
let p = project()
.file(
"Cargo.toml",
r#"
[workspace]
members = ["foo"]
resolver = "2"
[workspace.dependencies]
# Before default_features
a = {path = "a", default_features = false} # After default_features value
# After default_features line
"#,
)
.file(
"foo/Cargo.toml",
r#"
[package]
name = "foo"
edition = "2021"
"#,
)
.file("foo/src/lib.rs", "")
.file(
"a/Cargo.toml",
r#"
[package]
name = "a"
version = "0.0.1"
edition = "2015"
"#,
)
.file("a/src/lib.rs", "")
.build();

p.cargo("fix --edition --allow-no-vcs")
.with_stderr_data(str![[r#"
[MIGRATING] Cargo.toml from 2021 edition to 2024
[FIXED] Cargo.toml (1 fix)
[MIGRATING] foo/Cargo.toml from 2021 edition to 2024
[CHECKING] foo v0.0.0 ([ROOT]/foo/foo)
[MIGRATING] foo/src/lib.rs from 2021 edition to 2024
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
"#]])
.run();
assert_e2e().eq(
p.read_file("Cargo.toml"),
str![[r#"
[workspace]
members = ["foo"]
resolver = "2"
[workspace.dependencies]
# Before default_features
a = {path = "a", default-features = false} # After default_features value
# After default_features line
"#]],
);
assert_e2e().eq(
p.read_file("foo/Cargo.toml"),
str![[r#"
[package]
name = "foo"
edition = "2021"
"#]],
);
}

#[cargo_test]
fn remove_ignored_default_features() {
Package::new("dep_simple", "0.1.0").publish();
Expand Down

0 comments on commit 0b3ad41

Please sign in to comment.