Skip to content

Commit a2c6ec7

Browse files
borsbors
committed
Auto merge of #10103 - ehuss:tree-cycle, r=alexcrichton
Fix a couple issues with cyclic features and dev-dependencies This fixes two issues with cyclic features and dev-dependencies: * `cargo tree` would enter an infinite loop for cyclic features. * The resolver would return a confusing error if a cyclic dev-dependency attempted to enable a feature on its parent that resulted in a cycle. This fixes it to resolve correctly. Fixes #10101
2 parents df8cda0 + 2eee644 commit a2c6ec7

File tree

3 files changed

+258
-30
lines changed

3 files changed

+258
-30
lines changed

‎src/cargo/core/resolver/dep_cache.rs‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -280,10 +280,12 @@ pub fn resolve_features<'b>(
280280
// dep_name/feat_name` where `dep_name` does not exist. All other
281281
// validation is done either in `build_requirements` or
282282
// `build_feature_map`.
283-
for dep_name in reqs.deps.keys() {
284-
if !valid_dep_names.contains(dep_name) {
285-
let e = RequirementError::MissingDependency(*dep_name);
286-
return Err(e.into_activate_error(parent, s));
283+
if parent.is_none() {
284+
for dep_name in reqs.deps.keys() {
285+
if !valid_dep_names.contains(dep_name) {
286+
let e = RequirementError::MissingDependency(*dep_name);
287+
return Err(e.into_activate_error(parent, s));
288+
}
287289
}
288290
}
289291

‎src/cargo/ops/tree/graph.rs‎

Lines changed: 36 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -427,28 +427,32 @@ fn add_pkg(
427427
/// ```text
428428
/// from -Edge-> featname -Edge::Feature-> to
429429
/// ```
430+
///
431+
/// Returns a tuple `(missing, index)`.
432+
/// `missing` is true if this feature edge was already added.
433+
/// `index` is the index of the index in the graph of the `Feature` node.
430434
fn add_feature(
431435
graph: &mut Graph<'_>,
432436
name: InternedString,
433437
from: Option<usize>,
434438
to: usize,
435439
kind: EdgeKind,
436-
) -> usize {
440+
) -> (bool, usize) {
437441
// `to` *must* point to a package node.
438442
assert!(matches! {graph.nodes[to], Node::Package{..}});
439443
let node = Node::Feature {
440444
node_index: to,
441445
name,
442446
};
443-
let node_index = match graph.index.get(&node) {
444-
Some(idx) => *idx,
445-
None => graph.add_node(node),
447+
let (missing, node_index) = match graph.index.get(&node) {
448+
Some(idx) => (false, *idx),
449+
None => (true, graph.add_node(node)),
446450
};
447451
if let Some(from) = from {
448452
graph.edges[from].add_edge(kind, node_index);
449453
}
450454
graph.edges[node_index].add_edge(EdgeKind::Feature, to);
451-
node_index
455+
(missing, node_index)
452456
}
453457

454458
/// Adds nodes for features requested on the command-line for the given member.
@@ -480,7 +484,7 @@ fn add_cli_features(
480484
for fv in to_add {
481485
match fv {
482486
FeatureValue::Feature(feature) => {
483-
let index = add_feature(graph, feature, None, package_index, EdgeKind::Feature);
487+
let index = add_feature(graph, feature, None, package_index, EdgeKind::Feature).1;
484488
graph.cli_features.insert(index);
485489
}
486490
// This is enforced by CliFeatures.
@@ -511,10 +515,11 @@ fn add_cli_features(
511515
if is_optional {
512516
// Activate the optional dep on self.
513517
let index =
514-
add_feature(graph, dep_name, None, package_index, EdgeKind::Feature);
518+
add_feature(graph, dep_name, None, package_index, EdgeKind::Feature).1;
515519
graph.cli_features.insert(index);
516520
}
517-
let index = add_feature(graph, dep_feature, None, dep_index, EdgeKind::Feature);
521+
let index =
522+
add_feature(graph, dep_feature, None, dep_index, EdgeKind::Feature).1;
518523
graph.cli_features.insert(index);
519524
}
520525
}
@@ -571,21 +576,24 @@ fn add_feature_rec(
571576
for fv in fvs {
572577
match fv {
573578
FeatureValue::Feature(dep_name) => {
574-
let feat_index = add_feature(
579+
let (missing, feat_index) = add_feature(
575580
graph,
576581
*dep_name,
577582
Some(from),
578583
package_index,
579584
EdgeKind::Feature,
580585
);
581-
add_feature_rec(
582-
graph,
583-
resolve,
584-
*dep_name,
585-
package_id,
586-
feat_index,
587-
package_index,
588-
);
586+
// Don't recursive if the edge already exists to deal with cycles.
587+
if missing {
588+
add_feature_rec(
589+
graph,
590+
resolve,
591+
*dep_name,
592+
package_id,
593+
feat_index,
594+
package_index,
595+
);
596+
}
589597
}
590598
// Dependencies are already shown in the graph as dep edges. I'm
591599
// uncertain whether or not this might be confusing in some cases
@@ -628,21 +636,23 @@ fn add_feature_rec(
628636
EdgeKind::Feature,
629637
);
630638
}
631-
let feat_index = add_feature(
639+
let (missing, feat_index) = add_feature(
632640
graph,
633641
*dep_feature,
634642
Some(from),
635643
dep_index,
636644
EdgeKind::Feature,
637645
);
638-
add_feature_rec(
639-
graph,
640-
resolve,
641-
*dep_feature,
642-
dep_pkg_id,
643-
feat_index,
644-
dep_index,
645-
);
646+
if missing {
647+
add_feature_rec(
648+
graph,
649+
resolve,
650+
*dep_feature,
651+
dep_pkg_id,
652+
feat_index,
653+
dep_index,
654+
);
655+
}
646656
}
647657
}
648658
}

‎tests/testsuite/tree.rs‎

Lines changed: 216 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1831,3 +1831,219 @@ foo v0.1.0 ([..]/foo)
18311831
.with_status(101)
18321832
.run();
18331833
}
1834+
1835+
#[cargo_test]
1836+
fn cyclic_features() {
1837+
// Check for stack overflow with cyclic features (oops!).
1838+
let p = project()
1839+
.file(
1840+
"Cargo.toml",
1841+
r#"
1842+
[package]
1843+
name = "foo"
1844+
version = "1.0.0"
1845+
1846+
[features]
1847+
a = ["b"]
1848+
b = ["a"]
1849+
default = ["a"]
1850+
"#,
1851+
)
1852+
.file("src/lib.rs", "")
1853+
.build();
1854+
1855+
p.cargo("tree -e features")
1856+
.with_stdout("foo v1.0.0 ([ROOT]/foo)")
1857+
.run();
1858+
1859+
p.cargo("tree -e features -i foo")
1860+
.with_stdout(
1861+
"\
1862+
foo v1.0.0 ([ROOT]/foo)
1863+
├── foo feature \"a\"
1864+
│ ├── foo feature \"b\"
1865+
│ │ └── foo feature \"a\" (*)
1866+
│ └── foo feature \"default\" (command-line)
1867+
├── foo feature \"b\" (*)
1868+
└── foo feature \"default\" (command-line)
1869+
",
1870+
)
1871+
.run();
1872+
}
1873+
1874+
#[cargo_test]
1875+
fn dev_dep_cycle_with_feature() {
1876+
// Cycle with features and a dev-dependency.
1877+
let p = project()
1878+
.file(
1879+
"Cargo.toml",
1880+
r#"
1881+
[package]
1882+
name = "foo"
1883+
version = "1.0.0"
1884+
1885+
[dev-dependencies]
1886+
bar = { path = "bar" }
1887+
1888+
[features]
1889+
a = ["bar/feat1"]
1890+
"#,
1891+
)
1892+
.file("src/lib.rs", "")
1893+
.file(
1894+
"bar/Cargo.toml",
1895+
r#"
1896+
[package]
1897+
name = "bar"
1898+
version = "1.0.0"
1899+
1900+
[dependencies]
1901+
foo = { path = ".." }
1902+
1903+
[features]
1904+
feat1 = ["foo/a"]
1905+
"#,
1906+
)
1907+
.file("bar/src/lib.rs", "")
1908+
.build();
1909+
1910+
p.cargo("tree -e features --features a")
1911+
.with_stdout(
1912+
"\
1913+
foo v1.0.0 ([ROOT]/foo)
1914+
[dev-dependencies]
1915+
└── bar feature \"default\"
1916+
└── bar v1.0.0 ([ROOT]/foo/bar)
1917+
└── foo feature \"default\" (command-line)
1918+
└── foo v1.0.0 ([ROOT]/foo) (*)
1919+
",
1920+
)
1921+
.run();
1922+
1923+
p.cargo("tree -e features --features a -i foo")
1924+
.with_stdout(
1925+
"\
1926+
foo v1.0.0 ([ROOT]/foo)
1927+
├── foo feature \"a\" (command-line)
1928+
│ └── bar feature \"feat1\"
1929+
│ └── foo feature \"a\" (command-line) (*)
1930+
└── foo feature \"default\" (command-line)
1931+
└── bar v1.0.0 ([ROOT]/foo/bar)
1932+
├── bar feature \"default\"
1933+
│ [dev-dependencies]
1934+
│ └── foo v1.0.0 ([ROOT]/foo) (*)
1935+
└── bar feature \"feat1\" (*)
1936+
",
1937+
)
1938+
.run();
1939+
}
1940+
1941+
#[cargo_test]
1942+
fn dev_dep_cycle_with_feature_nested() {
1943+
// Checks for an issue where a cyclic dev dependency tries to activate a
1944+
// feature on its parent that tries to activate the feature back on the
1945+
// dev-dependency.
1946+
let p = project()
1947+
.file(
1948+
"Cargo.toml",
1949+
r#"
1950+
[package]
1951+
name = "foo"
1952+
version = "1.0.0"
1953+
1954+
[dev-dependencies]
1955+
bar = { path = "bar" }
1956+
1957+
[features]
1958+
a = ["bar/feat1"]
1959+
b = ["a"]
1960+
"#,
1961+
)
1962+
.file("src/lib.rs", "")
1963+
.file(
1964+
"bar/Cargo.toml",
1965+
r#"
1966+
[package]
1967+
name = "bar"
1968+
version = "1.0.0"
1969+
1970+
[dependencies]
1971+
foo = { path = ".." }
1972+
1973+
[features]
1974+
feat1 = ["foo/b"]
1975+
"#,
1976+
)
1977+
.file("bar/src/lib.rs", "")
1978+
.build();
1979+
1980+
p.cargo("tree -e features")
1981+
.with_stdout(
1982+
"\
1983+
foo v1.0.0 ([ROOT]/foo)
1984+
[dev-dependencies]
1985+
└── bar feature \"default\"
1986+
└── bar v1.0.0 ([ROOT]/foo/bar)
1987+
└── foo feature \"default\" (command-line)
1988+
└── foo v1.0.0 ([ROOT]/foo) (*)
1989+
",
1990+
)
1991+
.run();
1992+
1993+
p.cargo("tree -e features --features a -i foo")
1994+
.with_stdout(
1995+
"\
1996+
foo v1.0.0 ([ROOT]/foo)
1997+
├── foo feature \"a\" (command-line)
1998+
│ └── foo feature \"b\"
1999+
│ └── bar feature \"feat1\"
2000+
│ └── foo feature \"a\" (command-line) (*)
2001+
├── foo feature \"b\" (*)
2002+
└── foo feature \"default\" (command-line)
2003+
└── bar v1.0.0 ([ROOT]/foo/bar)
2004+
├── bar feature \"default\"
2005+
│ [dev-dependencies]
2006+
│ └── foo v1.0.0 ([ROOT]/foo) (*)
2007+
└── bar feature \"feat1\" (*)
2008+
",
2009+
)
2010+
.run();
2011+
2012+
p.cargo("tree -e features --features b -i foo")
2013+
.with_stdout(
2014+
"\
2015+
foo v1.0.0 ([ROOT]/foo)
2016+
├── foo feature \"a\"
2017+
│ └── foo feature \"b\" (command-line)
2018+
│ └── bar feature \"feat1\"
2019+
│ └── foo feature \"a\" (*)
2020+
├── foo feature \"b\" (command-line) (*)
2021+
└── foo feature \"default\" (command-line)
2022+
└── bar v1.0.0 ([ROOT]/foo/bar)
2023+
├── bar feature \"default\"
2024+
│ [dev-dependencies]
2025+
│ └── foo v1.0.0 ([ROOT]/foo) (*)
2026+
└── bar feature \"feat1\" (*)
2027+
",
2028+
)
2029+
.run();
2030+
2031+
p.cargo("tree -e features --features bar/feat1 -i foo")
2032+
.with_stdout(
2033+
"\
2034+
foo v1.0.0 ([ROOT]/foo)
2035+
├── foo feature \"a\"
2036+
│ └── foo feature \"b\"
2037+
│ └── bar feature \"feat1\" (command-line)
2038+
│ └── foo feature \"a\" (*)
2039+
├── foo feature \"b\" (*)
2040+
└── foo feature \"default\" (command-line)
2041+
└── bar v1.0.0 ([ROOT]/foo/bar)
2042+
├── bar feature \"default\"
2043+
│ [dev-dependencies]
2044+
│ └── foo v1.0.0 ([ROOT]/foo) (*)
2045+
└── bar feature \"feat1\" (command-line) (*)
2046+
",
2047+
)
2048+
.run();
2049+
}

0 commit comments

Comments
 (0)