Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow binary-only dependencies in metadata #9974

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion benches/benchsuite/benches/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -272,6 +273,7 @@ fn resolve_ws(c: &mut Criterion) {
specs,
*has_dev_units,
*force_all_targets,
BinaryOnlyDepsBehavior::Ignore,
)
.unwrap();
})
Expand Down
27 changes: 26 additions & 1 deletion src/bin/cargo/commands/metadata.rs
Original file line number Diff line number Diff line change
@@ -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")
Expand All @@ -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")
}

Expand All @@ -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)?;
Expand Down
1 change: 1 addition & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
7 changes: 6 additions & 1 deletion src/cargo/core/package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/cargo_compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
95 changes: 86 additions & 9 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down Expand Up @@ -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<'_>,
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -149,6 +189,7 @@ fn build_resolve_graph(
&package_map,
&target_data,
&requested_kinds,
metadata_opts.binary_deps,
);
}
// Get a Vec of Packages.
Expand All @@ -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;
Expand Down Expand Up @@ -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,
Expand All @@ -237,6 +313,7 @@ fn build_resolve_graph_r(
package_map,
target_data,
requested_kinds,
binary_deps,
);
}
}
1 change: 1 addition & 0 deletions src/cargo/ops/fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
6 changes: 4 additions & 2 deletions src/cargo/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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};

Expand Down
8 changes: 8 additions & 0 deletions src/cargo/ops/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
///
Expand All @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/cargo/ops/tree/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading