Do not read manifest if information from manifest is not required

Very few options require information from Cargo manifest, so it can
improve performance by avoiding reading and parsing them in many cases.

.github/workflows/ci.yml

@@ -31,6 +31,8 @@ jobs:
           # This is the minimum supported Rust version of this crate.
           # When updating this, the reminder to update the minimum supported Rust version in README.md.
           - 1.36.0
+          - 1.38.0
+          - 1.39.0
           - stable
           - beta
           - nightly

CHANGELOG.md

@@ -21,13 +21,16 @@ This project adheres to [Semantic Versioning](https://semver.org).
 
 * [Fix an issue where using `--features` with `--each-feature` or `--feature-powerset` together would result in the same feature combination being performed multiple times.][64]
 
+* [Improve performance by avoiding reading and parsing Cargo manifest.][73]
+
 [42]: https://github.com/taiki-e/cargo-hack/pull/42
 [61]: https://github.com/taiki-e/cargo-hack/pull/61
 [62]: https://github.com/taiki-e/cargo-hack/pull/62
 [63]: https://github.com/taiki-e/cargo-hack/pull/63
 [64]: https://github.com/taiki-e/cargo-hack/pull/64
 [65]: https://github.com/taiki-e/cargo-hack/pull/65
 [66]: https://github.com/taiki-e/cargo-hack/pull/66
+[73]: https://github.com/taiki-e/cargo-hack/pull/73
 
 ## [0.3.14] - 2020-10-10
 

src/cli.rs

@@ -233,6 +233,13 @@ pub(crate) struct Args<'a> {
     pub(crate) color: Option<Coloring>,
 }
 
