diff --git a/src/cargo/core/resolver/dep_cache.rs b/src/cargo/core/resolver/dep_cache.rs index 32bc6507670..c5bf602a2e1 100644 --- a/src/cargo/core/resolver/dep_cache.rs +++ b/src/cargo/core/resolver/dep_cache.rs @@ -280,10 +280,12 @@ pub fn resolve_features<'b>( // dep_name/feat_name` where `dep_name` does not exist. All other // validation is done either in `build_requirements` or // `build_feature_map`. - for dep_name in reqs.deps.keys() { - if !valid_dep_names.contains(dep_name) { - let e = RequirementError::MissingDependency(*dep_name); - return Err(e.into_activate_error(parent, s)); + if parent.is_none() { + for dep_name in reqs.deps.keys() { + if !valid_dep_names.contains(dep_name) { + let e = RequirementError::MissingDependency(*dep_name); + return Err(e.into_activate_error(parent, s)); + } } } diff --git a/src/cargo/ops/tree/graph.rs b/src/cargo/ops/tree/graph.rs index bc0fb73dd86..28338294961 100644 --- a/src/cargo/ops/tree/graph.rs +++ b/src/cargo/ops/tree/graph.rs @@ -427,28 +427,32 @@ fn add_pkg( /// ```text /// from -Edge-> featname -Edge::Feature-> to /// ``` +/// +/// Returns a tuple `(missing, index)`. +/// `missing` is true if this feature edge was already added. +/// `index` is the index of the index in the graph of the `Feature` node. fn add_feature( graph: &mut Graph<'_>, name: InternedString, from: Option, to: usize, kind: EdgeKind, -) -> usize { +) -> (bool, usize) { // `to` *must* point to a package node. assert!(matches! {graph.nodes[to], Node::Package{..}}); let node = Node::Feature { node_index: to, name, }; - let node_index = match graph.index.get(&node) { - Some(idx) => *idx, - None => graph.add_node(node), + let (missing, node_index) = match graph.index.get(&node) { + Some(idx) => (false, *idx), + None => (true, graph.add_node(node)), }; if let Some(from) = from { graph.edges[from].add_edge(kind, node_index); } graph.edges[node_index].add_edge(EdgeKind::Feature, to); - node_index + (missing, node_index) } /// Adds nodes for features requested on the command-line for the given member. @@ -480,7 +484,7 @@ fn add_cli_features( for fv in to_add { match fv { FeatureValue::Feature(feature) => { - let index = add_feature(graph, feature, None, package_index, EdgeKind::Feature); + let index = add_feature(graph, feature, None, package_index, EdgeKind::Feature).1; graph.cli_features.insert(index); } // This is enforced by CliFeatures. @@ -511,10 +515,11 @@ fn add_cli_features( if is_optional { // Activate the optional dep on self. let index = - add_feature(graph, dep_name, None, package_index, EdgeKind::Feature); + add_feature(graph, dep_name, None, package_index, EdgeKind::Feature).1; graph.cli_features.insert(index); } - let index = add_feature(graph, dep_feature, None, dep_index, EdgeKind::Feature); + let index = + add_feature(graph, dep_feature, None, dep_index, EdgeKind::Feature).1; graph.cli_features.insert(index); } } @@ -571,21 +576,24 @@ fn add_feature_rec( for fv in fvs { match fv { FeatureValue::Feature(dep_name) => { - let feat_index = add_feature( + let (missing, feat_index) = add_feature( graph, *dep_name, Some(from), package_index, EdgeKind::Feature, ); - add_feature_rec( - graph, - resolve, - *dep_name, - package_id, - feat_index, - package_index, - ); + // Don't recursive if the edge already exists to deal with cycles. + if missing { + add_feature_rec( + graph, + resolve, + *dep_name, + package_id, + feat_index, + package_index, + ); + } } // Dependencies are already shown in the graph as dep edges. I'm // uncertain whether or not this might be confusing in some cases @@ -628,21 +636,23 @@ fn add_feature_rec( EdgeKind::Feature, ); } - let feat_index = add_feature( + let (missing, feat_index) = add_feature( graph, *dep_feature, Some(from), dep_index, EdgeKind::Feature, ); - add_feature_rec( - graph, - resolve, - *dep_feature, - dep_pkg_id, - feat_index, - dep_index, - ); + if missing { + add_feature_rec( + graph, + resolve, + *dep_feature, + dep_pkg_id, + feat_index, + dep_index, + ); + } } } } diff --git a/tests/testsuite/tree.rs b/tests/testsuite/tree.rs index badfef90505..63115a85930 100644 --- a/tests/testsuite/tree.rs +++ b/tests/testsuite/tree.rs @@ -1831,3 +1831,219 @@ foo v0.1.0 ([..]/foo) .with_status(101) .run(); } + +#[cargo_test] +fn cyclic_features() { + // Check for stack overflow with cyclic features (oops!). + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "1.0.0" + + [features] + a = ["b"] + b = ["a"] + default = ["a"] + "#, + ) + .file("src/lib.rs", "") + .build(); + + p.cargo("tree -e features") + .with_stdout("foo v1.0.0 ([ROOT]/foo)") + .run(); + + p.cargo("tree -e features -i foo") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +├── foo feature \"a\" +│ ├── foo feature \"b\" +│ │ └── foo feature \"a\" (*) +│ └── foo feature \"default\" (command-line) +├── foo feature \"b\" (*) +└── foo feature \"default\" (command-line) +", + ) + .run(); +} + +#[cargo_test] +fn dev_dep_cycle_with_feature() { + // Cycle with features and a dev-dependency. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "1.0.0" + + [dev-dependencies] + bar = { path = "bar" } + + [features] + a = ["bar/feat1"] + "#, + ) + .file("src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "1.0.0" + + [dependencies] + foo = { path = ".." } + + [features] + feat1 = ["foo/a"] + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("tree -e features --features a") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +[dev-dependencies] +└── bar feature \"default\" + └── bar v1.0.0 ([ROOT]/foo/bar) + └── foo feature \"default\" (command-line) + └── foo v1.0.0 ([ROOT]/foo) (*) +", + ) + .run(); + + p.cargo("tree -e features --features a -i foo") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +├── foo feature \"a\" (command-line) +│ └── bar feature \"feat1\" +│ └── foo feature \"a\" (command-line) (*) +└── foo feature \"default\" (command-line) + └── bar v1.0.0 ([ROOT]/foo/bar) + ├── bar feature \"default\" + │ [dev-dependencies] + │ └── foo v1.0.0 ([ROOT]/foo) (*) + └── bar feature \"feat1\" (*) +", + ) + .run(); +} + +#[cargo_test] +fn dev_dep_cycle_with_feature_nested() { + // Checks for an issue where a cyclic dev dependency tries to activate a + // feature on its parent that tries to activate the feature back on the + // dev-dependency. + let p = project() + .file( + "Cargo.toml", + r#" + [package] + name = "foo" + version = "1.0.0" + + [dev-dependencies] + bar = { path = "bar" } + + [features] + a = ["bar/feat1"] + b = ["a"] + "#, + ) + .file("src/lib.rs", "") + .file( + "bar/Cargo.toml", + r#" + [package] + name = "bar" + version = "1.0.0" + + [dependencies] + foo = { path = ".." } + + [features] + feat1 = ["foo/b"] + "#, + ) + .file("bar/src/lib.rs", "") + .build(); + + p.cargo("tree -e features") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +[dev-dependencies] +└── bar feature \"default\" + └── bar v1.0.0 ([ROOT]/foo/bar) + └── foo feature \"default\" (command-line) + └── foo v1.0.0 ([ROOT]/foo) (*) +", + ) + .run(); + + p.cargo("tree -e features --features a -i foo") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +├── foo feature \"a\" (command-line) +│ └── foo feature \"b\" +│ └── bar feature \"feat1\" +│ └── foo feature \"a\" (command-line) (*) +├── foo feature \"b\" (*) +└── foo feature \"default\" (command-line) + └── bar v1.0.0 ([ROOT]/foo/bar) + ├── bar feature \"default\" + │ [dev-dependencies] + │ └── foo v1.0.0 ([ROOT]/foo) (*) + └── bar feature \"feat1\" (*) +", + ) + .run(); + + p.cargo("tree -e features --features b -i foo") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +├── foo feature \"a\" +│ └── foo feature \"b\" (command-line) +│ └── bar feature \"feat1\" +│ └── foo feature \"a\" (*) +├── foo feature \"b\" (command-line) (*) +└── foo feature \"default\" (command-line) + └── bar v1.0.0 ([ROOT]/foo/bar) + ├── bar feature \"default\" + │ [dev-dependencies] + │ └── foo v1.0.0 ([ROOT]/foo) (*) + └── bar feature \"feat1\" (*) +", + ) + .run(); + + p.cargo("tree -e features --features bar/feat1 -i foo") + .with_stdout( + "\ +foo v1.0.0 ([ROOT]/foo) +├── foo feature \"a\" +│ └── foo feature \"b\" +│ └── bar feature \"feat1\" (command-line) +│ └── foo feature \"a\" (*) +├── foo feature \"b\" (*) +└── foo feature \"default\" (command-line) + └── bar v1.0.0 ([ROOT]/foo/bar) + ├── bar feature \"default\" + │ [dev-dependencies] + │ └── foo v1.0.0 ([ROOT]/foo) (*) + └── bar feature \"feat1\" (command-line) (*) +", + ) + .run(); +}