Skip to content

Commit

Permalink
Fix panic in cargo doc with -Zfeatures=itarget
Browse files Browse the repository at this point in the history
  • Loading branch information
@ehuss
ehuss committed Oct 13, 2020
1 parent 31e1f1f commit 7f02a67
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 50 deletions.
109 changes: 59 additions & 50 deletions src/cargo/core/compiler/unit_dependencies.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::core::dependency::DepKind;
use crate::core::profiles::{Profile, Profiles, UnitFor};
use crate::core::resolver::features::{FeaturesFor, ResolvedFeatures};
use crate::core::resolver::Resolve;
use crate::core::{Package, PackageId, PackageSet, Target, Workspace};
use crate::core::{Dependency, Package, PackageId, PackageSet, Target, Workspace};
use crate::ops::resolve_all_features;
use crate::util::interning::InternedString;
use crate::util::Config;
Expand Down Expand Up @@ -223,50 +223,33 @@ fn compute_deps(
}

let id = unit.pkg.package_id();
let filtered_deps = state.resolve().deps(id).filter(|&(_id, deps)| {
assert!(!deps.is_empty());
deps.iter().any(|dep| {
// If this target is a build command, then we only want build
// dependencies, otherwise we want everything *other than* build
// dependencies.
if unit.target.is_custom_build() != dep.is_build() {
return false;
}

// If this dependency is **not** a transitive dependency, then it
// only applies to test/example targets.
if !dep.is_transitive()
&& !unit.target.is_test()
&& !unit.target.is_example()
&& !unit.mode.is_any_test()
{
return false;
}

// If this dependency is only available for certain platforms,
// make sure we're only enabling it for that platform.
if !state.target_data.dep_platform_activated(dep, unit.kind) {
return false;
}

// If this is an optional dependency, and the new feature resolver
// did not enable it, don't include it.
if dep.is_optional() {
let features_for = unit_for.map_to_features_for();
let filtered_deps = state
.deps(unit, unit_for)
.into_iter()
.filter(|&(_id, deps)| {
deps.iter().any(|dep| {
// If this target is a build command, then we only want build
// dependencies, otherwise we want everything *other than* build
// dependencies.
if unit.target.is_custom_build() != dep.is_build() {
return false;
}

let feats = state.activated_features(id, features_for);
if !feats.contains(&dep.name_in_toml()) {
// If this dependency is **not** a transitive dependency, then it
// only applies to test/example targets.
if !dep.is_transitive()
&& !unit.target.is_test()
&& !unit.target.is_example()
&& !unit.mode.is_any_test()
{
return false;
}
}

// If we've gotten past all that, then this dependency is
// actually used!
true
})
});
// Separate line to avoid rustfmt indentation. Must collect due to `state` capture.
let filtered_deps: Vec<_> = filtered_deps.collect();
// If we've gotten past all that, then this dependency is
// actually used!
true
})
});

let mut ret = Vec::new();
for (id, _) in filtered_deps {
Expand Down Expand Up @@ -410,16 +393,10 @@ fn compute_deps_custom_build(

/// Returns the dependencies necessary to document a package.
fn compute_deps_doc(unit: &Unit, state: &mut State<'_, '_>) -> CargoResult<Vec<UnitDep>> {
let target_data = state.target_data;
let deps = state
.resolve()
.deps(unit.pkg.package_id())
.filter(|&(_id, deps)| {
deps.iter().any(|dep| match dep.kind() {
DepKind::Normal => target_data.dep_platform_activated(dep, unit.kind),
_ => false,
})
});
.deps(unit, UnitFor::new_normal())
.into_iter()
.filter(|&(_id, deps)| deps.iter().any(|dep| dep.kind() == DepKind::Normal));

// To document a library, we depend on dependencies actually being
// built. If we're documenting *all* libraries, then we also depend on
Expand Down Expand Up @@ -741,4 +718,36 @@ impl<'a, 'cfg> State<'a, 'cfg> {
.get_one(id)
.unwrap_or_else(|_| panic!("expected {} to be downloaded", id))
}

/// Returns a filtered set of dependencies for the given unit.
fn deps(&self, unit: &Unit, unit_for: UnitFor) -> Vec<(PackageId, &HashSet<Dependency>)> {
let pkg_id = unit.pkg.package_id();
let kind = unit.kind;
self.resolve()
.deps(pkg_id)
.filter(|&(_id, deps)| {
assert!(!deps.is_empty());
deps.iter().any(|dep| {
// If this dependency is only available for certain platforms,
// make sure we're only enabling it for that platform.
if !self.target_data.dep_platform_activated(dep, kind) {
return false;
}

// If this is an optional dependency, and the new feature resolver
// did not enable it, don't include it.
if dep.is_optional() {
let features_for = unit_for.map_to_features_for();

let feats = self.activated_features(pkg_id, features_for);
if !feats.contains(&dep.name_in_toml()) {
return false;
}
}

true
})
})
.collect()
}
}
49 changes: 49 additions & 0 deletions tests/testsuite/features2.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1946,3 +1946,52 @@ fn test_proc_macro() {
.masquerade_as_nightly_cargo()
.run();
}

#[cargo_test]
fn doc_optional() {
// Checks for a bug where `cargo doc` was failing with an inactive target
// that enables a shared optional dependency.
Package::new("spin", "1.0.0").publish();
Package::new("bar", "1.0.0")
.add_dep(Dependency::new("spin", "1.0").optional(true))
.publish();
// The enabler package enables the `spin` feature, which we don't want.
Package::new("enabler", "1.0.0")
.feature_dep("bar", "1.0", &["spin"])
.publish();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
[target.'cfg(whatever)'.dependencies]
enabler = "1.0"
[dependencies]
bar = "1.0"
"#,
)
.file("src/lib.rs", "")
.build();

p.cargo("doc -Zfeatures=itarget")
.masquerade_as_nightly_cargo()
.with_stderr_unordered(
"\
[UPDATING] [..]
[DOWNLOADING] crates ...
[DOWNLOADED] spin v1.0.0 [..]
[DOWNLOADED] bar v1.0.0 [..]
[DOWNLOADING] crates ...
[DOWNLOADED] enabler v1.0.0 [..]
[DOCUMENTING] bar v1.0.0
[CHECKING] bar v1.0.0
[DOCUMENTING] foo v0.1.0 [..]
[FINISHED] [..]
",
)
.run();
}

0 comments on commit 7f02a67

Please sign in to comment.