[Notes] [Git][BuildStream/buildstream][richardmaw/shell-multi-stage] 6 c

richardmaw-codethink pushed to branch richardmaw/shell-multi-stage at BuildStream / buildstream

Commits:

21b6ba36

by Richard Maw at 2018-12-11T17:56:26Z

_stream: Move shell logic from Element and support multiple elements

There's redundancy in pre-staging sandbox creation logic
with Element._sandbox doing something similar,
but hopefully not too much.

10a5243b
by Richard Maw at 2018-12-11T17:56:26Z
```
element: Remove dead _shell method
```

f87b7fd3

by Richard Maw at 2018-12-11T17:56:26Z

cli: Support multiple elements in `bst shell`

2441ab39

by Richard Maw at 2018-12-11T17:56:26Z

tests: Move integration.bst to subdirectory

87b660fa

by Richard Maw at 2018-12-11T17:56:26Z

tests: Test multiple-element runtime and workspace shells

f7954ef5
by Richard Maw at 2018-12-11T17:56:26Z
```
Add to NEWS
```

9 changed files:

NEWS
buildstream/_frontend/app.py
buildstream/_frontend/cli.py
buildstream/_stream.py
buildstream/element.py
+ tests/integration/project/elements/shell/adds-bar.bst
+ tests/integration/project/elements/shell/adds-foo.bst
tests/integration/project/elements/integration.bst → tests/integration/project/elements/shell/integration.bst
tests/integration/shell.py

Changes:

NEWS

@@ -2,6 +2,12 @@
  buildstream 1.3.1
  =================
 +  o `bst shell` learned to accept multiple elements for staging
 +    provided the element's kind flags `BST_STAGE_INTEGRATES` as false.
++
 +    As a side-effect it is no longer possible to intersperse options and flags
 +    with arguments in the `bst shell` command-line.
++
    o All elements must now be suffixed with `.bst`
      Attempting to use an element that does not have the `.bst` extension,
      will result in a warning.

buildstream/_frontend/app.py

@@ -599,7 +599,7 @@ class App():
                      click.echo("\nDropping into an interactive shell in the failed build sandbox\n", err=True)
                      try:
                          prompt = self.shell_prompt(element)
 -                        self.stream.shell(element, Scope.BUILD, prompt, isolate=True)
 +                        self.stream.shell([(element, Scope.BUILD)], prompt, isolate=True)
                      except BstError as e:
                          click.echo("Error while attempting to create interactive shell: {}".format(e), err=True)
                  elif choice == 'log':

buildstream/_frontend/cli.py

@@ -573,7 +573,7 @@ def show(app, elements, deps, except_, order, format_):
  ##################################################################
  @cli.command(short_help="Shell into an element's sandbox environment")
  @click.option('--build', '-b', 'build_', is_flag=True, default=False,
 -              help='Stage dependencies and sources to build')
 +              help='Stage dependencies and sources to build the first element')
  @click.option('--sysroot', '-s', default=None,
                type=click.Path(exists=True, file_okay=False, readable=True),
                help="An existing sysroot")
@@ -582,11 +582,11 @@ def show(app, elements, deps, except_, order, format_):
                help="Mount a file or directory into the sandbox")
  @click.option('--isolate', is_flag=True, default=False,
                help='Create an isolated build sandbox')
 -@click.argument('element',
 -                type=click.Path(readable=False))
 -@click.argument('command', type=click.STRING, nargs=-1)
 +@click.argument('elements',
 +                type=click.Path(readable=False), nargs=-1,
 +                metavar="[ELEMENT]... [--] [COMMAND]...")
  @click.pass_obj
 -def shell(app, element, sysroot, mount, isolate, build_, command):
 +def shell(app, elements, sysroot, mount, isolate, build_):
      """Run a command in the target element's sandbox environment
      This will stage a temporary sysroot for running the target
@@ -600,6 +600,15 @@ def shell(app, element, sysroot, mount, isolate, build_, command):
      directory or with a checkout of the given target, in order
      to use a specific sysroot.
 +    Multiple element target names ending in .bst may be provided,
 +    optionally followed by the command to run in the shell.
++
 +    The first argument that doesn't end in .bst is assumed to be the beginning of COMMAND.
++
 +    A -- argument may be used to disambiguate between element target names and command arguments
 +    and should be used when being driven as part of a script, to prevent mis-parsing,
 +    in addition to a -- before the element list.
++
      If no COMMAND is specified, the default is to attempt
      to run an interactive shell.
      """
