[Notes] [Git][BuildStream/buildstream][jonathan/pickle-yaml] 5 commits:

Jonathan Maw pushed to branch jonathan/pickle-yaml at BuildStream / buildstream

Commits:

68ef69e4

by Tristan Van Berkom at 2018-09-21T05:20:46Z

source.py: Fix re-instantiation

This fixes an issue where the re-instantiated Source used with
Source mirroring enabled is not completely initialized.

Failing to load the ref from the project.refs file for instance,
will result in a crash at `fetch` time.

This fixes issue #666

662c729f

by Tristan Van Berkom at 2018-09-21T05:59:30Z

Merge branch 'tristan/fix-source-reinstantiation' into 'master'

source.py: Fix re-instantiation

Closes #666

See merge request BuildStream/buildstream!817

a6a1fca4

by Jonathan Maw at 2018-09-21T09:02:00Z

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.

1797ce99

by Jonathan Maw at 2018-09-21T09:02:00Z

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.

e29e78f4

by Jonathan Maw at 2018-09-27T14:38:10Z

yamlcache: Support files being added without projects

Changes:

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

@@ -30,6 +30,7 @@ from ..element import Element
  from .._profile import Topics, profile_start, profile_end
  from .._platform import Platform
  from .._includes import Includes
 +from .._yamlcache import YamlCache
  from .types import Symbol, Dependency
  from .loadelement import LoadElement
@@ -113,7 +114,8 @@ 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)
 +            with YamlCache.open(self._context) 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)
@@ -202,11 +204,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:
@@ -219,7 +222,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
@@ -262,13 +266,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,35 @@ 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:
 +            assert project
 +            key = yaml_cache.calculate_key(contents, copy_tree)
 +            data = yaml_cache.get(project, filename, key)
++
 +        if not data:
 +            data = load_data(contents, file, copy_tree=copy_tree)
++
 +        if yaml_cache:
 +            assert project
 +            yaml_cache.put(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
++
 +    # Writes the yaml cache to the specified path.
 +    def write(self):
 +        path = self._get_cache_file(self._context)
 +        parent_dir = os.path.dirname(path)
 +        os.makedirs(parent_dir, exist_ok=True)
 +        with open(path, "wb") as f:
 +            BstPickler(f).dump(self)
++
 +    # Gets a parsed file from the cache.
 +    #
 +    # 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
++
 +    # Put a parsed file into the cache.
 +    #
 +    # 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(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)
++
 +    # 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
++
 +    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
++
 +    # Return an instance of the YamlCache which writes to disk when it leaves scope.
 +    #
 +    # Args:
 +    #    context (Context): The context.
 +    #
 +    # Returns:
 +    #    (YamlCache): A YamlCache.
 +    @staticmethod
 +    @contextmanager
 +    def open(context):
 +        # Try to load from disk first
 +        cachefile = YamlCache._get_cache_file(context)
 +        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))
++
 +        if not cache:
 +            cache = YamlCache(context)
++
 +        yield cache
++
 +        cache.write()
++
 +    # Calculates a key for putting into the cache.
 +    @staticmethod
 +    def calculate_key(*args):
 +        string = pickle.dumps(args)
 +        return hashlib.sha1(string).hexdigest()
++
 +    # Retrieves a path to the yaml cache file.
 +    @staticmethod
 +    def _get_cache_file(context):
 +        toplevel_project = context.get_toplevel_project()
 +        return os.path.join(toplevel_project.directory, ".bst", YAML_CACHE_FILENAME)
++
++
 +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()

buildstream/source.py

@@ -930,6 +930,38 @@ class Source(Plugin):
      #                   Local Private Methods                   #
      #############################################################
 +    # __clone_for_uri()
 +    #
 +    # Clone the source with an alternative URI setup for the alias
 +    # which this source uses.
 +    #
 +    # This is used for iteration over source mirrors.
 +    #
 +    # Args:
 +    #    uri (str): The alternative URI for this source's alias
 +    #
 +    # Returns:
 +    #    (Source): A new clone of this Source, with the specified URI
 +    #              as the value of the alias this Source has marked as
 +    #              primary with either mark_download_url() or
 +    #              translate_url().
 +    #
 +    def __clone_for_uri(self, uri):
 +        project = self._get_project()
 +        context = self._get_context()
 +        alias = self._get_alias()
 +        source_kind = type(self)
++
 +        clone = source_kind(context, project, self.__meta, alias_override=(alias, uri))
++
 +        # Do the necessary post instantiation routines here
 +        #
 +        clone._preflight()
 +        clone._load_ref()
 +        clone._update_state()
++
 +        return clone
++
      # Tries to call fetch for every mirror, stopping once it succeeds
      def __do_fetch(self, **kwargs):
          project = self._get_project()
@@ -968,12 +1000,8 @@ class Source(Plugin):
                  self.fetch(**kwargs)
                  return
 -            context = self._get_context()
 -            source_kind = type(self)
              for uri in project.get_alias_uris(alias, first_pass=self.__first_pass):
 -                new_source = source_kind(context, project, self.__meta,
 -                                         alias_override=(alias, uri))
 -                new_source._preflight()
 +                new_source = self.__clone_for_uri(uri)
                  try:
                      new_source.fetch(**kwargs)
                  # FIXME: Need to consider temporary vs. permanent failures,
@@ -1006,9 +1034,7 @@ class Source(Plugin):
          # NOTE: We are assuming here that tracking only requires substituting the
          #       first alias used
          for uri in reversed(project.get_alias_uris(alias, first_pass=self.__first_pass)):
 -            new_source = source_kind(context, project, self.__meta,
 -                                     alias_override=(alias, uri))
 -            new_source._preflight()
 +            new_source = self.__clone_for_uri(uri)
              try:
                  ref = new_source.track(**kwargs)
              # FIXME: Need to consider temporary vs. permanent failures,

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)
 +    with YamlCache.open(context) 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):
 +    with open(input_file) as f:
 +        data = f.read()
 +    assert 'variables' not in data
 +    data += '\nvariables: {modified: True}\n'
 +    _, temppath = tempfile.mkstemp(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)
 +        contents = _yaml.load(temppath, copy_tree=False, project=prj)
 +        key = yamlcache_key(yc, element_path)
 +        yc.put(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):
 +    # inline and junction can only be changed by opening a workspace
 +    pass
++
++
 +@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_track_changed(cli, ref_storage, with_junction, move_project):
 +    # Skip inline junction, tracking those is forbidden
 +    pass

[Notes] [Git][BuildStream/buildstream][jonathan/pickle-yaml] 5 commits: source.py: Fix re-instantiation

Jonathan Maw pushed to branch jonathan/pickle-yaml at BuildStream / buildstream

Commits:

7 changed files:

Changes: