Tom Pollard pushed to branch tpollard/829 at BuildStream / buildstream
Commits:
3 changed files:
Changes:
| ... | ... | @@ -560,7 +560,7 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 560 | 560 | 
     else:
 | 
| 561 | 561 | 
         scope = Scope.RUN
 | 
| 562 | 562 | 
 | 
| 563 | 
-    use_buildtree = False
 | 
|
| 563 | 
+    use_buildtree = None
 | 
|
| 564 | 564 | 
 | 
| 565 | 565 | 
     with app.initialized():
 | 
| 566 | 566 | 
         if not element:
 | 
| ... | ... | @@ -568,7 +568,8 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 568 | 568 | 
             if not element:
 | 
| 569 | 569 | 
                 raise AppError('Missing argument "ELEMENT".')
 | 
| 570 | 570 | 
 | 
| 571 | 
-        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
 | 
|
| 571 | 
+        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE,
 | 
|
| 572 | 
+                                                 use_artifact_config=True)
 | 
|
| 572 | 573 | 
         element = dependencies[0]
 | 
| 573 | 574 | 
         prompt = app.shell_prompt(element)
 | 
| 574 | 575 | 
         mounts = [
 | 
| ... | ... | @@ -577,18 +578,19 @@ def shell(app, element, sysroot, mount, isolate, build_, cli_buildtree, command) | 
| 577 | 578 | 
         ]
 | 
| 578 | 579 | 
 | 
| 579 | 580 | 
         cached = element._cached_buildtree()
 | 
| 580 | 
-        if cli_buildtree == "always":
 | 
|
| 581 | 
-            if cached:
 | 
|
| 582 | 
-                use_buildtree = True
 | 
|
| 583 | 
-            else:
 | 
|
| 584 | 
-                raise AppError("No buildtree is cached but the use buildtree option was specified")
 | 
|
| 585 | 
-        elif cli_buildtree == "never":
 | 
|
| 586 | 
-            pass
 | 
|
| 587 | 
-        elif cli_buildtree == "try":
 | 
|
| 588 | 
-            use_buildtree = cached
 | 
|
| 581 | 
+        if cli_buildtree in ("always", "try"):
 | 
|
| 582 | 
+            use_buildtree = cli_buildtree
 | 
|
| 583 | 
+            if not cached and use_buildtree == "always":
 | 
|
| 584 | 
+                click.echo("Warning: buildtree is not cached locally, will attempt to pull from available remotes")
 | 
|
| 589 | 585 | 
         else:
 | 
| 590 | 
-            if app.interactive and cached:
 | 
|
| 591 | 
-                use_buildtree = bool(click.confirm('Do you want to use the cached buildtree?'))
 | 
|
| 586 | 
+            # If the value has defaulted to ask and in non interactive mode, don't consider the buildtree, this
 | 
|
| 587 | 
+            # being the default behaviour of the command
 | 
|
| 588 | 
+            if app.interactive and cli_buildtree == "ask":
 | 
|
| 589 | 
+                if cached and bool(click.confirm('Do you want to use the cached buildtree?')):
 | 
|
| 590 | 
+                    use_buildtree = "always"
 | 
|
| 591 | 
+                elif not cached and bool(click.confirm('Do you want to attempt to pull the cached buildtree?')):
 | 
|
| 592 | 
+                    use_buildtree = "try"
 | 
|
| 593 | 
+  | 
|
| 592 | 594 | 
         if use_buildtree and not element._cached_success():
 | 
| 593 | 595 | 
             click.echo("Warning: using a buildtree from a failed build.")
 | 
| 594 | 596 | 
 | 
| ... | ... | @@ -100,16 +100,19 @@ class Stream(): | 
| 100 | 100 | 
     #    targets (list of str): Targets to pull
 | 
| 101 | 101 | 
     #    selection (PipelineSelection): The selection mode for the specified targets
 | 
| 102 | 102 | 
     #    except_targets (list of str): Specified targets to except from fetching
 | 
| 103 | 
+    #    use_artifact_config (bool): If artifact remote config should be loaded
 | 
|
| 103 | 104 | 
     #
 | 
| 104 | 105 | 
     # Returns:
 | 
| 105 | 106 | 
     #    (list of Element): The selected elements
 | 
| 106 | 107 | 
     def load_selection(self, targets, *,
 | 
| 107 | 108 | 
                        selection=PipelineSelection.NONE,
 | 
| 108 | 
-                       except_targets=()):
 | 
|
| 109 | 
+                       except_targets=(),
 | 
|
| 110 | 
+                       use_artifact_config=False):
 | 
|
| 109 | 111 | 
         elements, _ = self._load(targets, (),
 | 
| 110 | 112 | 
                                  selection=selection,
 | 
| 111 | 113 | 
                                  except_targets=except_targets,
 | 
| 112 | 
-                                 fetch_subprojects=False)
 | 
|
| 114 | 
+                                 fetch_subprojects=False,
 | 
|
| 115 | 
+                                 use_artifact_config=use_artifact_config)
 | 
|
| 113 | 116 | 
         return elements
 | 
| 114 | 117 | 
 | 
| 115 | 118 | 
     # shell()
 | 
| ... | ... | @@ -124,7 +127,7 @@ class Stream(): | 
| 124 | 127 | 
     #    mounts (list of HostMount): Additional directories to mount into the sandbox
 | 
| 125 | 128 | 
     #    isolate (bool): Whether to isolate the environment like we do in builds
 | 
| 126 | 129 | 
     #    command (list): An argv to launch in the sandbox, or None
 | 
| 127 | 
-    #    usebuildtree (bool): Wheather to use a buildtree as the source.
 | 
|
| 130 | 
+    #    usebuildtree (str): Whether to use a buildtree as the source, given cli option
 | 
|
| 128 | 131 | 
     #
 | 
| 129 | 132 | 
     # Returns:
 | 
| 130 | 133 | 
     #    (int): The exit code of the launched shell
 | 
| ... | ... | @@ -134,7 +137,7 @@ class Stream(): | 
| 134 | 137 | 
               mounts=None,
 | 
| 135 | 138 | 
               isolate=False,
 | 
| 136 | 139 | 
               command=None,
 | 
| 137 | 
-              usebuildtree=False):
 | 
|
| 140 | 
+              usebuildtree=None):
 | 
|
| 138 | 141 | 
 | 
| 139 | 142 | 
         # Assert we have everything we need built, unless the directory is specified
 | 
| 140 | 143 | 
         # in which case we just blindly trust the directory, using the element
 | 
| ... | ... | @@ -149,8 +152,30 @@ class Stream(): | 
| 149 | 152 | 
                 raise StreamError("Elements need to be built or downloaded before staging a shell environment",
 | 
| 150 | 153 | 
                                   detail="\n".join(missing_deps))
 | 
| 151 | 154 | 
 | 
| 155 | 
+        # Check if we require a pull queue attempt, with given artifact state and context
 | 
|
| 156 | 
+        if usebuildtree:
 | 
|
| 157 | 
+            if not element._cached_buildtree():
 | 
|
| 158 | 
+                buildtree = False
 | 
|
| 159 | 
+                require_buildtree = self._buildtree_pull_required([element])
 | 
|
| 160 | 
+                # Attempt a pull queue for the given element if remote and context allow it
 | 
|
| 161 | 
+                if require_buildtree:
 | 
|
| 162 | 
+                    self._message(MessageType.INFO, "Attempting to fetch missing artifact buildtree")
 | 
|
| 163 | 
+                    self._add_queue(PullQueue(self._scheduler))
 | 
|
| 164 | 
+                    self._enqueue_plan(require_buildtree)
 | 
