Skip to content

Commit

Permalink
Auto merge of #13747 - epage:deprecated, r=weihanglo
Browse files Browse the repository at this point in the history
fix(toml): Error on `[project]` in Edition 2024

### 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 fix` support which is hard coded directly into the `cargo fix` command.

This is part of
- #13629
- rust-lang/rust#123754

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
  • Loading branch information
bors committed Apr 16, 2024
2 parents b9d913e + cbd9def commit 6f06fe9
Show file tree
Hide file tree
Showing 6 changed files with 268 additions and 56 deletions.
7 changes: 6 additions & 1 deletion src/bin/cargo/commands/fix.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use crate::command_prelude::*;

use cargo::core::Workspace;
use cargo::ops;

pub fn cli() -> Command {
Expand Down Expand Up @@ -60,7 +61,6 @@ pub fn cli() -> Command {
}

pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
let ws = args.workspace(gctx)?;
// This is a legacy behavior that causes `cargo fix` to pass `--test`.
let test = matches!(
args.get_one::<String>("profile").map(String::as_str),
Expand All @@ -70,6 +70,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {

// Unlike other commands default `cargo fix` to all targets to fix as much
// code as we can.
let root_manifest = args.root_manifest(gctx)?;
let mut ws = Workspace::new(&root_manifest, gctx)?;
ws.set_honor_rust_version(args.honor_rust_version());
let mut opts = args.compile_options(gctx, mode, Some(&ws), ProfileChecking::LegacyTestOnly)?;

if !opts.filter.is_specific() {
Expand All @@ -78,7 +81,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
}

ops::fix(
gctx,
&ws,
&root_manifest,
&mut ops::FixOptions {
edition: args.flag("edition"),
idioms: args.flag("edition-idioms"),
Expand Down
4 changes: 4 additions & 0 deletions src/cargo/core/workspace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -611,6 +611,10 @@ impl<'gctx> Workspace<'gctx> {
self.honor_rust_version = honor_rust_version;
}

pub fn honor_rust_version(&self) -> Option<bool> {
self.honor_rust_version
}

pub fn resolve_honors_rust_version(&self) -> bool {
self.gctx().cli_unstable().msrv_policy && self.honor_rust_version.unwrap_or(true)
}
Expand Down
84 changes: 78 additions & 6 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,8 @@ use tracing::{debug, trace, warn};
use crate::core::compiler::RustcTargetData;
use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver, FeaturesFor};
use crate::core::resolver::{HasDevUnits, Resolve, ResolveBehavior};
use crate::core::{Edition, MaybePackage, PackageId, Workspace};
use crate::core::PackageIdSpecQuery as _;
use crate::core::{Edition, MaybePackage, Package, PackageId, Workspace};
use crate::ops::resolve::WorkspaceResolve;
use crate::ops::{self, CompileOptions};
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
Expand Down Expand Up @@ -87,11 +88,26 @@ pub struct FixOptions {
pub broken_code: bool,
}

pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
check_version_control(ws.gctx(), opts)?;
pub fn fix(
gctx: &GlobalContext,
original_ws: &Workspace<'_>,
root_manifest: &Path,
opts: &mut FixOptions,
) -> CargoResult<()> {
check_version_control(gctx, opts)?;

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
.members()
.filter(|m| specs.iter().any(|spec| spec.matches(m.package_id())))
.collect();
migrate_manifests(original_ws, &members)?;

check_resolver_change(&original_ws, opts)?;
}
let mut ws = Workspace::new(&root_manifest, gctx)?;
ws.set_honor_rust_version(original_ws.honor_rust_version());

// Spin up our lock server, which our subprocesses will use to synchronize fixes.
let lock_server = LockServer::new()?;
Expand Down Expand Up @@ -128,7 +144,7 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
server.configure(&mut wrapper);
}

let rustc = ws.gctx().load_global_rustc(Some(ws))?;
let rustc = ws.gctx().load_global_rustc(Some(&ws))?;
wrapper.arg(&rustc.path);
// This is calling rustc in cargo fix-proxy-mode, so it also need to retry.
// The argfile handling are located at `FixArgs::from_args`.
Expand All @@ -138,7 +154,7 @@ pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
// repeating build until there are no more changes to be applied
opts.compile_opts.build_config.primary_unit_rustc = Some(wrapper);

ops::compile(ws, &opts.compile_opts)?;
ops::compile(&ws, &opts.compile_opts)?;
Ok(())
}

Expand Down Expand Up @@ -215,6 +231,62 @@ fn check_version_control(gctx: &GlobalContext, opts: &FixOptions) -> CargoResult
);
}

fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
for pkg in pkgs {
let existing_edition = pkg.manifest().edition();
let prepare_for_edition = existing_edition.saturating_next();
if existing_edition == prepare_for_edition
|| (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
{
continue;
}
let file = pkg.manifest_path();
let file = file.strip_prefix(ws.root()).unwrap_or(file);
let file = file.display();
ws.gctx().shell().status(
"Migrating",
format!("{file} from {existing_edition} edition to {prepare_for_edition}"),
)?;

if Edition::Edition2024 <= prepare_for_edition {
let mut document = pkg.manifest().document().clone().into_mut();
let mut fixes = 0;

let root = document.as_table_mut();
if rename_table(root, "project", "package") {
fixes += 1;
}

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

let s = document.to_string();
let new_contents_bytes = s.as_bytes();
cargo_util::paths::write_atomic(pkg.manifest_path(), new_contents_bytes)?;
}
}
}

Ok(())
}

fn rename_table(parent: &mut dyn toml_edit::TableLike, old: &str, new: &str) -> bool {
let Some(old_key) = parent.key(old).cloned() else {
return false;
};

let project = parent.remove(old).expect("returned early");
if !parent.contains_key(new) {
parent.insert(new, project);
let mut new_key = parent.key_mut(new).expect("just inserted");
*new_key.dotted_decor_mut() = old_key.dotted_decor().clone();
*new_key.leaf_decor_mut() = old_key.leaf_decor().clone();
}
true
}

fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<()> {
let root = ws.root_maybe();
match root {
Expand Down
33 changes: 13 additions & 20 deletions src/cargo/util/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,26 +935,9 @@ fn to_real_manifest(
);
};

let original_package = match (&original_toml.package, &original_toml.project) {
(Some(_), Some(project)) => {
warnings.push(format!(
"manifest at `{}` contains both `project` and `package`, \
this could become a hard error in the future",
package_root.display()
));
project.clone()
}
(Some(package), None) => package.clone(),
(None, Some(project)) => {
warnings.push(format!(
"manifest at `{}` contains `[project]` instead of `[package]`, \
this could become a hard error in the future",
package_root.display()
));
project.clone()
}
(None, None) => bail!("no `package` section found"),
};
let original_package = original_toml
.package()
.ok_or_else(|| anyhow::format_err!("no `package` section found"))?;

let package_name = &original_package.name;
if package_name.contains(':') {
Expand Down Expand Up @@ -1044,6 +1027,16 @@ fn to_real_manifest(
)));
}

if original_toml.project.is_some() {
if Edition::Edition2024 <= edition {
anyhow::bail!(
"`[project]` is not supported as of the 2024 Edition, please use `[package]`"
);
} else {
warnings.push(format!("`[project]` is deprecated in favor of `[package]`"));
}
}

if resolved_package.metabuild.is_some() {
features.require(Feature::metabuild())?;
}
Expand Down
85 changes: 58 additions & 27 deletions tests/testsuite/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -979,6 +979,63 @@ fn rustc_workspace_wrapper_excludes_published_deps() {
.run();
}

#[cargo_test]
fn warn_manifest_with_project() {
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
edition = "2015"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] `[project]` is deprecated in favor of `[package]`
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

#[cargo_test(nightly, reason = "edition2024")]
fn error_manifest_with_project_on_2024() {
let p = project()
.file(
"Cargo.toml",
r#"
cargo-features = ["edition2024"]
[project]
name = "foo"
version = "0.0.1"
edition = "2024"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.masquerade_as_nightly_cargo(&["edition2024"])
.with_status(101)
.with_stderr(
"\
[ERROR] failed to parse manifest at `[CWD]/Cargo.toml`
Caused by:
`[project]` is not supported as of the 2024 Edition, please use `[package]`
",
)
.run();
}

#[cargo_test]
fn warn_manifest_package_and_project() {
let p = project()
Expand All @@ -1002,7 +1059,7 @@ fn warn_manifest_package_and_project() {
p.cargo("check")
.with_stderr(
"\
[WARNING] manifest at `[CWD]` contains both `project` and `package`, this could become a hard error in the future
[WARNING] `[project]` is deprecated in favor of `[package]`
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
",
Expand Down Expand Up @@ -1065,32 +1122,6 @@ fn git_manifest_package_and_project() {
.run();
}

#[cargo_test]
fn warn_manifest_with_project() {
let p = project()
.file(
"Cargo.toml",
r#"
[project]
name = "foo"
version = "0.0.1"
edition = "2015"
"#,
)
.file("src/main.rs", "fn main() {}")
.build();

p.cargo("check")
.with_stderr(
"\
[WARNING] manifest at `[CWD]` contains `[project]` instead of `[package]`, this could become a hard error in the future
[CHECKING] foo v0.0.1 ([CWD])
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
",
)
.run();
}

#[cargo_test]
fn git_manifest_with_project() {
let p = project();
Expand Down
Loading

0 comments on commit 6f06fe9

Please sign in to comment.