fix infinite loop in cached_tasks transforms

Detect dependency cycles instead of looping forever, by using
taskgraph.graph.Graph instead of a custom graph traversal.
Update the graphs in the tests to not have multiple tasks with the same
label, as that breaks uniqueness assumptions.

Author: jcristau

src/taskgraph/transforms/cached_tasks.py

@@ -3,9 +3,8 @@
 # file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
 
-from collections import deque
-
 import taskgraph
+from taskgraph.graph import Graph
 from taskgraph.transforms.base import TransformSequence
 from taskgraph.util.cached_tasks import add_optimization
 
@@ -15,25 +14,19 @@
 def order_tasks(config, tasks):
     """Iterate image tasks in an order where parent tasks come first."""
     kind_prefix = config.kind + "-"
-
-    pending = deque(tasks)
-    task_labels = {task["label"] for task in pending}
-    emitted = set()
-    while True:
-        try:
-            task = pending.popleft()
-        except IndexError:
-            break
-        parents = {
-            task
-            for task in task.get("dependencies", {}).values()
-            if task.startswith(kind_prefix)
-        }
-        if parents and not emitted.issuperset(parents & task_labels):
-            pending.append(task)
-            continue
-        emitted.add(task["label"])
-        yield task
+    pending = {task["label"]: task for task in tasks}
+    nodes = set(pending)
+    edges = {
+        (label, dep, "")
+        for label in pending
+        for dep in pending[label].get("dependencies", {}).values()
+        if dep.startswith(kind_prefix)
+    }
+    graph = Graph(nodes, edges)
+
+    for label in graph.visit_postorder():
+        yield pending.pop(label)
+    assert not pending
 
 
 def format_task_digest(cached_task):

test/test_transforms_cached_tasks.py

@@ -55,6 +55,7 @@ def assert_cache_basic(tasks):
 def assert_cache_with_dependency(tasks):
     handle_exception(tasks)
     assert len(tasks) == 2
+    tasks = sorted(tasks, key=lambda t: t["label"])
     assert tasks[1] == {
         "attributes": {
             "cached_task": {
@@ -65,7 +66,7 @@ def assert_cache_with_dependency(tasks):
         },
         "dependencies": {"edge": "dep-cached"},
         "description": "description",
-        "label": "cached-task",
+        "label": "cached-with-dep",
         "optimization": {
             "index-search": [
                 "test-domain.cache.level-3.cached-task.v2.cache-foo.hash.db201e53944fccbb16736c8153a14de39748c0d290de84bd976c11ddcc413089",
@@ -92,15 +93,16 @@ def assert_cache_with_non_cached_dependency(e):
 
 def assert_chain_of_trust_influences_digest(tasks):
     assert len(tasks) == 3
-    # The first two tasks are chain-of-trust unspecified, and chain-of-trust: False
+    tasks = sorted(tasks, key=lambda t: t["label"])  # cot-false, cot-true, no-cot
+    # The first and third tasks are chain-of-trust: false, and chain-of-trust unspecified
     # which should result in the same digest.
     digest_0 = tasks[0]["attributes"]["cached_task"]["digest"]
-    digest_1 = tasks[1]["attributes"]["cached_task"]["digest"]
-    assert digest_0 == digest_1
-
-    # The third task is chain-of-trust: True, and should have a different digest
     digest_2 = tasks[2]["attributes"]["cached_task"]["digest"]
-    assert digest_0 != digest_2
+    assert digest_0 == digest_2
+
+    # The second task is chain-of-trust: True, and should have a different digest
+    digest_1 = tasks[1]["attributes"]["cached_task"]["digest"]
+    assert digest_0 != digest_1
 
 
 @pytest.mark.parametrize(
@@ -141,7 +143,8 @@ def assert_chain_of_trust_influences_digest(tasks):
                         "type": "cached-task.v2",
                         "name": "cache-foo",
                         "digest-data": ["abc"],
-                    }
+                    },
+                    "label": "cached-no-dep",
                 },
                 # This task has same digest-data, but a dependency on a cached task.
                 {
@@ -151,6 +154,7 @@ def assert_chain_of_trust_influences_digest(tasks):
                         "digest-data": ["abc"],
                     },
                     "dependencies": {"edge": "dep-cached"},
+                    "label": "cached-with-dep",
                 },
             ],
             # kind config
@@ -198,6 +202,7 @@ def assert_chain_of_trust_influences_digest(tasks):
                         "name": "cache-foo",
                         "digest-data": ["abc"],
                     },
+                    "label": "no-cot",
                     # no explicit chain of trust configuration; should be the
                     # same as when it is set to False
                 },
@@ -207,6 +212,7 @@ def assert_chain_of_trust_influences_digest(tasks):
                         "name": "cache-foo",
                         "digest-data": ["abc"],
                     },
+                    "label": "cot-false",
                     "worker": {
                         "chain-of-trust": False,
                     },
@@ -217,6 +223,7 @@ def assert_chain_of_trust_influences_digest(tasks):
                         "name": "cache-foo",
                         "digest-data": ["abc"],
                     },
+                    "label": "cot-true",
                     "worker": {"chain-of-trust": True},
                 },
             ],