|
| 165 | 
+                    self._run()
 | 
|
| 166 | 
+                    # Now check if the buildtree was successfully fetched
 | 
|
| 167 | 
+                    if element._cached_buildtree():
 | 
|
| 168 | 
+                        buildtree = True
 | 
|
| 169 | 
+                if not buildtree:
 | 
|
| 170 | 
+                    if usebuildtree == "always":
 | 
|
| 171 | 
+                        raise StreamError("Buildtree is not cached locally or in available remotes")
 | 
|
| 172 | 
+                    else:
 | 
|
| 173 | 
+                        self._message(MessageType.INFO, """Buildtree is not cached locally or in available remotes,
 | 
|
| 174 | 
+                                                        shell will be loaded without it""")
 | 
|
| 175 | 
+                        usebuildtree = False
 | 
|
| 176 | 
+  | 
|
| 152 | 177 | 
         return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command,
 | 
| 153 | 
-                              usebuildtree=usebuildtree)
 | 
|
| 178 | 
+                              usebuildtree=bool(usebuildtree))
 | 
|
| 154 | 179 | 
 | 
| 155 | 180 | 
     # build()
 | 
| 156 | 181 | 
     #
 | 
| ... | ... | @@ -140,7 +140,7 @@ def test_buildtree_pulled(cli, tmpdir, datafiles): | 
| 140 | 140 | 
         res.assert_success()
 | 
| 141 | 141 | 
 | 
| 142 | 142 | 
 | 
| 143 | 
-# This test checks for correct behaviour if a buildtree is not present.
 | 
|
| 143 | 
+# This test checks for correct behaviour if a buildtree is not present in the local cache.
 | 
|
| 144 | 144 | 
 @pytest.mark.datafiles(DATA_DIR)
 | 
| 145 | 145 | 
 @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 | 
| 146 | 146 | 
 def test_buildtree_options(cli, tmpdir, datafiles):
 | 
| ... | ... | @@ -155,6 +155,7 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 155 | 155 | 
         result = cli.run(project=project, args=['build', element_name])
 | 
| 156 | 156 | 
         result.assert_success()
 | 
| 157 | 157 | 
         assert cli.get_element_state(project, element_name) == 'cached'
 | 
| 158 | 
+        assert share.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 | 
|
| 158 | 159 | 
 | 
| 159 | 160 | 
         # Discard the cache
 | 
