Rework handling of cached failures

4 files changed, 23 insertions, 41 deletions

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index cf82a1636..ac33f041c 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -193,7 +193,6 @@ class Artifact:
         log_filename = context.messenger.get_log_filename()
         if log_filename:
             digest = self._cas.add_object(path=log_filename)
-            element._build_log_path = self._cas.objpath(digest)
             log = artifact.logs.add()
             log.name = os.path.basename(log_filename)
             log.digest.CopyFrom(digest)
diff --git a/src/buildstream/_exceptions.py b/src/buildstream/_exceptions.py
index 30fc26e71..755da3a9d 100644
--- a/src/buildstream/_exceptions.py
+++ b/src/buildstream/_exceptions.py
@@ -267,6 +267,14 @@ class AppError(BstError):
         super().__init__(message, detail=detail, domain=ErrorDomain.APP, reason=reason)
 
 
+# CachedFailure
+#
+# Raised from a child process within a job to indicate that the failure was cached
+#
+class CachedFailure(BstError):
+    pass
+
+
 # SkipJob
 #
 # Raised from a child process within a job when the job should be
diff --git a/src/buildstream/_scheduler/queues/buildqueue.py b/src/buildstream/_scheduler/queues/buildqueue.py
index 5cbd5af57..c7de0b880 100644
--- a/src/buildstream/_scheduler/queues/buildqueue.py
+++ b/src/buildstream/_scheduler/queues/buildqueue.py
@@ -18,11 +18,8 @@
 #        Tristan Van Berkom <tristan.vanberkom@codethink.co.uk>
 #        Jürg Billeter <juerg.billeter@codethink.co.uk>
 
-from datetime import timedelta
-
 from . import Queue, QueueStatus
 from ..resources import ResourceType
-from ..._message import MessageType
 from ..jobs import JobStatus
 
 
@@ -34,38 +31,6 @@ class BuildQueue(Queue):
     complete_name = "Built"
     resources = [ResourceType.PROCESS, ResourceType.CACHE]
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-        self._tried = set()
-
-    def enqueue(self, elts):
-        to_queue = []
-
-        for element in elts:
-            if not element._cached_failure() or element in self._tried:
-                to_queue.append(element)
-                continue
-
-            # XXX: Fix this, See https://mail.gnome.org/archives/buildstream-list/2018-September/msg00029.html
-            # Bypass queue processing entirely the first time it's tried.
-            self._tried.add(element)
-            _, description, detail = element._get_build_result()
-            logfile = element._get_build_log()
-            self._message(
-                element,
-                MessageType.FAIL,
-                description,
-                detail=detail,
-                action_name=self.action_name,
-                elapsed=timedelta(seconds=0),
-                logfile=logfile,
-            )
-            self._done_queue.append(element)
-            element_name = element._get_full_name()
-            self._task_group.add_failed_task(element_name)
-
-        return super().enqueue(to_queue)
-
     def get_process_func(self):
         return BuildQueue._assemble_element
 
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index a7a84a379..3cca7a081 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -91,7 +91,7 @@ from ruamel import yaml
 from . import _yaml
 from ._variables import Variables
 from ._versions import BST_CORE_ARTIFACT_VERSION
-from ._exceptions import BstError, LoadError, ImplError, SourceCacheError
+from ._exceptions import BstError, LoadError, ImplError, SourceCacheError, CachedFailure
 from .exceptions import ErrorDomain, LoadErrorReason
 from .utils import FileListResult, BST_ARBITRARY_TIMESTAMP
 from . import utils
@@ -256,7 +256,6 @@ class Element(Plugin):
         self.__required = False  # Whether the artifact is required in the current session
         self.__artifact_files_required = False  # Whether artifact files are required in the local cache
         self.__build_result = None  # The result of assembling this Element (success, description, detail)
-        self._build_log_path = None  # The path of the build log for this Element
         # Artifact class for direct artifact composite interaction
         self.__artifact = None  # type: Optional[Artifact]
         self.__strict_artifact = None  # Artifact for strict cache key
@@ -1558,6 +1557,20 @@ class Element(Plugin):
     #
     def _assemble(self):
 
+        # Only do this the first time around (i.e. __assemble_done is False)
+        # to allow for retrying the job
+        if self._cached_failure() and not self.__assemble_done:
+            with self._output_file() as output_file:
+                for log_path in self.__artifact.get_logs():
+                    with open(log_path) as log_file:
+                        output_file.write(log_file.read())
+
+            _, description, detail = self._get_build_result()
+            e = CachedFailure(description, detail=detail)
+            # Shelling into a sandbox is useful to debug this error
+            e.sandbox = True
+            raise e
+
         # Assert call ordering
         assert not self._cached_success()
 
@@ -1685,9 +1698,6 @@ class Element(Plugin):
 
         return artifact_size
 
-    def _get_build_log(self):
-        return self._build_log_path
-
     # _fetch_done()
     #
     # Indicates that fetching the sources for this element has been done.