Skip to content

Commit

Permalink
[guppy] handle platform dependencies + update feature graph construction
Browse files Browse the repository at this point in the history
This is a complete overhaul of the way platform-specific dependencies are
handled. Based on my experimentation and reading the Cargo source code, I
believe that this is now correct.

Doing so also required feature graph construction to be updated, so the
feature graph construction is ready for the new feature resolver, and is now
platform-sensitive as well. The APIs for the feature graph are still to
come, but making the data model be full-fidelity allows for both the current
and new feature resolvers to be implemented.

Now that target logic is internalized in guppy, cargo-guppy no longer needs
to do its own evaluation.

For more about the new feature resolver, see:

* https://doc.rust-lang.org/nightly/cargo/reference/unstable.html#features
* rust-lang/cargo#7914
* rust-lang/cargo#7915
* rust-lang/cargo#7916
  • Loading branch information
sunshowers committed Mar 28, 2020
1 parent a399ebf commit 80eadab
Show file tree
Hide file tree
Showing 14 changed files with 1,148 additions and 211 deletions.
1 change: 0 additions & 1 deletion cargo-guppy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,3 @@ itertools = "0.9.0"
serde = "1.0.40"
serde_json = "1.0.40"
structopt = "0.3.0"
target-spec = { version = "0.2.0", path = "../target-spec" }
20 changes: 13 additions & 7 deletions cargo-guppy/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@

use anyhow;
use clap::arg_enum;
use guppy::graph::EnabledStatus;
use guppy::{
graph::{DependencyLink, DotWrite, PackageDotVisitor, PackageGraph, PackageMetadata},
MetadataCommand, PackageId,
MetadataCommand, PackageId, Platform, TargetFeatures,
};
use itertools;
use std::cmp;
Expand All @@ -15,7 +16,6 @@ use std::fs;
use std::io::Write;
use std::iter;
use structopt::StructOpt;
use target_spec;

mod diff;

Expand Down Expand Up @@ -305,6 +305,13 @@ fn narrow_graph(pkg_graph: &mut PackageGraph, options: &FilterOptions) {
}
}

let platform = if let Some(ref target) = options.target {
// Accept all features for the target filtering below.
Some(Platform::new(target, TargetFeatures::All).unwrap())
} else {
None
};

pkg_graph.retain_edges(|_, DependencyLink { from, to, edge }| {
// filter by the kind of dependency (--kind)
// NOTE: We always retain all workspace deps in the graph, otherwise
Expand All @@ -316,12 +323,11 @@ fn narrow_graph(pkg_graph: &mut PackageGraph, options: &FilterOptions) {
};

// filter out irrelevant dependencies for a specific target (--target)
let include_target = if let Some(ref target) = options.target {
let include_target = if let Some(platform) = &platform {
edge.normal()
.and_then(|meta| meta.target())
.and_then(|edge_target| {
let res = target_spec::eval(edge_target, target).unwrap_or(true);
Some(res)
.map(|meta| {
// Include this dependency if it's optional or mandatory.
meta.enabled_on(platform).unwrap() != EnabledStatus::Never
})
.unwrap_or(true)
} else {
Expand Down
1 change: 1 addition & 0 deletions guppy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ proptest-derive = { version = "0.1.2", optional = true }
semver = "0.9.0"
serde = { version = "1.0.99", features = ["derive"] }
serde_json = "1.0.40"
target-spec = { version = "0.2.0", path = "../target-spec" }

[dev-dependencies]
assert_matches = "1.3.0"
Expand Down
1 change: 1 addition & 0 deletions guppy/fixtures/small/metadata_targets1.json

Large diffs are not rendered by default.

18 changes: 18 additions & 0 deletions guppy/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,16 @@ pub enum Error {
UnknownPackageId(PackageId),
/// A feature ID was unknown to this `FeatureGraph`.
UnknownFeatureId(PackageId, Option<String>),
/// The platform `guppy` is running on is unknown.
UnknownCurrentPlatform,
/// An error occurred while evaluating a target specification against the given platform.
#[non_exhaustive]
TargetEvalError {
/// The given platform.
platform: &'static str,
/// The error that occurred while evaluating the target specification.
err: Box<dyn error::Error + Sync + Send + 'static>,
},
/// An internal error occurred within this `PackageGraph`.
PackageGraphInternalError(String),
}
Expand All @@ -46,6 +56,12 @@ impl fmt::Display for Error {
Some(feature) => write!(f, "Unknown feature ID: '{}' '{}'", package_id, feature),
None => write!(f, "Unknown feature ID: '{}' (base)", package_id),
},
UnknownCurrentPlatform => write!(f, "Unknown current platform"),
TargetEvalError { platform, err } => write!(
f,
"Error while evaluating target specifications against platform '{}': {}",
platform, err
),
PackageGraphInternalError(msg) => write!(f, "Internal error in package graph: {}", msg),
}
}
Expand All @@ -59,6 +75,8 @@ impl error::Error for Error {
PackageGraphConstructError(_) => None,
UnknownPackageId(_) => None,
UnknownFeatureId(_, _) => None,
UnknownCurrentPlatform => None,
TargetEvalError { err, .. } => Some(err.as_ref()),
PackageGraphInternalError(_) => None,
}
}
Expand Down
Loading

0 comments on commit 80eadab

Please sign in to comment.