element.py: Only run `_schedule_assemble()` when necessary

1 files changed, 16 insertions, 25 deletions

diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 954f2e7d1..5cc7872f9 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1299,21 +1299,6 @@ class Element(Plugin):
     def _update_state(self):
         context = self._get_context()
 
-        # If the element wasn't assembled and isn't scheduled to be assemble,
-        # or cached, or waiting to be pulled but has an artifact then schedule
-        # the assembly.
-        scheduled = self._schedule_assemble()
-
-        if scheduled:
-            # If a build has been scheduled, we know that the element
-            # is not cached and can allow cache query even if the strict cache
-            # key is not available yet.
-            if self.__can_query_cache_callback is not None:
-                self.__can_query_cache_callback(self)
-                self.__can_query_cache_callback = None
-
-            return
-
         if not context.get_strict():
             self.__update_cache_key_non_strict()
 
@@ -1544,7 +1529,11 @@ class Element(Plugin):
         for dep in self.dependencies(Scope.RUN, recurse=False):
             dep._set_required()
 
-        self._update_state()
+        # When an element becomes required, it must be assembled for
+        # the current pipeline. `__schedule_assemble()` will abort if
+        # some other state prevents it from being built, and changes
+        # to such states will cause re-scheduling, so this is safe.
+        self._schedule_assemble()
 
         # Callback to the Queue
         if self.__required_callback is not None:
@@ -1581,19 +1570,19 @@ class Element(Plugin):
     def _artifact_files_required(self):
         return self.__artifact_files_required
 
-    # __can_schedule()
+    # __should_schedule()
     #
     # Returns:
     #     bool - Whether the element can be scheduled for a build.
     #
-    def __can_schedule(self):
+    def __should_schedule(self):
         # We're processing if we're already scheduled, we've
         # finished assembling or if we're waiting to pull.
         processing = (
             self.__assemble_scheduled or self.__assemble_done or self._pull_pending()
         )
 
-        # We can schedule when
+        # We should schedule a build when
         return (
             # We're not processing
             not processing and
@@ -1611,8 +1600,9 @@ class Element(Plugin):
     # in a subprocess.
     #
     def _schedule_assemble(self):
-        if not self.__can_schedule():
-            return False
+        if not self.__should_schedule():
+            self._update_state()
+            return
 
         self.__assemble_scheduled = True
 
@@ -1621,7 +1611,6 @@ class Element(Plugin):
             dep._set_required()
 
         self._update_state()
-        return True
 
     # _assemble_done():
     #
@@ -1869,7 +1858,9 @@ class Element(Plugin):
         self.__strict_artifact.reset_cached()
         self.__artifact.reset_cached()
 
-        self._update_state()
+        # We may not have actually pulled an artifact - the pull may
+        # have failed. We might therefore need to schedule assembly.
+        self._schedule_assemble()
         self._update_ready_for_runtime_and_cached()
 
     # _pull():
@@ -3213,7 +3204,7 @@ class Element(Plugin):
         if not context.get_strict() and not self.__artifact:
             # We've calculated the weak_key, so instantiate artifact instance member
             self.__artifact = Artifact(self, context, weak_key=self.__weak_cache_key)
-            self._update_state()
+            self._schedule_assemble()
 
         if not self.__strict_cache_key:
             return
@@ -3224,7 +3215,7 @@ class Element(Plugin):
 
             if context.get_strict():
                 self.__artifact = self.__strict_artifact
-                self._update_state()
+                self._schedule_assemble()
             else:
                 self.__update_cache_key_non_strict()