diff --git a/scarb/src/core/resolver.rs b/scarb/src/core/resolver.rs index e7a7e5af5..82ddd7cdd 100644 --- a/scarb/src/core/resolver.rs +++ b/scarb/src/core/resolver.rs @@ -4,7 +4,7 @@ use anyhow::{bail, Result}; use indoc::formatdoc; use itertools::Itertools; use petgraph::graphmap::DiGraphMap; -use petgraph::visit::{Dfs, EdgeFiltered, Walker}; +use petgraph::visit::{Dfs, EdgeFiltered, IntoNeighborsDirected, Walker}; use smallvec::SmallVec; use crate::core::lockfile::Lockfile; @@ -58,6 +58,20 @@ impl Resolve { self.graph .neighbors_directed(package_id, petgraph::Direction::Outgoing) } + + /// Collect [`PackageId`]s of directed dependencies of the package, that accept the given target kind. + pub fn package_dependencies_for_target_kind( + &self, + package_id: PackageId, + target_kind: &TargetKind, + ) -> Vec { + let filtered_graph = EdgeFiltered::from_fn(&self.graph, move |(node_a, _node_b, edge)| { + edge.accepts_target(target_kind.clone(), node_a == package_id) + }); + filtered_graph + .neighbors_directed(package_id, petgraph::Direction::Outgoing) + .collect_vec() + } } #[derive(Debug, Default, Clone, PartialEq, Eq)] diff --git a/scarb/src/ops/resolve.rs b/scarb/src/ops/resolve.rs index da0b99483..b0a822353 100644 --- a/scarb/src/ops/resolve.rs +++ b/scarb/src/ops/resolve.rs @@ -50,6 +50,19 @@ impl WorkspaceResolve { .map(|id| self.packages[id].clone()) .collect_vec() } + + pub fn package_dependencies( + &self, + package_id: PackageId, + target_kind: &TargetKind, + ) -> Vec { + assert!(self.packages.contains_key(&package_id)); + self.resolve + .package_dependencies_for_target_kind(package_id, target_kind) + .iter() + .map(|id| self.packages[id].clone()) + .collect_vec() + } } #[derive(Debug, Default)] @@ -398,55 +411,23 @@ fn cairo_compilation_unit_for_target( }; // Collect dependencies for the components. + let member_component = components + .iter() + .find(|component| component.package.id == member.id) + .unwrap(); + let mut test_package_deps = solution.component_dependencies(member_component, &components); + test_package_deps.push(member_component.id.clone()); + let dependencies_for_components: Vec<_> = components .iter() .map(|component| { - // Those are direct dependencies of the component. - let dependencies_summary: Vec<&ManifestDependency> = component - .package - .manifest - .summary - .full_dependencies() - .collect(); - - // We iterate over all the compilation unit components to get dependency's version. - let mut dependencies: Vec = components - .iter() - .filter(|component_as_dependency| { - dependencies_summary.iter().any(|dependency_summary| { - dependency_summary.name == component_as_dependency.package.id.name - }) || - // This is a hacky way of accommodating integration test components, - // which need to depend on the tested package. - component_as_dependency - .package - .manifest - .targets - .iter() - .filter(|target| target.kind.is_test()) - .any(|target| { - target.group_id.clone().unwrap_or(target.name.clone()) - == component.package.id.name.to_smol_str() - && component_as_dependency.cairo_package_name() != component.cairo_package_name() - }) - }) - .map(|compilation_unit_component| compilation_unit_component.id.clone() - ) - .collect(); - - // Adds itself to dependencies - let is_integration_test = if component.first_target().kind.is_test() { - let props: Option = component.first_target().props().ok(); - props - .map(|props| props.test_type == TestTargetType::Integration) - .unwrap_or_default() - } else { false }; - if !is_integration_test { - dependencies.push(component.id.clone()); + if component.package.id == test_package_id { + test_package_deps.clone() + } else { + solution.component_dependencies(component, &components) } - - dependencies - }).collect(); + }) + .collect(); for (component, dependencies) in zip(&mut components, dependencies_for_components) { component.dependencies = dependencies; @@ -629,6 +610,45 @@ impl<'a> PackageSolutionCollector<'a> { Ok((packages, cairo_plugins)) } + pub fn component_dependencies( + &self, + component: &CompilationUnitComponent, + components: &[CompilationUnitComponent], + ) -> Vec { + let package_id = component.id.package_id; + + // Those are direct dependencies of the component. + let dependencies_packages = self + .resolve + .package_dependencies(package_id, self.target_kind.as_ref().unwrap()); + + // We iterate over all the compilation unit components to get dependency's version. + let mut dependencies: Vec = components + .iter() + .filter(|component_as_dependency| { + dependencies_packages.iter().any(|dependency_summary| { + dependency_summary.id == component_as_dependency.package.id + }) + }) + .map(|compilation_unit_component| compilation_unit_component.id.clone()) + .collect(); + + // Adds itself to dependencies + let is_integration_test = if component.first_target().kind.is_test() { + let props: Option = component.first_target().props().ok(); + props + .map(|props| props.test_type == TestTargetType::Integration) + .unwrap_or_default() + } else { + false + }; + if !is_integration_test { + dependencies.push(component.id.clone()); + } + + dependencies + } + pub fn show_warnings(self) { for warning in self.warnings { self.ws.config().ui().warn(warning); diff --git a/scarb/tests/build_targets.rs b/scarb/tests/build_targets.rs index 630e1547e..edba3c70b 100644 --- a/scarb/tests/build_targets.rs +++ b/scarb/tests/build_targets.rs @@ -1047,3 +1047,46 @@ fn test_target_builds_external() { t.child("hello/target/dev/hello_unittest.test.starknet_artifacts.json") .assert_is_json::(); } + +#[test] +fn transitive_dev_deps_not_available() { + let t = TempDir::new().unwrap(); + + let first = &t.child("first"); + ProjectBuilder::start() + .lib_cairo(indoc! {r#" + pub fn forty_two() -> felt252 { 42 } + "#}) + .name("first") + .build(first); + let second = &t.child("second"); + ProjectBuilder::start() + .name("second") + .dep("first", first) + .build(second); + let hello = t.child("hello"); + ProjectBuilder::start() + .name("hello") + .lib_cairo(indoc! {r#" + use first::forty_two; + pub fn main() -> felt252 { forty_two() } + "#}) + .dep("second", second) + .dev_dep("first", first) + .build(&hello); + + Scarb::quick_snapbox() + .arg("check") + .current_dir(hello) + .assert() + .failure() + .stdout_matches(indoc! {r#" + [..]Checking hello v1.0.0 ([..]Scarb.toml) + error: Identifier not found. + --> [..]lib.cairo:1:5 + use first::forty_two; + ^***^ + + error: could not check `hello` due to previous error + "#}); +}