element.py: move _generate_key to _source_cached

This means that keys are only generated after tracking.

Part of a fix for !1124

2 files changed, 13 insertions, 21 deletions

diff --git a/buildstream/element.py b/buildstream/element.py
index e5bc0792e..3ddf40799 100644
--- a/buildstream/element.py
+++ b/buildstream/element.py
@@ -955,15 +955,11 @@ class Element(Plugin):
         cls.__instantiated_elements[meta] = element
 
         # Instantiate sources and generate their keys
-        previous_sources = []
         for meta_source in meta.sources:
             meta_source.first_pass = meta.kind == "junction"
             source = meta.project.create_source(meta_source,
                                                 first_pass=meta.first_pass)
 
-            source._generate_key(previous_sources)
-            previous_sources.append(source)
-
             redundant_ref = source._load_ref()
             element.__sources.append(source)
 
@@ -1367,12 +1363,6 @@ class Element(Plugin):
         self.__tracking_scheduled = False
         self.__tracking_done = True
 
-        # update keys
-        sources = list(self.sources())
-        if sources:
-            source = sources.pop()
-            source._generate_key(sources)
-
         self._update_state()
 
     # _track():
@@ -2148,12 +2138,13 @@ class Element(Plugin):
 
         return _cachekey.generate_key(cache_key_dict)
 
+    # Check if sources are cached, generating the source key if it hasn't been
     def _source_cached(self):
-        source = None
-        for source in self.sources():
-            pass
-        if source:
-            return self._get_context().sourcecache.contains(source)
+        sources = list(self.sources())
+        if sources:
+            if not sources[-1]._key:
+                sources[-1]._generate_key(sources[:-1])
+            return self._get_context().sourcecache.contains(sources[-1])
         else:
             return True
 
@@ -2909,11 +2900,12 @@ class Element(Plugin):
     # Caches the sources into the local CAS
     #
     def __cache_sources(self):
-        sources = list(self.sources())
-        if sources:
-            sourcecache = self._get_context().sourcecache
-            if not sourcecache.contains(sources[-1]):
-                sources[-1]._cache(sources[:-1])
+        if not self._source_cached():
+            sources = list(self.sources())
+            if sources:
+                sourcecache = self._get_context().sourcecache
+                if not sourcecache.contains(sources[-1]):
+                    sources[-1]._cache(sources[:-1])
 
 
 def _overlap_error_detail(f, forbidden_overlap_elements, elements):
diff --git a/tests/sourcecache/staging.py b/tests/sourcecache/staging.py
index b62bc3c2f..60852e238 100644
--- a/tests/sourcecache/staging.py
+++ b/tests/sourcecache/staging.py
@@ -70,8 +70,8 @@ def test_source_staged(tmpdir, cli, datafiles):
     # seems to be the only way to get the sources?
     element = project.load_elements(["import-bin.bst"])[0]
     source = list(element.sources())[0]
-    assert sourcecache.contains(source)
     assert element._source_cached()
+    assert sourcecache.contains(source)
 
     # Extract the file and check it's the same as the one we imported
     ref = source._get_source_name()