fix!: ensure if-dependencies apply at target phase

BREAKING CHANGE: Tasks will no longer pull `if-dependencies` in at the
target phase.

It's now possible for a task with multiple `if-dependencies` to run
after one task, without also causing the other to run.

Author: ahal

docs/reference/migrations.rst

@@ -3,6 +3,14 @@ Migration Guide
 
 This page can help when migrating Taskgraph across major versions.
 
+19.x -> 20.x
+------------
+
+* Tasks using ``if-dependencies`` will no longer pull these dependencies into
+  the graph during the target phase. If a desired dependency is no longer
+  showing up in the target graph, adjust your `target_tasks` logic to ensure
+  the dependency is explicitly selected.
+
 18.x -> 19.x
 ------------
 

src/taskgraph/generator.py

@@ -533,7 +533,22 @@ def load_tasks():
             f"Adding {len(always_target_tasks) - len(always_target_tasks & target_tasks)} tasks with `always_target` attribute"  # type: ignore
         )
         requested_tasks = target_tasks | always_target_tasks  # type: ignore
-        target_graph = full_task_graph.graph.transitive_closure(requested_tasks)
+
+        # Exclude unrequested if-dependency edges from transitive closure.
+        exclude_edges = set()
+        for task in all_tasks.values():
+            if missing_if_deps := set(task.if_dependencies) - requested_tasks:
+                exclude_edges.update(
+                    {
+                        (task.label, label, edge)
+                        for edge, label in task.dependencies.items()
+                        if label in missing_if_deps
+                    }
+                )
+
+        target_graph = full_task_graph.graph.transitive_closure(
+            requested_tasks, exclude_edges=exclude_edges
+        )
         target_task_graph = TaskGraph(
             {l: all_tasks[l] for l in target_graph.nodes},
             target_graph,

src/taskgraph/graph.py

@@ -34,14 +34,17 @@ class Graph(_Graph):
     def __init__(self, nodes, edges):
         super().__init__(frozenset(nodes), frozenset(edges))
 
-    def transitive_closure(self, nodes, reverse=False):
+    def transitive_closure(self, nodes, reverse=False, exclude_edges=None):
         """Return the transitive closure of <nodes>: the graph containing all
         specified nodes as well as any nodes reachable from them, and any
         intervening edges.
 
         If `reverse` is true, the "reachability" will be reversed and this
         will return the set of nodes that can reach the specified nodes.
 
+        If `exclude_edges` is provided, those edges will not be followed during
+        traversal.
+
         Example:
 
         .. code-block::
@@ -61,6 +64,8 @@ def transitive_closure(self, nodes, reverse=False):
                 f"Unknown nodes in transitive closure: {nodes - self.nodes}"
             )
 
+        exclude_edges = exclude_edges or set()
+
         # generate a new graph by expanding along edges until reaching a fixed
         # point
         new_nodes, new_edges = nodes, set()
@@ -71,6 +76,7 @@ def transitive_closure(self, nodes, reverse=False):
                 (left, right, name)
                 for (left, right, name) in self.edges
                 if (right if reverse else left) in nodes
+                and (left, right, name) not in exclude_edges
             }
             add_nodes = {(left if reverse else right) for (left, right, _) in add_edges}
             new_nodes = nodes | add_nodes

test/test_generator.py

@@ -414,3 +414,56 @@ def test_kind_graph_with_target_kinds(maketgg):
     # _fake3 and _other should not be included
     assert "_fake3" not in kind_graph.nodes
     assert "_other" not in kind_graph.nodes
+
+
+def test_if_dependencies_not_in_target_are_removed(maketgg):
+    "If-dependencies not in requested_tasks don't pull tasks into target graph"
+    tgg = maketgg(
+        target_tasks=["_other-t-2"],
+        kinds=[
+            ("_fake", {}),
+            (
+                "_other",
+                {
+                    "task-defaults": {
+                        "dependencies": {"if-dep": "_fake-t-0"},
+                        "if-dependencies": ["_fake-t-0"],
+                    }
+                },
+            ),
+        ],
+    )
+
+    full_task = tgg.full_task_set.tasks["_other-t-2"]
+    assert "_fake-t-0" in full_task.if_dependencies
+
+    target_graph = tgg.target_task_graph
+
+    assert "_other-t-2" in target_graph.tasks
+    assert "_other-t-1" in target_graph.tasks
+    assert "_other-t-0" in target_graph.tasks
+    assert "_fake-t-0" not in target_graph.tasks
+
+
+def test_if_dependencies_in_target_are_kept(maketgg):
+    "If-dependencies that are in requested_tasks are kept"
+    tgg = maketgg(
+        target_tasks=["_other-t-2", "_fake-t-0"],
+        kinds=[
+            ("_fake", {}),
+            (
+                "_other",
+                {
+                    "task-defaults": {
+                        "dependencies": {"if-dep": "_fake-t-0"},
+                        "if-dependencies": ["_fake-t-0"],
+                    }
+                },
+            ),
+        ],
+    )
+
+    target_graph = tgg.target_task_graph
+
+    assert "_other-t-2" in target_graph.tasks
+    assert "_fake-t-0" in target_graph.tasks

test/test_graph.py

@@ -134,6 +134,30 @@ def test_transitive_closure_loopy(self):
         "transitive closure of a loop is the whole loop"
         self.assertEqual(self.loopy.transitive_closure({"A"}), self.loopy)
 
+    def test_transitive_closure_exclude_edges(self):
+        "transitive closure with excluded edges doesn't follow those edges"
+        self.assertEqual(
+            self.multi_edges.transitive_closure(
+                {"3"}, exclude_edges={("3", "2", "green")}
+            ),
+            Graph(
+                {"1", "2", "3"},
+                {
+                    ("2", "1", "red"),
+                    ("2", "1", "blue"),
+                    ("3", "1", "red"),
+                    ("3", "2", "blue"),
+                },
+            ),
+        )
+
+        self.assertEqual(
+            self.multi_edges.transitive_closure(
+                {"3"}, exclude_edges={("3", "2", "blue"), ("3", "2", "green")}
+            ),
+            Graph({"1", "3"}, {("3", "1", "red")}),
+        )
+
     def test_visit_postorder_empty(self):
         "postorder visit of an empty graph is empty"
         self.assertEqual(list(Graph(set(), set()).visit_postorder()), [])