Skip to content

Commit c4adfd5

Browse files
committed
Auto merge of #9255 - ehuss:fix-deps-filtering, r=Eh2406
Fix issue with filtering exclusive target dependencies. #8777 incorrectly changed the filtering logic for dependencies. Essentially it split `filter(any(A && B && C && D))` into two parts `filter(any(A && B)).filter(any(C && D))` which doesn't have the same meaning. The solution here is to pass a closure so that the conditions are joined again. Fixes #9216
2 parents 80c47b7 + 8ce2a1b commit c4adfd5

File tree

2 files changed

+63
-30
lines changed

2 files changed

+63
-30
lines changed

src/cargo/core/compiler/unit_dependencies.rs

Lines changed: 30 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -235,33 +235,28 @@ fn compute_deps(
235235
}
236236

237237
let id = unit.pkg.package_id();
238-
let filtered_deps = state
239-
.deps(unit, unit_for)
240-
.into_iter()
241-
.filter(|&(_id, deps)| {
242-
deps.iter().any(|dep| {
243-
// If this target is a build command, then we only want build
244-
// dependencies, otherwise we want everything *other than* build
245-
// dependencies.
246-
if unit.target.is_custom_build() != dep.is_build() {
247-
return false;
248-
}
238+
let filtered_deps = state.deps(unit, unit_for, &|dep| {
239+
// If this target is a build command, then we only want build
240+
// dependencies, otherwise we want everything *other than* build
241+
// dependencies.
242+
if unit.target.is_custom_build() != dep.is_build() {
243+
return false;
244+
}
249245

250-
// If this dependency is **not** a transitive dependency, then it
251-
// only applies to test/example targets.
252-
if !dep.is_transitive()
253-
&& !unit.target.is_test()
254-
&& !unit.target.is_example()
255-
&& !unit.mode.is_any_test()
256-
{
257-
return false;
258-
}
246+
// If this dependency is **not** a transitive dependency, then it
247+
// only applies to test/example targets.
248+
if !dep.is_transitive()
249+
&& !unit.target.is_test()
250+
&& !unit.target.is_example()
251+
&& !unit.mode.is_any_test()
252+
{
253+
return false;
254+
}
259255

260-
// If we've gotten past all that, then this dependency is
261-
// actually used!
262-
true
263-
})
264-
});
256+
// If we've gotten past all that, then this dependency is
257+
// actually used!
258+
true
259+
});
265260

266261
let mut ret = Vec::new();
267262
let mut dev_deps = Vec::new();
@@ -419,10 +414,7 @@ fn compute_deps_doc(
419414
state: &mut State<'_, '_>,
420415
unit_for: UnitFor,
421416
) -> CargoResult<Vec<UnitDep>> {
422-
let deps = state
423-
.deps(unit, unit_for)
424-
.into_iter()
425-
.filter(|&(_id, deps)| deps.iter().any(|dep| dep.kind() == DepKind::Normal));
417+
let deps = state.deps(unit, unit_for, &|dep| dep.kind() == DepKind::Normal);
426418

427419
// To document a library, we depend on dependencies actually being
428420
// built. If we're documenting *all* libraries, then we also depend on
@@ -780,14 +772,22 @@ impl<'a, 'cfg> State<'a, 'cfg> {
780772
}
781773

782774
/// Returns a filtered set of dependencies for the given unit.
783-
fn deps(&self, unit: &Unit, unit_for: UnitFor) -> Vec<(PackageId, &HashSet<Dependency>)> {
775+
fn deps(
776+
&self,
777+
unit: &Unit,
778+
unit_for: UnitFor,
779+
filter: &dyn Fn(&Dependency) -> bool,
780+
) -> Vec<(PackageId, &HashSet<Dependency>)> {
784781
let pkg_id = unit.pkg.package_id();
785782
let kind = unit.kind;
786783
self.resolve()
787784
.deps(pkg_id)
788785
.filter(|&(_id, deps)| {
789786
assert!(!deps.is_empty());
790787
deps.iter().any(|dep| {
788+
if !filter(dep) {
789+
return false;
790+
}
791791
// If this dependency is only available for certain platforms,
792792
// make sure we're only enabling it for that platform.
793793
if !self.target_data.dep_platform_activated(dep, kind) {

tests/testsuite/cfg.rs

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,3 +440,36 @@ Caused by:
440440
)
441441
.run();
442442
}
443+
444+
#[cargo_test]
445+
fn exclusive_dep_kinds() {
446+
// Checks for a bug where the same package with different cfg expressions
447+
// was not being filtered correctly.
448+
Package::new("bar", "1.0.0").publish();
449+
let p = project()
450+
.file(
451+
"Cargo.toml",
452+
r#"
453+
[package]
454+
name = "foo"
455+
version = "0.1.0"
456+
457+
[target.'cfg(abc)'.dependencies]
458+
bar = "1.0"
459+
460+
[target.'cfg(not(abc))'.build-dependencies]
461+
bar = "1.0"
462+
"#,
463+
)
464+
.file("src/lib.rs", "")
465+
.file("build.rs", "extern crate bar; fn main() {}")
466+
.build();
467+
468+
p.cargo("check").run();
469+
p.change_file("src/lib.rs", "extern crate bar;");
470+
p.cargo("check")
471+
.with_status(101)
472+
// can't find crate for `bar`
473+
.with_stderr_contains("[..]E0463[..]")
474+
.run();
475+
}

0 commit comments

Comments
 (0)