Skip to content

Commit f0ccf88

Browse files
committed
[guppy] handle platform dependencies + update feature graph construction
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
1 parent a399ebf commit f0ccf88

File tree

14 files changed

+1149
-211
lines changed

14 files changed

+1149
-211
lines changed

cargo-guppy/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,4 +13,3 @@ itertools = "0.9.0"
1313
serde = "1.0.40"
1414
serde_json = "1.0.40"
1515
structopt = "0.3.0"
16-
target-spec = { version = "0.2.0", path = "../target-spec" }

cargo-guppy/src/lib.rs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,10 @@
33

44
use anyhow;
55
use clap::arg_enum;
6+
use guppy::graph::EnabledStatus;
67
use guppy::{
78
graph::{DependencyLink, DotWrite, PackageDotVisitor, PackageGraph, PackageMetadata},
8-
MetadataCommand, PackageId,
9+
MetadataCommand, PackageId, Platform, TargetFeatures,
910
};
1011
use itertools;
1112
use std::cmp;
@@ -15,7 +16,6 @@ use std::fs;
1516
use std::io::Write;
1617
use std::iter;
1718
use structopt::StructOpt;
18-
use target_spec;
1919

2020
mod diff;
2121

@@ -305,6 +305,13 @@ fn narrow_graph(pkg_graph: &mut PackageGraph, options: &FilterOptions) {
305305
}
306306
}
307307

308+
let platform = if let Some(ref target) = options.target {
309+
// Accept all features for the target filtering below.
310+
Some(Platform::new(target, TargetFeatures::All).unwrap())
311+
} else {
312+
None
313+
};
314+
308315
pkg_graph.retain_edges(|_, DependencyLink { from, to, edge }| {
309316
// filter by the kind of dependency (--kind)
310317
// NOTE: We always retain all workspace deps in the graph, otherwise
@@ -316,12 +323,11 @@ fn narrow_graph(pkg_graph: &mut PackageGraph, options: &FilterOptions) {
316323
};
317324

318325
// filter out irrelevant dependencies for a specific target (--target)
319-
let include_target = if let Some(ref target) = options.target {
326+
let include_target = if let Some(platform) = &platform {
320327
edge.normal()
321-
.and_then(|meta| meta.target())
322-
.and_then(|edge_target| {
323-
let res = target_spec::eval(edge_target, target).unwrap_or(true);
324-
Some(res)
328+
.map(|meta| {
329+
// Include this dependency if it's optional or mandatory.
330+
meta.enabled_on(platform).unwrap() != EnabledStatus::Never
325331
})
326332
.unwrap_or(true)
327333
} else {

guppy/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ proptest-derive = { version = "0.1.2", optional = true }
3939
semver = "0.9.0"
4040
serde = { version = "1.0.99", features = ["derive"] }
4141
serde_json = "1.0.40"
42+
target-spec = { version = "0.2.0", path = "../target-spec" }
4243

4344
[dev-dependencies]
4445
assert_matches = "1.3.0"

guppy/fixtures/small/metadata_targets1.json

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

guppy/src/errors.rs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,16 @@ pub enum Error {
2525
UnknownPackageId(PackageId),
2626
/// A feature ID was unknown to this `FeatureGraph`.
2727
UnknownFeatureId(PackageId, Option<String>),
28+
/// The platform `guppy` is running on is unknown.
29+
UnknownCurrentPlatform,
30+
/// An error occurred while evaluating a target specification against the given platform.
31+
#[non_exhaustive]
32+
TargetEvalError {
33+
/// The given platform.
34+
platform: &'static str,
35+
/// The error that occurred while evaluating the target specification.
36+
err: Box<dyn error::Error + Sync + Send + 'static>,
37+
},
2838
/// An internal error occurred within this `PackageGraph`.
2939
PackageGraphInternalError(String),
3040
}
@@ -46,6 +56,12 @@ impl fmt::Display for Error {
4656
Some(feature) => write!(f, "Unknown feature ID: '{}' '{}'", package_id, feature),
4757
None => write!(f, "Unknown feature ID: '{}' (base)", package_id),
4858
},
59+
UnknownCurrentPlatform => write!(f, "Unknown current platform"),
60+
TargetEvalError { platform, err } => write!(
61+
f,
62+
"Error while evaluating target specifications against platform '{}': {}",
63+
platform, err
64+
),
4965
PackageGraphInternalError(msg) => write!(f, "Internal error in package graph: {}", msg),
5066
}
5167
}
@@ -59,6 +75,8 @@ impl error::Error for Error {
5975
PackageGraphConstructError(_) => None,
6076
UnknownPackageId(_) => None,
6177
UnknownFeatureId(_, _) => None,
78+
UnknownCurrentPlatform => None,
79+
TargetEvalError { err, .. } => Some(err.as_ref()),
6280
PackageGraphInternalError(_) => None,
6381
}
6482
}

0 commit comments

Comments
 (0)