Merge branch 'tpollard/829' into 'master'

Download buildtrees on demand for bst shell --use-buildtree

Closes #829

See merge request BuildStream/buildstream!1050

3 files changed, 94 insertions, 30 deletions

diff --git a/buildstream/_frontend/cli.py b/buildstream/_frontend/cli.py
index ab190aae4..34217aee5 100644
--- a/buildstream/_frontend/cli.py
+++ b/buildstream/_frontend/cli.py
@@ -526,7 +526,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
     else:
         scope = Scope.RUN
 
-    use_buildtree = False
+    use_buildtree = None
 
     with app.initialized():
         if not element:
@@ -534,7 +534,8 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
             if not element:
                 raise AppError('Missing argument "ELEMENT".')
 
-        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
+        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE,
+                                                 use_artifact_config=True)
         element = dependencies[0]
         prompt = app.shell_prompt(element)
         mounts = [
@@ -543,20 +544,31 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command)
         ]
 
         cached = element._cached_buildtree()
-        if cli_buildtree == "always":
-            if cached:
-                use_buildtree = True
-            else:
-                raise AppError("No buildtree is cached but the use buildtree option was specified")
-        elif cli_buildtree == "never":
-            pass
-        elif cli_buildtree == "try":
-            use_buildtree = cached
+        if cli_buildtree in ("always", "try"):
+            use_buildtree = cli_buildtree
+            if not cached and use_buildtree == "always":
+                click.echo("WARNING: buildtree is not cached locally, will attempt to pull from available remotes",
+                           err=True)
         else:
-            if app.interactive and cached:
-                use_buildtree = bool(click.confirm('Do you want to use the cached buildtree?'))
+            # If the value has defaulted to ask and in non interactive mode, don't consider the buildtree, this
+            # being the default behaviour of the command
+            if app.interactive and cli_buildtree == "ask":
+                if cached and bool(click.confirm('Do you want to use the cached buildtree?')):
+                    use_buildtree = "always"
+                elif not cached:
+                    try:
+                        choice = click.prompt("Do you want to pull & use a cached buildtree?",
+                                              type=click.Choice(['try', 'always', 'never']),
+                                              err=True, show_choices=True)
+                    except click.Abort:
+                        click.echo('Aborting', err=True)
+                        sys.exit(-1)
+
+                    if choice != "never":
+                        use_buildtree = choice
+
         if use_buildtree and not element._cached_success():
-            click.echo("Warning: using a buildtree from a failed build.")
+            click.echo("WARNING: using a buildtree from a failed build.", err=True)
 
         try:
             exitcode = app.stream.shell(element, scope, prompt,
diff --git a/buildstream/_stream.py b/buildstream/_stream.py
index e77a19891..af736c96a 100644
--- a/buildstream/_stream.py
+++ b/buildstream/_stream.py
@@ -101,19 +101,22 @@ class Stream():
     #    targets (list of str): Targets to pull
     #    selection (PipelineSelection): The selection mode for the specified targets
     #    except_targets (list of str): Specified targets to except from fetching
+    #    use_artifact_config (bool): If artifact remote config should be loaded
     #
     # Returns:
     #    (list of Element): The selected elements
     def load_selection(self, targets, *,
                        selection=PipelineSelection.NONE,
-                       except_targets=()):
+                       except_targets=(),
+                       use_artifact_config=False):
 
         profile_start(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
 
         elements, _ = self._load(targets, (),
                                  selection=selection,
                                  except_targets=except_targets,
-                                 fetch_subprojects=False)
+                                 fetch_subprojects=False,
+                                 use_artifact_config=use_artifact_config)
 
         profile_end(Topics.LOAD_SELECTION, "_".join(t.replace(os.sep, '-') for t in targets))
 
@@ -131,7 +134,7 @@ class Stream():
     #    mounts (list of HostMount): Additional directories to mount into the sandbox
     #    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): Wheather to use a buildtree as the source.
+    #    usebuildtree (str): Whether to use a buildtree as the source, given cli option
     #
     # Returns:
     #    (int): The exit code of the launched shell
@@ -141,7 +144,7 @@ class Stream():
               mounts=None,
               isolate=False,
               command=None,
-              usebuildtree=False):
+              usebuildtree=None):
 
         # Assert we have everything we need built, unless the directory is specified
         # in which case we just blindly trust the directory, using the element
@@ -156,8 +159,31 @@ class Stream():
                 raise StreamError("Elements need to be built or downloaded before staging a shell environment",
                                   detail="\n".join(missing_deps))
 
