Use explicit element state updates

This adds the _update_state() method to the Element class to keep track
of element state and avoid calculating the same cache key multiple
times. This also consolidates the different get_cache_key
methods into a single method that always returns the cache key
calculated by _update_state(), if available.

Fixes #149, #173

2 files changed, 25 insertions, 14 deletions

diff --git a/buildstream/_artifactcache/ostreecache.py b/buildstream/_artifactcache/ostreecache.py
index a5a5d8d3b..ea93d228a 100644
--- a/buildstream/_artifactcache/ostreecache.py
+++ b/buildstream/_artifactcache/ostreecache.py
@@ -45,6 +45,9 @@ def buildref(element, key):
         for x in element.normal_name
     ])
 
+    if key is None:
+        raise ArtifactError('Cache key missing')
+
     # assume project and element names are not allowed to contain slashes
     return '{0}/{1}/{2}'.format(project.name, element_name, key)
 
@@ -100,7 +103,10 @@ class OSTreeCache(ArtifactCache):
         if strength is None:
             strength = _KeyStrength.STRONG if element._get_strict() else _KeyStrength.WEAK
 
-        key = element._get_cache_key(strength)
+        if strength == _KeyStrength.STRONG:
+            key = element._get_strict_cache_key()
+        else:
+            key = element._get_cache_key(strength)
         if not key:
             return False
 
@@ -140,7 +146,10 @@ class OSTreeCache(ArtifactCache):
         if strength is None:
             strength = _KeyStrength.STRONG if element._get_strict() else _KeyStrength.WEAK
 
-        key = element._get_cache_key(strength)
+        if strength == _KeyStrength.STRONG:
+            key = element._get_strict_cache_key()
+        else:
+            key = element._get_cache_key(strength)
         if not key:
             return False
 
@@ -163,7 +172,7 @@ class OSTreeCache(ArtifactCache):
     # Returns: path to extracted artifact
     #
     def extract(self, element):
-        ref = buildref(element, element._get_cache_key())
+        ref = buildref(element, element._get_strict_cache_key())
 
         # resolve ref to checksum
         rev = _ostree.checksum(self.repo, ref)
@@ -214,7 +223,7 @@ class OSTreeCache(ArtifactCache):
     #
     def commit(self, element, content):
         # tag with strong cache key based on dependency versions used for the build
-        ref = buildref(element, element._get_cache_key_for_build())
+        ref = buildref(element, element._get_cache_key())
 
         # also store under weak cache key
         weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK))
@@ -234,7 +243,7 @@ class OSTreeCache(ArtifactCache):
     #
     def pull(self, element, progress=None):
 
-        ref = buildref(element, element._get_cache_key())
+        ref = buildref(element, element._get_strict_cache_key())
         weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK))
 
         try:
@@ -257,8 +266,8 @@ class OSTreeCache(ArtifactCache):
                 rev = _ostree.checksum(self.repo, weak_ref)
 
                 # extract strong cache key from this newly fetched artifact
-                element._cached(recalculate=True)
-                ref = buildref(element, element._get_cache_key_from_artifact())
+                element._update_state()
+                ref = buildref(element, element._get_cache_key())
 
                 # create tag for strong cache key
                 _ostree.set_ref(self.repo, ref, rev)
@@ -288,7 +297,7 @@ class OSTreeCache(ArtifactCache):
         if len(self.push_urls) == 0:
             raise ArtifactError("Push is not enabled for any of the configured remote artifact caches.")
 
-        ref = buildref(element, element._get_cache_key_from_artifact())
+        ref = buildref(element, element._get_cache_key())
         weak_ref = buildref(element, element._get_cache_key(strength=_KeyStrength.WEAK))
 
         for push_url in self.push_urls:
diff --git a/buildstream/_artifactcache/tarcache.py b/buildstream/_artifactcache/tarcache.py
index 5f8fc7410..82487f077 100644
--- a/buildstream/_artifactcache/tarcache.py
+++ b/buildstream/_artifactcache/tarcache.py
@@ -252,8 +252,10 @@ class TarCache(ArtifactCache):
         if strength is None:
             strength = _KeyStrength.STRONG if element._get_strict() else _KeyStrength.WEAK
 
-        key = element._get_cache_key(strength)
-
+        if strength == _KeyStrength.STRONG:
+            key = element._get_strict_cache_key()
+        else:
+            key = element._get_cache_key(strength)
         if not key:
             return False
 
@@ -279,13 +281,13 @@ class TarCache(ArtifactCache):
     # Implements artifactcache.commit().
     #
     def commit(self, element, content):
-        ref = tarpath(element, element._get_cache_key_for_build())
+        ref = tarpath(element, element._get_cache_key())
         weak_ref = tarpath(element, element._get_cache_key(strength=_KeyStrength.WEAK))
 
         os.makedirs(os.path.join(self.tardir, element._get_project().name, element.normal_name), exist_ok=True)
 
         with utils._tempdir() as temp:
-            refdir = os.path.join(temp, element._get_cache_key_for_build())
+            refdir = os.path.join(temp, element._get_cache_key())
             shutil.copytree(content, refdir, symlinks=True)
 
             if ref != weak_ref:
@@ -293,7 +295,7 @@ class TarCache(ArtifactCache):
                 shutil.copytree(content, weak_refdir, symlinks=True)
 
             Tar.archive(os.path.join(self.tardir, ref),
-                        element._get_cache_key_for_build(),
+                        element._get_cache_key(),
                         temp)
 
             if ref != weak_ref:
@@ -307,7 +309,7 @@ class TarCache(ArtifactCache):
     #
     def extract(self, element):
 
-        key = element._get_cache_key()
+        key = element._get_strict_cache_key()
         ref = buildref(element, key)
         path = tarpath(element, key)