+impl Args<'_> {
+    /// Return `true` if options that require information from cargo manifest is specified.
+    pub(crate) fn require_manifest_info(&self, version: u32) -> bool {
+        (version < 39 && self.ignore_private) || self.no_dev_deps || self.remove_dev_deps
+    }
+}
+
 #[derive(Clone, Copy)]
 pub(crate) enum Coloring {
     Auto,

src/context.rs

@@ -4,34 +4,52 @@ use crate::{
     cli::{self, Args, Coloring, RawArgs},
     manifest::Manifest,
     metadata::{Metadata, Package, PackageId},
-    ProcessBuilder, Result,
+    version, ProcessBuilder, Result,
 };
 
 pub(crate) struct Context<'a> {
     args: Args<'a>,
     metadata: Metadata,
     manifests: HashMap<PackageId, Manifest>,
     cargo: OsString,
+    cargo_version: u32,
 }
 
 impl<'a> Context<'a> {
     pub(crate) fn new(args: &'a RawArgs, coloring: &mut Option<Coloring>) -> Result<Self> {
         let cargo = cargo_binary();
+
+        // If failed to determine cargo version, assign 0 to skip all version-dependent decisions.
+        let cargo_version = match version::from_path(&cargo) {
+            Ok(version) => version.minor,
+            Err(e) => {
+                warn!(*coloring, "{}", e);
+                0
+            }
+        };
+
         let args = cli::perse_args(args, coloring, &cargo)?;
         assert!(
             args.subcommand.is_some() || args.remove_dev_deps,
             "no subcommand or valid flag specified"
         );
-        let metadata = Metadata::new(&args, &cargo)?;
+        let metadata = Metadata::new(&args, &cargo, cargo_version)?;
 
-        let mut manifests = HashMap::with_capacity(metadata.workspace_members.len());
-        for id in &metadata.workspace_members {
-            let manifest_path = &metadata.packages[id].manifest_path;
-            let manifest = Manifest::new(manifest_path)?;
-            manifests.insert(id.clone(), manifest);
-        }
+        // Only a few options require information from cargo manifest.
+        // If manifest information is not required, do not read and parse them.
+        let manifests = if args.require_manifest_info(cargo_version) {
+            let mut manifests = HashMap::with_capacity(metadata.workspace_members.len());
+            for id in &metadata.workspace_members {
+                let manifest_path = &metadata.packages[id].manifest_path;
+                let manifest = Manifest::new(manifest_path)?;
+                manifests.insert(id.clone(), manifest);
+            }
+            manifests
+        } else {
+            HashMap::new()
+        };
 
-        Ok(Self { args, metadata, manifests, cargo })
+        Ok(Self { args, metadata, manifests, cargo, cargo_version })
     }
 
     // Accessor methods.
@@ -44,15 +62,23 @@ impl<'a> Context<'a> {
     // pub(crate) fn nodes(&self, id: &PackageId) -> &Node {
     //     &self.metadata.resolve.nodes[id]
     // }
-    pub(crate) fn current_manifest(&self) -> Option<&PackageId> {
+    pub(crate) fn current_package(&self) -> Option<&PackageId> {
         self.metadata.resolve.root.as_ref()
     }
     pub(crate) fn workspace_root(&self) -> &Path {
         &self.metadata.workspace_root
     }
     pub(crate) fn manifests(&self, id: &PackageId) -> &Manifest {
+        debug_assert!(self.require_manifest_info(self.cargo_version));
         &self.manifests[id]
     }
+    pub(crate) fn is_private(&self, id: &PackageId) -> bool {
+        if self.cargo_version >= 39 {
+            !self.packages(id).publish
+        } else {
+            !self.manifests(id).publish
+        }
+    }
 
     pub(crate) fn process(&self) -> ProcessBuilder<'_> {
         let mut command = ProcessBuilder::new(&self.cargo);

src/main.rs

@@ -18,6 +18,7 @@ mod metadata;
 mod process;
 mod remove_dev_deps;
 mod restore;
+mod version;
 
 use anyhow::{bail, Context as _};
 use std::{fmt::Write, fs};
@@ -86,7 +87,7 @@ enum Kind<'a> {
 }
 
 fn determine_kind<'a>(cx: &'a Context<'_>, id: &PackageId, progress: &mut Progress) -> Kind<'a> {
-    if cx.ignore_private && cx.manifests(id).is_private() {
+    if cx.ignore_private && cx.is_private(id) {
         return Kind::SkipAsPrivate;
     }
     if cx.subcommand.is_none() {
@@ -185,10 +186,10 @@ fn determine_package_list<'a>(
             .filter(|id| cx.package.contains(&&*cx.packages(id).name))
             .map(|id| (id, determine_kind(cx, id, progress)))
             .collect()
-    } else if cx.current_manifest().is_none() {
+    } else if cx.current_package().is_none() {
         cx.workspace_members().map(|id| (id, determine_kind(cx, id, progress))).collect()
     } else {
-        let current_package = &cx.manifests(cx.current_manifest().unwrap()).package.name;
+        let current_package = &cx.packages(cx.current_package().unwrap()).name;
         cx.workspace_members()
             .find(|id| cx.packages(id).name == *current_package)
             .map(|id| vec![(id, determine_kind(cx, id, progress))])

src/manifest.rs

@@ -6,10 +6,14 @@ use crate::Result;
 
 type ParseResult<T> = Result<T, &'static str>;
 
+// Refs:
+// * https://github.com/rust-lang/cargo/blob/0.47.0/src/cargo/util/toml/mod.rs
+// * https://gitlab.com/crates.rs/cargo_toml
+
 pub(crate) struct Manifest {
     pub(crate) raw: String,
-    // parsed manifest
-    pub(crate) package: Package,
+    // `metadata.package.publish` requires Rust 1.39
+    pub(crate) publish: bool,
 }
 
 impl Manifest {
@@ -18,78 +22,34 @@ impl Manifest {
             .with_context(|| format!("failed to read manifest from {}", path.display()))?;
         let toml = toml::from_str(&raw)
             .with_context(|| format!("failed to parse manifest as toml: {}", path.display()))?;
-        let package = Package::from_table(toml).map_err(|s| {
+        let package = Package::from_table(&toml).map_err(|s| {
             format_err!("failed to parse `{}` field from manifest: {}", s, path.display())
         })?;
-        Ok(Self { raw, package })
-    }
-
-    // `metadata.package.publish` requires Rust 1.39
-    pub(crate) fn is_private(&self) -> bool {
-        self.package.publish == false
+        Ok(Self { raw, publish: package.publish == true })
     }
 
     pub(crate) fn remove_dev_deps(&self) -> String {
         super::remove_dev_deps::remove_dev_deps(&self.raw)
     }
 }
 
-// Refs:
-// * https://github.com/rust-lang/cargo/blob/0.47.0/src/cargo/util/toml/mod.rs
-// * https://gitlab.com/crates.rs/cargo_toml
-
-pub(crate) struct Package {
-    pub(crate) name: String,
-    pub(crate) publish: Publish,
+struct Package {
+    publish: bool,
 }
 
 impl Package {
-    fn from_table(mut table: Table) -> ParseResult<Self> {
-        let package = table.get_mut("package").and_then(Value::as_table_mut).ok_or("package")?;
-        let name = package.remove("name").and_then(into_string).ok_or("name")?;
-        let publish = Publish::from_value(package.get("publish")).ok_or("publish")?;
-
-        Ok(Self { name, publish })
-    }
-}
-
-pub(crate) enum Publish {
-    Flag(bool),
-    Registry { is_empty: bool },
-}
-
-impl Publish {
-    fn from_value(value: Option<&Value>) -> Option<Self> {
-        Some(match value {
-            None => Self::default(),
-            Some(Value::Array(a)) => Publish::Registry { is_empty: a.is_empty() },
-            Some(Value::Boolean(b)) => Publish::Flag(*b),
-            Some(_) => return None,
+    fn from_table(table: &Table) -> ParseResult<Self> {
+        let package = table.get("package").and_then(Value::as_table).ok_or("package")?;
+        let _name = package.get("name").and_then(Value::as_str).ok_or("name")?;
+
+        Ok(Self {
+            // Publishing is unrestricted if `true`, and forbidden if `false` or the `Array` is empty.
+            publish: match package.get("publish") {
+                None => true,
+                Some(Value::Boolean(b)) => *b,
+                Some(Value::Array(a)) => !a.is_empty(),
+                Some(_) => return Err("publish"),
+            },
         })
     }
 }
-
-impl Default for Publish {
-    fn default() -> Self {
-        Publish::Flag(true)
-    }
-}
-
-impl PartialEq<Publish> for bool {
-    fn eq(&self, p: &Publish) -> bool {
-        match *p {
-            Publish::Flag(flag) => flag == *self,
-            Publish::Registry { is_empty } => is_empty != *self,
-        }
-    }
-}
-
-impl PartialEq<bool> for Publish {
-    fn eq(&self, b: &bool) -> bool {
-        b.eq(self)
-    }
-}
-
-fn into_string(value: Value) -> Option<String> {
-    if let Value::String(string) = value { Some(string) } else { None }
-}