You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by tv...@apache.org on 2021/10/10 02:41:33 UTC
[buildstream] 01/02: Remove `--pull` option from `bst shell`,
and pull dependencies by default
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 <tr...@codethink.co.uk>
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."
),
)
-@click.option("--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
-#
-@pytest.mark.datafiles(DATA_DIR)
-@pytest.mark.skipif(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")