[Notes] [Git][BuildStream/buildstream][jonathan/yamlcache-no-read] 15 co

Jonathan Maw pushed to branch jonathan/yamlcache-no-read at BuildStream / buildstream

Commits:

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

d4c06030

by Jonathan Maw at 2019-02-04T12:46:28Z

yamlcache: Lookup files in the cache without reading the file

For files that are not part of a junction (i.e. not checked-out into a
temporary directory), use the mtime to see if the file has changed.

For files that are, use the junction element's cache key.

17 changed files:

buildstream/_loader/loadelement.py
buildstream/_loader/loader.py
buildstream/_yaml.py
buildstream/_yamlcache.py
buildstream/plugins/elements/filter.py
buildstream/plugins/elements/filter.yaml
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/input-with-deps.bst
+ tests/elements/filter/basic/elements/output-include-with-indirect-deps.bst
tests/testutils/site.py
tox.ini

Changes:

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/_yaml.py

@@ -197,12 +197,12 @@ def load(filename, shortname=None, copy_tree=False, *, project=None, yaml_cache=
      try:
          data = None
 -        with open(filename) as f:
 -            contents = f.read()
 -        if yaml_cache:
 -            data, key = yaml_cache.get(project, filename, contents, copy_tree)
 +        if yaml_cache:
 +            data, key = yaml_cache.get(project, filename, copy_tree)
          if not data:
 +            with open(filename) as f:
 +                contents = f.read()
              data = load_data(contents, file, copy_tree=copy_tree)
              if yaml_cache:
                  yaml_cache.put_from_key(project, filename, key, data)

buildstream/_yamlcache.py

@@ -127,15 +127,14 @@ class YamlCache():
      # Args:
      #    project (Project) or None: The project this file is in, if it exists.
      #    filepath (str): The absolute path to the file.
 -    #    contents (str): The contents of the file to be cached
      #    copy_tree (bool): Whether the data should make a copy when it's being generated
      #                      (i.e. exactly as when called in yaml)
+     #
      # Returns:
      #    (decorated dict): The parsed yaml from the cache, or None if the file isn't in the cache.
      #    (str):            The key used to look up the parsed yaml in the cache
 -    def get(self, project, filepath, contents, copy_tree):
 -        key = self._calculate_key(contents, copy_tree)
 +    def get(self, project, filepath, copy_tree):
 +        key = self._calculate_key(project, filepath, copy_tree)
          data = self._get(project, filepath, key)
          return data, key
@@ -146,12 +145,11 @@ class YamlCache():
      # Args:
      #    project (Project): The project this file is in.
      #    filepath (str): The path to the file.
 -    #    contents (str): The contents of the file that has been cached
      #    copy_tree (bool): Whether the data should make a copy when it's being generated
      #                      (i.e. exactly as when called in yaml)
      #    value (decorated dict): The data to put into the cache.
 -    def put(self, project, filepath, contents, copy_tree, value):
 -        key = self._calculate_key(contents, copy_tree)
 +    def put(self, project, filepath, copy_tree, value):
 +        key = self._calculate_key(project, filepath, copy_tree)
          self.put_from_key(project, filepath, key, value)
      # put_from_key():
@@ -213,13 +211,23 @@ class YamlCache():
      # Calculates a key for putting into the cache.
+     #
      # Args:
 -    #    (basic object)... : Any number of strictly-ordered basic objects
 +    #    project (Project) or None: The project this file is in.
 +    #    filepath (str): The path to the file.
 +    #    copy_tree (bool): Whether the data should make a copy when it's being generated
 +    #                      (i.e. exactly as when called in yaml)
+     #
      # Returns:
      #   (str): A key made out of every arg passed in
      @staticmethod
 -    def _calculate_key(*args):
 -        string = pickle.dumps(args)
 +    def _calculate_key(project, filepath, copy_tree):
 +        if project and project.junction:
 +            # files in a junction only change if the junction element changes
 +            # NOTE: This may change when junction workspaces are revisited/fixed
 +            content_key = project.junction._get_key()  # XXX: Flagrant encapsulation violation!
 +        else:
 +            stat = os.stat(filepath)
 +            content_key = stat.st_mtime
 +        string = pickle.dumps(content_key, copy_tree)
          return hashlib.sha1(string).hexdigest()
      # _get():

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.
@@ -102,7 +206,7 @@ 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):
                  dep.stage_artifact(sandbox, include=self.include,
                                     exclude=self.exclude, orphans=self.include_orphans)
          return ""

buildstream/plugins/elements/filter.yaml

@@ -2,20 +2,21 @@
  # 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.
+   #
    # 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 +24,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

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,23 @@ 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"))

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-with-indirect-deps.bst

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

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][jonathan/yamlcache-no-read] 15 commits: filter.py: don't recurse when staging dependencies

Jonathan Maw pushed to branch jonathan/yamlcache-no-read at BuildStream / buildstream

Commits:

17 changed files:

Changes: