Skip to content

Commit

Permalink
[guppy] add support for evaluating against the union and intersection…
Browse files Browse the repository at this point in the history
… of platforms

What hakari really needs is a way to figure out which platforms are
included on all platforms. Add a `PlatformSpec` abstraction that can
represent this, as well as a single platform.
  • Loading branch information
sunshowers committed Sep 29, 2021
1 parent f189aa8 commit edf08c6
Show file tree
Hide file tree
Showing 23 changed files with 556 additions and 311 deletions.
7 changes: 0 additions & 7 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 13 additions & 19 deletions cargo-guppy/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,15 @@

//! Implementations for options shared by commands.

use anyhow::{anyhow, ensure};
use anyhow::{anyhow, ensure, Context};
use clap::arg_enum;
use guppy::{
graph::{
DependencyDirection, DependencyReq, EnabledTernary, PackageGraph, PackageLink, PackageQuery,
},
PackageId, Platform, TargetFeatures,
PackageId,
};
use guppy_cmdlib::string_to_platform_spec;
use std::collections::HashSet;
use structopt::StructOpt;

Expand Down Expand Up @@ -113,7 +114,7 @@ pub struct FilterOptions {
pub include_build: bool,

#[structopt(long)]
/// Target to filter, default is to match all targets
/// Target to filter, "current", "any" or "always" [default: any]
pub target: Option<String>,
}

Expand All @@ -122,34 +123,27 @@ impl FilterOptions {
pub fn make_resolver<'g>(
&'g self,
pkg_graph: &'g PackageGraph,
) -> impl Fn(&PackageQuery<'g>, PackageLink<'g>) -> bool + 'g {
) -> anyhow::Result<impl Fn(&PackageQuery<'g>, PackageLink<'g>) -> bool + 'g> {
let omitted_package_ids: HashSet<_> =
self.base_opts.omitted_package_ids(pkg_graph).collect();

let platform = self.target.clone().map(|triple_str| {
// The features are unknown.
Platform::new(triple_str, TargetFeatures::Unknown).unwrap()
});
let platform_spec = string_to_platform_spec(self.target.as_deref())
.with_context(|| "target platform isn't known")?;

move |_, link| {
let ret = move |_: &PackageQuery<'g>, link| {
// filter by the kind of dependency (--kind)
let include_kind = self.base_opts.kind.should_traverse(&link);

let include_type = if let Some(platform) = &platform {
// filter out irrelevant dependencies for a specific target (--target)
self.eval(link, |req| {
req.status().enabled_on(platform) != EnabledTernary::Disabled
})
} else {
// keep dependencies that are potentially enabled on any platform
self.eval(link, |req| req.is_present())
};
let include_type = self.eval(link, |req| {
req.status().enabled_on(&platform_spec.clone()) != EnabledTernary::Disabled
});

// filter out provided edge targets (--omit-edges-into)
let include_edge = !omitted_package_ids.contains(link.to().id());

include_kind && include_type && include_edge
}
};
Ok(ret)
}

/// Select normal, dev, or build dependencies as requested (--include-build, --include-dev), and
Expand Down
16 changes: 8 additions & 8 deletions cargo-guppy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use guppy::{
PackageId,
};
use guppy_cmdlib::{
triple_to_platform, CargoMetadataOptions, CargoResolverOpts, PackagesAndFeatures,
string_to_platform_spec, CargoMetadataOptions, CargoResolverOpts, PackagesAndFeatures,
};
use std::{
borrow::Cow,
Expand Down Expand Up @@ -137,7 +137,7 @@ pub fn cmd_dups(opts: &DupsOptions) -> Result<(), anyhow::Error> {
let command = opts.metadata_opts.make_command();
let pkg_graph = command.build_graph()?;

let resolver = opts.filter_opts.make_resolver(&pkg_graph);
let resolver = opts.filter_opts.make_resolver(&pkg_graph)?;
let selection = pkg_graph.query_workspace();

let mut dupe_map: HashMap<_, Vec<_>> = HashMap::new();
Expand Down Expand Up @@ -204,8 +204,8 @@ pub struct ResolveCargoOptions {
}

pub fn cmd_resolve_cargo(opts: &ResolveCargoOptions) -> Result<(), anyhow::Error> {
let target_platform = triple_to_platform(opts.target_platform.as_deref(), || None)?;
let host_platform = triple_to_platform(opts.host_platform.as_deref(), || None)?;
let target_platform = string_to_platform_spec(opts.target_platform.as_deref())?;
let host_platform = string_to_platform_spec(opts.host_platform.as_deref())?;
let command = opts.metadata_opts.make_command();
let pkg_graph = command.build_graph()?;

Expand All @@ -214,8 +214,8 @@ pub fn cmd_resolve_cargo(opts: &ResolveCargoOptions) -> Result<(), anyhow::Error
.set_include_dev(opts.resolver_opts.include_dev)
.set_resolver(opts.resolver_opts.resolver_version)
.set_initials_platform(opts.resolver_opts.initials_platform)
.set_target_platform(target_platform.as_ref())
.set_host_platform(host_platform.as_ref())
.set_target_platform(target_platform)
.set_host_platform(host_platform)
.add_omitted_packages(opts.base_filter_opts.omitted_package_ids(&pkg_graph));

let (initials, features_only) = opts.pf.make_feature_sets(&pkg_graph)?;
Expand Down Expand Up @@ -323,7 +323,7 @@ pub fn cmd_select(options: &CmdSelectOptions) -> Result<(), anyhow::Error> {
let pkg_graph = command.build_graph()?;

let query = options.query_opts.apply(&pkg_graph)?;
let resolver = options.filter_opts.make_resolver(&pkg_graph);
let resolver = options.filter_opts.make_resolver(&pkg_graph)?;
let package_set = query.resolve_with_fn(resolver);

for package_id in package_set.package_ids(options.output_direction) {
Expand Down Expand Up @@ -370,7 +370,7 @@ pub fn cmd_subtree_size(options: &SubtreeSizeOptions) -> Result<(), anyhow::Erro
let command = options.metadata_opts.make_command();
let pkg_graph = command.build_graph()?;

let resolver = options.filter_opts.make_resolver(&pkg_graph);
let resolver = options.filter_opts.make_resolver(&pkg_graph)?;

let mut dep_cache = pkg_graph.new_depends_cache();

Expand Down
5 changes: 3 additions & 2 deletions fixtures/src/dep_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ use guppy::{
DependencyDirection, DependencyReq, PackageGraph, PackageLink, PackageLinkPtrs,
PackageMetadata, PackageQuery, PackageSet,
},
DependencyKind, Error, PackageId, Platform,
platform::PlatformSpec,
DependencyKind, Error, PackageId,
};
use pretty_assertions::assert_eq;
use std::{
Expand Down Expand Up @@ -348,7 +349,7 @@ pub(crate) fn assert_all_links(graph: &PackageGraph, direction: DependencyDirect
}

fn assert_enabled_status_is_known(req: DependencyReq<'_>, msg: &str) {
let current_platform = Platform::current().expect("current platform is known");
let current_platform = PlatformSpec::current().expect("current platform is known");
assert!(
req.status().enabled_on(&current_platform).is_known(),
"{}: enabled status known for current platform",
Expand Down
15 changes: 10 additions & 5 deletions fixtures/src/details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use guppy::{
BuildTargetId, BuildTargetKind, DependencyDirection, EnabledStatus, EnabledTernary,
PackageGraph, PackageLink, PackageMetadata, PackageSource, Workspace,
},
platform::PlatformSpec,
DependencyKind, PackageId, Platform, Version,
};
use pretty_assertions::assert_eq;
Expand Down Expand Up @@ -524,22 +525,26 @@ impl LinkDetails {
}

pub fn assert_metadata(&self, link: PackageLink<'_>, msg: &str) {
let required_enabled = |status: EnabledStatus<'_>, platform: &Platform| {
(status.required_on(platform), status.enabled_on(platform))
let required_enabled = |status: EnabledStatus<'_>, platform_spec: &PlatformSpec| {
(
status.required_on(platform_spec),
status.enabled_on(platform_spec),
)
};

for (dep_kind, platform, results) in &self.platform_results {
let platform_spec = platform.clone().into();
let req = link.req_for_kind(*dep_kind);
assert_eq!(
required_enabled(req.status(), platform),
required_enabled(req.status(), &platform_spec),
results.status,
"{}: for platform '{}', kind {}, status is correct",
msg,
platform.triple_str(),
dep_kind,
);
assert_eq!(
required_enabled(req.default_features(), platform),
required_enabled(req.default_features(), &platform_spec),
results.default_features,
"{}: for platform '{}', kind {}, default features is correct",
msg,
Expand All @@ -548,7 +553,7 @@ impl LinkDetails {
);
for (feature, status) in &results.feature_statuses {
assert_eq!(
required_enabled(req.feature_status(feature), platform),
required_enabled(req.feature_status(feature), &platform_spec),
*status,
"{}: for platform '{}', kind {}, feature '{}' has correct status",
msg,
Expand Down
20 changes: 8 additions & 12 deletions guppy-cmdlib/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use guppy::{
feature::{feature_filter, FeatureSet, StandardFeatures},
PackageGraph,
},
platform::PlatformSpec,
MetadataCommand, Platform, TargetFeatures,
};
use std::{env, path::PathBuf};
Expand Down Expand Up @@ -176,17 +177,12 @@ impl CargoMetadataOptions {
/// Parse a given triple, the string "current", or "any", into a platform.
///
/// TODO: This should eventually support JSON specs as well, probably.
pub fn triple_to_platform(
triple: Option<&str>,
default_fn: impl FnOnce() -> Option<Platform>,
) -> Result<Option<Platform>> {
match triple {
Some("current") => Ok(Some(Platform::current()?)),
Some("any") => Ok(None),
Some(triple) => Ok(Some(Platform::new(
triple.to_owned(),
TargetFeatures::Unknown,
)?)),
None => Ok(default_fn()),
pub fn string_to_platform_spec(s: Option<&str>) -> Result<PlatformSpec> {
match s {
Some("current") => Ok(PlatformSpec::current()?),
Some("always") => Ok(PlatformSpec::Always),
Some("any") => Ok(PlatformSpec::Any),
Some(triple) => Ok(Platform::new(triple.to_owned(), TargetFeatures::Unknown)?.into()),
None => Ok(PlatformSpec::Any),
}
}
1 change: 0 additions & 1 deletion guppy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ semver = "1.0.4"
serde = { version = "1.0.130", features = ["derive"] }
serde_json = "1.0.67"
smallvec = "1.6.1"
supercow = "0.1.0"
target-spec = { version = "0.8.0", path = "../target-spec" }
toml = { version = "0.5.8", optional = true }

Expand Down
40 changes: 15 additions & 25 deletions guppy/src/graph/cargo/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ use crate::{
feature::{CrossLink, FeatureQuery, FeatureSet, StandardFeatures},
DependencyDirection, EnabledTernary, PackageGraph, PackageIx, PackageLink, PackageSet,
},
platform::PlatformSpec,
sorted_set::SortedSet,
DependencyKind, Error,
};
use fixedbitset::FixedBitSet;
use petgraph::{prelude::*, visit::VisitMap};
use target_spec::Platform;

pub(super) struct CargoSetBuildState<'a> {
opts: &'a CargoOptions<'a>,
Expand Down Expand Up @@ -151,7 +151,7 @@ impl<'a> CargoSetBuildState<'a> {
let is_enabled = |feature_set: &FeatureSet<'_>,
link: &PackageLink<'_>,
kind: DependencyKind,
platform: Option<&Platform>| {
platform_spec: &PlatformSpec| {
let (from, to) = link.endpoints();
let req_status = link.req_for_kind(kind).status();
// Check the complete set to figure out whether we look at required_on or
Expand All @@ -171,15 +171,10 @@ impl<'a> CargoSetBuildState<'a> {
false
});

match (consider_optional, platform) {
(true, Some(platform)) => {
req_status.enabled_on(platform) != EnabledTernary::Disabled
}
(true, None) => req_status.enabled_on_any(),
(false, Some(platform)) => {
req_status.required_on(platform) != EnabledTernary::Disabled
}
(false, None) => req_status.required_on_any(),
if consider_optional {
req_status.enabled_on(platform_spec) != EnabledTernary::Disabled
} else {
req_status.required_on(platform_spec) != EnabledTernary::Disabled
}
};

Expand All @@ -190,8 +185,8 @@ impl<'a> CargoSetBuildState<'a> {

// 2. Figure out what packages will be included on the target platform, i.e. normal + dev
// (if requested).
let target_platform = self.opts.target_platform();
let host_platform = self.opts.host_platform();
let target_platform = &self.opts.target_platform;
let host_platform = &self.opts.host_platform;

let target_packages = target_query.resolve_with_fn(|query, link| {
let (from, to) = link.endpoints();
Expand Down Expand Up @@ -363,16 +358,11 @@ impl<'a> CargoSetBuildState<'a> {
})
.collect();

let is_enabled = |link: &CrossLink<'_>,
kind: DependencyKind,
platform: Option<&Platform>| {
let platform_status = link.status_for_kind(kind);

match platform {
Some(platform) => platform_status.enabled_on(platform) != EnabledTernary::Disabled,
None => !platform_status.is_never(),
}
};
let is_enabled =
|link: &CrossLink<'_>, kind: DependencyKind, platform_spec: &PlatformSpec| {
let platform_status = link.status_for_kind(kind);
platform_status.enabled_on(platform_spec) != EnabledTernary::Disabled
};

let target_query = if self.opts.initials_platform == InitialsPlatform::Host {
// Empty query on the target.
Expand All @@ -385,8 +375,8 @@ impl<'a> CargoSetBuildState<'a> {
let target_query_2 = target_query.clone();

// 1. Perform a feature query for the target.
let target_platform = self.opts.target_platform();
let host_platform = self.opts.host_platform();
let target_platform = &self.opts.target_platform;
let host_platform = &self.opts.host_platform;
let target = target_query.resolve_with_fn(|query, link| {
let (from, to) = link.endpoints();

Expand Down
Loading

0 comments on commit edf08c6

Please sign in to comment.