[Notes] [Git][BuildStream/buildstream][lachlan/pickle-yaml-test-list-com

Lachlan pushed to branch lachlan/pickle-yaml-test-list-composite at BuildStream / buildstream

Commits:

3ca487b8

by Chandan Singh at 2018-10-08T18:25:39Z

bst-docker-import: Consistently use stderr for all logs

In !857, we added `contrib/bst-docker-import` script but it has a small
issue that some of logs go to stdout while others go to stderr. Fix it
so that all logging is done on stderr.

49df3d75

by Chandan Singh at 2018-10-08T18:58:54Z

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

bst-docker-import: Consistently use stderr for all logs

See merge request BuildStream/buildstream!863

d827dfae
by Angelos Evripiotis at 2018-10-09T09:12:45Z
```
contributing.rst: end lines with punctuation
```

9590e8ae

by Angelos Evripiotis at 2018-10-09T09:12:45Z

contributing.rst: fix whitespace

No tabs, no lines of only horizontal whitespace.

6d02e269
by Angelos Evripiotis at 2018-10-09T09:12:45Z
```
contributing.rst: fix typo of 'get_count'
```

3ed26a47

by Angelos Evripiotis at 2018-10-09T09:12:45Z

contributing.rst: de-paren subclass example

It seems easier to read when separated into two sentences.

5b22d850
by Angelos Evripiotis at 2018-10-09T09:12:45Z
```
contributing.rst: no spaces before '?'
```

ac0776f8

by Tristan Van Berkom at 2018-10-09T09:36:33Z

Merge branch 'aevri/contributing_fixups' into 'master'

Minor fixups to contributing.rst

See merge request BuildStream/buildstream!866

885bd946

by Tristan Van Berkom at 2018-10-09T09:39:24Z

CONTRIBUTING.rst: Added note about sphinx supporting docstrings on instance variables

Python does not natively support this, but sphinx does parse them and includes
these in the generated documentation.

120d8c73

by Tristan Van Berkom at 2018-10-09T09:45:23Z

CONTRIBUTING.rst: Fix typos in previous commit.

Ooops, that was done far too quickly.

37e65e47

by Jonathan Maw at 2018-10-09T10:51:32Z

Loader: Make _extract_depends_from_node not 'del' nonexistent fields

This is benign because a _yaml.ChainMap doesn't raise a
KeyError, but it's not required to be a ChainMap.

043701f3

by Jonathan Maw at 2018-10-09T10:51:32Z

Add a cache of parsed and provenanced yaml

Note that the ProvenanceFile's names will be incorrect after loading
from the cache, but this is currently only used for writeback, which
isn't used in junctions.

25a13793

by Jonathan Maw at 2018-10-09T10:51:32Z

yamlcache: Support files being added without projects

6c4c30b8

by Jonathan Maw at 2018-10-09T10:51:32Z

yamlcache: Fall back to current directory if not in a project

34af73c5
by Jonathan Maw at 2018-10-09T10:51:32Z
```
_yaml: Mix spurious check of project
```

d080cc57

by Jonathan Maw at 2018-10-09T10:51:32Z

yamlcache/tests: Make yamlcache.open pass a dir in instead of divining its own default

e081c52a
by Lachlan Mackenzie at 2018-10-09T10:51:32Z
```
Add loader for yaml test files cache option
```

963cabb0

by Lachlan Mackenzie at 2018-10-09T10:51:32Z

Add yaml cache testing to yaml list composition test

65b9c212

by Lachlan Mackenzie at 2018-10-09T10:51:32Z

Add yaml cache test to yaml list composition twice

dcfa4a7d

by Jonathan Maw at 2018-10-09T10:51:32Z

SQUASHME: yamlcache: hide key calculation and fix public/private

Previously, key calculation had to be done manually, creating
unnecessary boilerplate. This has now been subsumed into YamlCache.get
and YamlCache.put (Yamlcache._get and Yamlcache._put_from_key exist for
old behaviour).

YamlCache.write is now private.

Private methods have been put at the end of the class.

Changes:

CONTRIBUTING.rst

@@ -14,7 +14,7 @@ if no issue already exists.
  For policies on how to submit an issue and how to use our project labels,
  we recommend that you read the `policies guide
 -<https://gitlab.com/BuildStream/nosoftware/alignment/blob/master/BuildStream_policies.md>`_
 +<https://gitlab.com/BuildStream/nosoftware/alignment/blob/master/BuildStream_policies.md>`_.
  .. _contributing_fixing_bugs:
@@ -270,6 +270,11 @@ comments and docstrings.
      *Since: 1.2*
      """
 +.. note::
++
 +   Python does not support docstrings on instance variables, but sphinx does
 +   pick them up and includes them in the generated documentation.
++
  **Internal instance variable**::
    def __init__(self, context, element):
@@ -404,7 +409,7 @@ on a Python class in BuildStream::
           # Implementation of the "frobbish" abstract method
           # defined by the parent Bar class.
+          #
 -	 return True
 +         return True
        ################################################
        #                 Public Methods               #
@@ -445,7 +450,7 @@ on a Python class in BuildStream::
        # Returns:
        #    (int): The count of this foo
+       #
 -      def set_count(self, count):
 +      def get_count(self, count):
            return self._count
@@ -459,7 +464,7 @@ on a Python class in BuildStream::
        #       Even though these are private implementation
        #       details, they still MUST have API documenting
        #       comments on them.
+-
++
        # _do_frobbing()
+       #
        # Does the actual frobbing
@@ -494,10 +499,10 @@ reference on how the PEP-8 defines public and non-public.
    A private symbol must be denoted by a leading underscore.
 -* When a class can have subclasses (for example, the ``Sandbox`` or ``Platform``
 +* When a class can have subclasses, then private symbols should be denoted
 +  by two leading underscores. For example, the ``Sandbox`` or ``Platform``
    classes which have various implementations, or the ``Element`` and ``Source``
 -  classes which plugins derive from), then private symbols should be denoted
 -  by two leading underscores.
 +  classes which plugins derive from.
    The double leading underscore naming convention invokes Python's name
    mangling algorithm which helps prevent namespace collisions in the case
@@ -536,7 +541,7 @@ In order to disambiguate between:
  * Symbols which are publicly accessible details of the ``Element`` class, can
    be accessed by BuildStream internals, but must remain hidden from the
 -  *"Public API Surface"*
 +  *"Public API Surface"*.
  * Symbols which are private to the ``Element`` class, and cannot be accessed
    from outside of the ``Element`` class at all.
@@ -586,7 +591,7 @@ is found at ``_artifactcache/artifactcache.py``.
  Imports
  ~~~~~~~
 -Module imports inside BuildStream are done with relative ``.`` notation
 +Module imports inside BuildStream are done with relative ``.`` notation:
  **Good**::
@@ -628,12 +633,12 @@ which exposes an instance variable is the only one in control of the value of th
  variable.
  * If an instance variable is public and must be modified; then it must be
 -  modified using a :ref:`mutator <contributing_accessor_mutator>`
 +  modified using a :ref:`mutator <contributing_accessor_mutator>`.
  * Ideally for better encapsulation, all object state is declared as
    :ref:`private instance variables <contributing_public_and_private>` and can
    only be accessed by external classes via public :ref:`accessors and mutators
 -  <contributing_accessor_mutator>`
 +  <contributing_accessor_mutator>`.
  .. note::
@@ -720,10 +725,10 @@ In BuildStream, we use the term *"Abstract Method"*, to refer to
  a method which **can** be overridden by a subclass, whereas it
  is **illegal** to override any other method.
 -* Abstract methods are allowed to have default implementations
 +* Abstract methods are allowed to have default implementations.
  * Subclasses are not allowed to redefine the calling signature
 -  of an abstract method, or redefine the API contract in any way
 +  of an abstract method, or redefine the API contract in any way.
  * Subclasses are not allowed to override any other methods.
@@ -798,7 +803,7 @@ BstError parameters
  When raising ``BstError`` class exceptions, there are some common properties
  which can be useful to know about:
 -* **message:** The brief human readable error, will be formatted on one line in the frontend
 +* **message:** The brief human readable error, will be formatted on one line in the frontend.
  * **detail:** An optional detailed human readable message to accompany the **message** summary
    of the error. This is often used to recommend the user some course of action, or to provide
@@ -974,9 +979,9 @@ symbols to a minimum, this is important for both
  When anyone visits a file, there are two levels of comprehension:
 -* What do I need to know in order to *use* this object
 +* What do I need to know in order to *use* this object.
 -* What do I need to know in order to *modify* this object
 +* What do I need to know in order to *modify* this object.
  For the former, we want the reader to understand with as little effort
  as possible, what the public API contract is for a given object and consequently,
@@ -1001,9 +1006,9 @@ well documented and minimal.
  When adding new API to a given object for a new purpose, consider whether
  the new API is in any way redundant with other API (should this value now
 -go into the constructor, since we use it more than once ? could this
 +go into the constructor, since we use it more than once? could this
  value be passed along with another function, and the other function renamed,
 -to better suit the new purposes of this module/object ?) and repurpose
 +to better suit the new purposes of this module/object?) and repurpose
  the outward facing API of an object as a whole every time.
@@ -1183,7 +1188,7 @@ The BuildStream documentation style is as follows:
  * Cross references should be of the form ``:role:`target```.
    * Explicit anchors can be declared as ``.. _anchor_name:`` on a line by itself.
+-
++
    * To cross reference arbitrary locations with, for example, the anchor ``anchor_name``,
      always provide some explicit text in the link instead of deriving the text from
      the target, e.g.: ``:ref:`Link text <anchor_name>```.
@@ -1266,23 +1271,23 @@ Documentation Examples
  The examples section of the documentation contains a series of standalone
  examples, here are the criteria for an example addition.
 -* The example has a ``${name}``
 +* The example has a ``${name}``.
 -* The example has a project users can copy and use
 +* The example has a project users can copy and use.
 -  * This project is added in the directory ``doc/examples/${name}``
 +  * This project is added in the directory ``doc/examples/${name}``.
 -* The example has a documentation component
 +* The example has a documentation component.
    * This is added at ``doc/source/examples/${name}.rst``
    * A reference to ``examples/${name}`` is added to the toctree in ``doc/source/examples.rst``
    * This documentation discusses the project elements declared in the project and may
 -    provide some BuildStream command examples
 -  * This documentation links out to the reference manual at every opportunity
 +    provide some BuildStream command examples.
 +  * This documentation links out to the reference manual at every opportunity.
 -* The example has a CI test component
 +* The example has a CI test component.
 -  * This is an integration test added at ``tests/examples/${name}``
 +  * This is an integration test added at ``tests/examples/${name}``.
    * This test runs BuildStream in the ways described in the example
      and assert that we get the results which we advertize to users in
      the said examples.
@@ -1309,17 +1314,17 @@ The ``.run`` file format is just another YAML dictionary which consists of a
  Each *command* is a dictionary, the members of which are listed here:
 -* ``directory``: The input file relative project directory
 +* ``directory``: The input file relative project directory.
 -* ``output``: The input file relative output html file to generate (optional)
 +* ``output``: The input file relative output html file to generate (optional).
  * ``fake-output``: Don't really run the command, just pretend to and pretend
    this was the output, an empty string will enable this too.
 -* ``command``: The command to run, without the leading ``bst``
 +* ``command``: The command to run, without the leading ``bst``.
  * ``shell``: Specifying ``True`` indicates that ``command`` should be run as
 -  a shell command from the project directory, instead of a bst command (optional)
 +  a shell command from the project directory, instead of a bst command (optional).
  When adding a new ``.run`` file, one should normally also commit the new
  resulting generated ``.html`` file(s) into the ``doc/source/sessions-stored/``
@@ -1417,7 +1422,7 @@ a subdirectory beside your test in which to store data.
  When creating a test that needs data, use the datafiles extension
  to decorate your test case (again, examples exist in the existing
  tests for this), documentation on the datafiles extension can
 -be found here: https://pypi.python.org/pypi/pytest-datafiles
 +be found here: https://pypi.python.org/pypi/pytest-datafiles.
  Tests that run a sandbox should be decorated with::

buildstream/_loader/loadelement.py

@@ -185,6 +185,6 @@ def _extract_depends_from_node(node, *, key=None):
          output_deps.append(dependency)
      # Now delete the field, we dont want it anymore
 -    del node[key]
 +    node.pop(key, None)
      return output_deps

buildstream/_loader/loader.py

@@ -29,6 +29,7 @@ from .. import _yaml
  from ..element import Element
  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
@@ -112,7 +113,14 @@ class Loader():
              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)
++
 +            # XXX This will need to be changed to the context's top-level project if this method
 +            # is ever used for subprojects
 +            top_dir = self.project.directory
++
 +            cache_file = YamlCache.get_cache_file(top_dir)
 +            with YamlCache.open(self._context, cache_file) as yaml_cache:
 +                loader._load_file(name, rewritable, ticker, fetch_subprojects, yaml_cache)
              deps.append(Dependency(name, junction=junction))
              profile_end(Topics.LOAD_PROJECT, target)
@@ -201,11 +209,12 @@ class Loader():
      #    rewritable (bool): Whether we should load in round trippable mode
      #    ticker (callable): A callback to report loaded filenames to the frontend
      #    fetch_subprojects (bool): Whether to fetch subprojects while loading
 +    #    yaml_cache (YamlCache): A yaml cache
+     #
      # Returns:
      #    (LoadElement): A loaded LoadElement
+     #
 -    def _load_file(self, filename, rewritable, ticker, fetch_subprojects):
 +    def _load_file(self, filename, rewritable, ticker, fetch_subprojects, yaml_cache=None):
          # Silently ignore already loaded files
          if filename in self._elements:
@@ -218,7 +227,8 @@ class Loader():
          # Load the data and process any conditional statements therein
          fullpath = os.path.join(self._basedir, filename)
          try:
 -            node = _yaml.load(fullpath, shortname=filename, copy_tree=rewritable, project=self.project)
 +            node = _yaml.load(fullpath, shortname=filename, copy_tree=rewritable,
 +                              project=self.project, yaml_cache=yaml_cache)
          except LoadError as e:
              if e.reason == LoadErrorReason.MISSING_FILE:
                  # If we can't find the file, try to suggest plausible
@@ -261,13 +271,13 @@ class Loader():
          # Load all dependency files for the new LoadElement
          for dep in element.deps:
              if dep.junction:
 -                self._load_file(dep.junction, rewritable, ticker, fetch_subprojects)
 +                self._load_file(dep.junction, rewritable, ticker, fetch_subprojects, yaml_cache)
                  loader = self._get_loader(dep.junction, rewritable=rewritable, ticker=ticker,
                                            fetch_subprojects=fetch_subprojects)
              else:
                  loader = self
 -            dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects)
 +            dep_element = loader._load_file(dep.name, rewritable, ticker, fetch_subprojects, yaml_cache)
              if _yaml.node_get(dep_element.node, str, Symbol.KIND) == 'junction':
                  raise LoadError(LoadErrorReason.INVALID_DATA,

buildstream/_project.py

@@ -19,6 +19,7 @@
  #        Tiago Gomes <tiago gomes codethink co uk>
  import os
 +import hashlib
  from collections import Mapping, OrderedDict
  from pluginbase import PluginBase
  from . import utils

buildstream/_yaml.py

@@ -183,20 +183,32 @@ class CompositeTypeError(CompositeError):
  #    shortname (str): The filename in shorthand for error reporting (or None)
  #    copy_tree (bool): Whether to make a copy, preserving the original toplevels
  #                      for later serialization
 +#    yaml_cache (YamlCache): A yaml cache to consult rather than parsing
+ #
  # Returns (dict): A loaded copy of the YAML file with provenance information
+ #
  # Raises: LoadError
+ #
 -def load(filename, shortname=None, copy_tree=False, *, project=None):
 +def load(filename, shortname=None, copy_tree=False, *, project=None, yaml_cache=None):
      if not shortname:
          shortname = filename
      file = ProvenanceFile(filename, shortname, project)
      try:
 +        data = None
          with open(filename) as f:
 -            return load_data(f, file, copy_tree=copy_tree)
 +            contents = f.read()
 +        if yaml_cache:
 +            data, key = yaml_cache.get(project, filename, contents, copy_tree)
++
 +        if not data:
 +            data = load_data(contents, file, copy_tree=copy_tree)
++
 +        if yaml_cache:
 +            yaml_cache.put_from_key(project, filename, key, data)
++
 +        return data
      except FileNotFoundError as e:
          raise LoadError(LoadErrorReason.MISSING_FILE,
                          "Could not find file at {}".format(filename)) from e

buildstream/_yamlcache.py

 +#
 +#  Copyright 2018 Bloomberg Finance LP
 +#
 +#  This program is free software; you can redistribute it and/or
 +#  modify it under the terms of the GNU Lesser General Public
 +#  License as published by the Free Software Foundation; either
 +#  version 2 of the License, or (at your option) any later version.
 +#
 +#  This library is distributed in the hope that it will be useful,
 +#  but WITHOUT ANY WARRANTY; without even the implied warranty of
 +#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 +#  Lesser General Public License for more details.
 +#
 +#  You should have received a copy of the GNU Lesser General Public
 +#  License along with this library. If not, see <http://www.gnu.org/licenses/>.
 +#
 +#  Authors:
 +#        Jonathan Maw <jonathan maw codethink co uk>
++
 +import os
 +import pickle
 +import hashlib
 +import io
++
 +import sys
++
 +from contextlib import contextmanager
 +from collections import namedtuple
++
 +from ._cachekey import generate_key
 +from ._context import Context
 +from . import utils, _yaml
++
++
 +YAML_CACHE_FILENAME = "yaml_cache.pickle"
++
++
 +# YamlCache()
 +#
 +# A cache that wraps around the loading of yaml in projects.
 +#
 +# The recommended way to use a YamlCache is:
 +#   with YamlCache.open(context) as yamlcache:
 +#     # Load all the yaml
 +#     ...
 +#
 +# Args:
 +#    context (Context): The invocation Context
 +#
 +class YamlCache():
++
 +    def __init__(self, context):
 +        self._project_caches = {}
 +        self._context = context
 +    ##################
 +    # Public Methods #
 +    ##################
++
 +    # is_cached():
 +    #
 +    # Checks whether a file is cached.
 +    #
 +    # Args:
 +    #    project (Project): The project this file is in.
 +    #    filepath (str): The path to the file, *relative to the project's directory*.
 +    #
 +    # Returns:
 +    #    (bool): Whether the file is cached.
 +    def is_cached(self, project, filepath):
 +        cache_path = self._get_filepath(project, filepath)
 +        project_name = project.name if project else ""
 +        try:
 +            project_cache = self._project_caches[project_name]
 +            if cache_path in project_cache.elements:
 +                return True
 +        except KeyError:
 +            pass
 +        return False
++
 +    # open():
 +    #
 +    # Return an instance of the YamlCache which writes to disk when it leaves scope.
 +    #
 +    # Args:
 +    #    context (Context): The context.
 +    #    cachefile (str): The path to the cache file.
 +    #
 +    # Returns:
 +    #    (YamlCache): A YamlCache.
 +    @staticmethod
 +    @contextmanager
 +    def open(context, cachefile):
 +        # Try to load from disk first
 +        cache = None
 +        if os.path.exists(cachefile):
 +            try:
 +                with open(cachefile, "rb") as f:
 +                    cache = BstUnpickler(f, context).load()
 +            except pickle.UnpicklingError as e:
 +                sys.stderr.write("Failed to load YamlCache, {}\n".format(e))
++
 +        # Failed to load from disk, create a new one
 +        if not cache:
 +            cache = YamlCache(context)
++
 +        yield cache
++
 +        cache._write(cachefile)
++
 +    # get_cache_file():
 +    #
 +    # Retrieves a path to the yaml cache file.
 +    #
 +    # Returns:
 +    #   (str): The path to the cache file
 +    @staticmethod
 +    def get_cache_file(top_dir):
 +        return os.path.join(top_dir, ".bst", YAML_CACHE_FILENAME)
++
 +    # get():
 +    #
 +    # Gets a parsed file from the cache.
 +    #
 +    # 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)
 +        data = self._get(project, filepath, key)
 +        return data, key
++
 +    # put():
 +    #
 +    # Puts a parsed file into the cache.
 +    #
 +    # 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)
 +        self.put_from_key(project, filepath, key, value)
++
 +    # put_from_key():
 +    #
 +    # Put a parsed file into the cache when given a key.
 +    #
 +    # Args:
 +    #    project (Project): The project this file is in.
 +    #    filepath (str): The path to the file.
 +    #    key (str): The key to the file within the cache. Typically, this is the
 +    #               value of `calculate_key()` with the file's unparsed contents
 +    #               and any relevant metadata passed in.
 +    #    value (decorated dict): The data to put into the cache.
 +    def put_from_key(self, project, filepath, key, value):
 +        cache_path = self._get_filepath(project, filepath)
 +        project_name = project.name if project else ""
 +        try:
 +            project_cache = self._project_caches[project_name]
 +        except KeyError:
 +            project_cache = self._project_caches[project_name] = CachedProject({})
++
 +        project_cache.elements[cache_path] = CachedYaml(key, value)
++
++
 +    ###################
 +    # Private Methods #
 +    ###################
++
 +    # Writes the yaml cache to the specified path.
 +    #
 +    # Args:
 +    #    path (str): The path to the cache file.
 +    def _write(self, path):
 +        parent_dir = os.path.dirname(path)
 +        os.makedirs(parent_dir, exist_ok=True)
 +        with open(path, "wb") as f:
 +            BstPickler(f).dump(self)
++
 +    # _get_filepath():
 +    #
 +    # Returns a file path relative to a project if passed, or the original path if
 +    # the project is None
 +    #
 +    # Args:
 +    #    project (Project) or None: The project the filepath exists within
 +    #    full_path (str): The path that the returned path is based on
 +    #
 +    # Returns:
 +    #    (str): The path to the file, relative to a project if it exists
 +    def _get_filepath(self, project, full_path):
 +        if project:
 +            assert full_path.startswith(project.directory)
 +            filepath = os.path.relpath(full_path, project.directory)
 +        else:
 +            filepath = full_path
 +        return full_path
++
 +    # _calculate_key():
 +    #
 +    # Calculates a key for putting into the cache.
 +    #
 +    # Args:
 +    #    (basic object)... : Any number of strictly-ordered basic objects
 +    #
 +    # Returns:
 +    #   (str): A key made out of every arg passed in
 +    @staticmethod
 +    def _calculate_key(*args):
 +        string = pickle.dumps(args)
 +        return hashlib.sha1(string).hexdigest()
++
 +    # _get():
 +    #
 +    # Gets a parsed file from the cache when given a key.
 +    #
 +    # Args:
 +    #    project (Project): The project this file is in.
 +    #    filepath (str): The path to the file.
 +    #    key (str): The key to the file within the cache. Typically, this is the
 +    #               value of `calculate_key()` with the file's unparsed contents
 +    #               and any relevant metadata passed in.
 +    #
 +    # Returns:
 +    #    (decorated dict): The parsed yaml from the cache, or None if the file isn't in the cache.
 +    def _get(self, project, filepath, key):
 +        cache_path = self._get_filepath(project, filepath)
 +        project_name = project.name if project else ""
 +        try:
 +            project_cache = self._project_caches[project_name]
 +            try:
 +                cachedyaml = project_cache.elements[cache_path]
 +                if cachedyaml._key == key:
 +                    # We've unpickled the YamlCache, but not the specific file
 +                    if cachedyaml._contents is None:
 +                        cachedyaml._contents = BstUnpickler.loads(cachedyaml._pickled_contents, self._context)
 +                    return cachedyaml._contents
 +            except KeyError:
 +                pass
 +        except KeyError:
 +            pass
 +        return None
++
++
 +CachedProject = namedtuple('CachedProject', ['elements'])
++
++
 +class CachedYaml():
 +    def __init__(self, key, contents):
 +        self._key = key
 +        self.set_contents(contents)
++
 +    # Sets the contents of the CachedYaml.
 +    #
 +    # Args:
 +    #    contents (provenanced dict): The contents to put in the cache.
 +    #
 +    def set_contents(self, contents):
 +        self._contents = contents
 +        self._pickled_contents = BstPickler.dumps(contents)
++
 +    # Pickling helper method, prevents 'contents' from being serialised
 +    def __getstate__(self):
 +        data = self.__dict__.copy()
 +        data['_contents'] = None
 +        return data
++
++
 +# In _yaml.load, we have a ProvenanceFile that stores the project the file
 +# came from. Projects can't be pickled, but it's always going to be the same
 +# project between invocations (unless the entire project is moved but the
 +# file stayed in the same place)
 +class BstPickler(pickle.Pickler):
 +    def persistent_id(self, obj):
 +        if isinstance(obj, _yaml.ProvenanceFile):
 +            if obj.project:
 +                # ProvenanceFile's project object cannot be stored as it is.
 +                project_tag = obj.project.name
 +                # ProvenanceFile's filename must be stored relative to the
 +                # project, as the project dir may move.
 +                name = os.path.relpath(obj.name, obj.project.directory)
 +            else:
 +                project_tag = None
 +                name = obj.name
 +            return ("ProvenanceFile", name, obj.shortname, project_tag)
 +        elif isinstance(obj, Context):
 +            return ("Context",)
 +        else:
 +            return None
++
 +    @staticmethod
 +    def dumps(obj):
 +        stream = io.BytesIO()
 +        BstPickler(stream).dump(obj)
 +        stream.seek(0)
 +        return stream.read()
++
++
 +class BstUnpickler(pickle.Unpickler):
 +    def __init__(self, file, context):
 +        super().__init__(file)
 +        self._context = context
++
 +    def persistent_load(self, pid):
 +        if pid[0] == "ProvenanceFile":
 +            _, tagged_name, shortname, project_tag = pid
++
 +            if project_tag is not None:
 +                for p in self._context.get_projects():
 +                    if project_tag == p.name:
 +                        project = p
 +                        break
++
 +                name = os.path.join(project.directory, tagged_name)
++
 +                if not project:
 +                    projects = [p.name for p in self._context.get_projects()]
 +                    raise pickle.UnpicklingError("No project with name {} found in {}"
 +                                                 .format(key_id, projects))
 +            else:
 +                project = None
 +                name = tagged_name
++
 +            return _yaml.ProvenanceFile(name, shortname, project)
 +        elif pid[0] == "Context":
 +            return self._context
 +        else:
 +            raise pickle.UnpicklingError("Unsupported persistent object, {}".format(pid))
++
 +    @staticmethod
 +    def loads(text, context):
 +        stream = io.BytesIO()
 +        stream.write(bytes(text))
 +        stream.seek(0)
 +        return BstUnpickler(stream, context).load()

contrib/bst-docker-import

@@ -93,10 +93,10 @@ echo "INFO: Checking out $element ..." >&2
  $bst_cmd checkout --tar "$element" "$checkout_tar" || die "Failed to checkout $element"
  echo "INFO: Successfully checked out $element" >&2
 -echo "INFO: Importing Docker image ..."
 +echo "INFO: Importing Docker image ..." >&2
  "${docker_import_cmd[@]}" "$checkout_tar" "$docker_image_tag" || die "Failed to import Docker image from tarball"
 -echo "INFO: Successfully import Docker image $docker_image_tag"
 +echo "INFO: Successfully import Docker image $docker_image_tag" >&2
 -echo "INFO: Cleaning up ..."
 +echo "INFO: Cleaning up ..." >&2
  rm "$checkout_tar" || die "Failed to remove $checkout_tar"
 -echo "INFO: Clean up finished"
 +echo "INFO: Clean up finished" >&2

tests/frontend/yamlcache.py

 +import os
 +import pytest
 +import hashlib
 +import tempfile
 +from ruamel import yaml
++
 +from tests.testutils import cli, generate_junction, create_element_size, create_repo
 +from buildstream import _yaml
 +from buildstream._yamlcache import YamlCache
 +from buildstream._project import Project
 +from buildstream._context import Context
 +from contextlib import contextmanager
++
++
 +def generate_project(tmpdir, ref_storage, with_junction, name="test"):
 +    if with_junction == 'junction':
 +        subproject_dir = generate_project(
 +            tmpdir, ref_storage,
 +            'no-junction', name='test-subproject'
 +        )
++
 +    project_dir = os.path.join(tmpdir, name)
 +    os.makedirs(project_dir)
 +    # project.conf
 +    project_conf_path = os.path.join(project_dir, 'project.conf')
 +    elements_path = 'elements'
 +    project_conf = {
 +        'name': name,
 +        'element-path': elements_path,
 +        'ref-storage': ref_storage,
 +    }
 +    _yaml.dump(project_conf, project_conf_path)
++
 +    # elements
 +    if with_junction == 'junction':
 +        junction_name = 'junction.bst'
 +        junction_dir = os.path.join(project_dir, elements_path)
 +        junction_path = os.path.join(project_dir, elements_path, junction_name)
 +        os.makedirs(junction_dir)
 +        generate_junction(tmpdir, subproject_dir, junction_path)
 +        element_depends = [{'junction': junction_name, 'filename': 'test.bst'}]
 +    else:
 +        element_depends = []
++
 +    element_name = 'test.bst'
 +    create_element_size(element_name, project_dir, elements_path, element_depends, 1)
++
 +    return project_dir
++
++
 +@contextmanager
 +def with_yamlcache(project_dir):
 +    context = Context()
 +    project = Project(project_dir, context)
 +    cache_file = YamlCache.get_cache_file(project_dir)
 +    with YamlCache.open(context, cache_file) as yamlcache:
 +        yield yamlcache, project
++
++
 +def yamlcache_key(yamlcache, in_file, copy_tree=False):
 +    with open(in_file) as f:
 +        key = yamlcache._calculate_key(f.read(), copy_tree)
 +    return key
++
++
 +def modified_file(input_file, tmpdir):
 +    with open(input_file) as f:
 +        data = f.read()
 +    assert 'variables' not in data
 +    data += '\nvariables: {modified: True}\n'
 +    _, temppath = tempfile.mkstemp(dir=tmpdir, text=True)
 +    with open(temppath, 'w') as f:
 +        f.write(data)
++
 +    return temppath
++
++
 +@pytest.mark.parametrize('ref_storage', ['inline', 'project.refs'])
 +@pytest.mark.parametrize('with_junction', ['no-junction', 'junction'])
 +@pytest.mark.parametrize('move_project', ['move', 'no-move'])
 +def test_yamlcache_used(cli, tmpdir, ref_storage, with_junction, move_project):
 +    # Generate the project
 +    project = generate_project(str(tmpdir), ref_storage, with_junction)
 +    if with_junction == 'junction':
 +        result = cli.run(project=project, args=['fetch', '--track', 'junction.bst'])
 +        result.assert_success()
++
 +    # bst show to put it in the cache
 +    result = cli.run(project=project, args=['show', 'test.bst'])
 +    result.assert_success()
++
 +    element_path = os.path.join(project, 'elements', 'test.bst')
 +    with with_yamlcache(project) as (yc, prj):
 +        # Check that it's in the cache
 +        assert yc.is_cached(prj, element_path)
++
 +        # *Absolutely* horrible cache corruption to check it's being used
 +        # Modifying the data from the cache is fraught with danger,
 +        # so instead I'll load a modified version of the original file
 +        temppath = modified_file(element_path, str(tmpdir))
 +        contents = _yaml.load(temppath, copy_tree=False, project=prj)
 +        key = yamlcache_key(yc, element_path)
 +        yc.put_from_key(prj, element_path, key, contents)
++
 +    # Show that a variable has been added
 +    result = cli.run(project=project, args=['show', '--format', '%{vars}', 'test.bst'])
 +    result.assert_success()
 +    data = yaml.safe_load(result.output)
 +    assert 'modified' in data
 +    assert data['modified'] == 'True'
++
++
 +@pytest.mark.parametrize('ref_storage', ['inline', 'project.refs'])
 +@pytest.mark.parametrize('with_junction', ['junction', 'no-junction'])
 +@pytest.mark.parametrize('move_project', ['move', 'no-move'])
 +def test_yamlcache_changed_file(cli, ref_storage, with_junction, move_project):
 +    # i.e. a file is cached, the file is changed, loading the file (with cache) returns new data
 +    # inline and junction can only be changed by opening a workspace
 +    pass

tests/yaml/yaml.py

  import os
  import pytest
 +import tempfile
  from collections import Mapping
  from buildstream import _yaml
  from buildstream._exceptions import LoadError, LoadErrorReason
 +from buildstream._context import Context
 +from buildstream._yamlcache import YamlCache
  DATA_DIR = os.path.join(
      os.path.dirname(os.path.realpath(__file__)),
@@ -150,6 +153,21 @@ def test_composite_preserve_originals(datafiles):
      assert(_yaml.node_get(orig_extra, str, 'old') == 'new')
 +def load_yaml_file(filename, *, cache_path, shortname=None, from_cache='raw'):
++
 +    temppath = tempfile.mkstemp(dir=str(cache_path), text=True)
 +    context = Context()
++
 +    with YamlCache.open(context, str(temppath)) as yc:
 +        if from_cache == 'raw':
 +            return _yaml.load(filename, shortname)
 +        elif from_cache == 'cached':
 +            _yaml.load(filename, shortname, yaml_cache=yc)
 +            return _yaml.load(filename, shortname, yaml_cache=yc)
 +        else:
 +            assert False
++
++
  # Tests for list composition
+ #
  # Each test composits a filename on top of basics.yaml, and tests
@@ -165,6 +183,7 @@ def test_composite_preserve_originals(datafiles):
  #    prov_col: The expected provenance column of "mood"
+ #
  @pytest.mark.datafiles(os.path.join(DATA_DIR))
 +@pytest.mark.parametrize('caching', [('raw'), ('cached')])
  @pytest.mark.parametrize("filename,index,length,mood,prov_file,prov_line,prov_col", [
      # Test results of compositing with the (<) prepend directive
@@ -195,14 +214,15 @@ def test_composite_preserve_originals(datafiles):
      ('implicitoverwrite.yaml', 0, 2, 'overwrite1', 'implicitoverwrite.yaml', 4, 8),
      ('implicitoverwrite.yaml', 1, 2, 'overwrite2', 'implicitoverwrite.yaml', 6, 8),
  ])
 -def test_list_composition(datafiles, filename,
 +def test_list_composition(datafiles, filename, tmpdir,
                            index, length, mood,
 -                          prov_file, prov_line, prov_col):
 -    base = os.path.join(datafiles.dirname, datafiles.basename, 'basics.yaml')
 -    overlay = os.path.join(datafiles.dirname, datafiles.basename, filename)
 +                          prov_file, prov_line, prov_col, caching):
 +    base_file = os.path.join(datafiles.dirname, datafiles.basename, 'basics.yaml')
 +    overlay_file = os.path.join(datafiles.dirname, datafiles.basename, filename)
++
 +    base = load_yaml_file(base_file, cache_path=tmpdir, shortname='basics.yaml', from_cache=caching)
 +    overlay = load_yaml_file(overlay_file, cache_path=tmpdir, shortname=filename, from_cache=caching)
 -    base = _yaml.load(base, shortname='basics.yaml')
 -    overlay = _yaml.load(overlay, shortname=filename)
      _yaml.composite_dict(base, overlay)
      children = _yaml.node_get(base, list, 'children')
@@ -254,6 +274,7 @@ def test_list_deletion(datafiles):
  #    prov_col: The expected provenance column of "mood"
+ #
  @pytest.mark.datafiles(os.path.join(DATA_DIR))
 +@pytest.mark.parametrize('caching', [('raw'), ('cached')])
  @pytest.mark.parametrize("filename1,filename2,index,length,mood,prov_file,prov_line,prov_col", [
      # Test results of compositing literal list with (>) and then (<)
@@ -310,9 +331,9 @@ def test_list_deletion(datafiles):
      ('listoverwrite.yaml', 'listprepend.yaml', 2, 4, 'overwrite1', 'listoverwrite.yaml', 5, 10),
      ('listoverwrite.yaml', 'listprepend.yaml', 3, 4, 'overwrite2', 'listoverwrite.yaml', 7, 10),
  ])
 -def test_list_composition_twice(datafiles, filename1, filename2,
 +def test_list_composition_twice(datafiles, tmpdir, filename1, filename2,
                                  index, length, mood,
 -                                prov_file, prov_line, prov_col):
 +                                prov_file, prov_line, prov_col, caching):
      file_base = os.path.join(datafiles.dirname, datafiles.basename, 'basics.yaml')
      file1 = os.path.join(datafiles.dirname, datafiles.basename, filename1)
      file2 = os.path.join(datafiles.dirname, datafiles.basename, filename2)
@@ -320,9 +341,9 @@ def test_list_composition_twice(datafiles, filename1, filename2,
      #####################
      # Round 1 - Fight !
      #####################
 -    base = _yaml.load(file_base, shortname='basics.yaml')
 -    overlay1 = _yaml.load(file1, shortname=filename1)
 -    overlay2 = _yaml.load(file2, shortname=filename2)
 +    base = load_yaml_file(file_base, cache_path=tmpdir, shortname='basics.yaml', from_cache=caching)
 +    overlay1 = load_yaml_file(file1, cache_path=tmpdir, shortname=filename1, from_cache=caching)
 +    overlay2 = load_yaml_file(file2, cache_path=tmpdir, shortname=filename2, from_cache=caching)
      _yaml.composite_dict(base, overlay1)
      _yaml.composite_dict(base, overlay2)
@@ -337,9 +358,9 @@ def test_list_composition_twice(datafiles, filename1, filename2,
      #####################
      # Round 2 - Fight !
      #####################
 -    base = _yaml.load(file_base, shortname='basics.yaml')
 -    overlay1 = _yaml.load(file1, shortname=filename1)
 -    overlay2 = _yaml.load(file2, shortname=filename2)
 +    base = load_yaml_file(file_base, cache_path=tmpdir, shortname='basics.yaml', from_cache=caching)
 +    overlay1 = load_yaml_file(file1, cache_path=tmpdir, shortname=filename1, from_cache=caching)
 +    overlay2 = load_yaml_file(file2, cache_path=tmpdir, shortname=filename2, from_cache=caching)
      _yaml.composite_dict(overlay1, overlay2)
      _yaml.composite_dict(base, overlay1)

[Notes] [Git][BuildStream/buildstream][lachlan/pickle-yaml-test-list-composite] 20 commits: bst-docker-import: Consistently use stderr for all logs

Lachlan pushed to branch lachlan/pickle-yaml-test-list-composite at BuildStream / buildstream

Commits:

9 changed files:

Changes: