Allow binary-only dependencies in metadata

A number of projects, such as https://github.com/google/cargo-raze
consume the output of `cargo metadata` when generating input to other
build systems, such as Bazel and Buck. Typically, these projects use a
`Cargo.toml` file as the input to drive their work, and it can be useful
to express to these systems that a binary is required as part of the
build.

If such a dependency happens to have a lib target, sufficient dependency
information happens to be exposed via the resolve field to recreate the
dependency structure adequately.

However, for binary-only targets, that entire branch of the dependency
tree is cut off, making it hard to recreate this information.

This PR adds an option to `cargo metadata` to allow rendering these
subtrees, describing these deps as of kind "binary". This isn't really a
concept in cargo-proper, but may become one via
rust-lang/rfcs#3168 and/or
rust-lang/rfcs#3028 - this output format is
forwards-compatible with these proposals.

In an RFC 3028 world, binary deps would appear as `dep_kind` "binary",
and `cdylib` or `staticlib` deps would appear as new `DepKind` variants.
We'd probably add a new value of the flag, `declared`, and set that to
be the default. We may also want to deprecate the
`include-if-no-library-dep` value, and make these ignored binary deps a
hard error (replacing the existing warning log).

In an RFC 3168 world, there's an interesting metadata question to have -
there are (at least) three reasonable options in terms of handling
metadata:
1. Require a direct dependency on any package whose binary deps are
   being used from the crate which uses them - this gives a nicely
   restricted search space, and allows for nicely curated metadata
   output.
2. Allow any transitive package dependencies to be used as binary deps -
   this may significantly expand the output produced, but would model
   the implementation of the feature.
3. Require explicitly tagging binary deps as being used as binary deps -
   this looks a lot like RFC 3028, and would tend in that direction.

Author: illicitonion

benches/benchsuite/benches/resolve.rs

@@ -2,7 +2,7 @@ use cargo::core::compiler::{CompileKind, RustcTargetData};
 use cargo::core::resolver::features::{CliFeatures, FeatureOpts, FeatureResolver, ForceAllTargets};
 use cargo::core::resolver::{HasDevUnits, ResolveBehavior};
 use cargo::core::{PackageIdSpec, Workspace};
-use cargo::ops::WorkspaceResolve;
+use cargo::ops::{BinaryOnlyDepsBehavior, WorkspaceResolve};
 use cargo::Config;
 use criterion::{criterion_group, criterion_main, Criterion};
 use std::fs;
@@ -198,6 +198,7 @@ fn do_resolve<'cfg>(config: &'cfg Config, ws_root: &Path) -> ResolveInfo<'cfg> {
         &specs,
         has_dev_units,
         force_all_targets,
+        BinaryOnlyDepsBehavior::Ignore,
     )
     .unwrap();
     ResolveInfo {
@@ -272,6 +273,7 @@ fn resolve_ws(c: &mut Criterion) {
                     specs,
                     *has_dev_units,
                     *force_all_targets,
+                    BinaryOnlyDepsBehavior::Ignore,
                 )
                 .unwrap();
             })

src/bin/cargo/commands/metadata.rs

@@ -1,5 +1,6 @@
 use crate::command_prelude::*;
-use cargo::ops::{self, OutputMetadataOptions};
+use anyhow::anyhow;
+use cargo::ops::{self, BinaryDepsMode, OutputMetadataOptions};
 
 pub fn cli() -> App {
     subcommand("metadata")
@@ -26,6 +27,11 @@ pub fn cli() -> App {
                 .value_name("VERSION")
                 .possible_value("1"),
         )
+        .arg(
+            opt("binary-deps", "How to treat binary dependencies")
+                .possible_values(&["include-if-no-library-dep", "ignore"])
+                .default_value("ignore"),
+        )
         .after_help("Run `cargo help metadata` for more detailed information.\n")
 }
 
@@ -43,11 +49,30 @@ pub fn exec(config: &mut Config, args: &ArgMatches<'_>) -> CliResult {
         Some(version) => version.parse().unwrap(),
     };
 
+    let binary_deps = {
+        match args
+            .value_of("binary-deps")
+            .unwrap()
+            .to_ascii_lowercase()
+            .as_str()
+        {
+            "include-if-no-library-dep" => BinaryDepsMode::IncludeIfNoLibraryDep,
+            "ignore" => BinaryDepsMode::Ignore,
+            s => {
+                return Err(CliError::new(
+                    anyhow!("invalid binary-deps specifier: `{}`", s),
+                    1,
+                ))
+            }
+        }
+    };
+
     let options = OutputMetadataOptions {
         cli_features: args.cli_features()?,
         no_deps: args.is_present("no-deps"),
         filter_platforms: args._values_of("filter-platform"),
         version,
+        binary_deps,
     };
 
     let result = ops::output_metadata(&ws, &options)?;

src/cargo/core/compiler/standard_lib.rs

@@ -118,6 +118,7 @@ pub fn resolve_std<'cfg>(
         &specs,
         HasDevUnits::No,
         crate::core::resolver::features::ForceAllTargets::No,
+        ops::BinaryOnlyDepsBehavior::Warn,
     )?;
     Ok((
         resolve.pkg_set,

src/cargo/core/package.rs

@@ -569,6 +569,7 @@ impl<'cfg> PackageSet<'cfg> {
         requested_kinds: &[CompileKind],
         target_data: &RustcTargetData<'_>,
         force_all_targets: ForceAllTargets,
+        treat_binary_only_as_lib: bool,
     ) -> BTreeMap<PackageId, Vec<&Package>> {
         root_ids
             .iter()
@@ -583,7 +584,11 @@ impl<'cfg> PackageSet<'cfg> {
                 )
                 .filter_map(|package_id| {
                     if let Ok(dep_pkg) = self.get_one(package_id) {
-                        if !dep_pkg.targets().iter().any(|t| t.is_lib()) {
+                        if !dep_pkg
+                            .targets()
+                            .iter()
+                            .any(|t| t.is_lib() || (treat_binary_only_as_lib && t.is_bin()))
+                        {
                             Some(dep_pkg)
                         } else {
                             None

src/cargo/ops/cargo_compile.rs

@@ -377,6 +377,7 @@ pub fn create_bcx<'a, 'cfg>(
         &specs,
         has_dev_units,
         crate::core::resolver::features::ForceAllTargets::No,
+        ops::BinaryOnlyDepsBehavior::Warn,
     )?;
     let WorkspaceResolve {
         mut pkg_set,

src/cargo/ops/cargo_output_metadata.rs

@@ -7,7 +7,7 @@ use crate::ops::{self, Packages};
 use crate::util::interning::InternedString;
 use crate::util::CargoResult;
 use cargo_platform::Platform;
-use serde::Serialize;
+use serde::{Serialize, Serializer};
 use std::collections::BTreeMap;
 use std::path::PathBuf;
 
@@ -18,6 +18,7 @@ pub struct OutputMetadataOptions {
     pub no_deps: bool,
     pub version: u32,
     pub filter_platforms: Vec<String>,
+    pub binary_deps: BinaryDepsMode,
 }
 
 /// Loads the manifest, resolves the dependencies of the package to the concrete
@@ -88,19 +89,52 @@ struct Dep {
 
 #[derive(Serialize, PartialEq, Eq, PartialOrd, Ord)]
 struct DepKindInfo {
-    kind: DepKind,
+    kind: DepKindIncludingBinaryDeps,
     target: Option<Platform>,
 }
 
 impl From<&Dependency> for DepKindInfo {
     fn from(dep: &Dependency) -> DepKindInfo {
         DepKindInfo {
-            kind: dep.kind(),
+            kind: dep.kind().into(),
             target: dep.platform().cloned(),
         }
     }
 }
 
+#[derive(PartialEq, Eq, PartialOrd, Ord)]
+enum DepKindIncludingBinaryDeps {
+    Normal,
+    Development,
+    Build,
+    Binary,
+}
+
+impl From<DepKind> for DepKindIncludingBinaryDeps {
+    fn from(dep_kind: DepKind) -> Self {
+        match dep_kind {
+            DepKind::Normal => DepKindIncludingBinaryDeps::Normal,
+            DepKind::Development => DepKindIncludingBinaryDeps::Development,
+            DepKind::Build => DepKindIncludingBinaryDeps::Build,
+        }
+    }
+}
+
+impl Serialize for DepKindIncludingBinaryDeps {
+    fn serialize<S>(&self, s: S) -> Result<S::Ok, S::Error>
+    where
+        S: Serializer,
+    {
+        match *self {
+            DepKindIncludingBinaryDeps::Normal => None,
+            DepKindIncludingBinaryDeps::Development => Some("dev"),
+            DepKindIncludingBinaryDeps::Build => Some("build"),
+            DepKindIncludingBinaryDeps::Binary => Some("binary"),
+        }
+        .serialize(s)
+    }
+}
+
 /// Builds the resolve graph as it will be displayed to the user.
 fn build_resolve_graph(
     ws: &Workspace<'_>,
@@ -119,6 +153,11 @@ fn build_resolve_graph(
         crate::core::resolver::features::ForceAllTargets::No
     };
 
+    let binary_only_deps_behavior = match metadata_opts.binary_deps {
+        BinaryDepsMode::Ignore => ops::BinaryOnlyDepsBehavior::Warn,
+        BinaryDepsMode::IncludeIfNoLibraryDep => ops::BinaryOnlyDepsBehavior::Ignore,
+    };
+
     // Note that even with --filter-platform we end up downloading host dependencies as well,
     // as that is the behavior of download_accessible.
     let ws_resolve = ops::resolve_ws_with_opts(
@@ -129,6 +168,7 @@ fn build_resolve_graph(
         &specs,
         HasDevUnits::Yes,
         force_all,
+        binary_only_deps_behavior,
     )?;
 
     let package_map: BTreeMap<PackageId, Package> = ws_resolve
@@ -149,6 +189,7 @@ fn build_resolve_graph(
             &package_map,
             &target_data,
             &requested_kinds,
+            metadata_opts.binary_deps,
         );
     }
     // Get a Vec of Packages.
@@ -166,13 +207,20 @@ fn build_resolve_graph(
     Ok((actual_packages, mr))
 }
 
+#[derive(Clone, Copy)]
+pub enum BinaryDepsMode {
+    IncludeIfNoLibraryDep,
+    Ignore,
+}
+
 fn build_resolve_graph_r(
     node_map: &mut BTreeMap<PackageId, MetadataResolveNode>,
     pkg_id: PackageId,
     resolve: &Resolve,
     package_map: &BTreeMap<PackageId, Package>,
     target_data: &RustcTargetData<'_>,
     requested_kinds: &[CompileKind],
+    binary_deps: BinaryDepsMode,
 ) {
     if node_map.contains_key(&pkg_id) {
         return;
@@ -206,14 +254,42 @@ fn build_resolve_graph_r(
                 })
             }
         })
-        .filter_map(|(dep_id, deps)| {
-            let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
-            dep_kinds.sort();
+        .flat_map(|(dep_id, deps)| {
             package_map
                 .get(&dep_id)
-                .and_then(|pkg| pkg.targets().iter().find(|t| t.is_lib()))
-                .and_then(|lib_target| resolve.extern_crate_name(pkg_id, dep_id, lib_target).ok())
-                .map(|name| Dep {
+                .into_iter()
+                .flat_map(move |pkg| {
+                    if let Some(lib_target) = pkg.targets().iter().find(|t| t.is_lib()) {
+                        let mut dep_kinds: Vec<_> = deps.iter().map(DepKindInfo::from).collect();
+                        dep_kinds.sort();
+                        vec![(lib_target, dep_kinds)]
+                    } else {
+                        match binary_deps {
+                            BinaryDepsMode::IncludeIfNoLibraryDep => pkg
+                                .targets()
+                                .iter()
+                                .filter(|t| t.is_bin())
+                                .map(|t| {
+                                    (
+                                        t,
+                                        vec![DepKindInfo {
+                                            kind: DepKindIncludingBinaryDeps::Binary,
+                                            target: None,
+                                        }],
+                                    )
+                                })
+                                .collect(),
+                            BinaryDepsMode::Ignore => vec![],
+                        }
+                    }
+                })
+                .filter_map(move |(lib_target, dep_kinds)| {
+                    resolve
+                        .extern_crate_name(pkg_id, dep_id, lib_target)
+                        .ok()
+                        .map(|crate_name| (crate_name, dep_kinds))
+                })
+                .map(move |(name, dep_kinds)| Dep {
                     name,
                     pkg: normalize_id(dep_id),
                     dep_kinds,
@@ -237,6 +313,7 @@ fn build_resolve_graph_r(
             package_map,
             target_data,
             requested_kinds,
+            binary_deps,
         );
     }
 }

src/cargo/ops/fix.rs

@@ -252,6 +252,7 @@ fn check_resolver_change(ws: &Workspace<'_>, opts: &FixOptions) -> CargoResult<(
             &specs,
             has_dev_units,
             crate::core::resolver::features::ForceAllTargets::No,
+            ops::BinaryOnlyDepsBehavior::Warn,
         )?;
 
         let feature_opts = FeatureOpts::new_behavior(ResolveBehavior::V2, has_dev_units);

src/cargo/ops/mod.rs

@@ -12,7 +12,9 @@ pub use self::cargo_generate_lockfile::update_lockfile;
 pub use self::cargo_generate_lockfile::UpdateOptions;
 pub use self::cargo_install::{install, install_list};
 pub use self::cargo_new::{init, new, NewOptions, VersionControl};
-pub use self::cargo_output_metadata::{output_metadata, ExportInfo, OutputMetadataOptions};
+pub use self::cargo_output_metadata::{
+    output_metadata, BinaryDepsMode, ExportInfo, OutputMetadataOptions,
+};
 pub use self::cargo_package::{package, package_one, PackageOpts};
 pub use self::cargo_pkgid::pkgid;
 pub use self::cargo_read_manifest::{read_package, read_packages};
@@ -28,7 +30,7 @@ pub use self::registry::{needs_custom_http_transport, registry_login, registry_l
 pub use self::registry::{publish, registry_configuration, RegistryConfig};
 pub use self::resolve::{
     add_overrides, get_resolved_packages, resolve_with_previous, resolve_ws, resolve_ws_with_opts,
-    WorkspaceResolve,
+    BinaryOnlyDepsBehavior, WorkspaceResolve,
 };
 pub use self::vendor::{vendor, VendorOptions};
 

src/cargo/ops/resolve.rs

@@ -69,6 +69,12 @@ pub fn resolve_ws<'a>(ws: &Workspace<'a>) -> CargoResult<(PackageSet<'a>, Resolv
     Ok((packages, resolve))
 }
 
+#[derive(Eq, PartialEq)]
+pub enum BinaryOnlyDepsBehavior {
+    Warn,
+    Ignore,
+}
+
 /// Resolves dependencies for some packages of the workspace,
 /// taking into account `paths` overrides and activated features.
 ///
@@ -87,6 +93,7 @@ pub fn resolve_ws_with_opts<'cfg>(
     specs: &[PackageIdSpec],
     has_dev_units: HasDevUnits,
     force_all_targets: ForceAllTargets,
+    binary_only_deps_behaviour: BinaryOnlyDepsBehavior,
 ) -> CargoResult<WorkspaceResolve<'cfg>> {
     let mut registry = PackageRegistry::new(ws.config())?;
     let mut add_patches = true;
@@ -178,6 +185,7 @@ pub fn resolve_ws_with_opts<'cfg>(
         requested_targets,
         target_data,
         force_all_targets,
+        binary_only_deps_behaviour == BinaryOnlyDepsBehavior::Ignore,
     );
     for (pkg_id, dep_pkgs) in no_lib_pkgs {
         for dep_pkg in dep_pkgs {

src/cargo/ops/tree/mod.rs

@@ -158,6 +158,7 @@ pub fn build_and_print(ws: &Workspace<'_>, opts: &TreeOptions) -> CargoResult<()
         &specs,
         has_dev,
         force_all,
+        ops::BinaryOnlyDepsBehavior::Warn,
     )?;
 
     let package_map: HashMap<PackageId, &Package> = ws_resolve