Merge branch 'juerg/push' into 'master'

Pull missing artifacts in `bst artifact push`

See merge request BuildStream/buildstream!2077

5 files changed, 31 insertions, 66 deletions

diff --git a/src/buildstream/_scheduler/queues/artifactpushqueue.py b/src/buildstream/_scheduler/queues/artifactpushqueue.py
index 071c6fe74..79597c704 100644
--- a/src/buildstream/_scheduler/queues/artifactpushqueue.py
+++ b/src/buildstream/_scheduler/queues/artifactpushqueue.py
@@ -32,11 +32,16 @@ class ArtifactPushQueue(Queue):
     complete_name = "Artifacts Pushed"
     resources = [ResourceType.UPLOAD]
 
+    def __init__(self, scheduler, *, skip_uncached=False):
+        super().__init__(scheduler)
+
+        self._skip_uncached = skip_uncached
+
     def get_process_func(self):
         return ArtifactPushQueue._push_or_skip
 
     def status(self, element):
-        if element._skip_push():
+        if element._skip_push(skip_uncached=self._skip_uncached):
             return QueueStatus.SKIP
 
         return QueueStatus.READY
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index fbb2fca69..3b0a308e7 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -48,7 +48,7 @@ from .element import Element
 from ._pipeline import Pipeline
 from ._profile import Topics, PROFILER
 from ._state import State
-from .types import _KeyStrength, _PipelineSelection, _SchedulerErrorAction, _Scope
+from .types import _KeyStrength, _PipelineSelection, _Scope
 from .plugin import Plugin
 from . import utils, _yaml, _site
 
@@ -301,7 +301,7 @@ class Stream:
         self._add_queue(BuildQueue(self._scheduler))
 
         if self._artifacts.has_push_remotes():
-            self._add_queue(ArtifactPushQueue(self._scheduler))
+            self._add_queue(ArtifactPushQueue(self._scheduler, skip_uncached=True))
 
         if self._sourcecache.has_push_remotes():
             self._add_queue(SourcePushQueue(self._scheduler))
@@ -484,49 +484,11 @@ class Stream:
 
         self._pipeline.assert_consistent(elements)
 
-        # Check if we require a pull queue, with given artifact state and context
-        require_buildtrees = self._buildtree_pull_required(elements)
-        if require_buildtrees:
-            self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtrees")
-            self._add_queue(PullQueue(self._scheduler))
-            self._enqueue_plan(require_buildtrees)
-
-        # Before we try to push the artifacts, ensure they're cached
-        cached_elements = []
-        uncached_elements = []
-        self._message(MessageType.INFO, "Verifying that elements are cached")
-        for element in elements:
-            if element._cached():
-                cached_elements.append(element)
-            else:
-                msg = "{} is not cached".format(element.name)
-                if self._context.sched_error_action != _SchedulerErrorAction.CONTINUE:
-                    raise StreamError("Push failed: " + msg)
-
-                self._message(MessageType.WARN, msg)
-                uncached_elements.append(element)
-
-        if cached_elements:
-            self._scheduler.clear_queues()
-            push_queue = ArtifactPushQueue(self._scheduler)
-            self._add_queue(push_queue)
-            self._enqueue_plan(cached_elements, queue=push_queue)
-            self._run(announce_session=True)
-
-        # If the user has selected to continue on error, fail the command
-        # and print a summary of artifacts which could not be pushed
-        #
-        # NOTE: Usually we check the _SchedulerErrorAction when a *job* has failed.
-        #       However, we cannot create a PushQueue job unless we intentionally
-        #       ready an uncached element in the PushQueue.
-        if self._context.sched_error_action == _SchedulerErrorAction.CONTINUE and uncached_elements:
-            names = [element.name for element in uncached_elements]
-            fail_str = (
-                "Error while pushing. The following elements were not pushed as they are "
-                "not yet cached:\n\n\t{}\n".format("\n\t".join(names))
-            )
-
-            raise StreamError(fail_str)
+        self._scheduler.clear_queues()
+        self._add_queue(PullQueue(self._scheduler))
+        self._add_queue(ArtifactPushQueue(self._scheduler))
+        self._enqueue_plan(elements)
+        self._run(announce_session=True)
 
     # checkout()
     #
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 5118f0193..57aa37a85 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -1971,22 +1971,24 @@ class Element(Plugin):
     #
     # Determine whether we should create a push job for this element.
     #
+    # Args:
+    #    skip_uncached (bool): Whether to skip elements that aren't cached
+    #
     # Returns:
     #   (bool): True if this element does not need a push job to be created
     #
-    def _skip_push(self):
+    def _skip_push(self, *, skip_uncached):
         if not self.__artifacts.has_push_remotes(plugin=self):
             # No push remotes for this element's project
             return True
 
         # Do not push elements that aren't cached, or that are cached with a dangling buildtree
         # ref unless element type is expected to have an an empty buildtree directory
-        if not self._cached_buildtree() and self._buildtree_exists():
-            return True
-
-        # Do not push tainted artifact
-        if self.__get_tainted():
-            return True
+        if skip_uncached:
+            if not self._cached():
+                return True
+            if not self._cached_buildtree() and self._buildtree_exists():
+                return True
 
         return False
 
@@ -1999,7 +2001,13 @@ class Element(Plugin):
     #           and no updated was required
     #
     def _push(self):
-        self.__assert_cached()
+        if not self._cached():
+            raise ElementError("Push failed: {} is not cached".format(self.name))
+
+        # Do not push elements that are cached with a dangling buildtree ref
+        # unless element type is expected to have an an empty buildtree directory
+        if not self._cached_buildtree() and self._buildtree_exists():
+            raise ElementError("Push failed: buildtree of {} is not cached".format(self.name))
 
         if self.__get_tainted():
             self.warn("Not pushing tainted artifact.")
diff --git a/tests/frontend/push.py b/tests/frontend/push.py
index ffcc166f9..4b10b5bcd 100644
--- a/tests/frontend/push.py
+++ b/tests/frontend/push.py
@@ -239,16 +239,8 @@ def test_push_fails_with_on_error_continue(cli, tmpdir, datafiles):
         assert_shared(cli, share, project, "target.bst")
 
         assert_not_shared(cli, share, project, "import-dev.bst")
-        errors = [
-            "import-dev.bst is not cached",
-            (
-                "Error while pushing. The following elements were not pushed as they are not yet cached:\n"
-                "\n"
-                "\timport-dev.bst\n"
-            ),
-        ]
-        for error in errors:
-            assert error in result.stderr
+
+        assert "Push failed: import-dev.bst is not cached" in result.stderr
 
 
 # Tests that `bst artifact push --deps DEPS` pushes selected dependencies of
diff --git a/tests/integration/pullbuildtrees.py b/tests/integration/pullbuildtrees.py
index 6d9eefb26..5923ce544 100644
--- a/tests/integration/pullbuildtrees.py
+++ b/tests/integration/pullbuildtrees.py
@@ -142,7 +142,6 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
         assert element_name in result.get_pulled_elements()
         cli2.configure({"artifacts": {"url": share3.repo, "push": True}})
         result = cli2.run(project=project, args=["--pull-buildtrees", "artifact", "push", element_name])
-        assert "Attempting to fetch missing artifact buildtrees" in result.stderr
         assert element_name not in result.get_pulled_elements()
         with cli2.artifact.extract_buildtree(cwd, cwd, artifact_name) as buildtreedir:
             assert not buildtreedir
@@ -155,7 +154,6 @@ def test_pullbuildtrees(cli2, tmpdir, datafiles):
         # without exlipictly requiring a bst artifact pull.
         cli2.configure({"artifacts": [{"url": share1.repo, "push": False}, {"url": share3.repo, "push": True}]})
         result = cli2.run(project=project, args=["--pull-buildtrees", "artifact", "push", element_name])
-        assert "Attempting to fetch missing artifact buildtrees" in result.stderr
         assert element_name in result.get_pulled_elements()
         with cli2.artifact.extract_buildtree(cwd, cwd, artifact_name) as buildtreedir:
             assert os.path.isdir(buildtreedir)