Skip to content

Commit 6ffc01f

Browse files
authored
Add -Zfix-edition (#15596)
This adds a new `cargo fix -Zfix-edition` permanently unstable flag to assist with edition migration testing, particularly with crater. The commits and given documentation should explain how it works, but there will be forthcoming edition documentation (on the forge) once all the pieces with crater come together.
2 parents 8d17219 + 2162967 commit 6ffc01f

File tree

10 files changed

+395
-64
lines changed

10 files changed

+395
-64
lines changed

src/bin/cargo/commands/fix.rs

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -91,20 +91,24 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
9191

9292
let allow_dirty = args.flag("allow-dirty");
9393

94-
ops::fix(
95-
gctx,
96-
&ws,
97-
&root_manifest,
98-
&mut ops::FixOptions {
99-
edition: args.flag("edition"),
100-
idioms: args.flag("edition-idioms"),
101-
compile_opts: opts,
102-
allow_dirty,
103-
allow_staged: allow_dirty || args.flag("allow-staged"),
104-
allow_no_vcs: args.flag("allow-no-vcs"),
105-
broken_code: args.flag("broken-code"),
106-
requested_lockfile_path: lockfile_path,
107-
},
108-
)?;
94+
let mut opts = ops::FixOptions {
95+
edition: args
96+
.flag("edition")
97+
.then_some(ops::EditionFixMode::NextRelative),
98+
idioms: args.flag("edition-idioms"),
99+
compile_opts: opts,
100+
allow_dirty,
101+
allow_staged: allow_dirty || args.flag("allow-staged"),
102+
allow_no_vcs: args.flag("allow-no-vcs"),
103+
broken_code: args.flag("broken-code"),
104+
requested_lockfile_path: lockfile_path,
105+
};
106+
107+
if let Some(fe) = &gctx.cli_unstable().fix_edition {
108+
ops::fix_edition(gctx, &ws, &mut opts, fe)?;
109+
} else {
110+
ops::fix(gctx, &ws, &mut opts)?;
111+
}
112+
109113
Ok(())
110114
}

src/cargo/core/features.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,45 @@ impl FromStr for Edition {
362362
}
363363
}
364364

365+
/// The value for `-Zfix-edition`.
366+
#[derive(Debug, Deserialize)]
367+
pub enum FixEdition {
368+
/// `-Zfix-edition=start=$INITIAL`
369+
///
370+
/// This mode for `cargo fix` will just run `cargo check` if the current
371+
/// edition is equal to this edition. If it is a different edition, then
372+
/// it just exits with success. This is used for crater integration which
373+
/// needs to set a baseline for the "before" toolchain.
374+
Start(Edition),
375+
/// `-Zfix-edition=end=$INITIAL,$NEXT`
376+
///
377+
/// This mode for `cargo fix` will migrate to the `next` edition if the
378+
/// current edition is `initial`. After migration, it will update
379+
/// `Cargo.toml` and verify that that it works on the new edition. If the
380+
/// current edition is not `initial`, then it immediately exits with
381+
/// success since we just want to ignore those packages.
382+
End { initial: Edition, next: Edition },
383+
}
384+
385+
impl FromStr for FixEdition {
386+
type Err = anyhow::Error;
387+
fn from_str(s: &str) -> Result<Self, <Self as FromStr>::Err> {
388+
if let Some(start) = s.strip_prefix("start=") {
389+
Ok(FixEdition::Start(start.parse()?))
390+
} else if let Some(end) = s.strip_prefix("end=") {
391+
let (initial, next) = end
392+
.split_once(',')
393+
.ok_or_else(|| anyhow::format_err!("expected `initial,next`"))?;
394+
Ok(FixEdition::End {
395+
initial: initial.parse()?,
396+
next: next.parse()?,
397+
})
398+
} else {
399+
bail!("invalid `-Zfix-edition, expected start= or end=, got `{s}`");
400+
}
401+
}
402+
}
403+
365404
#[derive(Debug, PartialEq)]
366405
enum Status {
367406
Stable,
@@ -791,6 +830,7 @@ unstable_cli_options!(
791830
dual_proc_macros: bool = ("Build proc-macros for both the host and the target"),
792831
feature_unification: bool = ("Enable new feature unification modes in workspaces"),
793832
features: Option<Vec<String>>,
833+
fix_edition: Option<FixEdition> = ("Permanently unstable edition migration helper"),
794834
gc: bool = ("Track cache usage and \"garbage collect\" unused files"),
795835
#[serde(deserialize_with = "deserialize_git_features")]
796836
git: Option<GitFeatures> = ("Enable support for shallow git fetch operations"),
@@ -1298,6 +1338,12 @@ impl CliUnstable {
12981338
"doctest-xcompile" => stabilized_warn(k, "1.89", STABILIZED_DOCTEST_XCOMPILE),
12991339
"dual-proc-macros" => self.dual_proc_macros = parse_empty(k, v)?,
13001340
"feature-unification" => self.feature_unification = parse_empty(k, v)?,
1341+
"fix-edition" => {
1342+
let fe = v
1343+
.ok_or_else(|| anyhow::anyhow!("-Zfix-edition expected a value"))?
1344+
.parse()?;
1345+
self.fix_edition = Some(fe);
1346+
}
13011347
"gc" => self.gc = parse_empty(k, v)?,
13021348
"git" => {
13031349
self.git =

src/cargo/core/workspace.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,18 @@ impl<'gctx> Workspace<'gctx> {
301301
Ok(ws)
302302
}
303303

304+
/// Reloads the workspace.
305+
///
306+
/// This is useful if the workspace has been updated, such as with `cargo
307+
/// fix` modifying the `Cargo.toml` file.
308+
pub fn reload(&self, gctx: &'gctx GlobalContext) -> CargoResult<Workspace<'gctx>> {
309+
let mut ws = Workspace::new(self.root_manifest(), gctx)?;
310+
ws.set_resolve_honors_rust_version(Some(self.resolve_honors_rust_version));
311+
ws.set_resolve_feature_unification(self.resolve_feature_unification);
312+
ws.set_requested_lockfile_path(self.requested_lockfile_path.clone());
313+
Ok(ws)
314+
}
315+
304316
fn set_resolve_behavior(&mut self) -> CargoResult<()> {
305317
// - If resolver is specified in the workspace definition, use that.
306318
// - If the root package specifies the resolver, use that.

src/cargo/ops/fix/fix_edition.rs

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
//! Support for the permanently unstable `-Zfix-edition` flag.
2+
3+
use super::{EditionFixMode, FixOptions};
4+
use crate::core::features::{Edition, FixEdition};
5+
use crate::core::{Package, Workspace};
6+
use crate::ops;
7+
use crate::util::toml_mut::manifest::LocalManifest;
8+
use crate::{CargoResult, GlobalContext};
9+
use toml_edit::{Formatted, Item, Value};
10+
11+
/// Performs the actions for the `-Zfix-edition` flag.
12+
pub fn fix_edition(
13+
gctx: &GlobalContext,
14+
original_ws: &Workspace<'_>,
15+
opts: &mut FixOptions,
16+
fix_edition: &FixEdition,
17+
) -> CargoResult<()> {
18+
let packages = opts.compile_opts.spec.get_packages(original_ws)?;
19+
let skip_if_not_edition = |edition| -> CargoResult<bool> {
20+
if !packages.iter().all(|p| p.manifest().edition() == edition) {
21+
gctx.shell().status(
22+
"Skipping",
23+
&format!("not all packages are at edition {edition}"),
24+
)?;
25+
Ok(true)
26+
} else {
27+
Ok(false)
28+
}
29+
};
30+
31+
match fix_edition {
32+
FixEdition::Start(edition) => {
33+
// The start point just runs `cargo check` if the edition is the
34+
// starting edition. This is so that crater can set a baseline of
35+
// whether or not the package builds at all. For other editions,
36+
// we skip entirely since they are not of interest since we can't
37+
// migrate them.
38+
if skip_if_not_edition(*edition)? {
39+
return Ok(());
40+
}
41+
ops::compile(&original_ws, &opts.compile_opts)?;
42+
}
43+
FixEdition::End { initial, next } => {
44+
// Skip packages that are not the starting edition, since we can
45+
// only migrate from one edition to the next.
46+
if skip_if_not_edition(*initial)? {
47+
return Ok(());
48+
}
49+
// Do the edition fix.
50+
opts.edition = Some(EditionFixMode::OverrideSpecific(*next));
51+
opts.allow_dirty = true;
52+
opts.allow_no_vcs = true;
53+
opts.allow_staged = true;
54+
ops::fix(gctx, original_ws, opts)?;
55+
// Do `cargo check` with the new edition so that we can verify
56+
// that it also works on the next edition.
57+
replace_edition(&packages, *next)?;
58+
gctx.shell()
59+
.status("Updated", &format!("edition to {next}"))?;
60+
let ws = original_ws.reload(gctx)?;
61+
// Unset these since we just want to do a normal `cargo check`.
62+
*opts
63+
.compile_opts
64+
.build_config
65+
.rustfix_diagnostic_server
66+
.borrow_mut() = None;
67+
opts.compile_opts.build_config.primary_unit_rustc = None;
68+
69+
ops::compile(&ws, &opts.compile_opts)?;
70+
}
71+
}
72+
Ok(())
73+
}
74+
75+
/// Modifies the `edition` value of the given packages to the new edition.
76+
fn replace_edition(packages: &[&Package], to_edition: Edition) -> CargoResult<()> {
77+
for package in packages {
78+
let mut manifest_mut = LocalManifest::try_new(package.manifest_path())?;
79+
let document = &mut manifest_mut.data;
80+
let root = document.as_table_mut();
81+
// Update edition to the new value.
82+
if let Some(package) = root.get_mut("package").and_then(|t| t.as_table_like_mut()) {
83+
package.insert(
84+
"edition",
85+
Item::Value(Value::String(Formatted::new(to_edition.to_string()))),
86+
);
87+
}
88+
// If the edition is unstable, add it to cargo-features.
89+
if !to_edition.is_stable() {
90+
let feature = "unstable-editions";
91+
92+
if let Some(features) = root
93+
.entry("cargo-features")
94+
.or_insert_with(|| Item::Value(Value::Array(toml_edit::Array::new())))
95+
.as_array_mut()
96+
{
97+
if !features
98+
.iter()
99+
.any(|f| f.as_str().map_or(false, |f| f == feature))
100+
{
101+
features.push(feature);
102+
}
103+
}
104+
}
105+
manifest_mut.write()?;
106+
}
107+
Ok(())
108+
}

src/cargo/ops/fix.rs renamed to src/cargo/ops/fix/mod.rs

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,7 @@ use rustfix::CodeFix;
5151
use semver::Version;
5252
use tracing::{debug, trace, warn};
5353

54+
pub use self::fix_edition::fix_edition;
5455
use crate::core::compiler::CompileKind;
5556
use crate::core::compiler::RustcTargetData;
5657
use crate::core::resolver::features::{DiffMap, FeatureOpts, FeatureResolver, FeaturesFor};
@@ -66,6 +67,8 @@ use crate::util::GlobalContext;
6667
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
6768
use crate::{drop_eprint, drop_eprintln};
6869

70+
mod fix_edition;
71+
6972
/// **Internal only.**
7073
/// Indicates Cargo is in fix-proxy-mode if presents.
7174
/// The value of it is the socket address of the [`LockServer`] being used.
@@ -90,7 +93,7 @@ const IDIOMS_ENV_INTERNAL: &str = "__CARGO_FIX_IDIOMS";
9093
const SYSROOT_INTERNAL: &str = "__CARGO_FIX_RUST_SRC";
9194

9295
pub struct FixOptions {
93-
pub edition: bool,
96+
pub edition: Option<EditionFixMode>,
9497
pub idioms: bool,
9598
pub compile_opts: CompileOptions,
9699
pub allow_dirty: bool,
@@ -100,30 +103,66 @@ pub struct FixOptions {
100103
pub requested_lockfile_path: Option<PathBuf>,
101104
}
102105

106+
/// The behavior of `--edition` migration.
107+
#[derive(Clone, Copy)]
108+
pub enum EditionFixMode {
109+
/// Migrates the package from the current edition to the next.
110+
///
111+
/// This is the normal (stable) behavior of `--edition`.
112+
NextRelative,
113+
/// Migrates to a specific edition.
114+
///
115+
/// This is used by `-Zfix-edition` to force a specific edition like
116+
/// `future`, which does not have a relative value.
117+
OverrideSpecific(Edition),
118+
}
119+
120+
impl EditionFixMode {
121+
/// Returns the edition to use for the given current edition.
122+
pub fn next_edition(&self, current_edition: Edition) -> Edition {
123+
match self {
124+
EditionFixMode::NextRelative => current_edition.saturating_next(),
125+
EditionFixMode::OverrideSpecific(edition) => *edition,
126+
}
127+
}
128+
129+
/// Serializes to a string.
130+
fn to_string(&self) -> String {
131+
match self {
132+
EditionFixMode::NextRelative => "1".to_string(),
133+
EditionFixMode::OverrideSpecific(edition) => edition.to_string(),
134+
}
135+
}
136+
137+
/// Deserializes from the given string.
138+
fn from_str(s: &str) -> EditionFixMode {
139+
match s {
140+
"1" => EditionFixMode::NextRelative,
141+
edition => EditionFixMode::OverrideSpecific(edition.parse().unwrap()),
142+
}
143+
}
144+
}
145+
103146
pub fn fix(
104147
gctx: &GlobalContext,
105148
original_ws: &Workspace<'_>,
106-
root_manifest: &Path,
107149
opts: &mut FixOptions,
108150
) -> CargoResult<()> {
109151
check_version_control(gctx, opts)?;
110152

111153
let mut target_data =
112154
RustcTargetData::new(original_ws, &opts.compile_opts.build_config.requested_kinds)?;
113-
if opts.edition {
155+
if let Some(edition_mode) = opts.edition {
114156
let specs = opts.compile_opts.spec.to_package_id_specs(&original_ws)?;
115157
let members: Vec<&Package> = original_ws
116158
.members()
117159
.filter(|m| specs.iter().any(|spec| spec.matches(m.package_id())))
118160
.collect();
119-
migrate_manifests(original_ws, &members)?;
161+
migrate_manifests(original_ws, &members, edition_mode)?;
120162

121163
check_resolver_change(&original_ws, &mut target_data, opts)?;
122164
}
123-
let mut ws = Workspace::new(&root_manifest, gctx)?;
124-
ws.set_resolve_honors_rust_version(Some(original_ws.resolve_honors_rust_version()));
125-
ws.set_resolve_feature_unification(original_ws.resolve_feature_unification());
126-
ws.set_requested_lockfile_path(opts.requested_lockfile_path.clone());
165+
let ws = original_ws.reload(gctx)?;
127166

128167
// Spin up our lock server, which our subprocesses will use to synchronize fixes.
129168
let lock_server = LockServer::new()?;
@@ -137,8 +176,8 @@ pub fn fix(
137176
wrapper.env(BROKEN_CODE_ENV_INTERNAL, "1");
138177
}
139178

140-
if opts.edition {
141-
wrapper.env(EDITION_ENV_INTERNAL, "1");
179+
if let Some(mode) = &opts.edition {
180+
wrapper.env(EDITION_ENV_INTERNAL, mode.to_string());
142181
}
143182
if opts.idioms {
144183
wrapper.env(IDIOMS_ENV_INTERNAL, "1");
@@ -252,7 +291,11 @@ fn check_version_control(gctx: &GlobalContext, opts: &FixOptions) -> CargoResult
252291
);
253292
}
254293

255-
fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
294+
fn migrate_manifests(
295+
ws: &Workspace<'_>,
296+
pkgs: &[&Package],
297+
edition_mode: EditionFixMode,
298+
) -> CargoResult<()> {
256299
// HACK: Duplicate workspace migration logic between virtual manifests and real manifests to
257300
// reduce multiple Migrating messages being reported for the same file to the user
258301
if matches!(ws.root_maybe(), MaybePackage::Virtual(_)) {
@@ -263,7 +306,7 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
263306
.map(|p| p.manifest().edition())
264307
.max()
265308
.unwrap_or_default();
266-
let prepare_for_edition = highest_edition.saturating_next();
309+
let prepare_for_edition = edition_mode.next_edition(highest_edition);
267310
if highest_edition == prepare_for_edition
268311
|| (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
269312
{
@@ -308,7 +351,7 @@ fn migrate_manifests(ws: &Workspace<'_>, pkgs: &[&Package]) -> CargoResult<()> {
308351

309352
for pkg in pkgs {
310353
let existing_edition = pkg.manifest().edition();
311-
let prepare_for_edition = existing_edition.saturating_next();
354+
let prepare_for_edition = edition_mode.next_edition(existing_edition);
312355
if existing_edition == prepare_for_edition
313356
|| (!prepare_for_edition.is_stable() && !ws.gctx().nightly_features_allowed)
314357
{
@@ -1195,10 +1238,10 @@ impl FixArgs {
11951238
// ALLOWED: For the internal mechanism of `cargo fix` only.
11961239
// Shouldn't be set directly by anyone.
11971240
#[allow(clippy::disallowed_methods)]
1198-
let prepare_for_edition = env::var(EDITION_ENV_INTERNAL).ok().map(|_| {
1199-
enabled_edition
1200-
.unwrap_or(Edition::Edition2015)
1201-
.saturating_next()
1241+
let prepare_for_edition = env::var(EDITION_ENV_INTERNAL).ok().map(|v| {
1242+
let enabled_edition = enabled_edition.unwrap_or(Edition::Edition2015);
1243+
let mode = EditionFixMode::from_str(&v);
1244+
mode.next_edition(enabled_edition)
12021245
});
12031246

12041247
// ALLOWED: For the internal mechanism of `cargo fix` only.

0 commit comments

Comments
 (0)