+        buildtree = False
+        # Check if we require a pull queue attempt, with given artifact state and context
+        if usebuildtree:
+            if not element._cached_buildtree():
+                require_buildtree = self._buildtree_pull_required([element])
+                # Attempt a pull queue for the given element if remote and context allow it
+                if require_buildtree:
+                    self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtree")
+                    self._add_queue(PullQueue(self._scheduler))
+                    self._enqueue_plan(require_buildtree)
+                    self._run()
+                    # Now check if the buildtree was successfully fetched
+                    if element._cached_buildtree():
+                        buildtree = True
+                if not buildtree:
+                    if usebuildtree == "always":
+                        raise StreamError("Buildtree is not cached locally or in available remotes")
+                    else:
+                        self._message(MessageType.INFO, """Buildtree is not cached locally or in available remotes,
+                                                        shell will be loaded without it""")
+            else:
+                buildtree = True
+
         return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command,
-                              usebuildtree=usebuildtree)
+                              usebuildtree=buildtree)
 
     # build()
     #
diff --git a/tests/integration/build-tree.py b/tests/integration/build-tree.py
index b50d84152..b1a41aefe 100644
--- a/tests/integration/build-tree.py
+++ b/tests/integration/build-tree.py
@@ -101,7 +101,7 @@ def test_buildtree_from_failure(cli_integration, tmpdir, datafiles):
         'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
     ])
     res.assert_success()
-    assert "Warning: using a buildtree from a failed build" in res.output
+    assert "WARNING: using a buildtree from a failed build" in res.stderr
     assert 'Hi' in res.output
 
 
@@ -141,7 +141,7 @@ def test_buildtree_pulled(cli, tmpdir, datafiles):
         res.assert_success()
 
 
-# This test checks for correct behaviour if a buildtree is not present.
+# This test checks for correct behaviour if a buildtree is not present in the local cache.
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 def test_buildtree_options(cli, tmpdir, datafiles):
@@ -156,6 +156,7 @@ def test_buildtree_options(cli, tmpdir, datafiles):
         result = cli.run(project=project, args=['build', element_name])
         result.assert_success()
         assert cli.get_element_state(project, element_name) == 'cached'
+        assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 
         # Discard the cache
         cli.configure({
@@ -168,8 +169,6 @@ def test_buildtree_options(cli, tmpdir, datafiles):
         result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
         result.assert_success()
 
-        # The above is the simplest way I know to create a local cache without any buildtrees.
-
         # Check it's not using the cached build tree
         res = cli.run(project=project, args=[
             'shell', '--build', element_name, '--use-buildtree', 'never', '--', 'cat', 'test'
@@ -177,24 +176,51 @@ def test_buildtree_options(cli, tmpdir, datafiles):
         res.assert_shell_error()
         assert 'Hi' not in res.output
 
-        # Check it's not correctly handling the lack of buildtree
+        # Check it's not using the cached build tree, default is to ask, and fall back to not
+        # for non interactive behavior
         res = cli.run(project=project, args=[
-            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
+            'shell', '--build', element_name, '--', 'cat', 'test'
         ])
         res.assert_shell_error()
         assert 'Hi' not in res.output
 
-        # Check it's not using the cached build tree, default is to ask, and fall back to not
-        # for non interactive behavior
+        # Check correctly handling the lack of buildtree, with 'try' not attempting to
+        # pull the buildtree as the user context is by default set to not pull them
         res = cli.run(project=project, args=[
-            'shell', '--build', element_name, '--', 'cat', 'test'
+            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
         ])
-        res.assert_shell_error()
         assert 'Hi' not in res.output
+        assert 'Attempting to fetch missing artifact buildtrees' not in res.stderr
+        assert """Buildtree is not cached locally or in available remotes,
+                shell will be loaded without it"""
 
-        # Check it's using the cached build tree
+        # Check correctly handling the lack of buildtree, with 'try' attempting and succeeding
+        # to pull the buildtree as the user context allow the pulling of buildtrees and it is
+        # available in the remote
+        res = cli.run(project=project, args=[
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
+        ])
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
+        assert 'Hi' in res.output
+        shutil.rmtree(os.path.join(os.path.join(cli.directory, 'artifacts2')))
+        assert cli.get_element_state(project, element_name) != 'cached'
+
+        # Check it's not loading the shell at all with always set for the buildtree, when the
+        # user context does not allow for buildtree pulling
+        result = cli.run(project=project, args=['artifact', 'pull', '--deps', 'all', element_name])
+        result.assert_success()
         res = cli.run(project=project, args=[
             'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
         ])
         res.assert_main_error(ErrorDomain.PROG_NOT_FOUND, None)
+        assert 'Buildtree is not cached locally or in available remotes' in res.stderr
         assert 'Hi' not in res.output
+        assert 'Attempting to fetch missing artifact buildtree' not in res.stderr
+
+        # Check that when user context is set to pull buildtrees and a remote has the buildtree,
+        # 'always' will attempt and succeed at pulling the missing buildtree.
+        res = cli.run(project=project, args=[
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
+        ])
+        assert 'Hi' in res.output
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr