This is an automated email from the ASF dual-hosted git repository. tvb pushed a commit to branch tristan/shell-checkout-always-pull in repository https://gitbox.apache.org/repos/asf/buildstream.git
commit 5d35f50dbeb2fd13f6c7c2f93635bfb47a208190 Author: Tristan van Berkom <[email protected]> AuthorDate: Sat Oct 9 18:29:34 2021 +0900 Remove `--pull` option from `bst shell`, and pull dependencies by default Summary of changes: * _frontend/cli.py: Remove the `--pull` option * _stream.py: Remove `pull` option from `Stream.shell()`, and created a new method `_pull_missing_artifacts()` which is also used by Stream.checkout() * tests/integration/shell.py,shellbuildtrees.py: Updated test expectations This fixes #685 --- src/buildstream/_frontend/cli.py | 8 ++--- src/buildstream/_stream.py | 59 +++++++++++++++++++++--------------- tests/integration/shell.py | 8 +++-- tests/integration/shellbuildtrees.py | 57 ++++++---------------------------- 4 files changed, 51 insertions(+), 81 deletions(-) diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py index 4c3df63..c845cd4 100644 --- a/src/buildstream/_frontend/cli.py +++ b/src/buildstream/_frontend/cli.py @@ -633,12 +633,10 @@ def show(app, elements, deps, except_, order, format_): "cli_buildtree", is_flag=True, help=( - "Stage a buildtree. Will fail if a buildtree is not available." - " --pull and pull-buildtrees configuration is needed " - "if trying to query for remotely cached buildtrees." + "Stage a buildtree. Will fail if a buildtree is not available. " + "pull-buildtrees configuration is needed if the buildtree is not available locally." ), ) [email protected]("--pull", "pull_", is_flag=True, help="Attempt to pull missing or incomplete artifacts") @click.option( "--artifact-remote", "artifact_remotes", @@ -672,7 +670,6 @@ def shell( isolate, build_, cli_buildtree, - pull_, artifact_remotes, source_remotes, ignore_project_artifact_remotes, @@ -723,7 +720,6 @@ def shell( isolate=isolate, command=command, usebuildtree=cli_buildtree, - pull_=pull_, artifact_remotes=artifact_remotes, source_remotes=source_remotes, ignore_project_artifact_remotes=ignore_project_artifact_remotes, diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py index a1a12fe..51ddec6 100644 --- a/src/buildstream/_stream.py +++ b/src/buildstream/_stream.py @@ -246,7 +246,6 @@ class Stream: # isolate (bool): Whether to isolate the environment like we do in builds # command (list): An argv to launch in the sandbox, or None # usebuildtree (bool): Whether to use a buildtree as the source, given cli option - # pull_ (bool): Whether to attempt to pull missing or incomplete artifacts # artifact_remotes: Artifact cache remotes specified on the commmand line # source_remotes: Source cache remotes specified on the commmand line # ignore_project_artifact_remotes: Whether to ignore artifact remotes specified by projects @@ -266,7 +265,6 @@ class Stream: isolate: bool = False, command: Optional[List[str]] = None, usebuildtree: bool = False, - pull_: bool = False, artifact_remotes: Iterable[RemoteSpec] = (), source_remotes: Iterable[RemoteSpec] = (), ignore_project_artifact_remotes: bool = False, @@ -283,7 +281,7 @@ class Stream: elements = self.load_selection( (target,), selection=selection, - connect_artifact_cache=pull_, + connect_artifact_cache=True, connect_source_cache=True, artifact_remotes=artifact_remotes, source_remotes=source_remotes, @@ -297,17 +295,12 @@ class Stream: element = self.targets[0] element._set_required(scope) - self.query_cache([element] + elements) - - if pull_: - self._reset() - self._add_queue(PullQueue(self._scheduler)) - - # Pull the toplevel element regardless of whether it is in scope - plan = elements if element in elements else [element] + elements - - self._enqueue_plan(plan) - self._run() + # Check whether the required elements are cached, and then + # try to pull them if they are not already cached. + # + pull_elements = [element] + elements + self.query_cache(pull_elements) + self._pull_missing_artifacts(pull_elements) missing_deps = [dep for dep in _pipeline.dependencies([element], scope) if not dep._cached()] if missing_deps: @@ -320,12 +313,11 @@ class Stream: # Check if we require a pull queue attempt, with given artifact state and context if usebuildtree: if not element._cached_buildtree(): - remotes_message = " or in available remotes" if pull_ else "" if not element._cached(): - message = "Artifact not cached locally" + remotes_message + message = "Artifact not cached locally or in available remotes" reason = "missing-buildtree-artifact-not-cached" elif element._buildtree_exists(): - message = "Buildtree is not cached locally" + remotes_message + message = "Buildtree is not cached locally or in available remotes" reason = "missing-buildtree-artifact-buildtree-not-cached" else: message = "Artifact was created without buildtree" @@ -694,15 +686,11 @@ class Stream: self._check_location_writable(location, force=force, tar=tar) + # Check whether the required elements are cached, and then + # try to pull them if they are not already cached. + # self.query_cache(elements) - - uncached_elts = [elt for elt in elements if elt._pull_pending()] - if uncached_elts and pull: - self._context.messenger.info("Attempting to fetch missing or incomplete artifact") - self._reset() - self._add_queue(PullQueue(self._scheduler)) - self._enqueue_plan(uncached_elts) - self._run(announce_session=True) + self._pull_missing_artifacts(elements) try: scope = { @@ -1453,6 +1441,27 @@ class Stream: element._cached_remotely() task.add_current_progress() + # _pull_missing_artifacts() + # + # Pull missing artifacts from available remotes, this runs the scheduler + # just to pull the artifacts if any of the artifacts are missing locally, + # and is used in commands which need to use the artifacts. + # + # This function requires Stream.query_cache() to be called in advance + # in order to determine which artifacts to try and pull. + # + # Args: + # elements (list [Element]): The selected list of required elements + # + def _pull_missing_artifacts(self, elements): + uncached_elts = [elt for elt in elements if elt._pull_pending()] + if uncached_elts: + self._context.messenger.info("Attempting to fetch missing or incomplete artifact(s)") + self._reset() + self._add_queue(PullQueue(self._scheduler)) + self._enqueue_plan(uncached_elts) + self._run(announce_session=True) + # _load_tracking() # # A variant of _load() to be used when the elements should be used diff --git a/tests/integration/shell.py b/tests/integration/shell.py index ed7ed65..4e96fb8 100644 --- a/tests/integration/shell.py +++ b/tests/integration/shell.py @@ -391,12 +391,14 @@ def test_integration_partial_artifact(cli, datafiles, tmpdir, integration_cache) objpath = os.path.join(cachedir, "cas", "objects", digest[:2], digest[2:]) os.unlink(objpath) - # check shell doesn't work + # check shell doesn't work when it cannot pull the missing bits + cli.configure({"artifacts": {}}) result = cli.run(project=project, args=["shell", element_name, "--", "hello"]) result.assert_main_error(ErrorDomain.APP, "shell-missing-deps") - # check the artifact gets completed with '--pull' specified - result = cli.run(project=project, args=["shell", "--pull", element_name, "--", "hello"]) + # check the artifact gets completed with access to the remote + cli.configure({"artifacts": {"servers": [{"url": share.repo, "push": True}]}}) + result = cli.run(project=project, args=["shell", element_name, "--", "hello"]) result.assert_success() assert "autotools/amhello.bst" in result.get_pulled_elements() diff --git a/tests/integration/shellbuildtrees.py b/tests/integration/shellbuildtrees.py index 4c42551..d7e6bc3 100644 --- a/tests/integration/shellbuildtrees.py +++ b/tests/integration/shellbuildtrees.py @@ -247,6 +247,11 @@ def test_shell_use_cached_buildtree(share_with_buildtrees, datafiles, cli, pull_ # Optionally pull the buildtree along with `bst artifact pull` maybe_pull_deps(cli, project, element_name, pull_deps, pull_buildtree) + # Disable access to the artifact server after pulling, so that `bst shell` cannot automatically + # pull the missing bits, this should be equivalent to the missing bits being missing in a + # remote server + cli.configure({"artifacts": {}}) + # Run the shell without asking it to pull any buildtree, just asking to use a buildtree result = cli.run(project=project, args=["shell", "--build", element_name, "--use-buildtree", "--", "cat", "test"]) @@ -258,9 +263,10 @@ def test_shell_use_cached_buildtree(share_with_buildtrees, datafiles, cli, pull_ # -# Test behavior of launching a shell and requesting to use a buildtree, while -# also requesting to download any missing bits from the artifact server on the fly, -# again with various states of local cache (ranging from nothing cached to everything cached) +# Test behavior of launching a shell and requesting to use a buildtree, while allowing +# BuildStream to download any missing bits from the artifact server on the fly (which +# it will do by default) again with various states of local cache (ranging from nothing +# cached to everything cached) # @pytest.mark.datafiles(DATA_DIR) @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox") @@ -290,17 +296,7 @@ def test_shell_pull_cached_buildtree(share_with_buildtrees, datafiles, cli, pull # Run the shell and request that required artifacts and buildtrees should be pulled result = cli.run( project=project, - args=[ - "--pull-buildtrees", - "shell", - "--build", - element_name, - "--pull", - "--use-buildtree", - "--", - "cat", - "test", - ], + args=["--pull-buildtrees", "shell", "--build", element_name, "--use-buildtree", "--", "cat", "test",], ) # In this case, we should succeed every time, regardless of what was @@ -331,36 +327,3 @@ def test_shell_use_uncached_buildtree(share_without_buildtrees, datafiles, cli): # Sorry, a buildtree was never cached for this element result.assert_main_error(ErrorDomain.APP, "missing-buildtree-artifact-created-without-buildtree") - - -# -# Test behavior of launching a shell and requesting to use a buildtree. -# -# In this case we download everything we need first, but the buildtree was never cached at build time -# [email protected](DATA_DIR) [email protected](not HAVE_SANDBOX, reason="Only available with a functioning sandbox") -def test_shell_pull_uncached_buildtree(share_without_buildtrees, datafiles, cli): - project = str(datafiles) - element_name = "build-shell/buildtree.bst" - - cli.configure({"artifacts": {"servers": [{"url": share_without_buildtrees.repo}]}}) - - # Run the shell and request that required artifacts and buildtrees should be pulled - result = cli.run( - project=project, - args=[ - "--pull-buildtrees", - "shell", - "--build", - element_name, - "--pull", - "--use-buildtree", - "--", - "cat", - "test", - ], - ) - - # Sorry, a buildtree was never cached for this element - result.assert_main_error(ErrorDomain.APP, "missing-buildtree-artifact-created-without-buildtree")
