From 41d85cc7d369c0ae1dab936f9622d4c5e26cad5c Mon Sep 17 00:00:00 2001 From: Eric Huss Date: Thu, 17 Feb 2022 20:09:24 -0800 Subject: [PATCH] Simplify filtering. --- src/cargo/core/compiler/unit_dependencies.rs | 198 ++++++++----------- 1 file changed, 78 insertions(+), 120 deletions(-) diff --git a/src/cargo/core/compiler/unit_dependencies.rs b/src/cargo/core/compiler/unit_dependencies.rs index 48996d3330f..857436e9f90 100644 --- a/src/cargo/core/compiler/unit_dependencies.rs +++ b/src/cargo/core/compiler/unit_dependencies.rs @@ -262,22 +262,15 @@ fn compute_deps( return compute_deps_doc(unit, state, unit_for); } - let id = unit.pkg.package_id(); - - let filtered_deps = state.deps(unit, unit_for); - let mut ret = Vec::new(); let mut dev_deps = Vec::new(); - for (dep_pkg_id, deps) in filtered_deps { - let dep_pkg = state.get(dep_pkg_id); - let (could_have_non_artifact_lib, has_artifact_lib) = - calc_artifact_deps(unit, unit_for, dep_pkg_id, deps, state, &mut ret)?; - - let lib = package_lib(dep_pkg, could_have_non_artifact_lib, has_artifact_lib); - let dep_lib = match lib { - Some(t) => t, + for (dep_pkg_id, deps) in state.deps(unit, unit_for) { + let dep_lib = match calc_artifact_deps(unit, unit_for, dep_pkg_id, &deps, state, &mut ret)? + { + Some(lib) => lib, None => continue, }; + let dep_pkg = state.get(dep_pkg_id); let mode = check_or_build_mode(unit.mode, dep_lib); let dep_unit_for = unit_for.with_dependency(unit, dep_lib, unit_for.root_compile_kind()); @@ -357,6 +350,7 @@ fn compute_deps( && unit.mode.is_any_test() && (unit.target.is_test() || unit.target.is_bench()) { + let id = unit.pkg.package_id(); ret.extend( unit.pkg .targets() @@ -396,49 +390,28 @@ fn compute_deps( Ok(ret) } -/// Try to find a target in `dep_pkg` which is a library. -/// -/// `has_artifact` is true if `dep_pkg` has any artifact, and `artifact_lib` is true -/// if any of them wants a library to be available too. -fn package_lib( - dep_pkg: &Package, - could_have_non_artifact_lib: bool, - artifact_lib: bool, -) -> Option<&Target> { - dep_pkg - .targets() - .iter() - .find(|t| t.is_lib() && (could_have_non_artifact_lib || artifact_lib)) -} - /// Find artifacts for all `deps` of `unit` and add units that build these artifacts /// to `ret`. -fn calc_artifact_deps( +fn calc_artifact_deps<'a>( unit: &Unit, unit_for: UnitFor, dep_id: PackageId, - deps: &HashSet, - state: &State<'_, '_>, + deps: &[&Dependency], + state: &State<'a, '_>, ret: &mut Vec, -) -> CargoResult<(bool, bool)> { +) -> CargoResult> { let mut has_artifact_lib = false; - let mut num_artifacts = 0; + let mut maybe_non_artifact_lib = false; let artifact_pkg = state.get(dep_id); - let mut deps_past_filter = 0; - for (dep, artifact) in deps - .iter() - .filter(|dep| { - if non_custom_and_non_transitive_deps(unit, dep) { - deps_past_filter += 1; - true - } else { - false + for dep in deps { + let artifact = match dep.artifact() { + Some(a) => a, + None => { + maybe_non_artifact_lib = true; + continue; } - }) - .filter_map(|dep| dep.artifact().map(|a| (dep, a))) - { + }; has_artifact_lib |= artifact.is_lib(); - num_artifacts += 1; // Custom build scripts (build/compile) never get artifact dependencies, // but the run-build-script step does (where it is handled). if !unit.target.is_custom_build() { @@ -462,8 +435,11 @@ fn calc_artifact_deps( )?); } } - let could_be_non_artifact_lib = deps_past_filter != num_artifacts; - Ok((could_be_non_artifact_lib, has_artifact_lib)) + if has_artifact_lib || maybe_non_artifact_lib { + Ok(artifact_pkg.targets().iter().find(|t| t.is_lib())) + } else { + Ok(None) + } } /// Returns the dependencies needed to run a build script. @@ -519,10 +495,7 @@ fn compute_deps_custom_build( // // This is essentially the same as `calc_artifact_deps`, but there are some // subtle differences that require this to be implemented differently. - let artifact_build_deps = state.deps_filtered(unit, script_unit_for, &|_unit, dep| { - dep.kind() == DepKind::Build && dep.artifact().is_some() - }); - + // // Produce units that build all required artifact kinds (like binaries, // static libraries, etc) with the correct compile target. // @@ -532,10 +505,13 @@ fn compute_deps_custom_build( // `root_unit_compile_target` necessary. let root_unit_compile_target = unit_for.root_compile_kind(); let unit_for = UnitFor::new_host(/*host_features*/ true, root_unit_compile_target); - for (dep_pkg_id, deps) in artifact_build_deps { - let artifact_pkg = state.get(dep_pkg_id); - for build_dep in deps.iter().filter(|d| d.is_build()) { - let artifact = build_dep.artifact().expect("artifact dep"); + for (dep_pkg_id, deps) in state.deps(unit, script_unit_for) { + for dep in deps { + if dep.kind() != DepKind::Build || dep.artifact().is_none() { + continue; + } + let artifact_pkg = state.get(dep_pkg_id); + let artifact = dep.artifact().expect("artifact dep"); let resolved_artifact_compile_kind = artifact .target() .map(|target| target.to_resolved_compile_kind(root_unit_compile_target)); @@ -548,7 +524,7 @@ fn compute_deps_custom_build( state, resolved_artifact_compile_kind.unwrap_or(CompileKind::Host), artifact_pkg, - build_dep, + dep, )?); } } @@ -663,22 +639,16 @@ fn compute_deps_doc( state: &mut State<'_, '_>, unit_for: UnitFor, ) -> CargoResult> { - let deps = state.deps(unit, unit_for); - // To document a library, we depend on dependencies actually being // built. If we're documenting *all* libraries, then we also depend on // the documentation of the library being built. let mut ret = Vec::new(); - for (id, deps) in deps { - let (could_have_non_artifact_lib, has_artifact_lib) = - calc_artifact_deps(unit, unit_for, id, deps, state, &mut ret)?; - - let dep_pkg = state.get(id); - let lib = package_lib(dep_pkg, could_have_non_artifact_lib, has_artifact_lib); - let dep_lib = match lib { + for (id, deps) in state.deps(unit, unit_for) { + let dep_lib = match calc_artifact_deps(unit, unit_for, id, &deps, state, &mut ret)? { Some(lib) => lib, None => continue, }; + let dep_pkg = state.get(id); // Rustdoc only needs rmeta files for regular dependencies. // However, for plugins/proc macros, deps should be built like normal. let mode = check_or_build_mode(unit.mode, dep_lib); @@ -1098,73 +1068,61 @@ impl<'a, 'cfg> State<'a, 'cfg> { .unwrap_or_else(|_| panic!("expected {} to be downloaded", id)) } - /// Returns a set of dependencies for the given unit, with a default filter. - fn deps(&self, unit: &Unit, unit_for: UnitFor) -> Vec<(PackageId, &HashSet)> { - self.deps_filtered(unit, unit_for, &non_custom_and_non_transitive_deps) - } - /// Returns a filtered set of dependencies for the given unit. - fn deps_filtered( - &self, - unit: &Unit, - unit_for: UnitFor, - filter: &dyn Fn(&Unit, &Dependency) -> bool, - ) -> Vec<(PackageId, &HashSet)> { + fn deps(&self, unit: &Unit, unit_for: UnitFor) -> Vec<(PackageId, Vec<&Dependency>)> { let pkg_id = unit.pkg.package_id(); let kind = unit.kind; self.resolve() .deps(pkg_id) - .filter(|&(_id, deps)| { + .filter_map(|(id, deps)| { assert!(!deps.is_empty()); - deps.iter().any(|dep| { - if !filter(unit, dep) { - return false; - } + let deps: Vec<_> = deps + .iter() + .filter(|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 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 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_doc_scrape() + && !unit.mode.is_any_test() + { + 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(dep.artifact()); - if !self.is_dep_activated(pkg_id, features_for, dep.name_in_toml()) { + // 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 we've gotten past all that, then this dependency is - // actually used! - true - }) + // 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(dep.artifact()); + if !self.is_dep_activated(pkg_id, features_for, dep.name_in_toml()) { + return false; + } + } + + // If we've gotten past all that, then this dependency is + // actually used! + true + }) + .collect(); + if deps.is_empty() { + None + } else { + Some((id, deps)) + } }) .collect() } } - -fn non_custom_and_non_transitive_deps(unit: &Unit, dep: &Dependency) -> bool { - // 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_doc_scrape() - && !unit.mode.is_any_test() - { - return false; - } - - // If we've gotten past all that, then this dependency is - // actually used! - true -}