[Notes] [Git][BuildStream/buildstream][jonathan/debug-remote-failed-buil

Jonathan Maw pushed to branch jonathan/debug-remote-failed-builds at BuildStream / buildstream

Commits:

244ce39a

by Jonathan Maw at 2018-10-18T14:52:49Z

tests: Add a test for the interactive failure shell

This test checks that the interactive failure shell stages the cached
build tree.

01b6e3a7

by Jonathan Maw at 2018-10-18T14:52:49Z

sandbox.py: Remove redundant Sandbox.__directory

420f4c67

by Jonathan Maw at 2018-10-18T14:52:49Z

element: Make "--sysroot" take a bare directory

i.e. instead of taking a directory that must contain "root" and
"scratch", and treating "root" as the root, use the directory directly.

In element.py:
* __sandbox takes the `bare_sandbox` arg, to pass into the sandbox's constructor

In sandbox.py:
* If bare_sandbox, `_root` is the passed-in directory, and `__scratch`
  is None.
* Trying to use `__scratch` when bare_sandbox is True is a bug.

In _mount.py:
* Don't get the value of `__scratch` if it's not needed.

This is part of #539

470ffe8d

by Jonathan Maw at 2018-10-18T14:52:49Z

NEWS: Add item for bst shell --sysroot changes

4059148b

by Jonathan Maw at 2018-10-18T14:52:49Z

tests: Add tests for 'bst shell --sysroot'

This is related to #539

Changes:

NEWS

@@ -35,6 +35,10 @@ buildstream 1.3.1
      will now use the cached build tree. It is now *easier* to debug local build
      failures, and *possible* to debug remote build failures.
 +  o `bst shell --sysroot` now takes any directory that contains a sysroot,
 +    instead of just a specially-formatted build-root with a `root` and `scratch`
 +    subdirectory.
++
  =================
  buildstream 1.1.5

buildstream/element.py

@@ -1317,7 +1317,9 @@ class Element(Plugin):
      @contextmanager
      def _prepare_sandbox(self, scope, directory, deps='run', integrate=True):
          # bst shell and bst checkout require a local sandbox.
 -        with self.__sandbox(directory, config=self.__sandbox_config, allow_remote=False) as sandbox:
 +        bare_directory = True if directory else False
 +        with self.__sandbox(directory, config=self.__sandbox_config, allow_remote=False,
 +                            bare_directory=True if directory else False) as sandbox:
              # Configure always comes first, and we need it.
              self.configure_sandbox(sandbox)
@@ -2152,12 +2154,14 @@ class Element(Plugin):
      #    stderr (fileobject): The stream for stderr for the sandbox
      #    config (SandboxConfig): The SandboxConfig object
      #    allow_remote (bool): Whether the sandbox is allowed to be remote
 +    #    bare_directory (bool): Whether the directory is bare i.e. doesn't have
 +    #                           a separate 'root' subdir
+     #
      # Yields:
      #    (Sandbox): A usable sandbox
+     #
      @contextmanager
 -    def __sandbox(self, directory, stdout=None, stderr=None, config=None, allow_remote=True):
 +    def __sandbox(self, directory, stdout=None, stderr=None, config=None, allow_remote=True, bare_directory=False):
          context = self._get_context()
          project = self._get_project()
          platform = Platform.get_platform()
@@ -2188,6 +2192,7 @@ class Element(Plugin):
                                                stdout=stdout,
                                                stderr=stderr,
                                                config=config,
 +                                              bare_directory=bare_directory,
                                                allow_real_directory=not self.BST_VIRTUAL_DIRECTORY)
              yield sandbox
@@ -2197,7 +2202,7 @@ class Element(Plugin):
              # Recursive contextmanager...
              with self.__sandbox(rootdir, stdout=stdout, stderr=stderr, config=config,
 -                                allow_remote=allow_remote) as sandbox:
 +                                allow_remote=allow_remote, bare_directory=False) as sandbox:
                  yield sandbox
              # Cleanup the build dir

buildstream/sandbox/_mount.py

@@ -31,7 +31,6 @@ from .._fuse import SafeHardlinks
+ #
  class Mount():
      def __init__(self, sandbox, mount_point, safe_hardlinks, fuse_mount_options={}):
 -        scratch_directory = sandbox._get_scratch_directory()
          # Getting _get_underlying_directory() here is acceptable as
          # we're part of the sandbox code. This will fail if our
          # directory is CAS-based.
@@ -51,6 +50,7 @@ class Mount():
          #        a regular mount point within the parent's redirected mount.
+         #
          if self.safe_hardlinks:
 +            scratch_directory = sandbox._get_scratch_directory()
              # Redirected mount
              self.mount_origin = os.path.join(root_directory, mount_point.lstrip(os.sep))
              self.mount_base = os.path.join(scratch_directory, utils.url_directory_name(mount_point))

