You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@buildstream.apache.org by ro...@apache.org on 2020/12/29 13:40:19 UTC

[buildstream] branch jennis/pull_implicitly_in_checkout created (now 1e0a9cd)

This is an automated email from the ASF dual-hosted git repository.

root pushed a change to branch jennis/pull_implicitly_in_checkout
in repository https://gitbox.apache.org/repos/asf/buildstream.git.


      at 1e0a9cd  cli.py: Change artifact checkout to pull implicitly

This branch includes the following new commits:

     new 1c4fc02  _stream.py: Load the appropriate PipelineSelection in checkout
     new 1e0a9cd  cli.py: Change artifact checkout to pull implicitly

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[buildstream] 02/02: cli.py: Change artifact checkout to pull implicitly

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

root pushed a commit to branch jennis/pull_implicitly_in_checkout
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 1e0a9cd13ea30a19bd5a8c2a0ee79d4568242fa9
Author: James Ennis <ja...@codethink.co.uk>
AuthorDate: Wed Sep 11 18:22:01 2019 +0100

    cli.py: Change artifact checkout to pull implicitly
    
    This makes it consistent with source checkout
---
 src/buildstream/_frontend/cli.py |  5 +----
 src/buildstream/_stream.py       |  6 ++----
 tests/frontend/buildcheckout.py  | 12 +++++-------
 3 files changed, 8 insertions(+), 15 deletions(-)

diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py
index 6f40916..02133fe 100644
--- a/src/buildstream/_frontend/cli.py
+++ b/src/buildstream/_frontend/cli.py
@@ -1052,15 +1052,13 @@ def artifact_show(app, deps, artifacts):
 @click.option('--compression', default=None,
               type=click.Choice(['gz', 'xz', 'bz2']),
               help="The compression option of the tarball created.")
-@click.option('--pull', 'pull_', is_flag=True,
-              help="Pull the artifact if it's missing or incomplete.")
 @click.option('--directory', default=None,
               type=click.Path(file_okay=False),
               help="The directory to checkout the artifact to")
 @click.argument('target', required=False,
                 type=click.Path(readable=False))
 @click.pass_obj
-def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, pull_, directory, target):
+def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression, directory, target):
     """Checkout contents of an artifact
 
     When this command is executed from a workspace directory, the default
@@ -1111,7 +1109,6 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression,
                             deps=deps,
                             integrate=True if integrate is None else integrate,
                             hardlinks=hardlinks,
-                            pull=pull_,
                             compression=compression,
                             tar=bool(tar))
 
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index feb3efe..64ff3cf 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -530,8 +530,6 @@ class Stream():
     #                will be placed at the given location. If true and
     #                location is '-', the tarball will be dumped on the
     #                standard output.
-    #    pull (bool): If true will attempt to pull any missing or incomplete
-    #                 artifacts.
     #
     def checkout(self, target, *,
                  location=None,
@@ -540,7 +538,6 @@ class Stream():
                  integrate=True,
                  hardlinks=False,
                  compression='',
-                 pull=False,
                  tar=False):
 
         elements, _ = self._load((target,), (), selection=deps, use_artifact_config=True, load_refs=True)
@@ -553,8 +550,9 @@ class Stream():
 
         self._check_location_writable(location, force=force, tar=tar)
 
+        # Only try to pull elements which we know are not cached
         uncached_elts = [elt for elt in elements if not elt._cached()]
-        if uncached_elts and pull:
+        if uncached_elts:
             self._message(MessageType.INFO, "Attempting to fetch missing or incomplete artifact")
             self._scheduler.clear_queues()
             self._add_queue(PullQueue(self._scheduler))
diff --git a/tests/frontend/buildcheckout.py b/tests/frontend/buildcheckout.py
index c9a4223..f0fa821 100644
--- a/tests/frontend/buildcheckout.py
+++ b/tests/frontend/buildcheckout.py
@@ -1105,14 +1105,12 @@ def test_partial_artifact_checkout_fetch(cli, datafiles, tmpdir):
         os.unlink(objpath)
 
         # Verify that the build-only dependency is not (complete) in the local cache
-        result = cli.run(project=project, args=[
-            'artifact', 'checkout', build_elt,
-            '--directory', checkout_dir])
-        result.assert_main_error(ErrorDomain.STREAM, 'uncached-checkout-attempt')
+        assert cli.get_element_state(project, build_elt) != 'cached'
 
-        # Verify that the pull method fetches relevant artifacts in order to stage
+        # Verify that when we checkout, we implicitly pull the relevant
+        # artifacts in order to stage
         result = cli.run(project=project, args=[
-            'artifact', 'checkout', '--pull', build_elt,
+            'artifact', 'checkout', build_elt,
             '--directory', checkout_dir])
         result.assert_success()
 
@@ -1134,7 +1132,7 @@ def test_partial_checkout_fail(tmpdir, datafiles, cli):
         }})
 
         res = cli.run(project=project, args=[
-            'artifact', 'checkout', '--pull', build_elt, '--directory',
+            'artifact', 'checkout', build_elt, '--directory',
             checkout_dir])
         res.assert_main_error(ErrorDomain.STREAM, 'uncached-checkout-attempt')
         assert re.findall(r'Remote \((\S+)\) does not have artifact (\S+) cached', res.stderr)


[buildstream] 01/02: _stream.py: Load the appropriate PipelineSelection in checkout

Posted by ro...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

root pushed a commit to branch jennis/pull_implicitly_in_checkout
in repository https://gitbox.apache.org/repos/asf/buildstream.git

commit 1c4fc0277245a6d1fa206aa03ff32bce7041ea77
Author: James Ennis <ja...@codethink.co.uk>
AuthorDate: Wed Sep 11 17:48:49 2019 +0100

    _stream.py: Load the appropriate PipelineSelection in checkout
    
    This patch ensures checkout behaves like the rest of our
    commands which support --deps options. That is, we carry
    the "deps" string through to the Stream and load the
    corresponding PipelineSelection.
---
 src/buildstream/_frontend/cli.py |  5 +----
 src/buildstream/_stream.py       | 18 +++++++++---------
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/buildstream/_frontend/cli.py b/src/buildstream/_frontend/cli.py
index 1fdbbdc..6f40916 100644
--- a/src/buildstream/_frontend/cli.py
+++ b/src/buildstream/_frontend/cli.py
@@ -1066,8 +1066,6 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression,
     When this command is executed from a workspace directory, the default
     is to checkout the artifact of the workspace element.
     """
-    from ..element import Scope
-
     if hardlinks and tar:
         click.echo("ERROR: options --hardlinks and --tar conflict", err=True)
         sys.exit(-1)
@@ -1107,11 +1105,10 @@ def artifact_checkout(app, force, deps, integrate, hardlinks, tar, compression,
             if not target:
                 raise AppError('Missing argument "ELEMENT".')
 
-        scope = {'run': Scope.RUN, 'build': Scope.BUILD, 'none': Scope.NONE}
         app.stream.checkout(target,
                             location=location,
                             force=force,
-                            scope=scope[deps],
+                            deps=deps,
                             integrate=True if integrate is None else integrate,
                             hardlinks=hardlinks,
                             pull=pull_,
diff --git a/src/buildstream/_stream.py b/src/buildstream/_stream.py
index 293ba05..feb3efe 100644
--- a/src/buildstream/_stream.py
+++ b/src/buildstream/_stream.py
@@ -536,21 +536,20 @@ class Stream():
     def checkout(self, target, *,
                  location=None,
                  force=False,
-                 scope=Scope.RUN,
+                 deps='run',
                  integrate=True,
                  hardlinks=False,
                  compression='',
                  pull=False,
                  tar=False):
 
-        # if pulling we need to ensure dependency artifacts are also pulled
-        selection = PipelineSelection.RUN if pull else PipelineSelection.NONE
-        elements, _ = self._load((target,), (), selection=selection, use_artifact_config=True, load_refs=True)
-        target = elements[-1]
+        elements, _ = self._load((target,), (), selection=deps, use_artifact_config=True, load_refs=True)
 
-        # Verify that --deps run has not been specified for an ArtifactElement
-        if isinstance(target, ArtifactElement) and scope == Scope.RUN:
-            raise StreamError("Unable to determine the runtime dependencies of an ArtifactElement")
+        # self.targets contains a list of the loaded target objects
+        # if we specify --deps build, Stream._load() will return a list
+        # of build dependency objects, however, we need to prepare a sandbox
+        # with the target (which has had its appropriate dependencies loaded)
+        target = self.targets[0]
 
         self._check_location_writable(location, force=force, tar=tar)
 
@@ -563,7 +562,8 @@ class Stream():
             self._run()
 
         try:
-            with target._prepare_sandbox(scope=scope, directory=None,
+            scope = {'run': Scope.RUN, 'build': Scope.BUILD, 'none': Scope.NONE}
+            with target._prepare_sandbox(scope=scope[deps], directory=None,
                                          integrate=integrate) as sandbox:
                 # Copy or move the sandbox to the target directory
                 virdir = sandbox.get_virtual_directory()