[Notes] [Git][BuildStream/buildstream][tpollard/566] 19 commits: BREAK:r

Tom Pollard pushed to branch tpollard/566 at BuildStream / buildstream

Commits:

86c8e414

by Angelos Evripiotis at 2019-01-29T10:39:29Z

BREAK:remove unconditional 'are you sure?' prompts

This is a breaking change, as it affects behaviour that people might be
relying on. An entry has been added to NEWS.

As proposed on the mailing list, this change removes the unconditional
prompts on:

    o: bst workspace reset
    o: bst workspace close --remove-dir

If interactive, these commands would always interrupt you with a prompt
like this:

    This will remove all your changes, are you sure?

This seems like it may just save someone's work some time. It may also
condition folks to hit 'y' quickly without thinking.

This change also makes the non-interactive behaviour consistent with the
interactive behaviour in the default case. There is also the case of the
prompt configured by 'really-workspace-close-project-inaccessible',
which may be tackled in later work.

This change also removes the new config options to suppress those
prompts, and their associated news entry.

The relevant bit of the mailing list conversation is here:
https://mail.gnome.org/archives/buildstream-list/2018-December/msg00106.html

The issue to make interactive and non-interactive behaviour consistent
is here:
https://gitlab.com/BuildStream/buildstream/issues/744

aae35e13

by Angelos Evripiotis at 2019-01-29T12:08:07Z

Merge branch 'aevri/are_you_sure' into 'master'

BREAK:remove unconditional 'are you sure?' prompts

See merge request BuildStream/buildstream!1061

03111d39

by Dor Askayo at 2019-01-30T10:35:06Z

filter.py: don't recurse when staging dependencies

Also bump the element's version so cached artifacts would be
invalidated.

Fixes #883

7256bb0c

by James Ennis at 2019-01-30T11:34:04Z

Merge branch 'doraskayo/filter-indirect-deps-fix' into 'master'

filter.py: don't recurse when staging dependencies

Closes #883

See merge request BuildStream/buildstream!1110

36746730

by Chandan Singh at 2019-01-31T10:50:05Z

tox.ini: Specify minimum version of click-man

`click-man` versions < 0.3.0 do not properly support multiple
entrypoints. Since this was added to `tox` after `0.3.0` was released,
`tox` should never be pulling older versions. But, let's add it here for
documentation purposes.

See
https://gitlab.com/BuildStream/buildstream/merge_requests/1107#note_135187046
for some background on this.

fa4a21ce

by Chandan Singh at 2019-01-31T12:15:43Z

Merge branch 'chandan/min-version-click-man' into 'master'

tox.ini: Specify minimum version of click-man

See merge request BuildStream/buildstream!1120

dd791373

by Chandan Singh at 2019-01-31T14:32:44Z

testutils/site.py: Support parsing more exotic git versions

We use output of `git --version` to determine if we can run some tests
that rely on features from newer git versions. Usually, we expect the
output to be like:

    git version 2.17.2

On some platforms, like MacOS, there could be a suffix after the version
string, so that it looks something like:

    git version 2.17.2 (Apple Git-113)

This causes things to fail like so:

    ValueError: invalid literal for int() with base 10: '2 (Apple Git-113)\n'

Fix logic around `HAVE_OLD_GIT` such that we split the output of
`git --version` without limit on how many times we split. Previously we
used to split only twice so the suffixes like `(Apple Git-113)` are not
part of the parsed version.

96c0fbd6

by Chandan Singh at 2019-01-31T15:39:19Z

Merge branch 'chandan/fix-git-version-mac' into 'master'

testutils/site.py: Support parsing more exotic git versions

See merge request BuildStream/buildstream!1118

d25e2795

by Benjamin Schubert at 2019-01-31T17:06:23Z

Add LoadElement in dependency list for LoadElement idrectly

This removes the need for the 'Dependency' list to then be matched
with the corresponding LoadElement and will allow iterating the
graph more easily

2d0eebbf

by Benjamin Schubert at 2019-01-31T17:06:23Z

Pass element directly to collect_element

This simplifies the loading

583bd97d

by Benjamin Schubert at 2019-02-01T10:26:37Z

Merge branch 'bschubert/loader' into 'master'

Cleanup loader by linking LoadElements sooner

See merge request BuildStream/buildstream!1122

51cec3da

by Phil Dawson at 2019-02-01T14:25:44Z

tests/cachekey: Test cache keys are independent of target elements

2b38aabe

by Phil Dawson at 2019-02-01T15:33:00Z

Merge branch 'phil/cache-key-stability-test' into 'master'

tests/cachekey: Test cache keys are independent of target elements

See merge request BuildStream/buildstream!1123

dbb3d232

by James Ennis at 2019-02-01T15:51:32Z

filter.py/filter.yaml: Documentation improvements

7e4205cb

by James Ennis at 2019-02-01T15:51:32Z

filter.py: Add an example to the documentation

4109a34a

by James Ennis at 2019-02-01T17:11:29Z

Merge branch 'jennis/filter-docs' into 'master'

Improve our filter documentation

Closes #278

See merge request BuildStream/buildstream!1112

c9345014

by James Ennis at 2019-02-04T13:53:42Z

filter.py: Fail if declared domains do not exist in the parent element

This patch also uncovered the fact that our test_filter_deps_ok() test
has been inaccurate. Thus the element built in this test
(deps-permitted.bst) has been modified so that it build depends on the
input.bst element, as it should.

tests/filter.py: Ensure deps_ok test passes

3ab09651

by James Ennis at 2019-02-04T14:47:43Z

Merge branch 'jennis/warn_for_nonexistent_domains' into 'master'

Fail when we explictly try to include/exclude non-existent domains in a filter element

See merge request BuildStream/buildstream!1117

1522b9c6

by Tom Pollard at 2019-02-05T12:51:25Z

WIP: Make uploading of build trees configurable

29 changed files:

NEWS
buildstream/_artifactcache.py
buildstream/_cas/cascache.py
buildstream/_cas/casremote.py
buildstream/_context.py
buildstream/_frontend/cli.py
buildstream/_loader/loadelement.py
buildstream/_loader/loader.py
buildstream/data/userconfig.yaml
buildstream/element.py
buildstream/plugins/elements/filter.py
buildstream/plugins/elements/filter.yaml
doc/source/using_config.rst
tests/artifactcache/config.py
tests/cachekey/cachekey.py
+ tests/cachekey/project/elements/key-stability/aaa.bst
+ tests/cachekey/project/elements/key-stability/t1.bst
+ tests/cachekey/project/elements/key-stability/t2.bst
+ tests/cachekey/project/elements/key-stability/top-level.bst
+ tests/cachekey/project/elements/key-stability/zzz.bst
tests/elements/filter.py
tests/elements/filter/basic/elements/deps-permitted.bst
+ tests/elements/filter/basic/elements/input-with-deps.bst
+ tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst
+ tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst
+ tests/integration/pushbuildtrees.py
tests/testutils/runcli.py
tests/testutils/site.py
tox.ini

Changes:

NEWS

@@ -50,6 +50,11 @@ buildstream 1.3.1
      an error message and a hint instead, to avoid bothering folks that just
      made a mistake.
 +  o BREAKING CHANGE: The unconditional 'Are you sure?' prompts have been
 +    removed. These would always ask you if you were sure when running
 +    'bst workspace close --remove-dir' or 'bst workspace reset'. They got in
 +    the way too often.
++
    o Failed builds are included in the cache as well.
      `bst checkout` will provide anything in `%{install-root}`.
      A build including cached fails will cause any dependant elements
@@ -87,12 +92,6 @@ buildstream 1.3.1
      instead of just a specially-formatted build-root with a `root` and `scratch`
      subdirectory.
 -  o The buildstream.conf file learned new
 -    'prompt.really-workspace-close-remove-dir' and
 -    'prompt.really-workspace-reset-hard' options. These allow users to suppress
 -    certain confirmation prompts, e.g. double-checking that the user meant to
 -    run the command as typed.
+-
    o Due to the element `build tree` being cached in the respective artifact their
      size in some cases has significantly increased. In *most* cases the build trees
      are not utilised when building targets, as such by default bst 'pull' & 'build'

buildstream/_artifactcache.py

@@ -107,6 +107,7 @@ class ArtifactCache():
          self._has_fetch_remotes = False
          self._has_push_remotes = False
 +        self._has_partial_push_remotes = False
          os.makedirs(self.extractdir, exist_ok=True)
@@ -488,6 +489,9 @@ class ArtifactCache():
                  self._has_fetch_remotes = True
                  if remote_spec.push:
                      self._has_push_remotes = True
 +                    # Partial push requires generic push option to also be set
 +                    if remote_spec.partial_push:
 +                        self._has_partial_push_remotes = True
                  remotes[remote_spec.url] = CASRemote(remote_spec)
@@ -685,6 +689,32 @@ class ArtifactCache():
              remotes_for_project = self._remotes[element._get_project()]
              return any(remote.spec.push for remote in remotes_for_project)
 +    # has_partial_push_remotes():
 +    #
 +    # Check whether any remote repositories are available for pushing
 +    # non-complete artifacts. This option requires the generic push value
 +    # to also be set.
 +    #
 +    # Args:
 +    #     element (Element): The Element to check
 +    #
 +    # Returns:
 +    #   (bool): True if any remote repository is configured for optional
 +    #            partial pushes, False otherwise
 +    #
 +    def has_partial_push_remotes(self, *, element=None):
 +        # If there's no partial push remotes available, we can't partial push at all
 +        if not self._has_partial_push_remotes:
 +            return False
 +        elif element is None:
 +            # At least one remote is set to allow partial pushes
 +            return True
 +        else:
 +            # Check whether the specified element's project has push remotes configured
 +            # to not accept partial artifact pushes
 +            remotes_for_project = self._remotes[element._get_project()]
 +            return any(remote.spec.partial_push for remote in remotes_for_project)
++
      # push():
+     #
      # Push committed artifact to remote repository.
@@ -692,6 +722,8 @@ class ArtifactCache():
      # Args:
      #     element (Element): The Element whose artifact is to be pushed
      #     keys (list): The cache keys to use
 +    #     partial(bool): If the artifact is cached in a partial state
 +    #     subdir(string): Optional subdir to not push
+     #
      # Returns:
      #   (bool): True if any remote was updated, False if no pushes were required
@@ -699,12 +731,25 @@ class ArtifactCache():
      # Raises:
      #   (ArtifactError): if there was an error
+     #
 -    def push(self, element, keys):
 +    def push(self, element, keys, partial=False, subdir=None):
          refs = [self.get_artifact_fullname(element, key) for key in list(keys)]
          project = element._get_project()
 -        push_remotes = [r for r in self._remotes[project] if r.spec.push]
 +        push_remotes = []
 +        partial_remotes = []
++
 +        # Create list of remotes to push to, given current element and partial push config
 +        if not partial:
 +            push_remotes = [r for r in self._remotes[project] if (r.spec.push and not r.spec.partial_push)]
++
 +        if self._has_partial_push_remotes:
 +            # Create a specific list of the remotes expecting the artifact to be push in a partial
 +            # state. This list needs to be pushed in a partial state, without the optional subdir if
 +            # exists locally. No need to attempt pushing a partial artifact to a remote that is queued to
 +            # to also recieve a full artifact
 +            partial_remotes = [r for r in self._remotes[project] if (r.spec.partial_push and r.spec.push) and
 +                               r not in push_remotes]
          pushed = False
@@ -713,7 +758,9 @@ class ArtifactCache():
              display_key = element._get_brief_display_key()
              element.status("Pushing artifact {} -> {}".format(display_key, remote.spec.url))
 -            if self.cas.push(refs, remote):
 +            # Passing the optional subdir allows for remote artifacts that are cached in a 'partial'
 +            # state to be completed
 +            if self.cas.push(refs, remote, subdir=subdir):
                  element.info("Pushed artifact {} -> {}".format(display_key, remote.spec.url))
                  pushed = True
              else:
@@ -721,6 +768,19 @@ class ArtifactCache():
                      remote.spec.url, element._get_brief_display_key()
                  ))
 +        for remote in partial_remotes:
 +            remote.init()
 +            display_key = element._get_brief_display_key()
 +            element.status("Pushing partial artifact {} -> {}".format(display_key, remote.spec.url))
++
 +            if self.cas.push(refs, remote, excluded_subdirs=subdir):
 +                element.info("Pushed partial artifact {} -> {}".format(display_key, remote.spec.url))
 +                pushed = True
 +            else:
 +                element.info("Remote ({}) already has {} partial cached".format(
 +                    remote.spec.url, element._get_brief_display_key()
 +                ))
++
          return pushed
      # pull():
@@ -748,14 +808,23 @@ class ArtifactCache():
                  element.status("Pulling artifact {} <- {}".format(display_key, remote.spec.url))
                  if self.cas.pull(ref, remote, progress=progress, subdir=subdir, excluded_subdirs=excluded_subdirs):
 -                    element.info("Pulled artifact {} <- {}".format(display_key, remote.spec.url))
                      if subdir:
 -                        # Attempt to extract subdir into artifact extract dir if it already exists
 -                        # without containing the subdir. If the respective artifact extract dir does not
 -                        # exist a complete extraction will complete.
 -                        self.extract(element, key, subdir)
 -                    # no need to pull from additional remotes
 -                    return True
 +                        if not self.contains_subdir_artifact(element, key, subdir):
 +                            # The pull was expecting the specific subdir to be present in the remote, attempt
 +                            # to find it in other available remotes
 +                            element.info("Pulled partial artifact {} <- {}. Attempting to retrieve {} from remotes"
 +                                         .format(display_key, remote.spec.url, subdir))
 +                        else:
 +                            element.info("Pulled artifact {} <- {}".format(display_key, remote.spec.url))
 +                            # Attempt to extract subdir into artifact extract dir if it already exists
 +                            # without containing the subdir. If the respective artifact extract dir does not
 +                            # exist a complete extraction will complete.
 +                            self.extract(element, key, subdir)
 +                            # no need to pull from additional remotes
 +                            return True
 +                    else:
 +                        element.info("Pulled artifact {} <- {}".format(display_key, remote.spec.url))
 +                        return True
                  else:
                      element.info("Remote ({}) does not have {} cached".format(
                          remote.spec.url, element._get_brief_display_key()

buildstream/_cas/cascache.py

@@ -198,34 +198,47 @@ class CASCache():
      #   (bool): True if pull was successful, False if ref was not available
+     #
      def pull(self, ref, remote, *, progress=None, subdir=None, excluded_subdirs=None):
 -        try:
 -            remote.init()
 -            request = buildstream_pb2.GetReferenceRequest(instance_name=remote.spec.instance_name)
 -            request.key = ref
 -            response = remote.ref_storage.GetReference(request)
 +        tree_found = False
 -            tree = remote_execution_pb2.Digest()
 -            tree.hash = response.digest.hash
 -            tree.size_bytes = response.digest.size_bytes
 +        while True:
 +            try:
 +                if not tree_found:
 +                    remote.init()
 -            # Check if the element artifact is present, if so just fetch the subdir.
 -            if subdir and os.path.exists(self.objpath(tree)):
 -                self._fetch_subdir(remote, tree, subdir)
 -            else:
 -                # Fetch artifact, excluded_subdirs determined in pullqueue
 -                self._fetch_directory(remote, tree, excluded_subdirs=excluded_subdirs)
 +                    request = buildstream_pb2.GetReferenceRequest(instance_name=remote.spec.instance_name)
 +                    request.key = ref
 +                    response = remote.ref_storage.GetReference(request)
 -            self.set_ref(ref, tree)
 +                    tree = remote_execution_pb2.Digest()
 +                    tree.hash = response.digest.hash
 +                    tree.size_bytes = response.digest.size_bytes
 -            return True
 -        except grpc.RpcError as e:
 -            if e.code() != grpc.StatusCode.NOT_FOUND:
 -                raise CASCacheError("Failed to pull ref {}: {}".format(ref, e)) from e
 -            else:
 -                return False
 -        except BlobNotFound as e:
 -            return False
 +                # Check if the element artifact is present, if so just fetch the subdir.
 +                if subdir and os.path.exists(self.objpath(tree)):
 +                    self._fetch_subdir(remote, tree, subdir)
 +                else:
 +                    # Fetch artifact, excluded_subdirs determined in pullqueue
 +                    self._fetch_directory(remote, tree, excluded_subdirs=excluded_subdirs)
++
 +                self.set_ref(ref, tree)
++
 +                return True
 +            except grpc.RpcError as e:
 +                if e.code() != grpc.StatusCode.NOT_FOUND:
 +                    raise CASCacheError("Failed to pull ref {}: {}".format(ref, e)) from e
 +                else:
 +                    return False
 +            except BlobNotFound as e:
 +                if not excluded_subdirs and subdir:
 +                    # The remote has the top level digest but could not complete a full pull,
 +                    # attempt partial without the need to initialise and check for the artifact
 +                    # digest. This default behaviour of dropping back to partial pulls could
 +                    # be made a configurable warning given at artfictcache level.
 +                    tree_found = True
 +                    excluded_subdirs, subdir = subdir, excluded_subdirs
 +                else:
 +                    return False
      # pull_tree():
+     #
@@ -270,6 +283,8 @@ class CASCache():
      # Args:
      #     refs (list): The refs to push
      #     remote (CASRemote): The remote to push to
 +    #     subdir (string): Optional specific subdir to include in the push
 +    #     excluded_subdirs (list): The optional list of subdirs to not push
+     #
      # Returns:
      #   (bool): True if any remote was updated, False if no pushes were required
@@ -277,7 +292,7 @@ class CASCache():
      # Raises:
      #   (CASCacheError): if there was an error
+     #
 -    def push(self, refs, remote):
 +    def push(self, refs, remote, *, subdir=None, excluded_subdirs=None):
          skipped_remote = True
          try:
              for ref in refs:
@@ -291,15 +306,18 @@ class CASCache():
                      response = remote.ref_storage.GetReference(request)
                      if response.digest.hash == tree.hash and response.digest.size_bytes == tree.size_bytes:
 -                        # ref is already on the server with the same tree
 -                        continue
 +                        # ref is already on the server with the same tree, however it might be partially cached.
 +                        # If artifact is not set to be pushed partially attempt to 'complete' the remote artifact if
 +                        # needed, else continue.
 +                        if excluded_subdirs or remote.verify_digest_on_remote(self._get_subdir(tree, subdir)):
 +                            continue
                  except grpc.RpcError as e:
                      if e.code() != grpc.StatusCode.NOT_FOUND:
                          # Intentionally re-raise RpcError for outer except block.
                          raise
 -                self._send_directory(remote, tree)
 +                self._send_directory(remote, tree, excluded_dir=excluded_subdirs)
                  request = buildstream_pb2.UpdateReferenceRequest(instance_name=remote.spec.instance_name)
                  request.keys.append(ref)
@@ -782,10 +800,17 @@ class CASCache():
                  a += 1
                  b += 1
 -    def _reachable_refs_dir(self, reachable, tree, update_mtime=False):
 +    def _reachable_refs_dir(self, reachable, tree, update_mtime=False, subdir=False):
          if tree.hash in reachable:
              return
 +        # If looping through subdir digests, skip processing if
 +        # ref path does not exist, allowing for partial objects
 +        if subdir and not os.path.exists(self.objpath(tree)):
 +            return
++
 +        # Raises FileNotFound exception is path does not exist,
 +        # which should only be thrown on the top level digest
          if update_mtime:
              os.utime(self.objpath(tree))
@@ -802,9 +827,9 @@ class CASCache():
              reachable.add(filenode.digest.hash)
          for dirnode in directory.directories:
 -            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime)
 +            self._reachable_refs_dir(reachable, dirnode.digest, update_mtime=update_mtime, subdir=True)
 -    def _required_blobs(self, directory_digest):
 +    def _required_blobs(self, directory_digest, excluded_dir=None):
          # parse directory, and recursively add blobs
          d = remote_execution_pb2.Digest()
          d.hash = directory_digest.hash
@@ -823,7 +848,8 @@ class CASCache():
              yield d
          for dirnode in directory.directories:
 -            yield from self._required_blobs(dirnode.digest)
 +            if dirnode.name != excluded_dir:
 +                yield from self._required_blobs(dirnode.digest)
      # _ensure_blob():
+     #
@@ -928,6 +954,7 @@ class CASCache():
              objpath = self._ensure_blob(remote, dir_digest)
              directory = remote_execution_pb2.Directory()
++
              with open(objpath, 'rb') as f:
                  directory.ParseFromString(f.read())
@@ -970,8 +997,8 @@ class CASCache():
          return dirdigest
 -    def _send_directory(self, remote, digest, u_uid=uuid.uuid4()):
 -        required_blobs = self._required_blobs(digest)
 +    def _send_directory(self, remote, digest, u_uid=uuid.uuid4(), excluded_dir=None):
 +        required_blobs = self._required_blobs(digest, excluded_dir=excluded_dir)
          missing_blobs = dict()
          # Limit size of FindMissingBlobs request

buildstream/_cas/casremote.py

@@ -23,7 +23,8 @@ from .. import utils
  _MAX_PAYLOAD_BYTES = 1024 * 1024
 -class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key client_cert instance_name')):
 +class CASRemoteSpec(namedtuple('CASRemoteSpec',
 +                               'url push partial_push server_cert client_key client_cert instance_name')):
      # _new_from_config_node
+     #
@@ -31,9 +32,18 @@ class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key
+     #
      @staticmethod
      def _new_from_config_node(spec_node, basedir=None):
 -        _yaml.node_validate(spec_node, ['url', 'push', 'server-cert', 'client-key', 'client-cert', 'instance_name'])
 +        _yaml.node_validate(spec_node, ['url', 'push', 'allow-partial-push', 'server-cert', 'client-key',
 +                                        'client-cert', 'instance_name'])
          url = _yaml.node_get(spec_node, str, 'url')
          push = _yaml.node_get(spec_node, bool, 'push', default_value=False)
 +        partial_push = _yaml.node_get(spec_node, bool, 'allow-partial-push', default_value=False)
++
 +        # partial_push depends on push, raise error if not configured correctly
 +        if partial_push and not push:
 +            provenance = _yaml.node_get_provenance(spec_node, 'allow-partial-push')
 +            raise LoadError(LoadErrorReason.INVALID_DATA,
 +                            "{}: allow-partial-push also requires push to be set".format(provenance))
++
          if not url:
              provenance = _yaml.node_get_provenance(spec_node, 'url')
              raise LoadError(LoadErrorReason.INVALID_DATA,
@@ -63,10 +73,10 @@ class CASRemoteSpec(namedtuple('CASRemoteSpec', 'url push server_cert client_key
              raise LoadError(LoadErrorReason.INVALID_DATA,
                              "{}: 'client-cert' was specified without 'client-key'".format(provenance))
 -        return CASRemoteSpec(url, push, server_cert, client_key, client_cert, instance_name)
 +        return CASRemoteSpec(url, push, partial_push, server_cert, client_key, client_cert, instance_name)
 -CASRemoteSpec.__new__.__defaults__ = (None, None, None, None)
 +CASRemoteSpec.__new__.__defaults__ = (False, None, None, None, None)
  class BlobNotFound(CASRemoteError):

buildstream/_context.py

@@ -121,18 +121,10 @@ class Context():
          # Whether or not to attempt to pull build trees globally
          self.pull_buildtrees = None
 -        # Boolean, whether we double-check with the user that they meant to
 -        # remove a workspace directory.
 -        self.prompt_workspace_close_remove_dir = None
+-
          # Boolean, whether we double-check with the user that they meant to
          # close the workspace when they're using it to access the project.
          self.prompt_workspace_close_project_inaccessible = None
 -        # Boolean, whether we double-check with the user that they meant to do
 -        # a hard reset of a workspace, potentially losing changes.
 -        self.prompt_workspace_reset_hard = None
+-
          # Whether elements must be rebuilt when their dependencies have changed
          self._strict_build_plan = None
@@ -260,16 +252,10 @@ class Context():
          prompt = _yaml.node_get(
              defaults, Mapping, 'prompt')
          _yaml.node_validate(prompt, [
 -            'really-workspace-close-remove-dir',
              'really-workspace-close-project-inaccessible',
 -            'really-workspace-reset-hard',
          ])
 -        self.prompt_workspace_close_remove_dir = _node_get_option_str(
 -            prompt, 'really-workspace-close-remove-dir', ['ask', 'yes']) == 'ask'
          self.prompt_workspace_close_project_inaccessible = _node_get_option_str(
              prompt, 'really-workspace-close-project-inaccessible', ['ask', 'yes']) == 'ask'
 -        self.prompt_workspace_reset_hard = _node_get_option_str(
 -            prompt, 'really-workspace-reset-hard', ['ask', 'yes']) == 'ask'
          # Load per-projects overrides
          self._project_overrides = _yaml.node_get(defaults, Mapping, 'projects', default_value={})

buildstream/_frontend/cli.py

@@ -841,11 +841,6 @@ def workspace_close(app, remove_dir, all_, elements):
          if nonexisting:
              raise AppError("Workspace does not exist", detail="\n".join(nonexisting))
 -        if app.interactive and remove_dir and app.context.prompt_workspace_close_remove_dir:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          for element_name in elements:
              app.stream.workspace_close(element_name, remove_dir=remove_dir)
@@ -879,11 +874,6 @@ def workspace_reset(app, soft, track_, all_, elements):
          if all_ and not app.stream.workspace_exists():
              raise AppError("No open workspaces to reset")
 -        if app.interactive and not soft and app.context.prompt_workspace_reset_hard:
 -            if not click.confirm('This will remove all your changes, are you sure?'):
 -                click.echo('Aborting', err=True)
 -                sys.exit(-1)
+-
          if all_:
              elements = tuple(element_name for element_name, _ in app.context.get_workspaces().list())

buildstream/_loader/loadelement.py

@@ -39,6 +39,20 @@ from .types import Symbol, Dependency
  #    loader (Loader): The Loader object for this element
+ #
  class LoadElement():
 +    # Dependency():
 +    #
 +    # A link from a LoadElement to its dependencies.
 +    #
 +    # Keeps a link to one of the current Element's dependencies, together with
 +    # its dependency type.
 +    #
 +    # Args:
 +    #    element (LoadElement): a LoadElement on which there is a dependency
 +    #    dep_type (str): the type of dependency this dependency link is
 +    class Dependency:
 +        def __init__(self, element, dep_type):
 +            self.element = element
 +            self.dep_type = dep_type
      def __init__(self, node, filename, loader):
@@ -74,8 +88,11 @@ class LoadElement():
              'build-depends', 'runtime-depends',
          ])
 -        # Extract the Dependencies
 -        self.deps = _extract_depends_from_node(self.node)
 +        self.dependencies = []
++
 +    @property
 +    def junction(self):
 +        return self._loader.project.junction
      # depends():
+     #
@@ -101,8 +118,8 @@ class LoadElement():
              return
          self._dep_cache = {}
 -        for dep in self.deps:
 -            elt = self._loader.get_element_for_dep(dep)
 +        for dep in self.dependencies:
 +            elt = dep.element
              # Ensure the cache of the element we depend on
              elt._ensure_depends_cache()

buildstream/_loader/loader.py

@@ -19,7 +19,6 @@
  import os
  from functools import cmp_to_key
 -from collections import namedtuple
  from collections.abc import Mapping
  import tempfile
  import shutil
@@ -32,8 +31,8 @@ from .._profile import Topics, profile_start, profile_end
  from .._includes import Includes
  from .._yamlcache import YamlCache
 -from .types import Symbol, Dependency
 -from .loadelement import LoadElement
 +from .types import Symbol
 +from .loadelement import LoadElement, _extract_depends_from_node
  from . import MetaElement
  from . import MetaSource
  from ..types import CoreWarnings
@@ -112,7 +111,7 @@ class Loader():
          # First pass, recursively load files and populate our table of LoadElements
+         #
 -        deps = []
 +        target_elements = []
          # XXX This will need to be changed to the context's top-level project if this method
          # is ever used for subprojects
@@ -122,10 +121,10 @@ class Loader():
          with YamlCache.open(self._context, cache_file) as yaml_cache:
              for target in targets:
                  profile_start(Topics.LOAD_PROJECT, target)
 -                junction, name, loader = self._parse_name(target, rewritable, ticker,
 -                                                          fetch_subprojects=fetch_subprojects)
 -                loader._load_file(name, rewritable, ticker, fetch_subprojects, yaml_cache)
 -                deps.append(Dependency(name, junction=junction))
 +                _junction, name, loader = self._parse_name(target, rewritable, ticker,
 +                                                           fetch_subprojects=fetch_subprojects)
 +                element = loader._load_file(name, rewritable, ticker, fetch_subprojects, yaml_cache)
 +                target_elements.append(element)
                  profile_end(Topics.LOAD_PROJECT, target)
+         #
@@ -134,29 +133,29 @@ class Loader():
          # Set up a dummy element that depends on all top-level targets
          # to resolve potential circular dependencies between them
 -        DummyTarget = namedtuple('DummyTarget', ['name', 'full_name', 'deps'])
+-
 -        dummy = DummyTarget(name='', full_name='', deps=deps)
 -        self._elements[''] = dummy
 +        dummy_target = LoadElement("", "", self)
 +        dummy_target.dependencies.extend(
 +            LoadElement.Dependency(element, Symbol.RUNTIME)
 +            for element in target_elements
 +        )
          profile_key = "_".join(t for t in targets)
          profile_start(Topics.CIRCULAR_CHECK, profile_key)
 -        self._check_circular_deps('')
 +        self._check_circular_deps(dummy_target)
          profile_end(Topics.CIRCULAR_CHECK, profile_key)
          ret = []
+         #
          # Sort direct dependencies of elements by their dependency ordering
+         #
 -        for target in targets:
 -            profile_start(Topics.SORT_DEPENDENCIES, target)
 -            junction, name, loader = self._parse_name(target, rewritable, ticker,
 -                                                      fetch_subprojects=fetch_subprojects)
 -            loader._sort_dependencies(name)
 -            profile_end(Topics.SORT_DEPENDENCIES, target)
 +        for element in target_elements:
 +            loader = element._loader
 +            profile_start(Topics.SORT_DEPENDENCIES, element.name)
 +            loader._sort_dependencies(element)
 +            profile_end(Topics.SORT_DEPENDENCIES, element.name)
              # Finally, wrap what we have into LoadElements and return the target
+             #
 -            ret.append(loader._collect_element(name))
 +            ret.append(loader._collect_element(element))
          return ret
@@ -184,22 +183,6 @@ class Loader():
              if os.path.exists(self._tempdir):
                  shutil.rmtree(self._tempdir)
 -    # get_element_for_dep():
 -    #
 -    # Gets a cached LoadElement by Dependency object
 -    #
 -    # This is used by LoadElement
 -    #
 -    # Args:
 -    #    dep (Dependency): The dependency to search for
 -    #
 -    # Returns:
 -    #    (LoadElement): The cached LoadElement
 -    #
 -    def get_element_for_dep(self, dep):
 -        loader = self._get_loader_for_dep(dep)
 -        return loader._elements[dep.name]
+-
      ###########################################
      #            Private Methods              #
      ###########################################
@@ -272,8 +255,10 @@ class Loader():
          self._elements[filename] = element
 +        dependencies = _extract_depends_from_node(node)
++
          # Load all dependency files for the new LoadElement
 -        for dep in element.deps:
 +        for dep in dependencies:
              if dep.junction:
                  self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
                  loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
@@ -288,7 +273,9 @@ class Loader():
                                  "{}: Cannot depend on junction"
                                  .format(dep.provenance))
 -        deps_names = [dep.name for dep in element.deps]
 +            element.dependencies.append(LoadElement.Dependency(dep_element, dep.dep_type))
++
 +        deps_names = [dep.name for dep in dependencies]
          self._warn_invalid_elements(deps_names)
          return element
@@ -299,12 +286,12 @@ class Loader():
      # dependencies already resolved.
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to check
 +    #    element (str): The element to check
+     #
      # Raises:
      #    (LoadError): In case there was a circular dependency error
+     #
 -    def _check_circular_deps(self, element_name, check_elements=None, validated=None, sequence=None):
 +    def _check_circular_deps(self, element, check_elements=None, validated=None, sequence=None):
          if check_elements is None:
              check_elements = {}
@@ -313,38 +300,31 @@ class Loader():
          if sequence is None:
              sequence = []
 -        element = self._elements[element_name]
+-
 -        # element name must be unique across projects
 -        # to be usable as key for the check_elements and validated dicts
 -        element_name = element.full_name
+-
          # Skip already validated branches
 -        if validated.get(element_name) is not None:
 +        if validated.get(element) is not None:
              return
 -        if check_elements.get(element_name) is not None:
 +        if check_elements.get(element) is not None:
              # Create `chain`, the loop of element dependencies from this
              # element back to itself, by trimming everything before this
              # element from the sequence under consideration.
 -            chain = sequence[sequence.index(element_name):]
 -            chain.append(element_name)
 +            chain = sequence[sequence.index(element.full_name):]
 +            chain.append(element.full_name)
              raise LoadError(LoadErrorReason.CIRCULAR_DEPENDENCY,
                              ("Circular dependency detected at element: {}\n" +
                               "Dependency chain: {}")
 -                            .format(element.name, " -> ".join(chain)))
 +                            .format(element.full_name, " -> ".join(chain)))
          # Push / Check each dependency / Pop
 -        check_elements[element_name] = True
 -        sequence.append(element_name)
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            loader._check_circular_deps(dep.name, check_elements, validated, sequence)
 -        del check_elements[element_name]
 +        check_elements[element] = True
 +        sequence.append(element.full_name)
 +        for dep in element.dependencies:
 +            dep.element._loader._check_circular_deps(dep.element, check_elements, validated, sequence)
 +        del check_elements[element]
          sequence.pop()
          # Eliminate duplicate paths
 -        validated[element_name] = True
 +        validated[element] = True
      # _sort_dependencies():
+     #
@@ -357,28 +337,21 @@ class Loader():
      # sorts throughout the build process.
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to sort
 +    #    element (LoadElement): The element to sort
+     #
 -    def _sort_dependencies(self, element_name, visited=None):
 +    def _sort_dependencies(self, element, visited=None):
          if visited is None:
 -            visited = {}
 +            visited = set()
 -        element = self._elements[element_name]
+-
 -        # element name must be unique across projects
 -        # to be usable as key for the visited dict
 -        element_name = element.full_name
+-
 -        if visited.get(element_name) is not None:
 +        if element in visited:
              return
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            loader._sort_dependencies(dep.name, visited=visited)
 +        for dep in element.dependencies:
 +            dep.element._loader._sort_dependencies(dep.element, visited=visited)
          def dependency_cmp(dep_a, dep_b):
 -            element_a = self.get_element_for_dep(dep_a)
 -            element_b = self.get_element_for_dep(dep_b)
 +            element_a = dep_a.element
 +            element_b = dep_b.element
              # Sort on inter element dependency first
              if element_a.depends(element_b):
@@ -395,21 +368,21 @@ class Loader():
                      return -1
              # All things being equal, string comparison.
 -            if dep_a.name > dep_b.name:
 +            if element_a.name > element_b.name:
                  return 1
 -            elif dep_a.name < dep_b.name:
 +            elif element_a.name < element_b.name:
                  return -1
              # Sort local elements before junction elements
              # and use string comparison between junction elements
 -            if dep_a.junction and dep_b.junction:
 -                if dep_a.junction > dep_b.junction:
 +            if element_a.junction and element_b.junction:
 +                if element_a.junction > element_b.junction:
                      return 1
 -                elif dep_a.junction < dep_b.junction:
 +                elif element_a.junction < element_b.junction:
                      return -1
 -            elif dep_a.junction:
 +            elif element_a.junction:
                  return -1
 -            elif dep_b.junction:
 +            elif element_b.junction:
                  return 1
              # This wont ever happen
@@ -418,26 +391,23 @@ class Loader():
          # Now dependency sort, we ensure that if any direct dependency
          # directly or indirectly depends on another direct dependency,
          # it is found later in the list.
 -        element.deps.sort(key=cmp_to_key(dependency_cmp))
 +        element.dependencies.sort(key=cmp_to_key(dependency_cmp))
 -        visited[element_name] = True
 +        visited.add(element)
      # _collect_element()
+     #
      # Collect the toplevel elements we have
+     #
      # Args:
 -    #    element_name (str): The element-path relative element name to sort
 +    #    element (LoadElement): The element for which to load a MetaElement
+     #
      # Returns:
      #    (MetaElement): A recursively loaded MetaElement
+     #
 -    def _collect_element(self, element_name):
+-
 -        element = self._elements[element_name]
+-
 +    def _collect_element(self, element):
          # Return the already built one, if we already built it
 -        meta_element = self._meta_elements.get(element_name)
 +        meta_element = self._meta_elements.get(element.name)
          if meta_element:
              return meta_element
@@ -461,10 +431,10 @@ class Loader():
                  del source[Symbol.DIRECTORY]
              index = sources.index(source)
 -            meta_source = MetaSource(element_name, index, element_kind, kind, source, directory)
 +            meta_source = MetaSource(element.name, index, element_kind, kind, source, directory)
              meta_sources.append(meta_source)
 -        meta_element = MetaElement(self.project, element_name, element_kind,
 +        meta_element = MetaElement(self.project, element.name, element_kind,
                                     elt_provenance, meta_sources,
                                     _yaml.node_get(node, Mapping, Symbol.CONFIG, default_value={}),
                                     _yaml.node_get(node, Mapping, Symbol.VARIABLES, default_value={}),
@@ -475,12 +445,12 @@ class Loader():
                                     element_kind == 'junction')
          # Cache it now, make sure it's already there before recursing
 -        self._meta_elements[element_name] = meta_element
 +        self._meta_elements[element.name] = meta_element
          # Descend
 -        for dep in element.deps:
 -            loader = self._get_loader_for_dep(dep)
 -            meta_dep = loader._collect_element(dep.name)
 +        for dep in element.dependencies:
 +            loader = dep.element._loader
 +            meta_dep = loader._collect_element(dep.element)
              if dep.dep_type != 'runtime':
                  meta_element.build_dependencies.append(meta_dep)
              if dep.dep_type != 'build':
@@ -539,7 +509,7 @@ class Loader():
                  return None
          # meta junction element
 -        meta_element = self._collect_element(filename)
 +        meta_element = self._collect_element(self._elements[filename])
          if meta_element.kind != 'junction':
              raise LoadError(LoadErrorReason.INVALID_DATA,
                              "{}: Expected junction but element kind is {}".format(filename, meta_element.kind))
@@ -601,23 +571,6 @@ class Loader():
          return loader
 -    # _get_loader_for_dep():
 -    #
 -    # Gets the appropriate Loader for a Dependency object
 -    #
 -    # Args:
 -    #    dep (Dependency): A Dependency object
 -    #
 -    # Returns:
 -    #    (Loader): The Loader object to use for this Dependency
 -    #
 -    def _get_loader_for_dep(self, dep):
 -        if dep.junction:
 -            # junction dependency, delegate to appropriate loader
 -            return self._loaders[dep.junction]
 -        else:
 -            return self
+-
      # _parse_name():
+     #
      # Get junction and base name of element along with loader for the sub-project

buildstream/data/userconfig.yaml

@@ -112,14 +112,6 @@ logging:
+ #
  prompt:
 -  # Whether to really proceed with 'bst workspace close --remove-dir' removing
 -  # a workspace directory, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always remove, without asking.
 -  #
 -  really-workspace-close-remove-dir: ask
+-
    # Whether to really proceed with 'bst workspace close' when doing so would
    # stop them from running bst commands in this workspace.
+   #
@@ -127,11 +119,3 @@ prompt:
    #  yes - Always close, without asking.
+   #
    really-workspace-close-project-inaccessible: ask
+-
 -  # Whether to really proceed with 'bst workspace reset' doing a hard reset of
 -  # a workspace, potentially losing changes.
 -  #
 -  #  ask - Ask the user if they are sure.
 -  #  yes - Always hard reset, without asking.
 -  #
 -  really-workspace-reset-hard: ask

buildstream/element.py

@@ -1797,13 +1797,19 @@ class Element(Plugin):
      #   (bool): True if this element does not need a push job to be created
+     #
      def _skip_push(self):
++
          if not self.__artifacts.has_push_remotes(element=self):
              # No push remotes for this element's project
              return True
          # Do not push elements that aren't cached, or that are cached with a dangling buildtree
 -        # artifact unless element type is expected to have an an empty buildtree directory
 -        if not self._cached_buildtree():
 +        # artifact unless element type is expected to have an an empty buildtree directory. Check
 +        # that this default behaviour is not overriden via a remote configured to allow pushing
 +        # artifacts without their corresponding buildtree.
 +        if not self._cached():
 +            return True
++
 +        if not self._cached_buildtree() and not self.__artifacts.has_partial_push_remotes(element=self):
              return True
          # Do not push tainted artifact
@@ -1814,11 +1820,14 @@ class Element(Plugin):
      # _push():
+     #
 -    # Push locally cached artifact to remote artifact repository.
 +    # Push locally cached artifact to remote artifact repository. An attempt
 +    # will be made to push partial artifacts if given current config dictates.
 +    # If a remote set for 'full' artifact pushes is found to be cached partially
 +    # in the remote, an attempt will be made to 'complete' it.
+     #
      # Returns:
      #   (bool): True if the remote was updated, False if it already existed
 -    #           and no updated was required
 +    #           and no update was required
+     #
      def _push(self):
          self.__assert_cached()
@@ -1827,8 +1836,17 @@ class Element(Plugin):
              self.warn("Not pushing tainted artifact.")
              return False
 -        # Push all keys used for local commit
 -        pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit())
 +        # Push all keys used for local commit, this could be full or partial,
 +        # given previous _skip_push() logic. If buildtree isn't cached, then
 +        # set partial push
++
 +        partial = False
 +        subdir = 'buildtree'
 +        if not self._cached_buildtree():
 +            partial = True
++
 +        pushed = self.__artifacts.push(self, self.__get_cache_keys_for_commit(), partial=partial, subdir=subdir)
++
          if not pushed:
              return False

buildstream/plugins/elements/filter.py

@@ -20,25 +20,127 @@
  """
  filter - Extract a subset of files from another element
  =======================================================
 -This filters another element by producing an output that is a subset of
 -the filtered element.
 +Filter another element by producing an output that is a subset of
 +the parent element's output. Subsets are defined by the parent element's
 +:ref:`split rules <public_split_rules>`.
 -To specify the element to filter, specify it as the one and only build
 -dependency to filter. See :ref:`Dependencies <format_dependencies>`
 -for what dependencies are and how to specify them.
 +Overview
 +--------
 +A filter element must have exactly one *build* dependency, where said
 +dependency is the 'parent' element which we would like to filter.
 +Runtime dependencies may also be specified, which can be useful to propagate
 +forward from this filter element onto its reverse dependencies.
 +See :ref:`Dependencies <format_dependencies>` to see how we specify dependencies.
 -Dependencies aside from the filtered element may be specified, but
 -they must be runtime dependencies only. This can be useful to propagate
 -runtime dependencies forward from this filter element onto its reverse
 -dependencies.
 +When workspaces are opened, closed or reset on a filter element, or this
 +element is tracked, the filter element will transparently pass on the command
 +to its parent element (the sole build-dependency).
 -When workspaces are opened, closed or reset on this element, or this
 -element is tracked, instead of erroring due to a lack of sources, this
 -element will transparently pass on the command to its sole build-dependency.
 +Example
 +-------
 +Consider a simple import element, ``import.bst`` which imports the local files
 +'foo', 'bar' and 'baz' (each stored in ``files/``, relative to the project's root):
 -The default configuration and possible options are as such:
 -  .. literalinclude:: ../../../buildstream/plugins/elements/filter.yaml
 -     :language: yaml
 +.. code:: yaml
++
 +   kind: import
++
 +   # Specify sources to import
 +   sources:
 +   - kind: local
 +     path: files
++
 +   # Specify public domain data, visible to other elements
 +   public:
 +     bst:
 +       split-rules:
 +         foo:
 +         - /foo
 +         bar:
 +         - /bar
++
 +.. note::
++
 +   We can make an element's metadata visible to all reverse dependencies by making use
 +   of the ``public:`` field. See the :ref:`public data documentation <format_public>`
 +   for more information.
++
 +In this example, ``import.bst`` will serve as the 'parent' of the filter element, thus
 +its output will be filtered. It is important to understand that the artifact of the
 +above element will contain the files: 'foo', 'bar' and 'baz'.
++
 +Now, to produce an element whose artifact contains the file 'foo', and exlusively 'foo',
 +we can define the following filter, ``filter-foo.bst``:
++
 +.. code:: yaml
++
 +   kind: filter
++
 +   # Declare the sole build-dependency of the filter element
 +   depends:
 +   - filename: import.bst
 +     type: build
++
 +   # Declare a list of domains to include in the filter's artifact
 +   config:
 +     include:
 +     - foo
++
 +.. note::
++
 +   We can also specify build-dependencies with a 'build-depends' field which has been
 +   available since :ref:`format version 14 <project_format_version>`. See the
 +   :ref:`Build-Depends documentation <format_build_depends>` for more detail.
++
 +It should be noted that an 'empty' ``include:`` list would, by default, include all
 +split-rules specified in the parent element, which, in this example, would be the
 +files 'foo' and 'bar' (the file 'baz' was not covered by any split rules).
++
 +Equally, we can use the ``exclude:`` statement to create the same artifact (which
 +only contains the file 'foo') by declaring the following element, ``exclude-bar.bst``:
++
 +.. code:: yaml
++
 +   kind: filter
++
 +   # Declare the sole build-dependency of the filter element
 +   depends:
 +   - filename: import.bst
 +     type: build
++
 +   # Declare a list of domains to exclude in the filter's artifact
 +   config:
 +     exclude:
 +     - bar
++
 +In addition to the ``include:`` and ``exclude:`` fields, there exists an ``include-orphans:``
 +(Boolean) field, which defaults to ``False``. This will determine whether to include files
 +which are not present in the 'split-rules'. For example, if we wanted to filter out all files
 +which are not included as split rules we can define the following element, ``filter-misc.bst``:
++
 +.. code:: yaml
++
 +   kind: filter
++
 +   # Declare the sole build-dependency of the filter element
 +   depends:
 +   - filename: import.bst
 +     type: build
++
 +   # Filter out all files which are not declared as split rules
 +   config:
 +     exclude:
 +     - foo
 +     - bar
 +     include-orphans: True
++
 +The artifact of ``filter-misc.bst`` will only contain the file 'baz'.
++
 +Below is more information regarding the the default configurations and possible options
 +of the filter element:
++
 +.. literalinclude:: ../../../buildstream/plugins/elements/filter.yaml
 +   :language: yaml
  """
  from buildstream import Element, ElementError, Scope
@@ -47,6 +149,8 @@ from buildstream import Element, ElementError, Scope
  class FilterElement(Element):
      # pylint: disable=attribute-defined-outside-init
 +    BST_ARTIFACT_VERSION = 1
++
      # The filter element's output is its dependencies, so
      # we must rebuild if the dependencies change even when
      # not in strict build plans.
@@ -64,6 +168,8 @@ class FilterElement(Element):
          self.include = self.node_get_member(node, list, 'include')
          self.exclude = self.node_get_member(node, list, 'exclude')
          self.include_orphans = self.node_get_member(node, bool, 'include-orphans')
 +        self.include_provenance = self.node_provenance(node, member_name='include')
 +        self.exclude_provenance = self.node_provenance(node, member_name='exclude')
      def preflight(self):
          # Exactly one build-depend is permitted
@@ -102,7 +208,32 @@ class FilterElement(Element):
      def assemble(self, sandbox):
          with self.timed_activity("Staging artifact", silent_nested=True):
 -            for dep in self.dependencies(Scope.BUILD):
 +            for dep in self.dependencies(Scope.BUILD, recurse=False):
 +                # Check that all the included/excluded domains exist
 +                pub_data = dep.get_public_data('bst')
 +                split_rules = pub_data.get('split-rules', {})
 +                unfound_includes = []
 +                for domain in self.include:
 +                    if domain not in split_rules:
 +                        unfound_includes.append(domain)
 +                unfound_excludes = []
 +                for domain in self.exclude:
 +                    if domain not in split_rules:
 +                        unfound_excludes.append(domain)
++
 +                detail = []
 +                if unfound_includes:
 +                    detail.append("Unknown domains were used in {}".format(self.include_provenance))
 +                    detail.extend([' - {}'.format(domain) for domain in unfound_includes])
++
 +                if unfound_excludes:
 +                    detail.append("Unknown domains were used in {}".format(self.exclude_provenance))
 +                    detail.extend([' - {}'.format(domain) for domain in unfound_excludes])
++
 +                if detail:
 +                    detail = '\n'.join(detail)
 +                    raise ElementError("Unknown domains declared.", detail=detail)
++
                  dep.stage_artifact(sandbox, include=self.include,
                                     exclude=self.exclude, orphans=self.include_orphans)
          return ""

buildstream/plugins/elements/filter.yaml

@@ -2,20 +2,20 @@
  # Filter element configuration
  config:
 -  # A list of domains to include from each artifact, as
 -  # they were defined in the element's 'split-rules'.
 +  # A list of domains to include in each artifact, as
 +  # they were defined as public data in the parent
 +  # element's 'split-rules'.
+   #
 -  # Since domains can be added, it is not an error to
 -  # specify domains which may not exist for all of the
 -  # elements in this composition.
 +  # If a domain is specified that does not exist, the
 +  # filter element will fail to build.
+   #
    # The default empty list indicates that all domains
 -  # from each dependency should be included.
 +  # of the parent's artifact should be included.
+   #
    include: []
    # A list of domains to exclude from each artifact, as
 -  # they were defined in the element's 'split-rules'.
 +  # they were defined in the parent element's 'split-rules'.
+   #
    # In the case that a file is spoken for by a domain
    # in the 'include' list and another in the 'exclude'
@@ -23,7 +23,7 @@ config:
    exclude: []
    # Whether to include orphan files which are not
 -  # included by any of the 'split-rules' present on
 -  # a given element.
 +  # included by any of the 'split-rules' present in
 +  # the parent element.
+   #
    include-orphans: False

doc/source/using_config.rst

@@ -59,6 +59,15 @@ configuration:
       # Add another cache to pull from
       - url: https://anothercache.com/artifacts:8080
         server-cert: another_server.crt
 +     # Add a cache to push/pull to/from, specifying
 +       that you wish to push artifacts in a 'partial'
 +       state (this being without the respective buildtree).
 +       Note that allow-partial-push requires push to also
 +       be set.
 +     - url: https://anothercache.com/artifacts:11003
 +       push: true
 +       allow-partial-push: true
++
  .. note::
@@ -86,6 +95,14 @@ configuration:
           # Add another cache to pull from
           - url: https://ourprojectcache.com/artifacts:8080
             server-cert: project_server.crt
 +         # Add a cache to push/pull to/from, specifying
 +           that you wish to push artifacts in a 'partial'
 +           state (this being without the respective buildtree).
 +           Note that allow-partial-push requires push to also
 +           be set.
 +         - url: https://anothercache.com/artifacts:11003
 +           push: true
 +           allow-partial-push: true
  .. note::

tests/artifactcache/config.py

@@ -139,3 +139,28 @@ def test_missing_certs(cli, datafiles, config_key, config_value):
      # This does not happen for a simple `bst show`.
      result = cli.run(project=project, args=['artifact', 'pull', 'element.bst'])
      result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
++
++
 +# Assert that if allow-partial-push is specified as true without push also being
 +# set likewise, we get a comprehensive LoadError instead of an unhandled exception.
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_partial_push_error(cli, datafiles):
 +    project = os.path.join(datafiles.dirname, datafiles.basename, 'project', 'elements')
++
 +    project_conf = {
 +        'name': 'test',
++
 +        'artifacts': {
 +            'url': 'https://cache.example.com:12345',
 +            'allow-partial-push': 'True'
 +        }
 +    }
 +    project_conf_file = os.path.join(project, 'project.conf')
 +    _yaml.dump(project_conf, project_conf_file)
++
 +    # Use `pull` here to ensure we try to initialize the remotes, triggering the error
 +    #
 +    # This does not happen for a simple `bst show`.
 +    result = cli.run(project=project, args=['artifact', 'pull', 'target.bst'])
 +    result.assert_main_error(ErrorDomain.LOAD, LoadErrorReason.INVALID_DATA)
 +    assert "allow-partial-push also requires push to be set" in result.stderr

tests/cachekey/cachekey.py

@@ -214,3 +214,41 @@ def test_cache_key_fatal_warnings(cli, tmpdir, first_warnings, second_warnings,
      second_keys = run_get_cache_key("second", second_warnings)
      assert compare_cache_keys(first_keys, second_keys) == identical_keys
++
++
 +@pytest.mark.datafiles(DATA_DIR)
 +def test_keys_stable_over_targets(cli, datafiles):
 +    root_element = 'elements/key-stability/top-level.bst'
 +    target1 = 'elements/key-stability/t1.bst'
 +    target2 = 'elements/key-stability/t2.bst'
++
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    full_graph_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        root_element
 +    ])
 +    full_graph_result.assert_success()
 +    all_cache_keys = parse_output_keys(full_graph_result.output)
++
 +    ordering1_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        target1,
 +        target2
 +    ])
 +    ordering1_result.assert_success()
 +    ordering1_cache_keys = parse_output_keys(ordering1_result.output)
++
 +    ordering2_result = cli.run(project=project, args=[
 +        'show',
 +        '--format', '%{name}::%{full-key}',
 +        target2,
 +        target1
 +    ])
 +    ordering2_result.assert_success()
 +    ordering2_cache_keys = parse_output_keys(ordering2_result.output)
++
 +    for element in ordering1_cache_keys:
 +        assert ordering1_cache_keys[element] == ordering2_cache_keys[element]
 +        assert ordering1_cache_keys[element] == all_cache_keys[element]

tests/cachekey/project/elements/key-stability/aaa.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/aaa.bst

tests/cachekey/project/elements/key-stability/t1.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/t1.bst
 +depends:
 +- elements/key-stability/zzz.bst

tests/cachekey/project/elements/key-stability/t2.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/t2.bst
 +depends:
 +- elements/key-stability/aaa.bst
 +- elements/key-stability/zzz.bst

tests/cachekey/project/elements/key-stability/top-level.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/top-level.bst
 +depends:
 +- elements/key-stability/t1.bst
 +- elements/key-stability/t2.bst

tests/cachekey/project/elements/key-stability/zzz.bst

 +kind: import
 +sources:
 +- kind: local
 +  path: elements/key-stability/zzz.bst

tests/elements/filter.py

@@ -464,3 +464,34 @@ def test_filter_track_multi_exclude(datafiles, cli, tmpdir):
      assert "ref" not in new_input["sources"][0]
      new_input2 = _yaml.load(input2_file)
      assert new_input2["sources"][0]["ref"] == ref
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'basic'))
 +def test_filter_include_with_indirect_deps(datafiles, cli, tmpdir):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    result = cli.run(project=project, args=[
 +        'build', 'output-include-with-indirect-deps.bst'])
 +    result.assert_success()
++
 +    checkout = os.path.join(tmpdir.dirname, tmpdir.basename, 'checkout')
 +    result = cli.run(project=project, args=[
 +        'artifact', 'checkout', 'output-include-with-indirect-deps.bst', '--directory', checkout])
 +    result.assert_success()
++
 +    # direct dependencies should be staged and filtered
 +    assert os.path.exists(os.path.join(checkout, "baz"))
++
 +    # indirect dependencies shouldn't be staged and filtered
 +    assert not os.path.exists(os.path.join(checkout, "foo"))
 +    assert not os.path.exists(os.path.join(checkout, "bar"))
++
++
 +@pytest.mark.datafiles(os.path.join(DATA_DIR, 'basic'))
 +def test_filter_fails_for_nonexisting_domain(datafiles, cli, tmpdir):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    result = cli.run(project=project, args=['build', 'output-include-nonexistent-domain.bst'])
 +    result.assert_main_error(ErrorDomain.STREAM, None)
++
 +    error = "Unknown domains were used in output-include-nonexistent-domain.bst [line 7 column 2]"
 +    assert error in result.stderr
 +    assert '- unknown_file' in result.stderr

tests/elements/filter/basic/elements/deps-permitted.bst

  kind: filter
  depends:
 -- filename: output-include.bst
 +- filename: input.bst
    type: build
  - filename: output-exclude.bst
    type: runtime

tests/elements/filter/basic/elements/input-with-deps.bst

 +kind: import
++
 +depends:
 +- filename: input.bst
++
 +sources:
 +- kind: local
 +  path: files
++
 +public:
 +  bst:
 +    split-rules:
 +      baz:
 +      - /baz

tests/elements/filter/basic/elements/output-include-nonexistent-domain.bst

 +kind: filter
 +depends:
 +- filename: input.bst
 +  type: build
 +config:
 +  include:
 +  - unknown_file
++

tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst

 +kind: filter
++
 +depends:
 +- filename: input-with-deps.bst
 +  type: build

tests/integration/pushbuildtrees.py

 +import os
 +import shutil
 +import pytest
 +import subprocess
++
 +from buildstream import _yaml
 +from tests.testutils import cli_integration as cli, create_artifact_share
 +from tests.testutils.integration import assert_contains
 +from tests.testutils.site import HAVE_BWRAP, IS_LINUX
 +from buildstream._exceptions import ErrorDomain, LoadErrorReason
++
++
 +DATA_DIR = os.path.join(
 +    os.path.dirname(os.path.realpath(__file__)),
 +    "project"
 +)
++
++
 +# Remove artifact cache & set cli.config value of pull-buildtrees
 +# to false, which is the default user context. The cache has to be
 +# cleared as just forcefully removing the refpath leaves dangling objects.
 +def default_state(cli, tmpdir, share):
 +    shutil.rmtree(os.path.join(str(tmpdir), 'artifacts'))
 +    cli.configure({
 +        'artifacts': {'url': share.repo, 'push': False},
 +        'artifactdir': os.path.join(str(tmpdir), 'artifacts'),
 +        'cache': {'pull-buildtrees': False},
 +    })
++
++
 +# Tests to capture the integration of the optionl push of buildtrees.
 +# The behaviour should encompass pushing artifacts that are already cached
 +# without a buildtree as well as artifacts that are cached with their buildtree.
 +# This option is handled via 'allow-partial-push' on a per artifact remote config
 +# node basis. Multiple remote config nodes can point to the same url and as such can
 +# have different 'allow-partial-push' options, tests need to cover this using project
 +# confs.
 +@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_pushbuildtrees(cli, tmpdir, datafiles, integration_cache):
 +    project = os.path.join(datafiles.dirname, datafiles.basename)
 +    element_name = 'autotools/amhello.bst'
++
 +    # Create artifact shares for pull & push testing
 +    with create_artifact_share(os.path.join(str(tmpdir), 'share1')) as share1,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share2')) as share2,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share3')) as share3,\
 +        create_artifact_share(os.path.join(str(tmpdir), 'share4')) as share4:
++
 +        cli.configure({
 +            'artifacts': {'url': share1.repo, 'push': True},
 +            'artifactdir': os.path.join(str(tmpdir), 'artifacts')
 +        })
++
 +        cli.configure({'artifacts': [{'url': share1.repo, 'push': True},
 +                                     {'url': share2.repo, 'push': True, 'allow-partial-push': True}]})
++
 +        # Build autotools element, checked pushed, delete local.
 +        # As share 2 has push & allow-partial-push set a true, it
 +        # should have pushed the artifacts, without the cached buildtrees,
 +        # to it.
 +        result = cli.run(project=project, args=['build', element_name])
 +        assert result.exit_code == 0
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        elementdigest = share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        buildtreedir = os.path.join(str(tmpdir), 'artifacts', 'extract', 'test', 'autotools-amhello',
 +                                    elementdigest.hash, 'buildtree')
 +        assert os.path.isdir(buildtreedir)
 +        assert element_name in result.get_partial_pushed_elements()
 +        assert element_name in result.get_pushed_elements()
 +        assert share1.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        assert share2.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        default_state(cli, tmpdir, share1)
++
 +        # Check that after explictly pulling an artifact without it's buildtree,
 +        # we can push it to another remote that is configured to accept the partial
 +        # artifact
 +        result = cli.run(project=project, args=['pull', element_name])
 +        assert element_name in result.get_pulled_elements()
 +        cli.configure({'artifacts': {'url': share3.repo, 'push': True, 'allow-partial-push': True}})
 +        assert cli.get_element_state(project, element_name) == 'cached'
 +        assert not os.path.isdir(buildtreedir)
 +        result = cli.run(project=project, args=['push', element_name])
 +        assert result.exit_code == 0
 +        assert element_name in result.get_partial_pushed_elements()
 +        assert element_name not in result.get_pushed_elements()
 +        assert share3.has_artifact('test', element_name, cli.get_element_key(project, element_name))
 +        default_state(cli, tmpdir, share3)
++
 +        # Delete the local cache and pull the partial artifact from share 3,
 +        # this should not include the buildtree when extracted locally, even when
 +        # pull-buildtrees is given as a cli parameter as no available remotes will
 +        # contain the buildtree
 +        assert not os.path.isdir(buildtreedir)
 +        assert cli.get_element_state(project, element_name) != 'cached'
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
 +        assert element_name in result.get_partial_pulled_elements()
 +        assert not os.path.isdir(buildtreedir)
 +        default_state(cli, tmpdir, share3)
++
 +        # Delete the local cache and attempt to pull a 'full' artifact, including its
 +        # buildtree. As with before share3 being the first listed remote will not have
 +        # the buildtree available and should spawn a partial pull. Having share1 as the
 +        # second available remote should allow the buildtree to be pulled thus 'completing'
 +        # the artifact
 +        cli.configure({'artifacts': [{'url': share3.repo, 'push': True, 'allow-partial-push': True},
 +                                     {'url': share1.repo, 'push': True}]})
 +        assert cli.get_element_state(project, element_name) != 'cached'
 +        result = cli.run(project=project, args=['--pull-buildtrees', 'pull', element_name])
 +        assert element_name in result.get_partial_pulled_elements()
 +        assert element_name in result.get_pulled_elements()
 +        assert "Attempting to retrieve buildtree from remotes" in result.stderr
 +        assert os.path.isdir(buildtreedir)
 +        assert cli.get_element_state(project, element_name) == 'cached'
++
 +        # Test that we are able to 'complete' an artifact on a server which is cached partially,
 +        # but has now been configured for full artifact pushing. This should require only pushing
 +        # the missing blobs, which should be those of just the buildtree. In this case changing
 +        # share3 to full pushes should exercise this
 +        cli.configure({'artifacts': {'url': share3.repo, 'push': True}})
 +        result = cli.run(project=project, args=['push', element_name])
 +        assert element_name in result.get_pushed_elements()
++
 +        # Ensure that the same remote url can be defined multiple times with differing push
 +        # config. Buildstream supports the same remote having different configurations which
 +        # partial pushing could be different for elements defined at a top level project.conf to
 +        # those from a junctioned project. Assert that elements are pushed to the same remote in
 +        # a state defined via their respective project.confs
 +        default_state(cli, tmpdir, share1)
 +        cli.configure({'artifactdir': os.path.join(str(tmpdir), 'artifacts')}, reset=True)
 +        junction = os.path.join(project, 'elements', 'junction')
 +        os.mkdir(junction)
 +        shutil.copy2(os.path.join(project, 'elements', element_name), junction)
++
 +        junction_conf = {}
 +        project_conf = {}
 +        junction_conf['name'] = 'amhello'
 +        junction_conf['artifacts'] = {'url': share4.repo, 'push': True, 'allow-partial-push': True}
 +        _yaml.dump(junction_conf, os.path.join(junction, 'project.conf'))
 +        project_conf['artifacts'] = {'url': share4.repo, 'push': True}
++
 +        # Read project.conf, the junction project.conf and buildstream.conf
 +        # before running bst
 +        with open(os.path.join(project, 'project.conf'), 'r') as f:
 +            print(f.read())
 +        with open(os.path.join(junction, 'project.conf'), 'r') as f:
 +            print(f.read())
 +        with open(os.path.join(project, 'cache', 'buildstream.conf'), 'r') as f:
 +            print(f.read())
++
 +        result = cli.run(project=project, args=['build', 'junction/amhello.bst'], project_config=project_conf)
++
 +        # Read project.conf, the junction project.conf and buildstream.conf
 +        # after running bst
 +        with open(os.path.join(project, 'project.conf'), 'r') as f:
 +            print(f.read())
 +        with open(os.path.join(junction, 'project.conf'), 'r') as f:
 +            print(f.read())
 +        with open(os.path.join(project, 'cache', 'buildstream.conf'), 'r') as f:
 +            print(f.read())
++
 +        assert 'junction/amhello.bst' in result.get_partial_pushed_elements()
 +        assert 'base/base-alpine.bst' in result.get_pushed_elements()

tests/testutils/runcli.py

@@ -208,6 +208,13 @@ class Result():
          return list(pushed)
 +    def get_partial_pushed_elements(self):
 +        pushed = re.findall(r'\[\s*push:(\S+)\s*\]\s*INFO\s*Pushed partial artifact', self.stderr)
 +        if pushed is None:
 +            return []
++
 +        return list(pushed)
++
      def get_pulled_elements(self):
          pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Pulled artifact', self.stderr)
          if pulled is None:
@@ -215,6 +222,13 @@ class Result():
          return list(pulled)
 +    def get_partial_pulled_elements(self):
 +        pulled = re.findall(r'\[\s*pull:(\S+)\s*\]\s*INFO\s*Pulled partial artifact', self.stderr)
 +        if pulled is None:
 +            return []
++
 +        return list(pulled)
++
  class Cli():
@@ -235,11 +249,15 @@ class Cli():
+     #
      # Args:
      #    config (dict): The user configuration to use
 +    #    reset (bool): Optional reset of stored config
+     #
 -    def configure(self, config):
 +    def configure(self, config, reset=False):
          if self.config is None:
              self.config = {}
 +        if reset:
 +            self.config.clear()
++
          for key, val in config.items():
              self.config[key] = val

tests/testutils/site.py

@@ -18,7 +18,7 @@ try:
      utils.get_host_tool('git')
      HAVE_GIT = True
      out = str(subprocess.check_output(['git', '--version']), "utf-8")
 -    version = tuple(int(x) for x in out.split(' ', 2)[2].split('.'))
 +    version = tuple(int(x) for x in out.split(' ')[2].split('.'))
      HAVE_OLD_GIT = version < (1, 8, 5)
  except ProgramNotFoundError:
      HAVE_GIT = False

tox.ini

@@ -88,5 +88,5 @@ whitelist_externals =
  commands =
      python3 setup.py --command-packages=click_man.commands man_pages
  deps =
 -    click-man
 +    click-man >= 0.3.0
      -rrequirements/requirements.txt

[Notes] [Git][BuildStream/buildstream][tpollard/566] 19 commits: BREAK:remove unconditional 'are you sure?' prompts

Tom Pollard pushed to branch tpollard/566 at BuildStream / buildstream

Commits:

29 changed files:

Changes: