[Notes] [Git][BuildStream/buildstream][juerg/remote] 8 commits: element.

Jonathan Maw pushed to branch juerg/remote at BuildStream / buildstream

Commits:

c5778941

by Josh Smith at 2018-10-04T13:43:10Z

element.py: Do not save workspace on failed build

This fixes #634, BuildStream no longer saves workspace configuration on
a failed build.

788cde6a

by Josh Smith at 2018-10-04T13:43:10Z

testing: Add regression test for workspace bug

8630bac4

by Tristan Van Berkom at 2018-10-04T14:11:27Z

Merge branch 'Qinusty/634-workspace-failed-builds' into 'master'

Do not save workspace on failed build

Closes #634

See merge request BuildStream/buildstream!812

a7984218

by Valentin David at 2018-10-04T14:56:09Z

Catch correct exception from shutil.rmtree

Python documentation is not clear on what shutil.rmtree is supposed to
raise. However from the source code, it is obvious that OSError are
raised, but never shutil.Error.

It is not easy to test in normal condition because issues happen
usually in combination with a FUSE filesystem, a probably a race
condition where `fstatat` returns an error just before the filesystem
being unmounted.

Fixes #153.

b7b20d9e

by Valentin David at 2018-10-04T15:31:22Z

Merge branch 'valentindavid/rmtree_oserror' into 'master'

Catch correct exception from shutil.rmtree

Closes #153

See merge request BuildStream/buildstream!849

42ea143d

by Chandan Singh at 2018-10-05T12:40:48Z

Add contrib script to generate Docker images from bst checkout

This script can be useful to generate a Docker image from `bst checkout`
of an element as a single command. While this script does not eliminate
the need for a proper Docker/OCI element plugin that would probably also
support layering, it provides a cheap way to export the element into a
Docker image.

b38f9f9d

by Jonathan Maw at 2018-10-05T13:05:38Z

Merge branch 'chandan/bst-docker-import' into 'master'

Add contrib script to generate Docker images from bst checkout

See merge request BuildStream/buildstream!857

47df1e36

by Jürg Billeter at 2018-10-05T13:30:38Z

element.py: Prepare local sandbox for bst checkout and bst shell

A remote execution sandbox is not suitable for these commands.

Changes:

buildstream/element.py

@@ -212,7 +212,7 @@ class Element(Plugin):
          self.__staged_sources_directory = None  # Location where Element.stage_sources() was called
          self.__tainted = None                   # Whether the artifact is tainted and should not be shared
          self.__required = False                 # Whether the artifact is required in the current session
 -        self.__build_result = None              # The result of assembling this Element
 +        self.__build_result = None              # The result of assembling this Element (success, description, detail)
          self._build_log_path = None            # The path of the build log for this Element
          # hash tables of loaded artifact metadata, hashed by key
@@ -1316,7 +1316,8 @@ class Element(Plugin):
+     #
      @contextmanager
      def _prepare_sandbox(self, scope, directory, deps='run', integrate=True):
 -        with self.__sandbox(directory, config=self.__sandbox_config) as sandbox:
 +        # bst shell and bst checkout require a local sandbox.
 +        with self.__sandbox(directory, config=self.__sandbox_config, allow_remote=False) as sandbox:
              # Configure always comes first, and we need it.
              self.configure_sandbox(sandbox)
@@ -1479,11 +1480,13 @@ class Element(Plugin):
          self._update_state()
 -        if self._get_workspace() and self._cached():
 +        if self._get_workspace() and self._cached_success():
 +            assert utils._is_main_process(), \
 +                "Attempted to save workspace configuration from child process"
+             #
              # Note that this block can only happen in the
 -            # main process, since `self._cached()` cannot
 -            # be true when assembly is completed in the task.
 +            # main process, since `self._cached_success()` cannot
 +            # be true when assembly is successful in the task.
+             #
              # For this reason, it is safe to update and
              # save the workspaces configuration
@@ -2147,17 +2150,18 @@ class Element(Plugin):
      #    stdout (fileobject): The stream for stdout for the sandbox
      #    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
+     #
      # Yields:
      #    (Sandbox): A usable sandbox
+     #
      @contextmanager
 -    def __sandbox(self, directory, stdout=None, stderr=None, config=None):
 +    def __sandbox(self, directory, stdout=None, stderr=None, config=None, allow_remote=True):
          context = self._get_context()
          project = self._get_project()
          platform = Platform.get_platform()
 -        if directory is not None and self.__use_remote_execution():
 +        if directory is not None and allow_remote and self.__use_remote_execution():
              self.info("Using a remote sandbox for artifact {} with directory '{}'".format(self.name, directory))
@@ -2171,7 +2175,7 @@ class Element(Plugin):
              yield sandbox
          elif directory is not None and os.path.exists(directory):
 -            if self.__remote_execution_url:
 +            if allow_remote and self.__remote_execution_url:
                  self.warn("Artifact {} is configured to use remote execution but element plugin does not support it."
                            .format(self.name), detail="Element plugin '{kind}' does not support virtual directories."
                            .format(kind=self.get_kind()), warning_token="remote-failure")
@@ -2191,7 +2195,8 @@ class Element(Plugin):
              rootdir = tempfile.mkdtemp(prefix="{}-".format(self.normal_name), dir=context.builddir)
              # Recursive contextmanager...
 -            with self.__sandbox(rootdir, stdout=stdout, stderr=stderr, config=config) as sandbox:
 +            with self.__sandbox(rootdir, stdout=stdout, stderr=stderr, config=config,
 +                                allow_remote=allow_remote) as sandbox:
                  yield sandbox
              # Cleanup the build dir

