Skip to content

Commit

Permalink
fix(filter): dependencies filter should match transitive deps (#7886)
Browse files Browse the repository at this point in the history
### Description

Fixes bug from port where `...` filter would only select immediate
dependencies.

In Go we used
[`graph.Ancestors`](https://github.com/vercel/turbo/blob/v1.10.3/cli/internal/scope/filter/filter.go#L182)
which performs a full DFS, not just immediate ancestors. This PR updates
so we do the same in Rust.

### Testing Instructions

Added unit test that uses `...` filter on a project with transitive
dependencies.


We already have test cases for the reverse case of [including reverse
dependants](https://github.com/vercel/turbo/blob/main/crates/turborepo-lib/src/run/scope/filter.rs#L806)

For good measure did a manual check in repo with following dep chain:
`docs -> @repo/ui -> foo`

```
[0 olszewski@chriss-mbp] /tmp/dingus $ turbo_dev --skip-infer build --filter=docs... --output-logs=errors-only --no-daemon     
• Packages in scope: @repo/eslint-config, @repo/typescript-config, @repo/ui, docs, foo
• Running build in 5 packages
• Remote caching disabled

 Tasks:    2 successful, 2 total
Cached:    2 cached, 2 total
  Time:    149ms >>> FULL TURBO

[0 olszewski@chriss-mbp] /tmp/dingus $ turbo_dev --skip-infer build --filter=...foo --output-logs=errors-only --no-daemon  
• Packages in scope: @repo/ui, docs, foo, web
• Running build in 4 packages
• Remote caching disabled

 Tasks:    3 successful, 3 total
Cached:    2 cached, 3 total
  Time:    9.829s 
```

Closes TURBO-2749
  • Loading branch information
chris-olszewski committed Apr 2, 2024
1 parent 69f5007 commit 2f2406f
Showing 1 changed file with 19 additions and 10 deletions.
29 changes: 19 additions & 10 deletions crates/turborepo-lib/src/run/scope/filter.rs
Expand Up @@ -281,10 +281,10 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> {
let node = package_graph::PackageNode::Workspace(package.clone());

if selector.include_dependencies {
let dependencies = self.pkg_graph.immediate_dependencies(&node);
let dependencies = self.pkg_graph.dependencies(&node);
let dependencies = dependencies
.iter()
.flatten()
.filter(|node| !matches!(node, package_graph::PackageNode::Root))
.map(|i| i.as_package_name().to_owned())
.collect::<Vec<_>>();

Expand All @@ -303,11 +303,11 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> {
package_graph::PackageNode::Workspace(dependent.to_owned());

let dependent_dependencies =
self.pkg_graph.immediate_dependencies(&dependent_node);
self.pkg_graph.dependencies(&dependent_node);

let dependent_dependencies = dependent_dependencies
.iter()
.flatten()
.filter(|node| !matches!(node, package_graph::PackageNode::Root))
.map(|i| i.as_package_name().to_owned())
.collect::<HashSet<_>>();

Expand Down Expand Up @@ -401,7 +401,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> {
}

let workspace_node = package_graph::PackageNode::Workspace(package.clone());
let dependencies = self.pkg_graph.immediate_dependencies(&workspace_node);
let dependencies = self.pkg_graph.dependencies(&workspace_node);

for changed_package in &changed_packages {
if !selector.exclude_self && package.eq(changed_package) {
Expand All @@ -412,11 +412,7 @@ impl<'a, T: GitChangeDetector> FilterResolver<'a, T> {
let changed_node =
package_graph::PackageNode::Workspace(changed_package.to_owned());

if dependencies
.as_ref()
.map(|d| d.contains(&changed_node))
.unwrap_or_default()
{
if dependencies.contains(&changed_node) {
roots.insert(package.clone());
matched.insert(package);
break;
Expand Down Expand Up @@ -779,6 +775,19 @@ mod test {
&["project-1", "project-2", "project-4"] ;
"select package with dependencies"
)]
#[test_case(
vec![
TargetSelector {
exclude_self: false,
include_dependencies: true,
name_pattern: "project-0".to_string(),
..Default::default()
}
],
None,
&["project-0", "project-1", "project-2", "project-4", "project-5"] ;
"select package with transitive dependencies"
)]
#[test_case(
vec![
TargetSelector {
Expand Down

0 comments on commit 2f2406f

Please sign in to comment.