buildstream/sandbox/sandbox.py

@@ -98,16 +98,23 @@ class Sandbox():
          self.__config = kwargs['config']
          self.__stdout = kwargs['stdout']
          self.__stderr = kwargs['stderr']
 +        self.__bare_directory = kwargs['bare_directory']
          # Setup the directories. Root and output_directory should be
          # available to subclasses, hence being single-underscore. The
          # others are private to this class.
 -        self._root = os.path.join(directory, 'root')
 +        # If the directory is bare, it probably doesn't need scratch
 +        if self.__bare_directory:
 +            self._root = directory
 +            self.__scratch = None
 +            os.makedirs(self._root, exist_ok=True)
 +        else:
 +            self._root = os.path.join(directory, 'root')
 +            self.__scratch = os.path.join(directory, 'scratch')
 +            for directory_ in [self._root, self.__scratch]:
 +                os.makedirs(directory_, exist_ok=True)
++
          self._output_directory = None
 -        self.__directory = directory
 -        self.__scratch = os.path.join(self.__directory, 'scratch')
 -        for directory_ in [self._root, self.__scratch]:
 -            os.makedirs(directory_, exist_ok=True)
          self._vdir = None
          # This is set if anyone requests access to the underlying
@@ -334,6 +341,7 @@ class Sandbox():
      # Returns:
      #    (str): The sandbox scratch directory
      def _get_scratch_directory(self):
 +        assert not self.__bare_directory, "Scratch is not going to work with bare directories"
          return self.__scratch
      # _get_output()

setup.py

@@ -303,6 +303,7 @@ setup(name='BuildStream',
            'jinja2 >= 2.10',
            'protobuf >= 3.5',
            'grpcio >= 1.10',
 +          'pexpect',
        ],
        entry_points=bst_install_entry_points,
        setup_requires=['pytest-runner'],

tests/integration/build-tree.py

  import os
  import pytest
  import shutil
 +import pexpect
  from tests.testutils import cli, cli_integration, create_artifact_share
@@ -29,6 +30,31 @@ def test_buildtree_staged(cli_integration, tmpdir, datafiles):
      res.assert_success()
 +# Check that a failed build causes a shell that stages the buildtree
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_buildtree_staged_interactive(cli_integration, tmpdir, datafiles):
 +    # i.e. tests that cached build trees are staged by `bst shell --build`
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'build-shell/buildtree-fail.bst'
++
 +    # Start an interactive session and intercept the interactive prompt
 +    child = pexpect.spawn('bst', ['--no-colors', 'build', element_name], cwd=project)
++
 +    # Ensure starting a shell is a valid option
 +    child.expect('\(s\)hell')
++
 +    # Wait for the interactive prompt
 +    child.expect('Choice: \[continue\]:')
++
 +    # Start a shell and wait for the shell's prompt
 +    child.sendline('s')
 +    child.expect('\]\$')
++
 +    # Read the contents of the file we added
 +    child.sendline('cat test')
 +    child.expect('Hi')
++
++
  # Check that build shells work when pulled from a remote cache
  # This is to roughly simulate remote execution
  @pytest.mark.datafiles(DATA_DIR)

tests/integration/shell.py

@@ -302,6 +302,35 @@ def test_workspace_visible(cli, tmpdir, datafiles):
      assert result.output == workspace_hello
 +# Test that '--sysroot' works
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_sysroot(cli, tmpdir, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    base_element = "base/base-alpine.bst"
 +    # test element only needs to be something lightweight for this test
 +    test_element = "script/script.bst"
 +    checkout_dir = os.path.join(str(tmpdir), 'alpine-sysroot')
 +    test_file = 'hello'
++
 +    # Build and check out a sysroot
 +    res = cli.run(project=project, args=['build', base_element])
 +    res.assert_success()
 +    res = cli.run(project=project, args=['checkout', base_element, checkout_dir])
 +    res.assert_success()
++
 +    # Mutate the sysroot
 +    test_path = os.path.join(checkout_dir, test_file)
 +    with open(test_path, 'w') as f:
 +        f.write('hello\n')
++
 +    # Shell into the sysroot and check the test file exists
 +    res = cli.run(project=project, args=[
 +        'shell', '--build', '--sysroot', checkout_dir, test_element, '--',
 +        'grep', '-q', 'hello', '/' + test_file
 +    ])
 +    res.assert_success()
++
++
  # Test system integration commands can access devices in /dev
  @pytest.mark.datafiles(DATA_DIR)
  def test_integration_devices(cli, tmpdir, datafiles):

[Notes] [Git][BuildStream/buildstream][jonathan/debug-remote-failed-builds] 5 commits: tests: Add a test for the interactive failure shell

Jonathan Maw pushed to branch jonathan/debug-remote-failed-builds at BuildStream / buildstream

Commits:

7 changed files:

Changes: