diff options
-rw-r--r-- | NEWS | 9 | ||||
-rw-r--r-- | buildstream/_exceptions.py | 6 | ||||
-rw-r--r-- | buildstream/_frontend/app.py | 2 | ||||
-rw-r--r-- | buildstream/_frontend/widget.py | 11 | ||||
-rw-r--r-- | buildstream/_message.py | 2 | ||||
-rw-r--r-- | buildstream/element.py | 27 | ||||
-rw-r--r-- | buildstream/sandbox/_mount.py | 2 | ||||
-rw-r--r-- | buildstream/sandbox/sandbox.py | 18 | ||||
-rw-r--r-- | tests/integration/build-tree.py | 81 | ||||
-rw-r--r-- | tests/integration/project/elements/build-shell/buildtree-fail.bst | 13 | ||||
-rw-r--r-- | tests/integration/project/elements/build-shell/buildtree.bst | 11 | ||||
-rw-r--r-- | tests/integration/shell.py | 61 |
12 files changed, 174 insertions, 69 deletions
@@ -31,6 +31,15 @@ buildstream 1.3.1 new the `conf-root` variable to make the process easier. And there has been a bug fix to workspaces so they can be build in workspaces too. + o Creating a build shell through the interactive mode or `bst shell --build` + will now use the cached build tree. It is now easier to debug local 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 ================= diff --git a/buildstream/_exceptions.py b/buildstream/_exceptions.py index 19606776e..a1c26d38c 100644 --- a/buildstream/_exceptions.py +++ b/buildstream/_exceptions.py @@ -111,10 +111,8 @@ class BstError(Exception): # self.detail = detail - # The build sandbox in which the error occurred, if the - # error occurred at element assembly time. - # - self.sandbox = None + # A sandbox can be created to debug this error + self.sandbox = False # When this exception occurred during the handling of a job, indicate # whether or not there is any point retrying the job. diff --git a/buildstream/_frontend/app.py b/buildstream/_frontend/app.py index eeb5f3eb2..87db8076a 100644 --- a/buildstream/_frontend/app.py +++ b/buildstream/_frontend/app.py @@ -597,7 +597,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, directory=failure.sandbox, 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': diff --git a/buildstream/_frontend/widget.py b/buildstream/_frontend/widget.py index 478f0ff14..c0c45cec6 100644 --- a/buildstream/_frontend/widget.py +++ b/buildstream/_frontend/widget.py @@ -668,17 +668,6 @@ class LogLine(Widget): extra_nl = True - if message.sandbox is not None: - sandbox = self._indent + 'Sandbox directory: ' + message.sandbox - - text += '\n' - if message.message_type == MessageType.FAIL: - text += self._err_profile.fmt(sandbox, bold=True) - else: - text += self._detail_profile.fmt(sandbox) - text += '\n' - extra_nl = True - if message.scheduler and message.message_type == MessageType.FAIL: text += '\n' diff --git a/buildstream/_message.py b/buildstream/_message.py index 37630eb86..c2cdb8277 100644 --- a/buildstream/_message.py +++ b/buildstream/_message.py @@ -70,7 +70,7 @@ class Message(): self.elapsed = elapsed # The elapsed time, in timed messages self.depth = depth # The depth of a timed message self.logfile = logfile # The log file path where commands took place - self.sandbox = sandbox # The sandbox directory where an error occurred (if any) + self.sandbox = sandbox # The error that caused this message used a sandbox self.pid = os.getpid() # The process pid self.unique_id = unique_id # The plugin object ID issueing the message self.task_id = task_id # The plugin object ID of the task diff --git a/buildstream/element.py b/buildstream/element.py index 4d3e1bc75..27c5014a3 100644 --- a/buildstream/element.py +++ b/buildstream/element.py @@ -1318,7 +1318,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=bare_directory) as sandbox: # Configure always comes first, and we need it. self.configure_sandbox(sandbox) @@ -1385,6 +1387,7 @@ class Element(Plugin): # the same filing system as the rest of our cache. temp_staging_location = os.path.join(self._get_context().artifactdir, "staging_temp") temp_staging_directory = tempfile.mkdtemp(prefix=temp_staging_location) + import_dir = temp_staging_directory try: workspace = self._get_workspace() @@ -1395,12 +1398,16 @@ class Element(Plugin): with self.timed_activity("Staging local files at {}" .format(workspace.get_absolute_path())): workspace.stage(temp_staging_directory) + elif self._cached(): + # We have a cached buildtree to use, instead + artifact_base, _ = self.__extract() + import_dir = os.path.join(artifact_base, 'buildtree') else: # No workspace, stage directly for source in self.sources(): source._stage(temp_staging_directory) - vdirectory.import_files(temp_staging_directory) + vdirectory.import_files(import_dir) finally: # Staging may produce directories with less than 'rwx' permissions @@ -1566,9 +1573,8 @@ class Element(Plugin): collect = self.assemble(sandbox) # pylint: disable=assignment-from-no-return self.__set_build_result(success=True, description="succeeded") except BstError as e: - # If an error occurred assembling an element in a sandbox, - # then tack on the sandbox directory to the error - e.sandbox = rootdir + # Shelling into a sandbox is useful to debug this error + e.sandbox = True # If there is a workspace open on this element, it will have # been mounted for sandbox invocations instead of being staged. @@ -1683,8 +1689,8 @@ class Element(Plugin): "unable to collect artifact contents" .format(collect)) - # Finally cleanup the build dir - cleanup_rootdir() + # Finally cleanup the build dir + cleanup_rootdir() return artifact_size @@ -2152,12 +2158,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 +2196,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 +2206,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 diff --git a/buildstream/sandbox/_mount.py b/buildstream/sandbox/_mount.py index aff9e8a2d..c0f26c8d7 100644 --- a/buildstream/sandbox/_mount.py +++ b/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=None): - 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)) diff --git a/buildstream/sandbox/sandbox.py b/buildstream/sandbox/sandbox.py index 83714efdd..eabe57d75 100644 --- a/buildstream/sandbox/sandbox.py +++ b/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() diff --git a/tests/integration/build-tree.py b/tests/integration/build-tree.py new file mode 100644 index 000000000..df9006a27 --- /dev/null +++ b/tests/integration/build-tree.py @@ -0,0 +1,81 @@ +import os +import pytest +import shutil + +from tests.testutils import cli, cli_integration, create_artifact_share +from buildstream._exceptions import ErrorDomain + + +pytestmark = pytest.mark.integration + + +DATA_DIR = os.path.join( + os.path.dirname(os.path.realpath(__file__)), + "project" +) + + +@pytest.mark.datafiles(DATA_DIR) +def test_buildtree_staged(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.bst' + + res = cli_integration.run(project=project, args=['build', element_name]) + res.assert_success() + + res = cli_integration.run(project=project, args=[ + 'shell', '--build', element_name, '--', 'grep', '-q', 'Hi', 'test' + ]) + res.assert_success() + + +@pytest.mark.datafiles(DATA_DIR) +def test_buildtree_from_failure(cli_integration, tmpdir, datafiles): + # i.e. test that on a build failure, we can still shell into it + project = os.path.join(datafiles.dirname, datafiles.basename) + element_name = 'build-shell/buildtree-fail.bst' + + res = cli_integration.run(project=project, args=['build', element_name]) + res.assert_main_error(ErrorDomain.STREAM, None) + + # Assert that file has expected contents + res = cli_integration.run(project=project, args=[ + 'shell', '--build', element_name, '--', 'cat', 'test' + ]) + res.assert_success() + assert 'Hi' in res.output + + +# Check that build shells work when pulled from a remote cache +# This is to roughly simulate remote execution +@pytest.mark.datafiles(DATA_DIR) +def test_buildtree_pulled(cli, tmpdir, datafiles): + project = os.path.join(datafiles.dirname, datafiles.basename) + element_name = 'build-shell/buildtree.bst' + + with create_artifact_share(os.path.join(str(tmpdir), 'artifactshare')) as share: + # Build the element to push it to cache + cli.configure({ + 'artifacts': {'url': share.repo, 'push': True} + }) + result = cli.run(project=project, args=['build', element_name]) + result.assert_success() + assert cli.get_element_state(project, element_name) == 'cached' + + # Discard the cache + cli.configure({ + 'artifacts': {'url': share.repo, 'push': True}, + 'artifactdir': os.path.join(cli.directory, 'artifacts2') + }) + assert cli.get_element_state(project, element_name) != 'cached' + + # Pull from cache + result = cli.run(project=project, args=['pull', '--deps', 'all', element_name]) + result.assert_success() + + # Check it's using the cached build tree + res = cli.run(project=project, args=[ + 'shell', '--build', element_name, '--', 'grep', '-q', 'Hi', 'test' + ]) + res.assert_success() diff --git a/tests/integration/project/elements/build-shell/buildtree-fail.bst b/tests/integration/project/elements/build-shell/buildtree-fail.bst new file mode 100644 index 000000000..a3b515a9a --- /dev/null +++ b/tests/integration/project/elements/build-shell/buildtree-fail.bst @@ -0,0 +1,13 @@ +kind: manual +description: | + Puts a file in the build tree so that build tree caching and staging can be tested, + then deliberately failing to build so we can check the output. + +depends: + - filename: base.bst + type: build + +config: + build-commands: + - "echo 'Hi' > %{build-root}/test" + - "false" diff --git a/tests/integration/project/elements/build-shell/buildtree.bst b/tests/integration/project/elements/build-shell/buildtree.bst new file mode 100644 index 000000000..d5cf256be --- /dev/null +++ b/tests/integration/project/elements/build-shell/buildtree.bst @@ -0,0 +1,11 @@ +kind: manual +description: | + Puts a file in the build tree so that build tree caching and staging can be tested. + +depends: + - filename: base.bst + type: build + +config: + build-commands: + - "echo 'Hi' > %{build-root}/test" diff --git a/tests/integration/shell.py b/tests/integration/shell.py index 947650ff1..1db29a0c4 100644 --- a/tests/integration/shell.py +++ b/tests/integration/shell.py @@ -302,46 +302,33 @@ def test_workspace_visible(cli, tmpdir, datafiles): assert result.output == workspace_hello -# Test that we can see the workspace files in a shell -@pytest.mark.integration +# Test that '--sysroot' works @pytest.mark.datafiles(DATA_DIR) -def test_sysroot_workspace_visible(cli, tmpdir, datafiles): +def test_sysroot(cli, tmpdir, datafiles): project = os.path.join(datafiles.dirname, datafiles.basename) - workspace = os.path.join(cli.directory, 'workspace') - element_name = 'workspace/workspace-mount-fail.bst' - - # Open a workspace on our build failing element - # - res = cli.run(project=project, args=['workspace', 'open', element_name, workspace]) - assert res.exit_code == 0 - - # Ensure the dependencies of our build failing element are built - result = cli.run(project=project, args=['build', element_name]) - result.assert_main_error(ErrorDomain.STREAM, None) - - # Discover the sysroot of the failed build directory, after one - # failed build, there should be only one directory there. - # - build_base = os.path.join(cli.directory, 'build') - build_dirs = os.listdir(path=build_base) - assert len(build_dirs) == 1 - build_dir = os.path.join(build_base, build_dirs[0]) - - # Obtain a copy of the hello.c content from the workspace - # - workspace_hello_path = os.path.join(cli.directory, 'workspace', 'hello.c') - assert os.path.exists(workspace_hello_path) - with open(workspace_hello_path, 'r') as f: - workspace_hello = f.read() - - # Cat the hello.c file from a bst shell command, and assert - # that we got the same content here - # - result = cli.run(project=project, args=[ - 'shell', '--build', '--sysroot', build_dir, element_name, '--', 'cat', 'hello.c' + 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 ]) - assert result.exit_code == 0 - assert result.output == workspace_hello + res.assert_success() # Test system integration commands can access devices in /dev |