diff --git a/docs/reference/migrations.rst b/docs/reference/migrations.rst index 1cb078cef..9faa4fd66 100644 --- a/docs/reference/migrations.rst +++ b/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 ------------ diff --git a/src/taskgraph/generator.py b/src/taskgraph/generator.py index 044069ae5..e5ca4c9c6 100644 --- a/src/taskgraph/generator.py +++ b/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, diff --git a/src/taskgraph/graph.py b/src/taskgraph/graph.py index 573275087..343e7bbea 100644 --- a/src/taskgraph/graph.py +++ b/src/taskgraph/graph.py @@ -34,7 +34,7 @@ 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 : the graph containing all specified nodes as well as any nodes reachable from them, and any intervening edges. @@ -42,6 +42,9 @@ def transitive_closure(self, nodes, reverse=False): 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 diff --git a/test/test_generator.py b/test/test_generator.py index 0353295ab..33738d866 100644 --- a/test/test_generator.py +++ b/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 diff --git a/test/test_graph.py b/test/test_graph.py index f1d683cc4..14d306e4b 100644 --- a/test/test_graph.py +++ b/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()), [])