| 160 | 161 | 
         cli.configure({
 | 
| ... | ... | @@ -167,8 +168,6 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 167 | 168 | 
         result = cli.run(project=project, args=['pull', '--deps', 'all', element_name])
 | 
| 168 | 169 | 
         result.assert_success()
 | 
| 169 | 170 | 
 | 
| 170 | 
-        # The above is the simplest way I know to create a local cache without any buildtrees.
 | 
|
| 171 | 
-  | 
|
| 172 | 171 | 
         # Check it's not using the cached build tree
 | 
| 173 | 172 | 
         res = cli.run(project=project, args=[
 | 
| 174 | 173 | 
             'shell', '--build', element_name, '--use-buildtree', 'never', '--', 'cat', 'test'
 | 
| ... | ... | @@ -176,24 +175,51 @@ def test_buildtree_options(cli, tmpdir, datafiles): | 
| 176 | 175 | 
         res.assert_shell_error()
 | 
| 177 | 176 | 
         assert 'Hi' not in res.output
 | 
| 178 | 177 | 
 | 
| 179 | 
-        # Check it's not correctly handling the lack of buildtree
 | 
|
| 178 | 
+        # Check it's not using the cached build tree, default is to ask, and fall back to not
 | 
|
| 179 | 
+        # for non interactive behavior
 | 
|
| 180 | 180 | 
         res = cli.run(project=project, args=[
 | 
| 181 | 
-            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 181 | 
+            'shell', '--build', element_name, '--', 'cat', 'test'
 | 
|
| 182 | 182 | 
         ])
 | 
| 183 | 183 | 
         res.assert_shell_error()
 | 
| 184 | 184 | 
         assert 'Hi' not in res.output
 | 
| 185 | 185 | 
 | 
| 186 | 
-        # Check it's not using the cached build tree, default is to ask, and fall back to not
 | 
|
| 187 | 
-        # for non interactive behavior
 | 
|
| 186 | 
+        # Check correctly handling the lack of buildtree, with 'try' not attempting to
 | 
|
| 187 | 
+        # pull the buildtree as the user context is by default set to not pull them
 | 
|
| 188 | 188 | 
         res = cli.run(project=project, args=[
 | 
| 189 | 
-            'shell', '--build', element_name, '--', 'cat', 'test'
 | 
|
| 189 | 
+            'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 190 | 190 | 
         ])
 | 
| 191 | 
-        res.assert_shell_error()
 | 
|
| 192 | 191 | 
         assert 'Hi' not in res.output
 | 
| 192 | 
+        assert 'Attempting to fetch missing artifact buildtrees' not in res.stderr
 | 
|
| 193 | 
+        assert """Buildtree is not cached locally or in available remotes,
 | 
|
| 194 | 
+                shell will be loaded without it"""
 | 
|
| 193 | 195 | 
 | 
| 194 | 
-        # Check it's using the cached build tree
 | 
|
| 196 | 
+        # Check correctly handling the lack of buildtree, with 'try' attempting and succeeding
 | 
|
| 197 | 
+        # to pull the buildtree as the user context allow the pulling of buildtrees and it is
 | 
|
| 198 | 
+        # available in the remote
 | 
|
| 199 | 
+        res = cli.run(project=project, args=[
 | 
|
| 200 | 
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'try', '--', 'cat', 'test'
 | 
|
| 201 | 
+        ])
 | 
|
| 202 | 
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
 | 
|
| 203 | 
+        assert 'Hi' in res.output
 | 
|
| 204 | 
+        shutil.rmtree(os.path.join(os.path.join(cli.directory, 'artifacts2')))
 | 
|
| 205 | 
+        assert cli.get_element_state(project, element_name) != 'cached'
 | 
|
| 206 | 
+  | 
|
| 207 | 
+        # Check it's not loading the shell at all with always set for the buildtree, when the
 | 
|
| 208 | 
+        # user context does not allow for buildtree pulling
 | 
|
| 209 | 
+        result = cli.run(project=project, args=['pull', '--deps', 'all', element_name])
 | 
|
| 210 | 
+        result.assert_success()
 | 
|
| 195 | 211 | 
         res = cli.run(project=project, args=[
 | 
| 196 | 212 | 
             'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 | 
| 197 | 213 | 
         ])
 | 
| 198 | 214 | 
         res.assert_main_error(ErrorDomain.PROG_NOT_FOUND, None)
 | 
| 215 | 
+        assert 'Buildtree is not cached locally or in available remotes' in res.stderr
 | 
|
| 199 | 216 | 
         assert 'Hi' not in res.output
 | 
| 217 | 
+        assert 'Attempting to fetch missing artifact buildtree' not in res.stderr
 | 
|
| 218 | 
+  | 
|
| 219 | 
+        # Check that when user context is set to pull buildtrees and a remote has the buildtree,
 | 
|
| 220 | 
+        # 'always' will attempt and succeed at pulling the missing buildtree.
 | 
|
| 221 | 
+        res = cli.run(project=project, args=[
 | 
|
| 222 | 
+            '--pull-buildtrees', 'shell', '--build', element_name, '--use-buildtree', 'always', '--', 'cat', 'test'
 | 
|
| 223 | 
+        ])
 | 
|
| 224 | 
+        assert 'Hi' in res.output
 | 
|
| 225 | 
+        assert 'Attempting to fetch missing artifact buildtree' in res.stderr
 | 
