Merge branch 'tristan/fix-non-strict-push' into 'master'

Fix non strict push

Closes #990

See merge request BuildStream/buildstream!1290

2 files changed, 69 insertions, 0 deletions

diff --git a/buildstream/_stream.py b/buildstream/_stream.py
index 64a578c92..fa57f4468 100644
--- a/buildstream/_stream.py
+++ b/buildstream/_stream.py
@@ -441,6 +441,20 @@ class Stream():
             self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtrees")
             self._add_queue(PullQueue(self._scheduler))
             self._enqueue_plan(require_buildtrees)
+        else:
+            # FIXME: This hack should be removed as a result of refactoring
+            #        Element._update_state()
+            #
+            # This workaround marks all dependencies of all selected elements as
+            # "pulled" before trying to push.
+            #
+            # Instead of lying to the elements and telling them they have already
+            # been pulled, we should have something more consistent with how other
+            # state bits are handled; and explicitly tell the elements that they
+            # need to be pulled with something like Element._schedule_pull().
+            #
+            for element in elements:
+                element._pull_done()
 
         push_queue = ArtifactPushQueue(self._scheduler)
         self._add_queue(push_queue)
diff --git a/tests/frontend/push.py b/tests/frontend/push.py
index 7b4e944d4..b2991ada7 100644
--- a/tests/frontend/push.py
+++ b/tests/frontend/push.py
@@ -454,3 +454,58 @@ def test_build_remote_option(caplog, cli, tmpdir, datafiles):
             assert_shared(cli, sharecli, project, element_name)
             assert_not_shared(cli, shareuser, project, element_name)
             assert_not_shared(cli, shareproject, project, element_name)
+
+
+# This test ensures that we are able to run `bst artifact push` in non strict mode
+# and that we do not crash when trying to push elements even though they
+# have not yet been pulled.
+#
+# This is a regression test for issue #990
+#
+@pytest.mark.datafiles(DATA_DIR)
+@pytest.mark.parametrize("buildtrees", [('buildtrees'), ('normal')])
+def test_push_no_strict(caplog, cli, tmpdir, datafiles, buildtrees):
+    project = os.path.join(datafiles.dirname, datafiles.basename)
+    caplog.set_level(1)
+
+    with create_artifact_share(os.path.join(str(tmpdir), 'artifactshare')) as share:
+        cli.configure({
+            'artifacts': {
+                'url': share.repo,
+                'push': True
+            },
+            'projects': {
+                'test': {
+                    'strict': False
+                }
+            }
+        })
+
+        # First get us a build
+        result = cli.run(project=project, args=['build', 'target.bst'])
+        result.assert_success()
+
+        # Now cause one of the dependenies to change their cache key
+        #
+        # Here we just add a file, causing the strong cache key of the
+        # import-bin.bst element to change due to the local files it
+        # imports changing.
+        path = os.path.join(project, 'files', 'bin-files', 'newfile')
+        with open(path, 'w') as f:
+            f.write("PONY !")
+
+        # Now build again after having changed the dependencies
+        result = cli.run(project=project, args=['build', 'target.bst'])
+        result.assert_success()
+
+        # Now run `bst artifact push`.
+        #
+        # Optionally try it with --pull-buildtrees, since this causes
+        # a pull queue to be added to the `push` command, the behavior
+        # around this is different.
+        args = []
+        if buildtrees == 'buildtrees':
+            args += ['--pull-buildtrees']
+        args += ['artifact', 'push', '--deps', 'all', 'target.bst']
+        result = cli.run(project=project, args=args)
+        result.assert_success()