@@ -607,13 +616,22 @@ def shell(app, element, sysroot, mount, isolate, build_, command):
      from .._project import HostMount
      from .._pipeline import PipelineSelection
 -    if build_:
 -        scope = Scope.BUILD
 -    else:
 -        scope = Scope.RUN
 +    targets = []
 +    command = []
 +    for i, arg in enumerate(elements):
 +        if arg == '--':
 +            command = elements[i + 1:]
 +            break
 +        if not arg.endswith('.bst'):
 +            command = elements[i:]
 +            break
 +        targets.append(arg)
++
 +    if not targets:
 +        raise AppError('No elements specified to open a shell in')
      with app.initialized():
 -        dependencies = app.stream.load_selection((element,), selection=PipelineSelection.NONE)
 +        dependencies = app.stream.load_selection(targets, selection=PipelineSelection.NONE)
          element = dependencies[0]
          prompt = app.shell_prompt(element)
          mounts = [
@@ -621,7 +639,9 @@ def shell(app, element, sysroot, mount, isolate, build_, command):
              for host_path, path in mount
+         ]
          try:
 -            exitcode = app.stream.shell(element, scope, prompt,
 +            elements = tuple((e, Scope.BUILD if i == 0 and build_ else Scope.RUN)
 +                             for (i, e) in enumerate(dependencies))
 +            exitcode = app.stream.shell(elements, prompt,
                                          directory=sysroot,
                                          mounts=mounts,
                                          isolate=isolate,

buildstream/_stream.py

@@ -25,15 +25,17 @@ import stat
  import shlex
  import shutil
  import tarfile
 -from contextlib import contextmanager
 +from contextlib import contextmanager, ExitStack
  from tempfile import TemporaryDirectory
  from ._exceptions import StreamError, ImplError, BstError, set_last_task_error
  from ._message import Message, MessageType
 -from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
  from ._pipeline import Pipeline, PipelineSelection
 +from ._platform import Platform
 +from .sandbox._config import SandboxConfig
 +from ._scheduler import Scheduler, SchedStatus, TrackQueue, FetchQueue, BuildQueue, PullQueue, PushQueue
  from . import utils, _yaml, _site
 -from . import Scope, Consistency
 +from . import SandboxFlags, Scope, Consistency
  # Stream()
@@ -117,8 +119,7 @@ class Stream():
      # Run a shell
+     #
      # Args:
 -    #    element (Element): An Element object to run the shell for
 -    #    scope (Scope): The scope for the shell (Scope.BUILD or Scope.RUN)
 +    #    elements (List of (Element, Scope) tuples): Elements to run the shell for and their dependency scopes.
      #    prompt (str): The prompt to display in the shell
      #    directory (str): A directory where an existing prestaged sysroot is expected, or None
      #    mounts (list of HostMount): Additional directories to mount into the sandbox
@@ -128,7 +129,7 @@ class Stream():
      # Returns:
      #    (int): The exit code of the launched shell
+     #
 -    def shell(self, element, scope, prompt, *,
 +    def shell(self, elements, prompt, *,
                directory=None,
                mounts=None,
                isolate=False,
@@ -138,16 +139,118 @@ class Stream():
          # in which case we just blindly trust the directory, using the element
          # definitions to control the execution environment only.
          if directory is None:
 -            missing_deps = [
 -                dep._get_full_name()
 -                for dep in self._pipeline.dependencies([element], scope)
 -                if not dep._cached()
 -            ]
 +            missing_deps = []
 +            for element, scope in elements:
 +                missing_deps.extend(
 +                    dep._get_full_name()
 +                    for dep in self._pipeline.dependencies((element,), scope)
 +                    if not dep._cached())
              if missing_deps:
                  raise StreamError("Elements need to be built or downloaded before staging a shell environment",
                                    detail="\n".join(missing_deps))
 -        return element._shell(scope, directory, mounts=mounts, isolate=isolate, prompt=prompt, command=command)
 +        # Assert we're not mixing virtual directory compatible
 +        # and non-virtual directory compatible elements
 +        if (any(e.BST_VIRTUAL_DIRECTORY for (e, _) in elements) and
 +                not all(e.BST_VIRTUAL_DIRECTORY for (e, _) in elements)):
 +            raise StreamError(
 +                "Elements do not support multiple-element staging",
 +                detail=("Multi-element staging is not supported" +
 +                        " because elements {} support BST_VIRTUAL_DIRECTORY and {} do not.").format(
 +                            ', '.join(e.name for (e, _) in elements if e.BST_VIRTUAL_DIRECTORY),
 +                            ', '.join(e.name for (e, _) in elements if not e.BST_VIRTUAL_DIRECTORY)))
++
 +        with ExitStack() as stack:
 +            # Creation logic duplicated from Element.__sandbox
 +            # since most of it is creating the tmpdir
 +            # and deciding whether to make a remote sandbox,
 +            # which we don't want to.
 +            if directory is None:
 +                os.makedirs(self._context.builddir, exist_ok=True)
 +                rootdir = stack.enter_context(TemporaryDirectory(dir=self._context.builddir))
 +            else:
 +                rootdir = directory
++
 +            # SandboxConfig comes from project, element defaults and MetaElement sandbox config
 +            # In the absence of it being exposed to other APIs and a merging strategy
 +            # just make it from the project sandbox config.
 +            sandbox_config = SandboxConfig(_yaml.node_get(self._project._sandbox, int, 'build-uid'),
 +                                           _yaml.node_get(self._project._sandbox, int, 'build-gid'))
 +            platform = Platform.get_platform()
 +            sandbox = platform.create_sandbox(context=self._context,
 +                                              project=self._project,
 +                                              directory=rootdir,
 +                                              stdout=None, stderr=None, config=sandbox_config,
 +                                              bare_directory=directory is not None,
 +                                              allow_real_directory=not any(e.BST_VIRTUAL_DIRECTORY
 +                                                                           for (e, _) in elements))
++
 +            # Configure the sandbox with the last element taking precedence for config.
 +            for e, _ in elements:
 +                e.configure_sandbox(sandbox)
++
 +            # Stage contents if not passed --sysroot
 +            if not directory:
 +                if any(e.BST_STAGE_INTEGRATES for (e, _) in elements):
 +                    if len(elements) > 1:
 +                        raise StreamError(
 +                            "Elements do not support multiple-element staging",
 +                            detail=("Elements {} do not support multi-element staging " +
 +                                    " because element kinds {} set BST_STAGE_INTEGRATES").format(
 +                                        ', '.join(e.name for (e, _) in elements if e.BST_STAGE_INTEGRATES),
 +                                        ', '.join(set(e.get_kind() for (e, _) in elements))))
 +                    elements[0][0].stage(sandbox)
 +                else:
 +                    visited = {}
 +                    for e, scope in elements:
 +                        if scope is Scope.BUILD:
 +                            e.stage(sandbox, visited=visited)
 +                        else:
 +                            e.stage_dependency_artifacts(sandbox, scope, visited=visited)
++
 +                    visited = {}
 +                    for e, scope in elements:
 +                        e.integrate_dependency_artifacts(sandbox, scope, visited=visited)
++
 +            environment = {}
 +            for e, _ in elements:
 +                environment.update(e.get_environment())
 +            flags = SandboxFlags.INTERACTIVE | SandboxFlags.ROOT_READ_ONLY
 +            shell_command, shell_environment, shell_host_files = self._project.get_shell_config()
 +            environment['PS1'] = prompt
 +            # Special configurations for non-isolated sandboxes
 +            if not isolate:
++
 +                # Open the network, and reuse calling uid/gid
 +                #
 +                flags |= SandboxFlags.NETWORK_ENABLED | SandboxFlags.INHERIT_UID
++
 +                # Apply project defined environment vars to set for a shell
 +                for key, value in _yaml.node_items(shell_environment):
 +                    environment[key] = value
++
 +                # Setup any requested bind mounts
 +                if mounts is None:
 +                    mounts = []
++
 +                for mount in shell_host_files + mounts:
 +                    if not os.path.exists(mount.host_path):
 +                        if not mount.optional:
 +                            self._message(MessageType.WARN,
 +                                          "Not mounting non-existing host file: {}".format(mount.host_path))
 +                    else:
 +                        sandbox.mark_directory(mount.path)
 +                        sandbox._set_mount_source(mount.path, mount.host_path)
++
 +            if command:
 +                argv = list(command)
 +            else:
 +                argv = shell_command
++
 +            self._message(MessageType.STATUS, "Running command", detail=" ".join(argv))
++
 +            # Run shells with network enabled and readonly root.
 +            return sandbox.run(argv, flags, env=environment)
      # build()
+     #

buildstream/element.py

@@ -75,7 +75,6 @@ Class Reference
  import os
  import re
  import stat
 -import copy
  from collections import OrderedDict
  from collections.abc import Mapping
  import contextlib
@@ -1382,7 +1381,7 @@ class Element(Plugin):
      # is used to stage things by the `bst checkout` codepath
+     #
      @contextmanager
 -    def _prepare_sandbox(self, scope, directory, shell=False, integrate=True):
 +    def _prepare_sandbox(self, scope, directory, integrate=True):
          # bst shell and bst checkout require a local sandbox.
          bare_directory = True if directory else False
          with self.__sandbox(directory, config=self.__sandbox_config, allow_remote=False,
@@ -1393,20 +1392,15 @@ class Element(Plugin):
              # Stage something if we need it
              if not directory:
 -                if shell and scope == Scope.BUILD:
 -                    self.stage(sandbox)
 -                    if not self.BST_STAGE_INTEGRATES:
 +                # Stage deps in the sandbox root
 +                with self.timed_activity("Staging dependencies", silent_nested=True):
 +                    self.stage_dependency_artifacts(sandbox, scope)
++
 +                # Run any integration commands provided by the dependencies
 +                # once they are all staged and ready
 +                if integrate:
 +                    with self.timed_activity("Integrating sandbox"):
                          self.integrate_dependency_artifacts(sandbox, scope)
 -                else:
 -                    # Stage deps in the sandbox root
 -                    with self.timed_activity("Staging dependencies", silent_nested=True):
 -                        self.stage_dependency_artifacts(sandbox, scope)
+-
 -                    # Run any integration commands provided by the dependencies
 -                    # once they are all staged and ready
 -                    if integrate:
 -                        with self.timed_activity("Integrating sandbox"):
 -                            self.integrate_dependency_artifacts(sandbox, scope)
              yield sandbox
@@ -1885,71 +1879,6 @@ class Element(Plugin):
          # Notify successful upload
          return True
 -    # _shell():
 -    #
 -    # Connects the terminal with a shell running in a staged
 -    # environment
 -    #
 -    # Args:
 -    #    scope (Scope): Either BUILD or RUN scopes are valid, or None
 -    #    directory (str): A directory to an existing sandbox, or None
 -    #    mounts (list): A list of (str, str) tuples, representing host/target paths to mount
 -    #    isolate (bool): Whether to isolate the environment like we do in builds
 -    #    prompt (str): A suitable prompt string for PS1
 -    #    command (list): An argv to launch in the sandbox
 -    #
 -    # Returns: Exit code
 -    #
 -    # If directory is not specified, one will be staged using scope
 -    def _shell(self, scope=None, directory=None, *, mounts=None, isolate=False, prompt=None, command=None):
+-
 -        with self._prepare_sandbox(scope, directory, shell=True) as sandbox:
 -            environment = self.get_environment()
 -            environment = copy.copy(environment)
 -            flags = SandboxFlags.INTERACTIVE | SandboxFlags.ROOT_READ_ONLY
+-
 -            # Fetch the main toplevel project, in case this is a junctioned
 -            # subproject, we want to use the rules defined by the main one.
 -            context = self._get_context()
 -            project = context.get_toplevel_project()
 -            shell_command, shell_environment, shell_host_files = project.get_shell_config()
+-
 -            if prompt is not None:
 -                environment['PS1'] = prompt
+-
 -            # Special configurations for non-isolated sandboxes
 -            if not isolate:
+-
 -                # Open the network, and reuse calling uid/gid
 -                #
 -                flags |= SandboxFlags.NETWORK_ENABLED | SandboxFlags.INHERIT_UID
+-
 -                # Apply project defined environment vars to set for a shell
 -                for key, value in _yaml.node_items(shell_environment):
 -                    environment[key] = value
+-
 -                # Setup any requested bind mounts
 -                if mounts is None:
 -                    mounts = []
+-
 -                for mount in shell_host_files + mounts:
 -                    if not os.path.exists(mount.host_path):
 -                        if not mount.optional:
 -                            self.warn("Not mounting non-existing host file: {}".format(mount.host_path))
 -                    else:
 -                        sandbox.mark_directory(mount.path)
 -                        sandbox._set_mount_source(mount.path, mount.host_path)
+-
 -            if command:
 -                argv = [arg for arg in command]
 -            else:
 -                argv = shell_command
+-
 -            self.status("Running command", detail=" ".join(argv))
+-
 -            # Run shells with network enabled and readonly root.
 -            return sandbox.run(argv, flags, env=environment)
+-
      # _open_workspace():
+     #
      # "Open" a workspace for this element

tests/integration/project/elements/shell/adds-bar.bst

 +kind: manual
 +depends:
 +- base.bst
++
 +config:
 +  install-commands:
 +  - |
 +    install -D -m775 /proc/self/fd/0 %{install-root}%{bindir}/bar <<\EOF
 +    #!/bin/sh
 +    echo bar
 +    EOF

tests/integration/project/elements/shell/adds-foo.bst

 +kind: manual
 +depends:
 +- base.bst
++
 +config:
 +  install-commands:
 +  - |
 +    install -D -m775 /proc/self/fd/0 %{install-root}%{bindir}/foo <<\EOF
 +    #!/bin/sh
 +    echo foo
 +    EOF

tests/integration/project/elements/integration.bst → tests/integration/project/elements/shell/integration.bst

tests/integration/shell.py

@@ -28,11 +28,17 @@ DATA_DIR = os.path.join(
  #    config (dict): A project.conf dictionary to composite over the default
  #    mount (tuple): A (host, target) tuple for the `--mount` option
  #    element (str): The element to build and run a shell with
 +#    elements (list): Other elements to build and run a shell with
  #    isolate (bool): Whether to pass --isolate to `bst shell`
+ #
 -def execute_shell(cli, project, command, *, config=None, mount=None, element='base.bst', isolate=False):
 +def execute_shell(cli, project, command, *, config=None, mount=None, elements=None, isolate=False):
      # Ensure the element is built
 -    result = cli.run(project=project, project_config=config, args=['build', element])
 +    if elements is None:
 +        elements = ('base.bst',)
++
 +    args = ['build', '--']
 +    args.extend(elements)
 +    result = cli.run(project=project, project_config=config, args=args)
      assert result.exit_code == 0
      args = ['shell']
@@ -41,7 +47,9 @@ def execute_shell(cli, project, command, *, config=None, mount=None, element='ba
      if mount is not None:
          host_path, target_path = mount
          args += ['--mount', host_path, target_path]
 -    args += [element, '--'] + command
 +    args.append('--')
 +    args.extend(elements)
 +    args += ['--'] + command
      return cli.run(project=project, project_config=config, args=args)
@@ -158,7 +166,7 @@ def test_no_shell(cli, tmpdir, datafiles):
      os.makedirs(os.path.dirname(os.path.join(element_path, element_name)), exist_ok=True)
      _yaml.dump(element, os.path.join(element_path, element_name))
 -    result = execute_shell(cli, project, ['/bin/echo', 'Pegasissies!'], element=element_name)
 +    result = execute_shell(cli, project, ['/bin/echo', 'Pegasissies!'], elements=(element_name,))
      assert result.exit_code == 0
      assert result.output == "Pegasissies!\n"
@@ -345,11 +353,56 @@ def test_sysroot(cli, tmpdir, datafiles):
  # Test system integration commands can access devices in /dev
 +@pytest.mark.integration
  @pytest.mark.datafiles(DATA_DIR)
  @pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
  def test_integration_devices(cli, tmpdir, datafiles):
      project = os.path.join(datafiles.dirname, datafiles.basename)
 -    element_name = 'integration.bst'
 +    element_name = 'shell/integration.bst'
++
 +    result = execute_shell(cli, project, ["true"], elements=(element_name,))
 +    assert result.exit_code == 0
++
++
 +# Test multiple element shell
 +@pytest.mark.integration
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 +def test_shell_multiple_elements(cli, tmpdir, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
++
 +    result = execute_shell(cli, project, ["sh", "-c", "foo && bar"],
 +                           elements=["shell/adds-foo.bst", "shell/adds-bar.bst"])
 +    assert result.exit_code == 0
++
++
 +# Test multiple element build shell
 +@pytest.mark.integration
 +@pytest.mark.datafiles(DATA_DIR)
 +@pytest.mark.skipif(IS_LINUX and not HAVE_BWRAP, reason='Only available with bubblewrap on Linux')
 +def test_shell_multiple_workspace(cli, tmpdir, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    elements = {'workspace/workspace-mount.bst': os.path.join(cli.directory, 'workspace-mount'),
 +                'make/makehello.bst': os.path.join(cli.directory, 'makehello')}
++
 +    for element, workspace in elements.items():
 +        res = cli.run(project=project, args=['workspace', 'open', element, '--directory', workspace])
 +        assert res.exit_code == 0
++
 +    for workspace in elements.values():
 +        with open(os.path.join(workspace, "workspace-exists"), "w") as f:
 +            pass
++
 +    # Ensure the dependencies of our build failing element are built
 +    result = cli.run(project=project, args=['build', 'base.bst', 'make/makehello.bst'])
 +    assert result.exit_code == 0
 -    result = execute_shell(cli, project, ["true"], element=element_name)
 +    # Test that only the first workspace exists, since only the first element may be staged for build,
 +    # additional elements may only be staged as extra dependencies.
 +    args = ['shell', '--build', '--'] + list(elements)
 +    args += ['--', 'sh', '-c',
 +             'test -e /buildstream/test/workspace/workspace-mount.bst/workspace-exists && \
 +              test ! -e /buildstream/test/make/makehello.bst/workspace-exists']
 +    result = cli.run(project=project, args=args)
      assert result.exit_code == 0
 +    assert result.output == ''

[Notes] [Git][BuildStream/buildstream][richardmaw/shell-multi-stage] 6 commits: _stream: Move shell logic from Element and support multiple elements

richardmaw-codethink pushed to branch richardmaw/shell-multi-stage at BuildStream / buildstream

Commits:

9 changed files:

Changes: