Only run `element.__update_source_state` when necessary

5 files changed, 24 insertions, 7 deletions

diff --git a/src/buildstream/_artifactelement.py b/src/buildstream/_artifactelement.py
index 1c1c5db46..ef3d67032 100644
--- a/src/buildstream/_artifactelement.py
+++ b/src/buildstream/_artifactelement.py
@@ -80,6 +80,7 @@ class ArtifactElement(Element):
         artifact_element = ArtifactElement(context, ref)
         # XXX: We need to call update state as it is responsible for
         # initialising an Element/ArtifactElement's Artifact (__artifact)
+        artifact_element._update_source_state()
         artifact_element._update_state()
         cls.__instantiated_artifacts[ref] = artifact_element
 
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index da0c0fb29..56128e99f 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -633,6 +633,10 @@ class Loader:
             raise LoadError("Dependencies are forbidden for 'junction' elements", LoadErrorReason.INVALID_JUNCTION)
 
         element = Element._new_from_meta(meta_element)
+        element._update_source_state()
+        # FIXME: We're doubly updating here for the moment; this
+        # should be removed once we don't need the entirety of
+        # _update_state() anymore
         element._update_state()
 
         # If this junction element points to a sub-sub-project, we need to
diff --git a/src/buildstream/_pipeline.py b/src/buildstream/_pipeline.py
index 0b9ab5f24..c754ce78f 100644
--- a/src/buildstream/_pipeline.py
+++ b/src/buildstream/_pipeline.py
@@ -147,6 +147,7 @@ class Pipeline:
             for element in self.dependencies(targets, Scope.ALL):
                 # Determine initial element state.
                 if not element._resolved_initial_state:
+                    element._update_source_state()
                     element._update_state()
 
                 # We may already have Elements which are cached and have their runtimes
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 5927c8533..362938ca5 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1233,9 +1233,6 @@ class Element(Plugin):
             self._resolved_initial_state = True
         context = self._get_context()
 
-        # Compute and determine consistency of sources
-        self.__update_source_state()
-
         if self._get_consistency() == Consistency.INCONSISTENT:
             # Tracking may still be pending
             return
@@ -1338,7 +1335,9 @@ class Element(Plugin):
 
         self.__tracking_scheduled = False
 
-        self._update_state()
+        # Tracking may change the sources' refs, and therefore the
+        # source state. We need to update source state.
+        self._update_source_state()
 
     # _track():
     #
@@ -1784,7 +1783,7 @@ class Element(Plugin):
         # Fetching cannot change the source state from INCONSISTENT to CACHED because
         # we prevent fetching when it's INCONSISTENT.
         # Therefore, only the source state will change.
-        self.__update_source_state()
+        self._update_source_state()
 
     # _pull_pending()
     #
@@ -2387,7 +2386,7 @@ class Element(Plugin):
     #                   Private Local Methods                   #
     #############################################################
 
-    # __update_source_state()
+    # _update_source_state()
     #
     # Updates source consistency state
     #
@@ -2395,19 +2394,24 @@ class Element(Plugin):
     # cache keys, because the source's ref, whether defined in yaml or
     # from the workspace, is a component of the element's cache keys.
     #
-    def __update_source_state(self):
+    def _update_source_state(self):
 
         # Cannot resolve source state until tracked
         if self.__tracking_scheduled:
             return
 
+        old_consistency = self.__consistency
         self.__consistency = Consistency.CACHED
 
         # Determine overall consistency of the element
         for source in self.__sources:
+            # FIXME: It'd be nice to remove this eventually
             source._update_state()
             self.__consistency = min(self.__consistency, source._get_consistency())
 
+        if old_consistency != self.__consistency:
+            self._update_state()
+
     # __can_build_incrementally()
     #
     # Check if the element can be built incrementally, this
@@ -3238,6 +3242,9 @@ class Element(Plugin):
                     assert not rdep.__build_deps_without_strict_cache_key < 0
 
                     if rdep.__build_deps_without_strict_cache_key == 0:
+                        # FIXME: Get to the bottom of why we need
+                        # source cache keys to be updated here
+                        rdep._update_source_state()
                         rdep._update_state()
 
     # __update_ready_for_runtime()
@@ -3273,6 +3280,9 @@ class Element(Plugin):
                     assert not rdep.__build_deps_without_cache_key < 0
 
                     if rdep.__build_deps_without_cache_key == 0:
+                        # FIXME: Get to the bottom of why we need
+                        # source cache keys to be updated here
+                        rdep._update_source_state()
                         rdep._update_state()
 
                 # If the element is cached, and has all of its runtime dependencies cached,
diff --git a/tests/artifactcache/push.py b/tests/artifactcache/push.py
index 238d5f7ef..e5a332dea 100644
--- a/tests/artifactcache/push.py
+++ b/tests/artifactcache/push.py
@@ -37,6 +37,7 @@ def _push(cli, cache_dir, project_dir, config_file, target):
         # as this test does not use the cli frontend.
         for e in element.dependencies(Scope.ALL):
             # Determine initial element state.
+            e._update_source_state()
             e._update_state()
 
         # Manually setup the CAS remotes