refactor: include review comments and docs

README.md

@@ -5,7 +5,14 @@ However, it doesn't offer any filtering; today cargo includes
 all platforms, but some projects only care about Linux
 for example.
 
-More information: https://github.com/rust-lang/cargo/issues/7058
+More information: <https://github.com/rust-lang/cargo/issues/7058>
+
+Additionally some projects are not interested by vendoring test code
+or development dependencies of used crates and these filters
+are also not supported with no development planned yet.
+
+More information: <https://github.com/rust-lang/cargo/issues/13474>
+or <https://github.com/rust-lang/cargo/issues/7065>
 
 ## Generating a vendor/ directory with filtering
 
@@ -26,13 +33,15 @@ $ cargo vendor-filterer --tier=2
 Currently this will drop out crates such as `redox_syscall`.
 
 You can also declaratively specify the desired vendor configuration via the [Cargo metadata](https://doc.rust-lang.org/cargo/reference/manifest.html#the-metadata-table)
-key `package.metadata.vendor-filter`.  In this example, we include only tier 1 and 2 Linux platforms, and additionally remove some vendored C sources and `tests` folders from all crates:
+key `package.metadata.vendor-filter`.  In this example, we include only tier 1 and 2 Linux platforms, and additionally remove some vendored C sources, `tests` folders
+and development dependencies from all crates:
 
 ```toml
 [package.metadata.vendor-filter]
 platforms = ["*-unknown-linux-gnu"]
 tier = "2"
 all-features = true
+keep_dep_kinds = "no-dev"
 exclude-crate-paths = [ { name = "curl-sys", exclude = "curl" },
                         { name = "libz-sys", exclude = "src/zlib" },
                         { name = "libz-sys", exclude = "src/smoke.c" },
@@ -51,6 +60,8 @@ key `workspace.metadata.vendor-filter`.
   and `*` wildcards are supported.  For example, `*-unknown-linux-gnu`.
 - `tier`: This can be either "1" or "2".  It may be specified in addition to `platforms`.
 - `all-features`: Enable all features of the current crate when vendoring.
+- `keep_dep_kinds`: Specify which dependencies kinds to keep.
+  Can be one of: all, normal, build, dev, no-normal, no-build, no-dev
 - `exclude-crate-paths`: Remove files and directories from target crates.  A key
   use case for this is removing the vendored copy of C libraries embedded in
   crates like `libz-sys`, when you only want to support dynamically linking.

src/dep_kinds.rs → src/dep_kinds_filtering.rs

@@ -1,5 +1,5 @@
 use crate::{Args, VendorFilter};
-use anyhow::Result;
+use anyhow::{Context, Result};
 use camino::Utf8Path;
 use clap::{builder::PossibleValue, ValueEnum};
 use serde::{Deserialize, Serialize};
@@ -8,7 +8,7 @@ use std::{
     collections::{HashMap, HashSet},
 };
 
-/// Kind of dependencies that shall be included.
+/// Kinds of dependencies that shall be included.
 #[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)]
 #[serde(rename_all = "kebab-case")]
 pub enum DepKinds {
@@ -59,8 +59,8 @@ impl std::fmt::Display for DepKinds {
 /// Filter out unwanted dependency kinds.
 ///
 /// Replicates logic from add_packages_for_platform() but uses cargo tree
-/// because cargo metadata does not implement dependency kind filtering.
-/// Ref: <https://github.com/rust-lang/cargo/issues/7065>
+/// because cargo metadata does not implement dependency kinds filtering.
+/// Ref: <https://github.com/rust-lang/cargo/issues/10718>
 /// Cargo tree is NOT intended for automatic processing so this function
 /// explicitly does not replace the add_packages_for_platform() entirely.
 pub(crate) fn filter_dep_kinds(
@@ -69,8 +69,8 @@ pub(crate) fn filter_dep_kinds(
     packages: &mut HashMap<cargo_metadata::PackageId, &cargo_metadata::Package>,
     platform: Option<&str>,
 ) -> Result<()> {
-    // exit early when no dependency kind filtering is requested
-    match config.dep_kinds {
+    // exit early when no dependency kinds filtering is requested
+    match config.keep_dep_kinds {
         None | Some(DepKinds::All) => return Ok(()),
         Some(_) => (),
     };
@@ -98,14 +98,14 @@ fn get_required_packages<'a>(
     config: &VendorFilter,
     platform: Option<&str>,
 ) -> Result<HashSet<(Cow<'a, str>, Cow<'a, cargo_metadata::semver::Version>)>> {
-    let dep_kinds = config.dep_kinds.expect("dep_kinds not set");
+    let keep_dep_kinds = config.keep_dep_kinds.expect("keep_dep_kinds not set");
     let mut required_packages = HashSet::new();
     for manifest_path in manifest_paths {
         let mut cargo_tree = std::process::Command::new("cargo");
         cargo_tree
             .arg("tree")
             .args(["--quiet", "--prefix", "none"]) // ignore non-relevant output
-            .args(["--edges", &dep_kinds.to_string()]); // key filter not available with metadata
+            .args(["--edges", &keep_dep_kinds.to_string()]); // key filter not available with metadata
         if offline {
             cargo_tree.arg("--offline");
         }
@@ -142,16 +142,18 @@ fn get_required_packages<'a>(
                 anyhow::bail!("Incomplete output {line} received from cargo tree");
             }
             let package = tokens[0].to_string();
+            if tokens[1].len() < 5 || tokens[1].contains("feature") {
+                continue; // skip invalid entries and "feature" list
+            }
             // need to remove the initial "v" character that the cargo tree is printing in package name
             // Ref: <https://doc.rust-lang.org/cargo/commands/cargo-tree.html>
             // The PR requesting the v to be removed (or configurable) was closed:
             // <https://github.com/rust-lang/cargo/issues/13120>
-            if tokens[1].len() < 5 || tokens[1].contains("feature") {
-                continue; // skip invalid entries and "feature" list
-            }
-            let version = &tokens[1][1..tokens[1].len()];
+            let version = tokens[1]
+                .strip_prefix("v")
+                .with_context(|| format!("Invalid version: {}", tokens[1]))?;
             let version = cargo_metadata::semver::Version::parse(version)
-                .unwrap_or_else(|_| panic!("Cannot parse version {version} for {package}"));
+                .with_context(|| format!("Cannot parse version {version} for {package}"))?;
             required_packages.insert((Cow::Owned(package), Cow::Owned(version)));
         }
     }
@@ -171,7 +173,7 @@ mod tests {
         let rp = get_required_packages(
             &vec![Some(&own_cargo_toml)],
             false,
-            &serde_json::from_value(json!({ "dep-kinds": "dev"})).unwrap(),
+            &serde_json::from_value(json!({ "keep-dep-kinds": "dev"})).unwrap(),
             Some("x86_64-pc-windows-gnu"),
         );
         match rp {
@@ -187,8 +189,9 @@ mod tests {
         let rp = get_required_packages(
             &vec![Some(&own_cargo_toml)],
             false,
-            &serde_json::from_value(json!({ "dep-kinds": "all", "--all-features": true})).unwrap(),
-            None,
+            &serde_json::from_value(json!({ "keep-dep-kinds": "all", "--all-features": true}))
+                .unwrap(),
+            None, // all platforms
         );
         match rp {
             Ok(rp) => assert!(rp.len() > 90), // all features, all platforms list is long
@@ -204,25 +207,55 @@ mod tests {
         let rp_normal = get_required_packages(
             &vec![Some(&own_cargo_toml)],
             false,
-            &serde_json::from_value(json!({ "dep-kinds": "normal"})).unwrap(),
+            &serde_json::from_value(json!({ "keep-dep-kinds": "normal"})).unwrap(),
             Some("x86_64-pc-windows-gnu"),
         );
 
-        // no-build => normal + dev dependencies, so including once_call and serial_test
+        // no-build => normal + dev dependencies, so including once_call, serial_test...
         let rp_no_build = get_required_packages(
             &vec![Some(&own_cargo_toml)],
             false,
-            &serde_json::from_value(json!({ "dep-kinds": "no-build"})).unwrap(),
+            &serde_json::from_value(json!({ "keep-dep-kinds": "no-build"})).unwrap(),
             Some("x86_64-pc-windows-gnu"),
         );
 
         // if once_cell is also a normal dependency, it is not removed from the list
         match (rp_normal, rp_no_build) {
-            (Ok(rp_normal), Ok(rp_all)) => assert!(
-                rp_normal.len() < rp_all.len(),
+            (Ok(rp_normal), Ok(rp_no_build)) => assert!(
+                rp_normal.len() < rp_no_build.len(),
                 "Filtering does not work. Got {} normal and {} no-build dependencies",
                 rp_normal.len(),
-                rp_all.len()
+                rp_no_build.len()
+            ),
+            _ => panic!("One of get_required_packages() calls failed"),
+        }
+    }
+
+    #[test]
+    fn test_dep_kind_build_vs_no_dev() {
+        let mut own_cargo_toml = Utf8PathBuf::from(env!("CARGO_MANIFEST_DIR"));
+        own_cargo_toml.push("Cargo.toml");
+
+        let rp_build = get_required_packages(
+            &vec![Some(&own_cargo_toml)],
+            false,
+            &serde_json::from_value(json!({ "keep-dep-kinds": "normal"})).unwrap(),
+            Some("x86_64-unknown-linux-gnu"),
+        );
+
+        // no-dev => build + normal so the list shall be larger
+        let rp_no_dev = get_required_packages(
+            &vec![Some(&own_cargo_toml)],
+            false,
+            &serde_json::from_value(json!({ "keep-dep-kinds": "no-dev"})).unwrap(),
+            Some("x86_64-unknown-linux-gnu"),
+        );
+        match (rp_build, rp_no_dev) {
+            (Ok(rp_build), Ok(rp_no_dev)) => assert!(
+                rp_build.len() < rp_no_dev.len(),
+                "Filtering does not work. Got {} build and {} no-dev dependencies",
+                rp_build.len(),
+                rp_no_dev.len()
             ),
             _ => panic!("One of get_required_packages() calls failed"),
         }

src/main.rs

@@ -16,7 +16,7 @@ use std::vec;
 
 use cargo_vendor_filterer::*;
 
-mod dep_kinds;
+mod dep_kinds_filtering;
 mod tiers;
 
 /// This is the .cargo-checksum.json in a crate/package.
@@ -132,7 +132,7 @@ struct VendorFilter {
     #[serde(skip_serializing_if = "Vec::is_empty", default)]
     features: Vec<String>,
     exclude_crate_paths: Option<HashSet<CrateExclude>>,
-    dep_kinds: Option<dep_kinds::DepKinds>,
+    keep_dep_kinds: Option<dep_kinds_filtering::DepKinds>,
 }
 
 #[derive(Parser, Debug)]
@@ -181,11 +181,11 @@ struct Args {
     #[arg(long, short = 'F')]
     features: Vec<String>,
 
-    /// Kinds of dependencies to keep (all, normal, build, dev, no-normal, no-build, no-dev)
-    ///
-    /// Syntax based on edges option of cargo tree.
+    /// Dependencies kinds you want to keep: normal, build and/or development (dev).
+    /// Possible values: all (default), normal, build, dev, no-normal, no-build, no-dev
+    /// Ref: <https://doc.rust-lang.org/cargo/reference/specifying-dependencies.html>
     #[arg(long)]
-    dep_kinds: Option<dep_kinds::DepKinds>,
+    keep_dep_kinds: Option<dep_kinds_filtering::DepKinds>,
 
     /// Pick the output format.
     #[arg(long, default_value = "dir")]
@@ -346,7 +346,7 @@ impl VendorFilter {
             && !args.no_default_features
             && args.features.is_empty()
             && args.exclude_crate_path.is_none()
-            && args.dep_kinds.is_none();
+            && args.keep_dep_kinds.is_none();
         let exclude_crate_paths = args
             .exclude_crate_path
             .as_ref()
@@ -366,7 +366,7 @@ impl VendorFilter {
             no_default_features: args.no_default_features,
             features: args.features.clone(),
             exclude_crate_paths,
-            dep_kinds: args.dep_kinds,
+            keep_dep_kinds: args.keep_dep_kinds,
         });
         Ok(r)
     }
@@ -872,12 +872,12 @@ fn run() -> Result<()> {
                 &mut packages,
                 Some(platform),
             )?;
-            dep_kinds::filter_dep_kinds(&args, &config, &mut packages, Some(platform))?;
+            dep_kinds_filtering::filter_dep_kinds(&args, &config, &mut packages, Some(platform))?;
         }
         expanded_platforms = Some(platforms);
     } else {
         add_packages_for_platform(&args, &config, &all_packages, &mut packages, None)?;
-        dep_kinds::filter_dep_kinds(&args, &config, &mut packages, None)?;
+        dep_kinds_filtering::filter_dep_kinds(&args, &config, &mut packages, None)?;
     }
 
     // Run `cargo vendor` which will capture all dependencies.
@@ -951,8 +951,8 @@ fn run() -> Result<()> {
     } else if let Some(platforms) = expanded_platforms {
         eprintln!("Filtered to target platforms: {:?}", platforms);
     }
-    if let Some(dep_kinds) = config.dep_kinds {
-        eprintln!("Filtered to dependency kinds: {dep_kinds}");
+    if let Some(keep_dep_kinds) = config.keep_dep_kinds {
+        eprintln!("Filtered to dependency kinds: {keep_dep_kinds}");
     }
 
     eprintln!("Generated: {final_output_path}");
@@ -976,12 +976,12 @@ fn test_parse_config() {
     let valid = vec![
         json!({}),
         json!({ "platforms": ["aarch64-unknown-linux-gnu"]}),
-        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "dep-kinds": "all"}),
-        json!({ "platforms": ["*-unknown-linux-gnu"], "tier": "2", "no-default-features": false, "dep-kinds": "normal"}),
-        json!({ "platforms": ["*-unknown-linux-gnu"], "tier": "Two", "no-default-features": false, "dep-kinds": "build"}),
-        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "all-features": true, "no-default-features": false, "dep-kinds": "dev"}),
-        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "dep-kinds": "no-build"}),
-        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "features": ["first-feature", "second-feature"], "dep-kinds": "no-build"}),
+        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "keep-dep-kinds": "all"}),
+        json!({ "platforms": ["*-unknown-linux-gnu"], "tier": "2", "no-default-features": false, "keep-dep-kinds": "normal"}),
+        json!({ "platforms": ["*-unknown-linux-gnu"], "tier": "Two", "no-default-features": false, "keep-dep-kinds": "build"}),
+        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "all-features": true, "no-default-features": false, "keep-dep-kinds": "dev"}),
+        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "keep-dep-kinds": "no-build"}),
+        json!({ "platforms": ["aarch64-unknown-linux-gnu"], "no-default-features": true, "features": ["first-feature", "second-feature"], "keep-dep-kinds": "no-build"}),
     ];
     for case in valid {
         let _: VendorFilter = serde_json::from_value(case).unwrap();

tests/vendor_filterer/common.rs

@@ -69,7 +69,7 @@ pub(crate) struct VendorOptions<'a, 'b, 'c, 'd, 'e> {
     pub manifest_path: Option<&'d Utf8Path>,
     pub sync: Vec<&'e Utf8Path>,
     pub versioned_dirs: bool,
-    pub dep_kinds: Option<&'static str>,
+    pub keep_dep_kinds: Option<&'static str>,
 }
 
 /// Run a vendoring process
@@ -106,8 +106,8 @@ pub(crate) fn vendor(options: VendorOptions) -> Result<Output> {
     for s in options.sync {
         cmd.arg(format!("--sync={s}"));
     }
-    if let Some(dep_kinds) = options.dep_kinds {
-        cmd.args(["--dep-kinds", dep_kinds]);
+    if let Some(keep_dep_kinds) = options.keep_dep_kinds {
+        cmd.args(["--keep-dep-kinds", keep_dep_kinds]);
     }
     if let Some(output) = options.output {
         cmd.arg(output);

tests/vendor_filterer/exclude.rs

@@ -28,7 +28,7 @@ fn windows_with_dep_kind_filter_normal() {
     let output = vendor(VendorOptions {
         output: Some(&test_folder),
         platforms: Some(&["x86_64-pc-windows-gnu"]),
-        dep_kinds: Some("normal"),
+        keep_dep_kinds: Some("normal"),
         ..Default::default()
     })
     .unwrap();