Skip to content

Commit 90691f2

Browse files
committed
Auto merge of #9268 - ehuss:2021-fix-edition-v2-warning, r=alexcrichton
Add report if `cargo fix --edition` changes features. This adds a small report if using `cargo fix --edition` to transition from 2018 to 2021 to determine if the new resolver will result in different features. There is a gauntlet of checks that must pass before it even tries to show the differences: * If the resolver is already specified, skip. * If in a virtual workspace, skip (no way to specify global edition). * If not migrating the root package, skip. The edition only changes the resolver in the root package. * If not migrating all targets, skip. It's uncertain if the user intends to set the package edition or not. Closes #9048
2 parents 841179e + 6c69e9d commit 90691f2

File tree

7 files changed

+266
-13
lines changed

7 files changed

+266
-13
lines changed

src/cargo/core/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ pub use self::resolver::{Resolve, ResolveVersion};
1010
pub use self::shell::{Shell, Verbosity};
1111
pub use self::source::{GitReference, Source, SourceId, SourceMap};
1212
pub use self::summary::{FeatureMap, FeatureValue, Summary};
13-
pub use self::workspace::{Members, Workspace, WorkspaceConfig, WorkspaceRootConfig};
13+
pub use self::workspace::{MaybePackage, Members, Workspace, WorkspaceConfig, WorkspaceRootConfig};
1414

1515
pub mod compiler;
1616
pub mod dependency;

src/cargo/core/resolver/features.rs

Lines changed: 78 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ use crate::core::resolver::{Resolve, ResolveBehavior};
4242
use crate::core::{FeatureValue, PackageId, PackageIdSpec, PackageSet, Workspace};
4343
use crate::util::interning::InternedString;
4444
use crate::util::CargoResult;
45-
use std::collections::{BTreeSet, HashMap, HashSet};
45+
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
4646
use std::rc::Rc;
4747

4848
/// Map of activated features.
@@ -71,7 +71,7 @@ pub struct ResolvedFeatures {
7171

7272
/// Options for how the feature resolver works.
7373
#[derive(Default)]
74-
struct FeatureOpts {
74+
pub struct FeatureOpts {
7575
/// Use the new resolver instead of the old one.
7676
new_resolver: bool,
7777
/// Build deps and proc-macros will not share share features with other dep kinds.
@@ -123,7 +123,7 @@ impl FeaturesFor {
123123
}
124124

125125
impl FeatureOpts {
126-
fn new(
126+
pub fn new(
127127
ws: &Workspace<'_>,
128128
has_dev_units: HasDevUnits,
129129
force_all_targets: ForceAllTargets,
@@ -180,6 +180,20 @@ impl FeatureOpts {
180180
}
181181
Ok(opts)
182182
}
183+
184+
/// Creates a new FeatureOpts for the given behavior.
185+
pub fn new_behavior(behavior: ResolveBehavior, has_dev_units: HasDevUnits) -> FeatureOpts {
186+
match behavior {
187+
ResolveBehavior::V1 => FeatureOpts::default(),
188+
ResolveBehavior::V2 => FeatureOpts {
189+
new_resolver: true,
190+
decouple_host_deps: true,
191+
decouple_dev_deps: has_dev_units == HasDevUnits::No,
192+
ignore_inactive_targets: true,
193+
compare: false,
194+
},
195+
}
196+
}
183197
}
184198

185199
/// Features flags requested for a package.
@@ -286,6 +300,66 @@ impl ResolvedFeatures {
286300
}
287301
}
288302
}
303+
304+
/// Compares the result against the original resolver behavior.
305+
///
306+
/// Used by `cargo fix --edition` to display any differences.
307+
pub fn compare_legacy(&self, legacy: &ResolvedFeatures) -> FeatureDifferences {
308+
let legacy_features = legacy.legacy_features.as_ref().unwrap();
309+
let features = self
310+
.activated_features
311+
.iter()
312+
.filter_map(|((pkg_id, for_host), new_features)| {
313+
let old_features = match legacy_features.get(pkg_id) {
314+
Some(feats) => feats.iter().cloned().collect(),
315+
None => BTreeSet::new(),
316+
};
317+
// The new resolver should never add features.
318+
assert_eq!(new_features.difference(&old_features).next(), None);
319+
let removed_features: BTreeSet<_> =
320+
old_features.difference(&new_features).cloned().collect();
321+
if removed_features.is_empty() {
322+
None
323+
} else {
324+
Some(((*pkg_id, *for_host), removed_features))
325+
}
326+
})
327+
.collect();
328+
let legacy_deps = legacy.legacy_dependencies.as_ref().unwrap();
329+
let optional_deps = self
330+
.activated_dependencies
331+
.iter()
332+
.filter_map(|((pkg_id, for_host), new_deps)| {
333+
let old_deps = match legacy_deps.get(pkg_id) {
334+
Some(deps) => deps.iter().cloned().collect(),
335+
None => BTreeSet::new(),
336+
};
337+
// The new resolver should never add dependencies.
338+
assert_eq!(new_deps.difference(&old_deps).next(), None);
339+
let removed_deps: BTreeSet<_> = old_deps.difference(&new_deps).cloned().collect();
340+
if removed_deps.is_empty() {
341+
None
342+
} else {
343+
Some(((*pkg_id, *for_host), removed_deps))
344+
}
345+
})
346+
.collect();
347+
FeatureDifferences {
348+
features,
349+
optional_deps,
350+
}
351+
}
352+
}
353+
354+
/// Map of differences.
355+
///
356+
/// Key is `(pkg_id, for_host)`. Value is a set of features or dependencies removed.
357+
pub type DiffMap = BTreeMap<(PackageId, bool), BTreeSet<InternedString>>;
358+
359+
/// Differences between resolvers.
360+
pub struct FeatureDifferences {
361+
pub features: DiffMap,
362+
pub optional_deps: DiffMap,
289363
}
290364

291365
pub struct FeatureResolver<'a, 'cfg> {
@@ -334,13 +408,11 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
334408
requested_features: &RequestedFeatures,
335409
specs: &[PackageIdSpec],
336410
requested_targets: &[CompileKind],
337-
has_dev_units: HasDevUnits,
338-
force_all_targets: ForceAllTargets,
411+
opts: FeatureOpts,
339412
) -> CargoResult<ResolvedFeatures> {
340413
use crate::util::profile;
341414
let _p = profile::start("resolve features");
342415

343-
let opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
344416
if !opts.new_resolver {
345417
// Legacy mode.
346418
return Ok(ResolvedFeatures {

src/cargo/core/workspace.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ struct Packages<'cfg> {
103103
}
104104

105105
#[derive(Debug)]
106-
enum MaybePackage {
106+
pub enum MaybePackage {
107107
Package(Package),
108108
Virtual(VirtualManifest),
109109
}
@@ -342,7 +342,7 @@ impl<'cfg> Workspace<'cfg> {
342342
}
343343

344344
/// Returns the root Package or VirtualManifest.
345-
fn root_maybe(&self) -> &MaybePackage {
345+
pub fn root_maybe(&self) -> &MaybePackage {
346346
self.packages.get(self.root_manifest())
347347
}
348348

src/cargo/ops/cargo_compile.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -803,6 +803,15 @@ impl CompileFilter {
803803
}
804804
}
805805

806+
pub fn is_all_targets(&self) -> bool {
807+
match *self {
808+
CompileFilter::Only {
809+
all_targets: true, ..
810+
} => true,
811+
_ => false,
812+
}
813+
}
814+
806815
pub(crate) fn contains_glob_patterns(&self) -> bool {
807816
match self {
808817
CompileFilter::Default { .. } => false,

src/cargo/ops/fix.rs

Lines changed: 110 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,16 @@ use log::{debug, trace, warn};
5050
use rustfix::diagnostics::Diagnostic;
5151
use rustfix::{self, CodeFix};
5252

53-
use crate::core::{Edition, Workspace};
53+
use crate::core::compiler::RustcTargetData;
54+
use crate::core::resolver::features::{FeatureOpts, FeatureResolver, RequestedFeatures};
55+
use crate::core::resolver::{HasDevUnits, ResolveBehavior, ResolveOpts};
56+
use crate::core::{Edition, MaybePackage, Workspace};
5457
use crate::ops::{self, CompileOptions};
5558
use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
5659
use crate::util::errors::CargoResult;
5760
use crate::util::{self, paths, Config, ProcessBuilder};
5861
use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
62+
use crate::{drop_eprint, drop_eprintln};
5963

6064
const FIX_ENV: &str = "__CARGO_FIX_PLZ";
6165
const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
@@ -74,6 +78,9 @@ pub struct FixOptions {
7478

7579
pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
7680
check_version_control(ws.config(), opts)?;
81+
if opts.edition {
82+
check_resolver_change(ws, opts)?;
83+
}
7784

7885
// Spin up our lock server, which our subprocesses will use to synchronize fixes.
7986
let lock_server = LockServer::new()?;
@@ -193,6 +200,108 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()>
193200
);
194201
}
195202

203+
fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<()> {
204+
let root = ws.root_maybe();
205+
match root {
206+
MaybePackage::Package(root_pkg) => {
207+
if root_pkg.manifest().resolve_behavior().is_some() {
208+
// If explicitly specified by the user, no need to check.
209+
return Ok(());
210+
}
211+
// Only trigger if updating the root package from 2018.
212+
let pkgs = opts.compile_opts.spec.get_packages(ws)?;
213+
if !pkgs.iter().any(|&pkg| pkg == root_pkg) {
214+
// The root is not being migrated.
215+
return Ok(());
216+
}
217+
if root_pkg.manifest().edition() != Edition::Edition2018 {
218+
// V1 to V2 only happens on 2018 to 2021.
219+
return Ok(());
220+
}
221+
}
222+
MaybePackage::Virtual(_vm) => {
223+
// Virtual workspaces don't have a global edition to set (yet).
224+
return Ok(());
225+
}
226+
}
227+
// 2018 without `resolver` set must be V1
228+
assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1);
229+
let specs = opts.compile_opts.spec.to_package_id_specs(ws)?;
230+
let resolve_opts = ResolveOpts::new(
231+
/*dev_deps*/ true,
232+
RequestedFeatures::from_command_line(
233+
&opts.compile_opts.features,
234+
opts.compile_opts.all_features,
235+
!opts.compile_opts.no_default_features,
236+
),
237+
);
238+
let target_data = RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
239+
// HasDevUnits::No because that may uncover more differences.
240+
// This is not the same as what `cargo fix` is doing, since it is doing
241+
// `--all-targets` which includes dev dependencies.
242+
let ws_resolve = ops::resolve_ws_with_opts(
243+
ws,
244+
&target_data,
245+
&opts.compile_opts.build_config.requested_kinds,
246+
&resolve_opts,
247+
&specs,
248+
HasDevUnits::No,
249+
crate::core::resolver::features::ForceAllTargets::No,
250+
)?;
251+
252+
let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, HasDevUnits::No);
253+
let v2_features = FeatureResolver::resolve(
254+
ws,
255+
&target_data,
256+
&ws_resolve.targeted_resolve,
257+
&ws_resolve.pkg_set,
258+
&resolve_opts.features,
259+
&specs,
260+
&opts.compile_opts.build_config.requested_kinds,
261+
feature_opts,
262+
)?;
263+
264+
let differences = v2_features.compare_legacy(&ws_resolve.resolved_features);
265+
if differences.features.is_empty() && differences.optional_deps.is_empty() {
266+
// Nothing is different, nothing to report.
267+
return Ok(());
268+
}
269+
let config = ws.config();
270+
config.shell().note(
271+
"Switching to Edition 2021 will enable the use of the version 2 feature resolver in Cargo.",
272+
)?;
273+
drop_eprintln!(
274+
config,
275+
"This may cause dependencies to resolve with a different set of features."
276+
);
277+
drop_eprintln!(
278+
config,
279+
"More information about the resolver changes may be found \
280+
at https://doc.rust-lang.org/cargo/reference/features.html#feature-resolver-version-2"
281+
);
282+
drop_eprintln!(
283+
config,
284+
"The following differences were detected with the current configuration:\n"
285+
);
286+
let report = |changes: crate::core::resolver::features::DiffMap, what| {
287+
for ((pkg_id, for_host), removed) in changes {
288+
drop_eprint!(config, " {}", pkg_id);
289+
if for_host {
290+
drop_eprint!(config, " (as build dependency)");
291+
}
292+
if !removed.is_empty() {
293+
let joined: Vec<_> = removed.iter().map(|s| s.as_str()).collect();
294+
drop_eprint!(config, " removed {} `{}`", what, joined.join(","));
295+
}
296+
drop_eprint!(config, "\n");
297+
}
298+
};
299+
report(differences.features, "features");
300+
report(differences.optional_deps, "optional dependency");
301+
drop_eprint!(config, "\n");
302+
Ok(())
303+
}
304+
196305
/// Entry point for `cargo` running as a proxy for `rustc`.
197306
///
198307
/// This is called every time `cargo` is run to check if it is in proxy mode.

src/cargo/ops/resolve.rs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
1313
use crate::core::compiler::{CompileKind, RustcTargetData};
1414
use crate::core::registry::PackageRegistry;
15-
use crate::core::resolver::features::{FeatureResolver, ForceAllTargets, ResolvedFeatures};
15+
use crate::core::resolver::features::{
16+
FeatureOpts, FeatureResolver, ForceAllTargets, ResolvedFeatures,
17+
};
1618
use crate::core::resolver::{self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion};
1719
use crate::core::summary::Summary;
1820
use crate::core::Feature;
@@ -143,6 +145,7 @@ pub fn resolve_ws_with_opts<'cfg>(
143145
force_all_targets,
144146
)?;
145147

148+
let feature_opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
146149
let resolved_features = FeatureResolver::resolve(
147150
ws,
148151
target_data,
@@ -151,8 +154,7 @@ pub fn resolve_ws_with_opts<'cfg>(
151154
&opts.features,
152155
specs,
153156
requested_targets,
154-
has_dev_units,
155-
force_all_targets,
157+
feature_opts,
156158
)?;
157159

158160
Ok(WorkspaceResolve {

0 commit comments

Comments
 (0)