Auto merge of #8997 - ehuss:stabilize-features, r=alexcrichton

Stabilize -Zfeatures and -Zpackage-features.

This follows through with [RFC 2957](rust-lang/rfcs#2957) to stabilize the new feature resolver, and `-Zpackage-features` command-line changes.

This also rewrites the "Features" chapter to try to expand it a little.

I decided to leave the `-Zfeatures` flag in for now for testing, but it can be removed at a later date.

There is a code change related to the `package-name/feature-name` syntax for the `--features` flag. I wanted to stabilize that for `resolver = "1"`, but I previously neglected to separate that behavior out, so it required change to `Workspace::members_with_features` to make that work (see the `resolver1_member_features` test).

Closes #4328
Closes #5364
Closes #7914
Closes #7915
Closes #7916
Closes #8088
Closes #8431

Author: bors

src/cargo/core/compiler/unit_dependencies.rs

@@ -518,12 +518,12 @@ fn dep_build_script(
             // build.rs unit use the same features. This is because some
             // people use `cfg!` and `#[cfg]` expressions to check for enabled
             // features instead of just checking `CARGO_FEATURE_*` at runtime.
-            // In the case with `-Zfeatures=host_dep`, and a shared
-            // dependency has different features enabled for normal vs. build,
-            // then the build.rs script will get compiled twice. I believe it
-            // is not feasible to only build it once because it would break a
-            // large number of scripts (they would think they have the wrong
-            // set of features enabled).
+            // In the case with the new feature resolver (decoupled host
+            // deps), and a shared dependency has different features enabled
+            // for normal vs. build, then the build.rs script will get
+            // compiled twice. I believe it is not feasible to only build it
+            // once because it would break a large number of scripts (they
+            // would think they have the wrong set of features enabled).
             let script_unit_for = UnitFor::new_host(unit_for.is_for_host_features());
             new_unit_dep_with_profile(
                 state,

src/cargo/core/features.rs

@@ -210,7 +210,7 @@ features! {
         [unstable] named_profiles: bool,
 
         // Opt-in new-resolver behavior.
-        [unstable] resolver: bool,
+        [stable] resolver: bool,
 
         // Allow to specify whether binaries should be stripped.
         [unstable] strip: bool,
@@ -338,7 +338,6 @@ pub struct CliUnstable {
     pub no_index_update: bool,
     pub avoid_dev_deps: bool,
     pub minimal_versions: bool,
-    pub package_features: bool,
     pub advanced_env: bool,
     pub config_include: bool,
     pub dual_proc_macros: bool,
@@ -445,7 +444,6 @@ impl CliUnstable {
             "no-index-update" => self.no_index_update = parse_empty(k, v)?,
             "avoid-dev-deps" => self.avoid_dev_deps = parse_empty(k, v)?,
             "minimal-versions" => self.minimal_versions = parse_empty(k, v)?,
-            "package-features" => self.package_features = parse_empty(k, v)?,
             "advanced-env" => self.advanced_env = parse_empty(k, v)?,
             "config-include" => self.config_include = parse_empty(k, v)?,
             "dual-proc-macros" => self.dual_proc_macros = parse_empty(k, v)?,

src/cargo/core/resolver/features.rs

@@ -1,11 +1,8 @@
 //! Feature resolver.
 //!
 //! This is a new feature resolver that runs independently of the main
-//! dependency resolver. It is intended to make it easier to experiment with
-//! new behaviors. When `-Zfeatures` is not used, it will fall back to using
-//! the original `Resolve` feature computation. With `-Zfeatures` enabled,
-//! this will walk the dependency graph and compute the features using a
-//! different algorithm.
+//! dependency resolver. It is enabled when the user specifies `resolver =
+//! "2"` in `Cargo.toml`.
 //!
 //! One of its key characteristics is that it can avoid unifying features for
 //! shared dependencies in some situations. See `FeatureOpts` for the
@@ -61,11 +58,11 @@ pub struct ResolvedFeatures {
     ///
     /// The value is the `name_in_toml` of the dependencies.
     activated_dependencies: ActivateMap,
-    /// This is only here for legacy support when `-Zfeatures` is not enabled.
+    /// This is only here for legacy support when the new resolver is not enabled.
     ///
     /// This is the set of features enabled for each package.
     legacy_features: Option<HashMap<PackageId, Vec<InternedString>>>,
-    /// This is only here for legacy support when `-Zfeatures` is not enabled.
+    /// This is only here for legacy support when the new resolver is not enabled.
     ///
     /// This is the set of optional dependencies enabled for each package.
     legacy_dependencies: Option<HashMap<PackageId, HashSet<InternedString>>>,
@@ -75,7 +72,7 @@ pub struct ResolvedFeatures {
 /// Options for how the feature resolver works.
 #[derive(Default)]
 struct FeatureOpts {
-    /// -Zfeatures is enabled, use new resolver.
+    /// 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.
     decouple_host_deps: bool,

src/cargo/core/workspace.rs

@@ -637,8 +637,15 @@ impl<'cfg> Workspace<'cfg> {
         self.resolve_behavior.unwrap_or(ResolveBehavior::V1)
     }
 
-    pub fn allows_unstable_package_features(&self) -> bool {
-        self.config().cli_unstable().package_features
+    /// Returns `true` if this workspace uses the new CLI features behavior.
+    ///
+    /// The old behavior only allowed choosing the features from the package
+    /// in the current directory, regardless of which packages were chosen
+    /// with the -p flags. The new behavior allows selecting features from the
+    /// packages chosen on the command line (with -p or --workspace flags),
+    /// ignoring whatever is in the current directory.
+    pub fn allows_new_cli_feature_behavior(&self) -> bool {
+        self.is_virtual()
             || match self.resolve_behavior() {
                 ResolveBehavior::V1 => false,
                 ResolveBehavior::V2 => true,
@@ -947,15 +954,16 @@ impl<'cfg> Workspace<'cfg> {
                 .map(|m| (m, RequestedFeatures::new_all(true)))
                 .collect());
         }
-        if self.allows_unstable_package_features() {
-            self.members_with_features_pf(specs, requested_features)
+        if self.allows_new_cli_feature_behavior() {
+            self.members_with_features_new(specs, requested_features)
         } else {
-            self.members_with_features_stable(specs, requested_features)
+            self.members_with_features_old(specs, requested_features)
         }
     }
 
-    /// New command-line feature selection with -Zpackage-features.
-    fn members_with_features_pf(
+    /// New command-line feature selection behavior with resolver = "2" or the
+    /// root of a virtual workspace. See `allows_new_cli_feature_behavior`.
+    fn members_with_features_new(
         &self,
         specs: &[PackageIdSpec],
         requested_features: &RequestedFeatures,
@@ -1053,30 +1061,69 @@ impl<'cfg> Workspace<'cfg> {
         Ok(members)
     }
 
-    /// This is the current "stable" behavior for command-line feature selection.
-    fn members_with_features_stable(
+    /// This is the "old" behavior for command-line feature selection.
+    /// See `allows_new_cli_feature_behavior`.
+    fn members_with_features_old(
         &self,
         specs: &[PackageIdSpec],
         requested_features: &RequestedFeatures,
     ) -> CargoResult<Vec<(&Package, RequestedFeatures)>> {
+        // Split off any features with the syntax `member-name/feature-name` into a map
+        // so that those features can be applied directly to those workspace-members.
+        let mut member_specific_features: HashMap<&str, BTreeSet<InternedString>> = HashMap::new();
+        // Features for the member in the current directory.
+        let mut cwd_features = BTreeSet::new();
+        for feature in requested_features.features.iter() {
+            if let Some(index) = feature.find('/') {
+                let name = &feature[..index];
+                if specs.iter().any(|spec| spec.name() == name) {
+                    member_specific_features
+                        .entry(name)
+                        .or_default()
+                        .insert(InternedString::new(&feature[index + 1..]));
+                } else {
+                    cwd_features.insert(*feature);
+                }
+            } else {
+                cwd_features.insert(*feature);
+            };
+        }
+
         let ms = self.members().filter_map(|member| {
             let member_id = member.package_id();
             match self.current_opt() {
                 // The features passed on the command-line only apply to
                 // the "current" package (determined by the cwd).
                 Some(current) if member_id == current.package_id() => {
-                    Some((member, requested_features.clone()))
+                    let feats = RequestedFeatures {
+                        features: Rc::new(cwd_features.clone()),
+                        all_features: requested_features.all_features,
+                        uses_default_features: requested_features.uses_default_features,
+                    };
+                    Some((member, feats))
                 }
                 _ => {
                     // Ignore members that are not enabled on the command-line.
                     if specs.iter().any(|spec| spec.matches(member_id)) {
-                        // -p for a workspace member that is not the
-                        // "current" one, don't use the local
-                        // `--features`, only allow `--all-features`.
-                        Some((
-                            member,
-                            RequestedFeatures::new_all(requested_features.all_features),
-                        ))
+                        // -p for a workspace member that is not the "current"
+                        // one.
+                        //
+                        // The odd behavior here is due to backwards
+                        // compatibility. `--features` and
+                        // `--no-default-features` used to only apply to the
+                        // "current" package. As an extension, this allows
+                        // member-name/feature-name to set member-specific
+                        // features, which should be backwards-compatible.
+                        let feats = RequestedFeatures {
+                            features: Rc::new(
+                                member_specific_features
+                                    .remove(member.name().as_str())
+                                    .unwrap_or_default(),
+                            ),
+                            uses_default_features: true,
+                            all_features: requested_features.all_features,
+                        };
+                        Some((member, feats))
                     } else {
                         // This member was not requested on the command-line, skip.
                         None

src/cargo/util/command_prelude.rs

@@ -310,20 +310,6 @@ pub trait ArgMatchesExt {
         if config.cli_unstable().avoid_dev_deps {
             ws.set_require_optional_deps(false);
         }
-        if ws.is_virtual() && !ws.allows_unstable_package_features() {
-            // --all-features is actually honored. In general, workspaces and
-            // feature flags are a bit of a mess right now.
-            for flag in &["features", "no-default-features"] {
-                if self._is_present(flag) {
-                    bail!(
-                        "--{} is not allowed in the root of a virtual workspace\n\
-                         note: while this was previously accepted, it didn't actually do anything\n\
-                         help: change the current directory to the package directory, or use the --manifest-path flag to the path of the package",
-                        flag
-                    );
-                }
-            }
-        }
         Ok(ws)
     }
 

src/cargo/util/toml/mod.rs

@@ -883,19 +883,7 @@ impl TomlManifest {
             .unwrap()
             .clone();
         package.workspace = None;
-        let mut cargo_features = self.cargo_features.clone();
         package.resolver = ws.resolve_behavior().to_manifest();
-        if package.resolver.is_some() {
-            // This should be removed when stabilizing.
-            match &mut cargo_features {
-                None => cargo_features = Some(vec!["resolver".to_string()]),
-                Some(feats) => {
-                    if !feats.iter().any(|feat| feat == "resolver") {
-                        feats.push("resolver".to_string());
-                    }
-                }
-            }
-        }
         if let Some(license_file) = &package.license_file {
             let license_path = Path::new(&license_file);
             let abs_license_path = paths::normalize_path(&package_root.join(license_path));
@@ -977,7 +965,7 @@ impl TomlManifest {
             patch: None,
             workspace: None,
             badges: self.badges.clone(),
-            cargo_features,
+            cargo_features: self.cargo_features.clone(),
         });
 
         fn map_deps(

src/doc/man/generated_txt/cargo-bench.txt

@@ -163,28 +163,25 @@ OPTIONS
            --tests --benches --examples.
 
    Feature Selection
-       The feature flags allow you to control the enabled features for the
-       "current" package. The "current" package is the package in the current
-       directory, or the one specified in --manifest-path. If running in the
-       root of a virtual workspace, then the default features are selected for
-       all workspace members, or all features if --all-features is specified.
+       The feature flags allow you to control which features are enabled. When
+       no feature options are given, the default feature is activated for every
+       selected package.
 
-       When no feature options are given, the default feature is activated for
-       every selected package.
+       See the features documentation
+       <https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
+       for more details.
 
        --features features
-           Space or comma separated list of features to activate. These
-           features only apply to the current directory's package. Features of
-           direct dependencies may be enabled with <dep-name>/<feature-name>
+           Space or comma separated list of features to activate. Features of
+           workspace members may be enabled with package-name/feature-name
            syntax. This flag may be specified multiple times, which enables all
            specified features.
 
        --all-features
            Activate all available features of all selected packages.
 
        --no-default-features
-           Do not activate the default feature of the current directory's
-           package.
+           Do not activate the default feature of the selected packages.
 
    Compilation Options
        --target triple

src/doc/man/generated_txt/cargo-build.txt

@@ -106,28 +106,25 @@ OPTIONS
            --tests --benches --examples.
 
    Feature Selection
-       The feature flags allow you to control the enabled features for the
-       "current" package. The "current" package is the package in the current
-       directory, or the one specified in --manifest-path. If running in the
-       root of a virtual workspace, then the default features are selected for
-       all workspace members, or all features if --all-features is specified.
+       The feature flags allow you to control which features are enabled. When
+       no feature options are given, the default feature is activated for every
+       selected package.
 
-       When no feature options are given, the default feature is activated for
-       every selected package.
+       See the features documentation
+       <https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
+       for more details.
 
        --features features
-           Space or comma separated list of features to activate. These
-           features only apply to the current directory's package. Features of
-           direct dependencies may be enabled with <dep-name>/<feature-name>
+           Space or comma separated list of features to activate. Features of
+           workspace members may be enabled with package-name/feature-name
            syntax. This flag may be specified multiple times, which enables all
            specified features.
 
        --all-features
            Activate all available features of all selected packages.
 
        --no-default-features
-           Do not activate the default feature of the current directory's
-           package.
+           Do not activate the default feature of the selected packages.
 
    Compilation Options
        --target triple

src/doc/man/generated_txt/cargo-check.txt

@@ -112,28 +112,25 @@ OPTIONS
            --tests --benches --examples.
 
    Feature Selection
-       The feature flags allow you to control the enabled features for the
-       "current" package. The "current" package is the package in the current
-       directory, or the one specified in --manifest-path. If running in the
-       root of a virtual workspace, then the default features are selected for
-       all workspace members, or all features if --all-features is specified.
+       The feature flags allow you to control which features are enabled. When
+       no feature options are given, the default feature is activated for every
+       selected package.
 
-       When no feature options are given, the default feature is activated for
-       every selected package.
+       See the features documentation
+       <https://doc.rust-lang.org/cargo/reference/features.html#command-line-feature-options>
+       for more details.
 
        --features features
-           Space or comma separated list of features to activate. These
-           features only apply to the current directory's package. Features of
-           direct dependencies may be enabled with <dep-name>/<feature-name>
+           Space or comma separated list of features to activate. Features of
+           workspace members may be enabled with package-name/feature-name
            syntax. This flag may be specified multiple times, which enables all
            specified features.
 
        --all-features
            Activate all available features of all selected packages.
 
        --no-default-features
-           Do not activate the default feature of the current directory's
-           package.
+           Do not activate the default feature of the selected packages.
 
    Compilation Options
        --target triple