Auto merge of #8799 - ehuss:new-namespaced, r=alexcrichton

New namespaced features implementation.

This is a new implementation for namespaced features (#5565). See the `unstable.md` docs for a description of the new behavior. This is intended to fix several issues with the existing design, and to make it backwards compatible so that it does not require an opt-in flag.

This also includes tangentially-related changes to the new feature resolver. The changes are:

* `crate_name/feat_name` syntax will now always enable the feature `crate_name`, even if it is an inactive optional dependency (such as a dependency for another platform). The intent here is to have a certain amount of consistency whereby "features" are always activated, but individual crates will still not be activated.
* `--all-features` will now enable features for inactive optional dependencies. This is more consistent with `--features foo` enabling the `foo` feature, even when the `foo` dep is not activated.

I'm still very conflicted on how that should work, but I think it is better from a simplicity/consistency perspective. I still think it may be confusing if someone has a `cfg(some_dep)` in their code, and `some_dep` isn't built, it will error. The intent is that `cfg(accessible(some_dep))` will be the preferred method in the future, or using platform `cfg` expression like `cfg(windows)` to match whatever is in Cargo.toml.

Closes #8044
Closes #8046
Closes #8047
Closes #8316

## Questions
- For various reasons, I changed the way dependency conflict errors are collected. One slightly negative consequence is that it will raise an error for the first problem it detects (like a "missing feature"). Previously it would collect a list of all missing features and display all of them in the error message. Let me know if I should retain the old behavior. I think it will make the code more complicated and brittle, but it shouldn't be too hard (essentially `Requirements` will need to collect a list of errors, and then `resolve_features` would need to check if the list is non-empty, and then aggregate the errors).

- Should `cargo metadata` show the implicit features in the "features" table? Currently it does not, and I think that is probably best (it mirrors what is in `Cargo.toml`), but I could potentially see an argument to show how cargo sees the implicit features.

Author: bors

crates/cargo-test-support/src/lib.rs

@@ -1218,7 +1218,7 @@ enum MatchKind {
 
 /// Compares a line with an expected pattern.
 /// - Use `[..]` as a wildcard to match 0 or more characters on the same line
-///   (similar to `.*` in a regex).
+///   (similar to `.*` in a regex). It is non-greedy.
 /// - Use `[EXE]` to optionally add `.exe` on Windows (empty string on other
 ///   platforms).
 /// - There is a wide range of macros (such as `[COMPILING]` or `[WARNING]`)

crates/resolver-tests/src/lib.rs

@@ -170,14 +170,7 @@ pub fn resolve_with_config_raw(
         list: registry,
         used: HashSet::new(),
     };
-    let summary = Summary::new(
-        pkg_id("root"),
-        deps,
-        &BTreeMap::<String, Vec<String>>::new(),
-        None::<&String>,
-        false,
-    )
-    .unwrap();
+    let summary = Summary::new(pkg_id("root"), deps, &BTreeMap::new(), None::<&String>).unwrap();
     let opts = ResolveOpts::everything();
     let start = Instant::now();
     let resolve = resolver::resolve(
@@ -571,14 +564,7 @@ pub fn pkg_dep<T: ToPkgId>(name: T, dep: Vec<Dependency>) -> Summary {
     } else {
         None
     };
-    Summary::new(
-        name.to_pkgid(),
-        dep,
-        &BTreeMap::<String, Vec<String>>::new(),
-        link,
-        false,
-    )
-    .unwrap()
+    Summary::new(name.to_pkgid(), dep, &BTreeMap::new(), link).unwrap()
 }
 
 pub fn pkg_id(name: &str) -> PackageId {
@@ -599,14 +585,7 @@ pub fn pkg_loc(name: &str, loc: &str) -> Summary {
     } else {
         None
     };
-    Summary::new(
-        pkg_id_loc(name, loc),
-        Vec::new(),
-        &BTreeMap::<String, Vec<String>>::new(),
-        link,
-        false,
-    )
-    .unwrap()
+    Summary::new(pkg_id_loc(name, loc), Vec::new(), &BTreeMap::new(), link).unwrap()
 }
 
 pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
@@ -616,9 +595,8 @@ pub fn remove_dep(sum: &Summary, ind: usize) -> Summary {
     Summary::new(
         sum.package_id(),
         deps,
-        &BTreeMap::<String, Vec<String>>::new(),
+        &BTreeMap::new(),
         sum.links().map(|a| a.as_str()),
-        sum.namespaced_features(),
     )
     .unwrap()
 }

crates/resolver-tests/tests/resolve.rs

@@ -228,6 +228,7 @@ proptest! {
 }
 
 #[test]
+#[should_panic(expected = "pub dep")] // The error handling is not yet implemented.
 fn pub_fail() {
     let input = vec![
         pkg!(("a", "0.0.4")),

src/bin/cargo/cli.rs

@@ -35,13 +35,14 @@ pub fn main(config: &mut Config) -> CliResult {
             "
 Available unstable (nightly-only) flags:
 
-    -Z avoid-dev-deps   -- Avoid installing dev-dependencies if possible
-    -Z minimal-versions -- Install minimal dependency versions instead of maximum
-    -Z no-index-update  -- Do not update the registry, avoids a network request for benchmarking
-    -Z unstable-options -- Allow the usage of unstable options
-    -Z timings          -- Display concurrency information
-    -Z doctest-xcompile -- Compile and run doctests for non-host target using runner config
-    -Z terminal-width   -- Provide a terminal width to rustc for error truncation
+    -Z avoid-dev-deps      -- Avoid installing dev-dependencies if possible
+    -Z minimal-versions    -- Install minimal dependency versions instead of maximum
+    -Z no-index-update     -- Do not update the registry, avoids a network request for benchmarking
+    -Z unstable-options    -- Allow the usage of unstable options
+    -Z timings             -- Display concurrency information
+    -Z doctest-xcompile    -- Compile and run doctests for non-host target using runner config
+    -Z terminal-width      -- Provide a terminal width to rustc for error truncation
+    -Z namespaced-features -- Allow features with `dep:` prefix
 
 Run with 'cargo -Z [FLAG] [SUBCOMMAND]'"
         );

src/bin/cargo/commands/read_manifest.rs

@@ -15,6 +15,6 @@ Deprecated, use `cargo metadata --no-deps` instead.\
 
 pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
     let ws = args.workspace(config)?;
-    config.shell().print_json(&ws.current()?);
+    config.shell().print_json(&ws.current()?.serialized(config));
     Ok(())
 }

src/cargo/core/compiler/mod.rs

@@ -1034,7 +1034,7 @@ pub fn extern_args(
             if unit
                 .pkg
                 .manifest()
-                .features()
+                .unstable_features()
                 .require(Feature::public_dependency())
                 .is_ok()
                 && !dep.public

src/cargo/core/compiler/unit_dependencies.rs

@@ -713,6 +713,16 @@ impl<'a, 'cfg> State<'a, 'cfg> {
         features.activated_features(pkg_id, features_for)
     }
 
+    fn is_dep_activated(
+        &self,
+        pkg_id: PackageId,
+        features_for: FeaturesFor,
+        dep_name: InternedString,
+    ) -> bool {
+        self.features()
+            .is_dep_activated(pkg_id, features_for, dep_name)
+    }
+
     fn get(&self, id: PackageId) -> &'a Package {
         self.package_set
             .get_one(id)
@@ -738,9 +748,7 @@ impl<'a, 'cfg> State<'a, 'cfg> {
                     // did not enable it, don't include it.
                     if dep.is_optional() {
                         let features_for = unit_for.map_to_features_for();
-
-                        let feats = self.activated_features(pkg_id, features_for);
-                        if !feats.contains(&dep.name_in_toml()) {
+                        if !self.is_dep_activated(pkg_id, features_for, dep.name_in_toml()) {
                             return false;
                         }
                     }

src/cargo/core/features.rs

@@ -25,7 +25,7 @@
 //! use core::{Feature, Features};
 //!
 //! let feature = Feature::launch_into_space();
-//! package.manifest().features().require(feature).chain_err(|| {
+//! package.manifest().unstable_features().require(feature).chain_err(|| {
 //!     "launching Cargo into space right now is unstable and may result in \
 //!      unintended damage to your codebase, use with caution"
 //! })?;
@@ -197,9 +197,6 @@ features! {
         // Overriding profiles for dependencies.
         [stable] profile_overrides: bool,
 
-        // Separating the namespaces for features and dependencies
-        [unstable] namespaced_features: bool,
-
         // "default-run" manifest option,
         [stable] default_run: bool,
 
@@ -360,6 +357,7 @@ pub struct CliUnstable {
     pub multitarget: bool,
     pub rustdoc_map: bool,
     pub terminal_width: Option<Option<usize>>,
+    pub namespaced_features: bool,
 }
 
 fn deserialize_build_std<'de, D>(deserializer: D) -> Result<Option<Vec<String>>, D::Error>
@@ -465,6 +463,7 @@ impl CliUnstable {
             "multitarget" => self.multitarget = parse_empty(k, v)?,
             "rustdoc-map" => self.rustdoc_map = parse_empty(k, v)?,
             "terminal-width" => self.terminal_width = Some(parse_usize_opt(v)?),
+            "namespaced-features" => self.namespaced_features = parse_empty(k, v)?,
             _ => bail!("unknown `-Z` flag specified: {}", k),
         }
 

src/cargo/core/manifest.rs

@@ -42,7 +42,7 @@ pub struct Manifest {
     patch: HashMap<Url, Vec<Dependency>>,
     workspace: WorkspaceConfig,
     original: Rc<TomlManifest>,
-    features: Features,
+    unstable_features: Features,
     edition: Edition,
     im_a_teapot: Option<bool>,
     default_run: Option<String>,
@@ -371,7 +371,7 @@ impl Manifest {
         replace: Vec<(PackageIdSpec, Dependency)>,
         patch: HashMap<Url, Vec<Dependency>>,
         workspace: WorkspaceConfig,
-        features: Features,
+        unstable_features: Features,
         edition: Edition,
         im_a_teapot: Option<bool>,
         default_run: Option<String>,
@@ -393,7 +393,7 @@ impl Manifest {
             replace,
             patch,
             workspace,
-            features,
+            unstable_features,
             edition,
             original,
             im_a_teapot,
@@ -467,8 +467,9 @@ impl Manifest {
         &self.workspace
     }
 
-    pub fn features(&self) -> &Features {
-        &self.features
+    /// Unstable, nightly features that are enabled in this manifest.
+    pub fn unstable_features(&self) -> &Features {
+        &self.unstable_features
     }
 
     /// The style of resolver behavior to use, declared with the `resolver` field.
@@ -487,7 +488,7 @@ impl Manifest {
 
     pub fn feature_gate(&self) -> CargoResult<()> {
         if self.im_a_teapot.is_some() {
-            self.features
+            self.unstable_features
                 .require(Feature::test_dummy_unstable())
                 .chain_err(|| {
                     anyhow::format_err!(
@@ -578,7 +579,7 @@ impl VirtualManifest {
         &self.warnings
     }
 
-    pub fn features(&self) -> &Features {
+    pub fn unstable_features(&self) -> &Features {
         &self.features
     }