[buildstream] 04/04: _artifact.py, element.py, buildqueue.py: Don't try to calculate artifact size

[tv...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tvb pushed a commit to branch tristan/full-build-tree
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 95d66a9b23f73b9e29237ae967a026f02f9f2ad8
Author: Tristan van Berkom <tr...@codethink.co.uk>
AuthorDate: Sun Jul 31 16:18:22 2022 +0900

    _artifact.py, element.py, buildqueue.py: Don't try to calculate artifact size
    
    The artifact caching codepath was trying to calculate artifact size, in
    a way that it is impossible to accurately calculate. Furthermore the
    returned size is propagated through some scheduler codepaths and then
    simply ignored.
    
    Lets drop this deadcode.
---
 src/buildstream/_artifact.py                    | 8 --------
 src/buildstream/_scheduler/queues/buildqueue.py | 2 +-
 src/buildstream/element.py                      | 9 ++-------
 3 files changed, 3 insertions(+), 16 deletions(-)

diff --git a/src/buildstream/_artifact.py b/src/buildstream/_artifact.py
index a433b1700..8a1e4540d 100644
--- a/src/buildstream/_artifact.py
+++ b/src/buildstream/_artifact.py
@@ -191,9 +191,6 @@ class Artifact:
     #    environment (dict): dict of the element's environment variables
     #    sandboxconfig (SandboxConfig): The element's SandboxConfig
     #
-    # Returns:
-    #    (int): The size of the newly cached artifact
-    #
     def cache(
         self,
         *,
@@ -284,7 +281,6 @@ class Artifact:
             assert len(files_to_capture) == len(digests)
             for entry, digest in zip(files_to_capture, digests):
                 entry[1].CopyFrom(digest)
-                size += digest.size_bytes
 
         # store build dependencies
         for e in element._dependencies(_Scope.BUILD):
@@ -299,12 +295,10 @@ class Artifact:
             buildtreevdir = CasBasedDirectory(cas_cache=self._cas)
             buildtreevdir._import_files_internal(sandbox_build_dir, properties=properties, collect_result=False)
             artifact.buildtree.CopyFrom(buildtreevdir._get_digest())
-            size += buildtreevdir._get_size()
 
         # Store sources
         if sourcesvdir is not None:
             artifact.sources.CopyFrom(sourcesvdir._get_digest())
-            size += sourcesvdir._get_size()
 
         os.makedirs(os.path.dirname(os.path.join(self._artifactdir, element.get_artifact_name())), exist_ok=True)
         keys = utils._deduplicate([self._cache_key, self._weak_cache_key])
@@ -313,8 +307,6 @@ class Artifact:
             with utils.save_file_atomic(path, mode="wb") as f:
                 f.write(artifact.SerializeToString())
 
-        return size
-
     # cached_buildtree()
     #
     # Check if artifact is cached with expected buildtree. A
diff --git a/src/buildstream/_scheduler/queues/buildqueue.py b/src/buildstream/_scheduler/queues/buildqueue.py
index 5a6ca75bc..6a05594d0 100644
--- a/src/buildstream/_scheduler/queues/buildqueue.py
+++ b/src/buildstream/_scheduler/queues/buildqueue.py
@@ -54,4 +54,4 @@ class BuildQueue(Queue):
 
     @staticmethod
     def _assemble_element(element):
-        return element._assemble()
+        element._assemble()
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 7fbadc20b..f6c5d6c5c 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1639,9 +1639,6 @@ class Element(Plugin):
     #   - Call the public abstract methods for the build phase
     #   - Cache the resulting artifact
     #
-    # Returns:
-    #    (int): The size of the newly cached artifact
-    #
     def _assemble(self):
 
         # Only do this the first time around (i.e. __assemble_done is False)
@@ -1708,7 +1705,7 @@ class Element(Plugin):
 
                     raise
                 else:
-                    return self._cache_artifact(sandbox, collect)
+                    self._cache_artifact(sandbox, collect)
 
     def _cache_artifact(self, sandbox, collect):
 
@@ -1755,7 +1752,7 @@ class Element(Plugin):
         assert self.__artifact._cache_key is not None
 
         with self.timed_activity("Caching artifact"):
-            artifact_size = self.__artifact.cache(
+            self.__artifact.cache(
                 sandbox_build_dir=sandbox_build_dir,
                 collectvdir=collectvdir,
                 sourcesvdir=sourcesvdir,
@@ -1772,8 +1769,6 @@ class Element(Plugin):
                 "unable to collect artifact contents".format(collect)
             )
 
-        return artifact_size
-
     # _fetch_done()
     #
     # Indicates that fetching the sources for this element has been done.