Skip to content

Commit

Permalink
[guppy] fix an edge case with feature activation
Browse files Browse the repository at this point in the history
As explained in the comment -- same-package features should not activate build
dependencies.

Discovered via upgrade to Cargo 0.81.0.
  • Loading branch information
sunshowers committed Jul 29, 2024
1 parent c34e155 commit 8666ebc
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 3 deletions.
21 changes: 20 additions & 1 deletion guppy/src/graph/cargo/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,26 @@ impl<'a> CargoSetBuildState<'a> {
let proc_macro_redirect = follow_target && to.package().is_proc_macro();

// Build dependencies are evaluated against the host platform.
let build_dep_redirect = is_enabled(&link, DependencyKind::Build, host_platform);
let build_dep_redirect = {
// If this is a dependency like:
//
// ```
// [build-dependencies]
// cc = { version = "1.0", optional = true }
//
// [features]
// bundled = ["cc"]
// ```
//
// Then, there is an implicit named feature here called "cc" on the target platform,
// which enables the optional dependency "cc". But this does not mean that this
// package itself is built on the host platform!
//
// Detect this situation by ensuring that the package ID of the `from` and `to`
// nodes are different.
from.package_id() != to.package_id()
&& is_enabled(&link, DependencyKind::Build, host_platform)
};

// Finally, process what needs to be done.
if build_dep_redirect || proc_macro_redirect {
Expand Down
24 changes: 22 additions & 2 deletions guppy/src/graph/feature/graph_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -593,7 +593,7 @@ impl<'g> fmt::Display for FeatureLabel<'g> {
}

/// Metadata for a feature within a package.
#[derive(Clone, Copy, Debug)]
#[derive(Clone, Copy)]
pub struct FeatureMetadata<'g> {
graph: DebugIgnore<FeatureGraph<'g>>,
node: FeatureNode,
Expand Down Expand Up @@ -641,6 +641,14 @@ impl<'g> FeatureMetadata<'g> {
}
}

impl fmt::Debug for FeatureMetadata<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("FeatureMetadata")
.field("id", &self.feature_id())
.finish()
}
}

/// A graph representing every possible feature of every package, and the connections between them.
#[derive(Clone, Debug)]
pub(in crate::graph) struct FeatureGraphImpl {
Expand Down Expand Up @@ -722,7 +730,7 @@ impl FeatureGraphImpl {
/// If a dependency, for example `unix-dep` above, is optional, an implicit feature is created in
/// the package `main` with the name `unix-dep`. In this case, the dependency from `main/feat` to
/// `main/unix-dep` is also a `ConditionalLink` representing the same `cfg(unix)` condition.
#[derive(Copy, Clone, Debug)]
#[derive(Copy, Clone)]
pub struct ConditionalLink<'g> {
graph: DebugIgnore<FeatureGraph<'g>>,
from: &'g FeatureMetadataImpl,
Expand Down Expand Up @@ -832,6 +840,18 @@ impl<'g> ConditionalLink<'g> {
}
}

impl fmt::Debug for ConditionalLink<'_> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
f.debug_struct("ConditionalLink")
.field("from", &self.from())
.field("to", &self.to())
.field("normal", &self.normal())
.field("build", &self.build())
.field("dev", &self.dev())
.finish()
}
}

// ---

/// A combination of a package ID and a feature name, forming a node in a `FeatureGraph`.
Expand Down

0 comments on commit 8666ebc

Please sign in to comment.