Skip to content

Commit

Permalink
Map dependencies to crates (#90)
Browse files Browse the repository at this point in the history
* Checkpoint

* Make cargo_metadata optional

* Fix fmt

* Fixup

* Fixup
  • Loading branch information
Jake-Shadle authored Jun 26, 2024
1 parent 3e5d62c commit aac2fcd
Show file tree
Hide file tree
Showing 12 changed files with 1,198 additions and 54 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-22.04]
features: ["--features targets", null]
features: ["--features targets", "--features metadata", null]
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@v4
Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

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

11 changes: 8 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,19 +21,24 @@ rust-version = "1.65.0"
default = []
# Adds support for filtering target specific dependencies
targets = ["cfg-expr/targets"]
# Enables interop with cargo_metadata, if not enabled uses types defined within krates
metadata = ["dep:cargo_metadata", "dep:cargo-platform"]

[dependencies]
camino = { version = "1.1", features = ["serde1"] }
# Used for acquiring and/or deserializing `cargo metadata` output
cargo_metadata = "0.18"
cargo_metadata = { version = "0.18", default-features = false, optional = true }
# We need to use a type from this because it use part of the public API of cargo_metadata
# ...but it's not actually in the public API :p
cargo-platform = "0.1"
cargo-platform = { version = "0.1", default-features = false, optional = true }
# Used to parse and evaluate cfg() expressions for dependencies
cfg-expr = "0.15"
# Used to create and traverse graph structures
petgraph = "0.6"
# Used for checking version requirements
semver = "1.0"
semver = { version = "1.0", features = ["serde"] }
serde = "1.0"
serde_json = "1.0"

[dev-dependencies]
# Example CLI
Expand Down
82 changes: 42 additions & 40 deletions src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ pub(crate) mod features;

pub mod index;

use crate::{DepKind, Edge, Error, Kid, Krates};
use cargo_metadata as cm;
use crate::{cm, DepKind, Edge, Error, Kid, Krates, MdTarget};
use features::{Feature, ParsedFeature};
use std::{
collections::{BTreeMap, BTreeSet},
Expand Down Expand Up @@ -568,7 +567,7 @@ impl Builder {
on_filter: F,
) -> Result<Krates<N, E>, Error>
where
N: From<cargo_metadata::Package>,
N: From<crate::Package>,
E: From<Edge>,
F: OnFilter,
{
Expand Down Expand Up @@ -600,11 +599,11 @@ impl Builder {
/// ```
pub fn build_with_metadata<N, E, F>(
self,
md: cargo_metadata::Metadata,
md: crate::Metadata,
mut on_filter: F,
) -> Result<Krates<N, E>, Error>
where
N: From<cargo_metadata::Package>,
N: From<crate::Package>,
E: From<Edge>,
F: OnFilter,
{
Expand Down Expand Up @@ -689,7 +688,7 @@ impl Builder {
#[derive(Debug)]
struct DepKindInfo {
kind: DepKind,
cfg: Option<(String, cargo_platform::Platform)>,
cfg: Option<MdTarget>,
}

#[derive(Debug)]
Expand Down Expand Up @@ -756,7 +755,7 @@ impl Builder {
.into_iter()
.map(|dk| DepKindInfo {
kind: dk.kind.into(),
cfg: dk.target.map(|t| (t.to_string(), t)),
cfg: dk.target.map(MdTarget::from),
})
.collect();

Expand Down Expand Up @@ -835,6 +834,7 @@ impl Builder {
kind: DepKind,
cfg: Option<String>,
features: Vec<usize>,
dep_index: usize,
}

#[derive(Debug)]
Expand Down Expand Up @@ -1138,6 +1138,7 @@ impl Builder {
kind: DepKind,
cfg: Option<&'d str>,
features: &'d [String],
dep_index: usize,
uses_default_features: bool,
}

Expand Down Expand Up @@ -1193,13 +1194,14 @@ impl Builder {
return false;
}

dk.cfg.as_ref().map(|(_, p)| p) == dep.target.as_ref()
crate::targets_eq(&dk.cfg, &dep.target)
}).count() > 1;

let dep = krate
let (dep_index, dep) = krate
.dependencies
.iter()
.find(|dep| {
.enumerate()
.find(|(_, dep)| {
if dk.kind != dep.kind {
return false;
}
Expand All @@ -1218,7 +1220,7 @@ impl Builder {
return false;
}

if dk.cfg.as_ref().map(|(_, p)| p) != dep.target.as_ref() {
if !crate::targets_eq(&dk.cfg, &dep.target) {
return false;
}

Expand Down Expand Up @@ -1258,51 +1260,42 @@ impl Builder {
return None;
}

let cfg = if let Some(cfg) = dk.cfg.as_ref().map(|(c, _)| c.as_str()) {
let cfg = if let Some(cfg) = &dk.cfg {
if !include_all_targets {
let matched = if cfg.starts_with("cfg(") {
match cfg_expr::Expression::parse(cfg) {
Ok(expr) => {
// We only need to focus on target predicates because they are
// the only type of predicate allowed by cargo at the moment

// While it might be nicer to evaluate all the targets for each predicate
// it would lead to weird situations where an expression could evaluate to true
// (or false) with a combination of platform, that would otherwise be impossible,
// eg cfg(all(windows, target_env = "musl")) could evaluate to true
targets
.iter()
.any(|target| expr.eval(|pred| target.eval(pred)))
}
Err(_pe) => {
// TODO: maybe log a warning if we somehow fail to parse the cfg?
true
}
}
let matched = if let Some(expr) = &cfg.cfg {
// We only need to focus on target predicates because they are
// the only type of predicate allowed by cargo at the moment

// While it might be nicer to evaluate all the targets for each predicate
// it would lead to weird situations where an expression could evaluate to true
// (or false) with a combination of platform, that would otherwise be impossible,
// eg cfg(all(windows, target_env = "musl")) could evaluate to true
targets
.iter()
.any(|target| expr.eval(|pred| target.eval(pred)))
} else {
// If it's not a cfg expression, it's just a fully specified target triple,
// so we just do a string comparison
targets.iter().any(|target| target.matches_triple(cfg))
targets.iter().any(|target| target.matches_triple(&cfg.inner))
};

if !matched {
return None;
}
} else if cfg.starts_with("cfg(") {
} else if let Some(expr) = &cfg.cfg {
// This is _basically_ a tortured way to evaluate `cfg(any())`, which is always false but
// is used by eg. serde -> serde_derive. If not filtering targets this would mean that
// serde_derive and all of its dependencies would be pulled into the graph, even if the
// only edge was the cfg(any()).
if let Ok(expr) = cfg_expr::Expression::parse(cfg) {
// We can't just do an eval and always return true, as that then would cause any
// not() expressions to evaluate to false
if expr.predicates().count() == 0 && !expr.eval(|_| true) {
return None;
}

// We can't just do an eval and always return true, as that then would cause any
// not() expressions to evaluate to false
if expr.predicates().count() == 0 && !expr.eval(|_| true) {
return None;
}
}

Some(cfg)
Some(cfg.inner.as_str())
} else {
None
};
Expand All @@ -1311,6 +1304,7 @@ impl Builder {
kind: dk.kind,
cfg,
features: &dep.features,
dep_index,
// Dependencies will default to saying "uses_default_features" on edges,
// even if the crate in question doesn't actually have a "default" feature,
// so check that it actually does
Expand Down Expand Up @@ -1362,6 +1356,7 @@ impl Builder {

let edge = DependencyEdge {
kind: edge.kind,
dep_index: edge.dep_index,
cfg: edge.cfg.map(|s| s.into()),
features,
};
Expand Down Expand Up @@ -1404,10 +1399,13 @@ impl Builder {
.collect()
};

let dep_mapping = vec![None; krate.dependencies.len()];

let krate = crate::Node::Krate {
id,
krate: N::from(krate),
features,
dep_mapping,
};

graph.add_node(krate);
Expand Down Expand Up @@ -1539,6 +1537,10 @@ impl Builder {
);
}

if let crate::Node::Krate { dep_mapping, .. } = &mut graph[srcid] {
dep_mapping[edge.dep_index] = Some(target_krate);
}

if attach_direct_edge {
graph.add_edge(
srcid,
Expand Down
11 changes: 6 additions & 5 deletions src/builder/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,12 @@ impl CachingIndex {
}
}

/// Correct features with index information
///
/// Due to <https://github.com/rust-lang/cargo/issues/11319>, we can't actually
/// trust cargo to give us the correct package metadata, so we instead use the
/// (presumably) correct data from the the index
pub(super) fn fix_features(index: &CachingIndex, krate: &mut cargo_metadata::Package) {
pub(super) fn fix_features(index: &CachingIndex, krate: &mut crate::Package) {
if krate
.source
.as_ref()
Expand All @@ -62,10 +64,9 @@ pub(super) fn fix_features(index: &CachingIndex, krate: &mut cargo_metadata::Pac
}
}

// The index entry features might not have the `dep:<crate>`
// used with weak features if the crate version was
// published with cargo <1.60.0 version, so we need to
// manually fix that up since we depend on that format
// The index entry features might not have the `dep:<crate>` used with weak
// features if the crate version was published with cargo <1.60.0 version,
// so we need to manually fix that up since we depend on that format
let missing_deps: Vec<_> = krate
.features
.iter()
Expand Down
Loading

0 comments on commit aac2fcd

Please sign in to comment.