buildstream/utils.py

@@ -686,7 +686,7 @@ def _force_rmtree(rootpath, **kwargs):
      try:
          shutil.rmtree(rootpath, **kwargs)
 -    except shutil.Error as e:
 +    except OSError as e:
          raise UtilError("Failed to remove cache directory '{}': {}"
                          .format(rootpath, e))

contrib/bst-docker-import

 +#!/bin/bash
 +#
 +#  Copyright 2018 Bloomberg Finance LP
 +#
 +#  This program is free software; you can redistribute it and/or
 +#  modify it under the terms of the GNU Lesser General Public
 +#  License as published by the Free Software Foundation; either
 +#  version 2 of the License, or (at your option) any later version.
 +#
 +#  This library is distributed in the hope that it will be useful,
 +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
 +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 +#  Lesser General Public License for more details.
 +#
 +#  You should have received a copy of the GNU Lesser General Public
 +#  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 +#
 +#  Authors:
 +#        Chadnan Singh <csingh43 bloomberg net>
++
 +# This is a helper script to generate Docker images using checkouts of
 +# BuildStream elements.
++
 +usage() {
 +    cat <<EOF
++
 +USAGE: $(basename "$0") [-c BST_CMD] [-m MESSAGE] [-t TAG] [-h] ELEMENT
++
 +Create a Docker image from bst checkout of an element.
++
 +OPTIONS:
 +    -c BST_CMD    Path to BuildStream command (default: bst).
 +    -m MESSAGE    Commit message for the imported image.
 +    -t TAG        Tag of the imported image.
 +    -h            Print this help text and exit.
++
 +EXAMPLES:
++
 +    # Import hello.bst as a Docker image with tag "bst-hello" and message "hello"
 +    $(basename "$0") -m hello -t bst-hello hello.bst
++
 +    # Import hello.bst as a Docker image with tag "bst-hello" using bst-here
 +    $(basename "$0") -c bst-here -t bst-hello hello.bst
++
 +EOF
 +    exit "$1"
 +}
++
 +die() {
 +    echo "FATAL: $1" >&2
 +    exit 1
 +}
++
 +bst_cmd=bst
 +docker_import_cmd=(docker import)
 +docker_image_tag=
++
 +while getopts c:m:t:h arg
 +do
 +    case $arg in
 +    c)
 +        bst_cmd="$OPTARG"
 +        ;;
 +    m)
 +        docker_import_cmd+=('-m' "$OPTARG")
 +        ;;
 +    t)
 +        docker_image_tag="$OPTARG"
 +        ;;
 +    h)
 +        usage 0
 +        ;;
 +    \?)
 +        usage 1
 +    esac
 +done
++
 +shift $((OPTIND-1))
 +if [[ "$#" != 1 ]]; then
 +    echo "$0: No element specified" >&2
 +    usage 1
 +fi
 +element="$1"
++
 +# Dump to a temporary file in the current directory.
 +# NOTE: We use current directory to try to ensure compatibility with scripts
 +# like bst-here, assuming that the current working directory is mounted
 +# inside the container.
++
 +checkout_tar="bst-checkout-$(basename "$element")-$RANDOM.tar"
++
 +echo "INFO: Checking out $element ..." >&2
 +$bst_cmd checkout --tar "$element" "$checkout_tar" || die "Failed to checkout $element"
 +echo "INFO: Successfully checked out $element" >&2
++
 +echo "INFO: Importing Docker image ..."
 +"${docker_import_cmd[@]}" "$checkout_tar" "$docker_image_tag" || die "Failed to import Docker image from tarball"
 +echo "INFO: Successfully import Docker image $docker_image_tag"
++
 +echo "INFO: Cleaning up ..."
 +rm "$checkout_tar" || die "Failed to remove $checkout_tar"
 +echo "INFO: Clean up finished"

tests/frontend/workspace.py

@@ -43,7 +43,8 @@ DATA_DIR = os.path.join(
+ )
 -def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None, project_path=None):
 +def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir=None,
 +                   project_path=None, element_attrs=None):
      if not workspace_dir:
          workspace_dir = os.path.join(str(tmpdir), 'workspace{}'.format(suffix))
      if not project_path:
@@ -69,6 +70,8 @@ def open_workspace(cli, tmpdir, datafiles, kind, track, suffix='', workspace_dir
              repo.source_config(ref=ref)
+         ]
+     }
 +    if element_attrs:
 +        element = {**element, **element_attrs}
      _yaml.dump(element,
                 os.path.join(element_path,
                              element_name))
@@ -854,3 +857,22 @@ def test_cache_key_workspace_in_dependencies(cli, tmpdir, datafiles, strict):
      # Check that the original /usr/bin/hello is not in the checkout
      assert not os.path.exists(os.path.join(checkout, 'usr', 'bin', 'hello'))
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_multiple_failed_builds(cli, tmpdir, datafiles):
 +    element_config = {
 +        "kind": "manual",
 +        "config": {
 +            "configure-commands": [
 +                "unknown_command_that_will_fail"
 +            ]
 +        }
 +    }
 +    element_name, project, _ = open_workspace(cli, tmpdir, datafiles,
 +                                              "git", False, element_attrs=element_config)
++
 +    for _ in range(2):
 +        result = cli.run(project=project, args=["build", element_name])
 +        assert "BUG" not in result.stderr
 +        assert cli.get_element_state(project, element_name) != "cached"

[Notes] [Git][BuildStream/buildstream][juerg/remote] 8 commits: element.py: Do not save workspace on failed build

Jonathan Maw pushed to branch juerg/remote at BuildStream / buildstream

Commits:

4 changed files:

Changes: