Add report if cargo fix --edition changes features.

ehuss · ehuss · commit 501499c5f917 · 2021-03-16T11:39:21.000-07:00
diff --git a/src/cargo/core/mod.rs b/src/cargo/core/mod.rs
@@ -10,7 +10,7 @@ pub use self::resolver::{Resolve, ResolveVersion};
 pub use self::shell::{Shell, Verbosity};
 pub use self::source::{GitReference, Source, SourceId, SourceMap};
 pub use self::summary::{FeatureMap, FeatureValue, Summary};
-pub use self::workspace::{Members, Workspace, WorkspaceConfig, WorkspaceRootConfig};
+pub use self::workspace::{MaybePackage, Members, Workspace, WorkspaceConfig, WorkspaceRootConfig};
 
 pub mod compiler;
 pub mod dependency;
diff --git a/src/cargo/core/resolver/features.rs b/src/cargo/core/resolver/features.rs
@@ -42,7 +42,7 @@ use crate::core::resolver::{Resolve, ResolveBehavior};
 use crate::core::{FeatureValue, PackageId, PackageIdSpec, PackageSet, Workspace};
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
-use std::collections::{BTreeSet, HashMap, HashSet};
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::rc::Rc;
 
 /// Map of activated features.
@@ -71,7 +71,7 @@ pub struct ResolvedFeatures {
 
 /// Options for how the feature resolver works.
 #[derive(Default)]
-struct FeatureOpts {
+pub struct FeatureOpts {
     /// Use the new resolver instead of the old one.
     new_resolver: bool,
     /// Build deps and proc-macros will not share share features with other dep kinds.
@@ -123,7 +123,7 @@ impl FeaturesFor {
 }
 
 impl FeatureOpts {
-    fn new(
+    pub fn new(
         ws: &Workspace<'_>,
         has_dev_units: HasDevUnits,
         force_all_targets: ForceAllTargets,
@@ -180,6 +180,20 @@ impl FeatureOpts {
         }
         Ok(opts)
     }
+
+    /// Creates a new FeatureOpts for the given behavior.
+    pub fn new_behavior(behavior: ResolveBehavior, has_dev_units: HasDevUnits) -> FeatureOpts {
+        match behavior {
+            ResolveBehavior::V1 => FeatureOpts::default(),
+            ResolveBehavior::V2 => FeatureOpts {
+                new_resolver: true,
+                decouple_host_deps: true,
+                decouple_dev_deps: has_dev_units == HasDevUnits::No,
+                ignore_inactive_targets: true,
+                compare: false,
+            },
+        }
+    }
 }
 
 /// Features flags requested for a package.
@@ -286,6 +300,66 @@ impl ResolvedFeatures {
             }
         }
     }
+
+    /// Compares the result against the original resolver behavior.
+    ///
+    /// Used by `cargo fix --edition` to display any differences.
+    pub fn compare_legacy(&self, legacy: &ResolvedFeatures) -> FeatureDifferences {
+        let legacy_features = legacy.legacy_features.as_ref().unwrap();
+        let features = self
+            .activated_features
+            .iter()
+            .filter_map(|((pkg_id, for_host), new_features)| {
+                let old_features = match legacy_features.get(pkg_id) {
+                    Some(feats) => feats.iter().cloned().collect(),
+                    None => BTreeSet::new(),
+                };
+                // The new resolver should never add features.
+                assert_eq!(new_features.difference(&old_features).next(), None);
+                let removed_features: BTreeSet<_> =
+                    old_features.difference(&new_features).cloned().collect();
+                if removed_features.is_empty() {
+                    None
+                } else {
+                    Some(((*pkg_id, *for_host), removed_features))
+                }
+            })
+            .collect();
+        let legacy_deps = legacy.legacy_dependencies.as_ref().unwrap();
+        let optional_deps = self
+            .activated_dependencies
+            .iter()
+            .filter_map(|((pkg_id, for_host), new_deps)| {
+                let old_deps = match legacy_deps.get(pkg_id) {
+                    Some(deps) => deps.iter().cloned().collect(),
+                    None => BTreeSet::new(),
+                };
+                // The new resolver should never add dependencies.
+                assert_eq!(new_deps.difference(&old_deps).next(), None);
+                let removed_deps: BTreeSet<_> = old_deps.difference(&new_deps).cloned().collect();
+                if removed_deps.is_empty() {
+                    None
+                } else {
+                    Some(((*pkg_id, *for_host), removed_deps))
+                }
+            })
+            .collect();
+        FeatureDifferences {
+            features,
+            optional_deps,
+        }
+    }
+}
+
+/// Map of differences.
+///
+/// Key is `(pkg_id, for_host)`. Value is a set of features or dependencies removed.
+pub type DiffMap = BTreeMap<(PackageId, bool), BTreeSet<InternedString>>;
+
+/// Differences between resolvers.
+pub struct FeatureDifferences {
+    pub features: DiffMap,
+    pub optional_deps: DiffMap,
 }
 
 pub struct FeatureResolver<'a, 'cfg> {
@@ -334,13 +408,11 @@ impl<'a, 'cfg> FeatureResolver<'a, 'cfg> {
         requested_features: &RequestedFeatures,
         specs: &[PackageIdSpec],
         requested_targets: &[CompileKind],
-        has_dev_units: HasDevUnits,
-        force_all_targets: ForceAllTargets,
+        opts: FeatureOpts,
     ) -> CargoResult<ResolvedFeatures> {
         use crate::util::profile;
         let _p = profile::start("resolve features");
 
-        let opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
         if !opts.new_resolver {
             // Legacy mode.
             return Ok(ResolvedFeatures {
diff --git a/src/cargo/core/workspace.rs b/src/cargo/core/workspace.rs
@@ -103,7 +103,7 @@ struct Packages<'cfg> {
 }
 
 #[derive(Debug)]
-enum MaybePackage {
+pub enum MaybePackage {
     Package(Package),
     Virtual(VirtualManifest),
 }
@@ -342,7 +342,7 @@ impl<'cfg> Workspace<'cfg> {
     }
 
     /// Returns the root Package or VirtualManifest.
-    fn root_maybe(&self) -> &MaybePackage {
+    pub fn root_maybe(&self) -> &MaybePackage {
         self.packages.get(self.root_manifest())
     }
 
diff --git a/src/cargo/ops/cargo_compile.rs b/src/cargo/ops/cargo_compile.rs
@@ -803,6 +803,15 @@ impl CompileFilter {
         }
     }
 
+    pub fn is_all_targets(&self) -> bool {
+        match *self {
+            CompileFilter::Only {
+                all_targets: true, ..
+            } => true,
+            _ => false,
+        }
+    }
+
     pub(crate) fn contains_glob_patterns(&self) -> bool {
         match self {
             CompileFilter::Default { .. } => false,
diff --git a/src/cargo/ops/fix.rs b/src/cargo/ops/fix.rs
@@ -50,12 +50,16 @@ use log::{debug, trace, warn};
 use rustfix::diagnostics::Diagnostic;
 use rustfix::{self, CodeFix};
 
-use crate::core::{Edition, Workspace};
+use crate::core::compiler::RustcTargetData;
+use crate::core::resolver::features::{FeatureOpts, FeatureResolver, RequestedFeatures};
+use crate::core::resolver::{HasDevUnits, ResolveBehavior, ResolveOpts};
+use crate::core::{Edition, MaybePackage, Workspace};
 use crate::ops::{self, CompileOptions};
 use crate::util::diagnostic_server::{Message, RustfixDiagnosticServer};
 use crate::util::errors::CargoResult;
 use crate::util::{self, paths, Config, ProcessBuilder};
 use crate::util::{existing_vcs_repo, LockServer, LockServerClient};
+use crate::{drop_eprint, drop_eprintln};
 
 const FIX_ENV: &str = "__CARGO_FIX_PLZ";
 const BROKEN_CODE_ENV: &str = "__CARGO_FIX_BROKEN_CODE";
@@ -74,6 +78,9 @@ pub struct FixOptions {
 
 pub fn fix(ws: &Workspace<'_>, opts: &mut FixOptions) -> CargoResult<()> {
     check_version_control(ws.config(), opts)?;
+    if opts.edition {
+        check_resolver_change(ws, opts)?;
+    }
 
     // Spin up our lock server, which our subprocesses will use to synchronize fixes.
     let lock_server = LockServer::new()?;
@@ -193,6 +200,114 @@ fn check_version_control(config: &Config, opts: &FixOptions) -> CargoResult<()>
     );
 }
 
+fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<()> {
+    let root = ws.root_maybe();
+    match root {
+        MaybePackage::Package(root_pkg) => {
+            if root_pkg.manifest().resolve_behavior().is_some() {
+                // If explicitly specified by the user, no need to check.
+                return Ok(());
+            }
+            // Only trigger if updating the root package from 2018.
+            let pkgs = opts.compile_opts.spec.get_packages(ws)?;
+            if !pkgs.iter().any(|&pkg| pkg == root_pkg) {
+                // The root is not being migrated.
+                return Ok(());
+            }
+            if root_pkg.manifest().edition() != Edition::Edition2018 {
+                // V1 to V2 only happens on 2018 to 2021.
+                return Ok(());
+            }
+        }
+        MaybePackage::Virtual(_vm) => {
+            // Virtual workspaces don't have a global edition to set (yet).
+            return Ok(());
+        }
+    }
+    if !opts.compile_opts.filter.is_all_targets() {
+        // When migrating specific targets, we can't know if the user intends
+        // to set the global edition at this time or not. Conservatively
+        // assume the user will figure things out.
+        return Ok(());
+    }
+    // 2018 without `resolver` set must be V1
+    assert_eq!(ws.resolve_behavior(), ResolveBehavior::V1);
+    let specs = opts.compile_opts.spec.to_package_id_specs(ws)?;
+    let resolve_opts = ResolveOpts::new(
+        /*dev_deps*/ true,
+        RequestedFeatures::from_command_line(
+            &opts.compile_opts.features,
+            opts.compile_opts.all_features,
+            !opts.compile_opts.no_default_features,
+        ),
+    );
+    let target_data = RustcTargetData::new(ws, &opts.compile_opts.build_config.requested_kinds)?;
+    // HasDevUnits::No because that may uncover more differences.
+    // This is not the same as what `cargo fix` is doing, since it is doing
+    // `--all-targets` which includes dev dependencies.
+    let ws_resolve = ops::resolve_ws_with_opts(
+        ws,
+        &target_data,
+        &opts.compile_opts.build_config.requested_kinds,
+        &resolve_opts,
+        &specs,
+        HasDevUnits::No,
+        crate::core::resolver::features::ForceAllTargets::No,
+    )?;
+
+    let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, HasDevUnits::No);
+    let v2_features = FeatureResolver::resolve(
+        ws,
+        &target_data,
+        &ws_resolve.targeted_resolve,
+        &ws_resolve.pkg_set,
+        &resolve_opts.features,
+        &specs,
+        &opts.compile_opts.build_config.requested_kinds,
+        feature_opts,
+    )?;
+
+    let differences = v2_features.compare_legacy(&ws_resolve.resolved_features);
+    if differences.features.is_empty() && differences.optional_deps.is_empty() {
+        // Nothing is different, nothing to report.
+        return Ok(());
+    }
+    let config = ws.config();
+    config.shell().note(
+        "Switching to Edition 2021 will enable the use of the version 2 feature resolver in Cargo.",
+    )?;
+    drop_eprintln!(
+        config,
+        "This may cause dependencies to resolve with a different set of features."
+    );
+    drop_eprintln!(
+        config,
+        "More information about the resolver changes may be found \
+         at https://doc.rust-lang.org/cargo/reference/features.html#feature-resolver-version-2"
+    );
+    drop_eprintln!(
+        config,
+        "The following differences were detected with the current configuration:\n"
+    );
+    let report = |changes: crate::core::resolver::features::DiffMap, what| {
+        for ((pkg_id, for_host), removed) in changes {
+            drop_eprint!(config, "  {}", pkg_id);
+            if for_host {
+                drop_eprint!(config, " (as build dependency)");
+            }
+            if !removed.is_empty() {
+                let joined: Vec<_> = removed.iter().map(|s| s.as_str()).collect();
+                drop_eprint!(config, " removed {} `{}`", what, joined.join(","));
+            }
+            drop_eprint!(config, "\n");
+        }
+    };
+    report(differences.features, "features");
+    report(differences.optional_deps, "optional dependency");
+    drop_eprint!(config, "\n");
+    Ok(())
+}
+
 /// Entry point for `cargo` running as a proxy for `rustc`.
 ///
 /// This is called every time `cargo` is run to check if it is in proxy mode.
diff --git a/src/cargo/ops/resolve.rs b/src/cargo/ops/resolve.rs
@@ -12,7 +12,9 @@
 
 use crate::core::compiler::{CompileKind, RustcTargetData};
 use crate::core::registry::PackageRegistry;
-use crate::core::resolver::features::{FeatureResolver, ForceAllTargets, ResolvedFeatures};
+use crate::core::resolver::features::{
+    FeatureOpts, FeatureResolver, ForceAllTargets, ResolvedFeatures,
+};
 use crate::core::resolver::{self, HasDevUnits, Resolve, ResolveOpts, ResolveVersion};
 use crate::core::summary::Summary;
 use crate::core::Feature;
@@ -143,6 +145,7 @@ pub fn resolve_ws_with_opts<'cfg>(
         force_all_targets,
     )?;
 
+    let feature_opts = FeatureOpts::new(ws, has_dev_units, force_all_targets)?;
     let resolved_features = FeatureResolver::resolve(
         ws,
         target_data,
@@ -151,8 +154,7 @@ pub fn resolve_ws_with_opts<'cfg>(
         &opts.features,
         specs,
         requested_targets,
-        has_dev_units,
-        force_all_targets,
+        feature_opts,
     )?;
 
     Ok(WorkspaceResolve {
diff --git a/tests/testsuite/fix.rs b/tests/testsuite/fix.rs

Original file line number	Diff line number	Diff line change
`@@ -103,7 +103,7 @@ struct Packages<'cfg> {`
`103`	`103`	`}`
`104`	`104`
`105`	`105`	`#[derive(Debug)]`
`106`		`-enum MaybePackage {`
	`106`	`+pub enum MaybePackage {`
`107`	`107`	`Package(Package),`
`108`	`108`	`Virtual(VirtualManifest),`
`109`	`109`	`}`
`@@ -342,7 +342,7 @@ impl<'cfg> Workspace<'cfg> {`
`342`	`342`	`}`
`343`	`343`
`344`	`344`	`/// Returns the root Package or VirtualManifest.`
`345`		`- fn root_maybe(&self) -> &MaybePackage {`
	`345`	`+ pub fn root_maybe(&self) -> &MaybePackage {`
`346`	`346`	`self.packages.get(self.root_manifest())`
`347`	`347`	`}`
`348`	`348`