wip

CHANGELOG.md

@@ -10,6 +10,10 @@ Note: In this file, do not use the hard wrap in the middle of a sentence for com
 
 ## [Unreleased]
 
+- Fix handling of weak dependency features when namespaced features is not used together.
+
+- Improve performance by passing `--no-deps` to `cargo metadata`  except when using `--include-deps-features`.
+
 ## [0.6.10] - 2023-10-17
 
 - Fix compatibility with old Cargo. This fixes a regression introduced in 0.6.9.

src/context.rs

@@ -10,7 +10,6 @@ use std::{
 };
 
 use anyhow::{bail, Result};
-use cargo_config2::Config;
 
 use crate::{
     cargo,
@@ -30,6 +29,7 @@ pub(crate) struct Context {
     pub(crate) cargo_version: u32,
     pub(crate) restore: restore::Manager,
     pub(crate) current_dir: PathBuf,
+    pub(crate) current_package: Option<PackageId>,
 }
 
 impl Context {
@@ -48,20 +48,10 @@ impl Context {
             .map(|v| v.minor)
             .unwrap_or(0);
 
-        let config = Config::load()?;
-        let targets = config.build_target_for_cli(&args.target)?;
-        let host = config.host_triple()?;
-
         // if `--remove-dev-deps` flag is off, restore manifest file.
         let restore = restore::Manager::new(!args.remove_dev_deps);
-        let metadata = Metadata::new(
-            args.manifest_path.as_deref(),
-            &cargo,
-            cargo_version,
-            &targets,
-            host,
-            &restore,
-        )?;
+        let metadata =
+            Metadata::new(args.manifest_path.as_deref(), &cargo, cargo_version, &args, &restore)?;
         if metadata.cargo_version < 41 && args.include_deps_features {
             bail!("--include-deps-features requires Cargo 1.41 or later");
         }
@@ -77,6 +67,23 @@ impl Context {
             pkg_features.insert(id.clone(), features);
         }
 
+        let mut cmd = cmd!(&cargo, "locate-project", "--message-format", "plain");
+        if let Some(manifest_path) = &args.manifest_path {
+            cmd.arg("--manifest-path");
+            cmd.arg(manifest_path);
+        }
+        let locate_project = &cmd.read()?;
+        let mut current_package = None;
+        for id in &metadata.workspace_members {
+            let manifest_path = &metadata.packages[id].manifest_path;
+            // no need to use same_file as cargo-metadata and cargo-locate-project
+            // as they return absolute paths resolved in the same way.
+            if Path::new(locate_project) == manifest_path {
+                current_package = Some(id.clone());
+                break;
+            }
+        }
+
         let this = Self {
             args,
             metadata,
@@ -86,6 +93,7 @@ impl Context {
             cargo_version,
             restore,
             current_dir: env::current_dir()?,
+            current_package,
         };
 
         // TODO: Ideally, we should do this, but for now, we allow it as cargo-hack
@@ -110,7 +118,7 @@ impl Context {
     }
 
     pub(crate) fn current_package(&self) -> Option<&PackageId> {
-        self.metadata.resolve.root.as_ref()
+        self.current_package.as_ref()
     }
 
     pub(crate) fn workspace_root(&self) -> &Path {

src/features.rs

@@ -22,35 +22,31 @@ impl Features {
         include_deps_features: bool,
     ) -> Self {
         let package = &metadata.packages[id];
-        let node = &metadata.resolve.nodes[id];
 
-        let mut features = Vec::with_capacity(package.features.len());
-        let mut optional_deps = vec![];
-        let mut namespaced_features = vec![]; // features with `dep:` prefix
+        let mut features: BTreeSet<_> = manifest.features.keys().map(Feature::from).collect();
+        let mut has_namespaced_features = false; // features with `dep:` prefix
 
         // package.features.values() does not provide a way to determine the `dep:` specified by the user.
         for names in manifest.features.values() {
             for name in names {
-                if let Some(name) = name.strip_prefix("dep:") {
-                    namespaced_features.push(name);
+                if name.starts_with("dep:") {
+                    has_namespaced_features = true;
+                    break;
                 }
             }
         }
-        for name in package.optional_deps() {
-            if !namespaced_features.contains(&name) {
-                optional_deps.push(name);
-            }
-        }
-        for name in package.features.keys() {
-            if !optional_deps.contains(&&**name) {
-                features.push(name.into());
+        let optional_deps_start = features.len();
+        // When namespace dependency is used, other optional dependencies are also not
+        // treated as implicit features.
+        if !has_namespaced_features {
+            for name in package.optional_deps() {
+                features.insert(name.into());
             }
         }
-        let optional_deps_start = features.len();
-        features.extend(optional_deps.into_iter().map(Into::into));
         let deps_features_start = features.len();
 
         if include_deps_features {
+            let node = &metadata.resolve.nodes[id];
             // TODO: Unpublished dependencies are not included in `node.deps`.
             for dep in node.deps.iter().filter(|dep| {
                 // ignore if `dep_kinds` is empty (i.e., not Rust 1.41+), target specific or not a normal dependency.
@@ -68,7 +64,7 @@ impl Features {
             }
         }
 
-        Self { features, optional_deps_start, deps_features_start }
+        Self { features: features.into_iter().collect(), optional_deps_start, deps_features_start }
     }
 
     pub(crate) fn normal(&self) -> &[Feature] {
@@ -89,6 +85,7 @@ impl Features {
 }
 
 /// The representation of Cargo feature.
+#[derive(PartialEq, Eq, PartialOrd, Ord)]
 pub(crate) enum Feature {
     /// A feature of the current crate.
     Normal {

src/metadata.rs

@@ -14,9 +14,10 @@ use std::{
 };
 
 use anyhow::{format_err, Context as _, Result};
+use cargo_config2::Config;
 use serde_json::{Map, Value};
 
-use crate::{cargo, fs, process::ProcessBuilder, restore, term};
+use crate::{cargo, cli::Args, fs, process::ProcessBuilder, restore, term};
 
 type Object = Map<String, Value>;
 type ParseResult<T> = Result<T, &'static str>;
@@ -52,13 +53,22 @@ impl Metadata {
         manifest_path: Option<&str>,
         cargo: &OsStr,
         mut cargo_version: u32,
-        targets: &[String],
-        host: &str,
+        args: &Args,
         restore: &restore::Manager,
     ) -> Result<Self> {
         let stable_cargo_version =
             cargo::version(cmd!("rustup", "run", "stable", "cargo")).map(|v| v.minor).unwrap_or(0);
 
+        let config;
+        let include_deps_features = if args.include_deps_features {
+            config = Config::load()?;
+            let targets = config.build_target_for_cli(&args.target)?;
+            let host = config.host_triple()?;
+            Some((targets, host))
+        } else {
+            None
+        };
+
         let mut cmd;
         let append_metadata_args = |cmd: &mut ProcessBuilder<'_>| {
             cmd.arg("metadata");
@@ -67,14 +77,21 @@ impl Metadata {
                 cmd.arg("--manifest-path");
                 cmd.arg(manifest_path);
             }
-            if targets.is_empty() {
-                cmd.arg("--filter-platform");
-                cmd.arg(host);
-            } else {
-                for target in targets {
+            if let Some((targets, host)) = &include_deps_features {
+                if targets.is_empty() {
                     cmd.arg("--filter-platform");
-                    cmd.arg(target);
+                    cmd.arg(host);
+                } else {
+                    for target in targets {
+                        cmd.arg("--filter-platform");
+                        cmd.arg(target);
+                    }
                 }
+                // features-related flags are unneeded for --no-deps.
+                // TODO:
+                // cmd.arg("--all-features");
+            } else {
+                cmd.arg("--no-deps");
             }
         };
         let json = if stable_cargo_version > cargo_version {
@@ -83,7 +100,8 @@ impl Metadata {
                 cmd.arg("--manifest-path");
                 cmd.arg(manifest_path);
             }
-            let no_deps: Object = serde_json::from_str(&cmd.read()?)
+            let no_deps_raw = cmd.read()?;
+            let no_deps: Object = serde_json::from_str(&no_deps_raw)
                 .with_context(|| format!("failed to parse output from {cmd}"))?;
             let lockfile =
                 Path::new(no_deps["workspace_root"].as_str().unwrap()).join("Cargo.lock");
@@ -111,10 +129,14 @@ impl Metadata {
                     json
                 }
                 Err(_e) => {
-                    // If failed, try again with the version of cargo we will actually use.
-                    cmd = cmd!(cargo);
-                    append_metadata_args(&mut cmd);
-                    cmd.read()?
+                    if include_deps_features.is_some() {
+                        // If failed, try again with the version of cargo we will actually use.
+                        cmd = cmd!(cargo);
+                        append_metadata_args(&mut cmd);
+                        cmd.read()?
+                    } else {
+                        no_deps_raw
+                    }
                 }
             }
         } else {
@@ -143,7 +165,10 @@ impl Metadata {
                 .map(|v| Package::from_value(v, cargo_version))
                 .collect::<Result<_, _>>()?,
             workspace_members,
-            resolve: Resolve::from_obj(map.remove_object("resolve")?, cargo_version)?,
+            resolve: match map.remove_nullable("resolve", into_object)? {
+                Some(resolve) => Resolve::from_obj(resolve, cargo_version)?,
+                None => Resolve { nodes: HashMap::new() },
+            },
             workspace_root: map.remove_string("workspace_root")?,
         })
     }
@@ -152,10 +177,9 @@ impl Metadata {
 /// The resolved dependency graph for the entire workspace.
 pub(crate) struct Resolve {
     /// Nodes in a dependency graph.
+    ///
+    /// This is always empty if cargo-metadata is run with --no-deps.
     pub(crate) nodes: HashMap<PackageId, Node>,
-    /// The crate for which the metadata was read.
-    /// This is `None` if the metadata was read in a virtual workspace.
-    pub(crate) root: Option<PackageId>,
 }
 
 impl Resolve {
@@ -166,7 +190,6 @@ impl Resolve {
                 .into_iter()
                 .map(|v| Node::from_value(v, cargo_version))
                 .collect::<Result<_, _>>()?,
-            root: map.remove_nullable("root", into_string)?,
         })
     }
 }

tests/fixtures/namespaced_features/Cargo.toml

@@ -13,5 +13,8 @@ easytime = ["dep:easytime"]
 [dependencies]
 # easytime 0.2.6 requires Rust 1.58
 easytime = { version = "=0.2.5", optional = true, default-features = false }
+# When namespace dependency is used, other optional dependencies are also not
+# treated as implicit features.
+portable-atomic = { version = "1", optional = true }
 
 [dev-dependencies]

tests/fixtures/weak_dep_features/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "weak_dep_features"
+version = "0.0.0"
+edition = "2021"
+publish = false
+
+[workspace]
+resolver = "2"
+
+[features]
+easytime = ["easytime/std"]
+default = ["easytime?/default"]
+
+[dependencies]
+# easytime 0.2.6 requires Rust 1.58
+easytime = { version = "=0.2.5", optional = true, default-features = false }
+
+[dev-dependencies]

tests/fixtures/weak_dep_features/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/weak_dep_features_implicit/Cargo.toml

@@ -0,0 +1,17 @@
+[package]
+name = "weak_dep_features_implicit"
+version = "0.0.0"
+edition = "2021"
+publish = false
+
+[workspace]
+resolver = "2"
+
+[features]
+default = ["easytime?/default"]
+
+[dependencies]
+# easytime 0.2.6 requires Rust 1.58
+easytime = { version = "=0.2.5", optional = true, default-features = false }
+
+[dev-dependencies]

tests/fixtures/weak_dep_features_implicit/src/lib.rs

@@ -0,0 +1 @@
+

tests/fixtures/weak_dep_features_namespaced/Cargo.toml

@@ -0,0 +1,18 @@
+[package]
+name = "weak_dep_features_namespaced"
+version = "0.0.0"
+edition = "2021"
+publish = false
+
+[workspace]
+resolver = "2"
+
+[features]
+easytime = ["dep:easytime", "easytime/std"]
+default = ["easytime?/default"]
+
+[dependencies]
+# easytime 0.2.6 requires Rust 1.58
+easytime = { version = "=0.2.5", optional = true, default-features = false }
+
+[dev-dependencies]

tests/fixtures/weak_dep_features_namespaced/src/lib.rs

@@ -0,0 +1 @@
+