From 138083752bc74f64c7bcce2f633f2114a3a6efa5 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Mon, 15 Apr 2019 17:47:12 +0900 Subject: _stream.py: Mark all elements as pulled in Stream.push() Marking all elements as pulled in Stream.push() ensures that cache keys are resolved before pushing elements, otherwise state is left unresolved in non-strict mode while elements are awaiting to download an artifact by it's strict cache key. Fixes #990 --- buildstream/_stream.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) 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) -- cgit v1.2.1 From 188c0983aa610b42925c4ec120f3caa9f28bf8c8 Mon Sep 17 00:00:00 2001 From: Tristan Van Berkom Date: Mon, 15 Apr 2019 18:32:32 +0900 Subject: tests/frontend/push.py: Test pushing artifacts in non-strict mode This is a regression test for issue #990 --- tests/frontend/push.py | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) 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() -- cgit v1.2.1