From 39e360f93a374314c28716ded3b1533d66cd62be Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Sun, 27 Dec 2015 17:51:11 -0800 Subject: Add custom YAML loader & dumper to track lines & maintain dict order. - Configs are now parsed with `spack.util.spack_yaml.load/dump` - Parser annotates returned data with `_start_mark` and `_end_mark` properties, so that we can recover what lines/files they came from. - Parser uses `OrderedDict` instead of `dict`. This will help maintain some sanity when round-tripping config files. --- lib/spack/spack/config.py | 77 ++++++----- lib/spack/spack/test/__init__.py | 3 +- lib/spack/spack/test/config.py | 8 +- lib/spack/spack/test/mock_packages_test.py | 12 +- lib/spack/spack/test/yaml.py | 93 +++++++++++++ lib/spack/spack/util/spack_yaml.py | 201 +++++++++++++++++++++++++++++ 6 files changed, 351 insertions(+), 43 deletions(-) create mode 100644 lib/spack/spack/test/yaml.py create mode 100644 lib/spack/spack/util/spack_yaml.py (limited to 'lib') diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index aa6afd183e..9e3b44085f 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -1,5 +1,5 @@ ############################################################################## -# Copyright (c) 2013, Lawrence Livermore National Security, LLC. +# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC. # Produced at the Lawrence Livermore National Laboratory. # # This file is part of Spack. @@ -120,8 +120,10 @@ the site configuration will be ignored. import os import sys import copy + import yaml from yaml.error import MarkedYAMLError +from ordereddict_backport import OrderedDict import llnl.util.tty as tty from llnl.util.filesystem import mkdirp @@ -129,11 +131,20 @@ from llnl.util.filesystem import mkdirp import spack from spack.error import SpackError -"""List of valid config sections.""" -valid_sections = ('compilers', 'mirrors', 'repos') +# Hacked yaml for configuration files preserves line numbers. +import spack.util.spack_yaml as syaml + + +"""Dict from section names -> function to check section YAML format.""" +valid_sections = ['compilers', 'mirrors', 'repos'] + +"""OrderedDict of config scopes keyed by name. + Later scopes will override earlier scopes. +""" +config_scopes = OrderedDict() -def check_section(section): +def validate_section(section): """Raise a ValueError if the section is not a valid section.""" if section not in valid_sections: raise ValueError("Invalid config section: '%s'. Options are %s." @@ -146,14 +157,19 @@ class ConfigScope(object): A scope is one directory containing named configuration files. Each file is a config "section" (e.g., mirrors, compilers, etc). """ + def __init__(self, name, path): self.name = name # scope name. self.path = path # path to directory containing configs. self.sections = {} # sections read from config files. + # Register in a dict of all ConfigScopes + # TODO: make this cleaner. Mocking up for testing is brittle. + global config_scopes + config_scopes[name] = self def get_section_filename(self, section): - check_section(section) + validate_section(section) return os.path.join(self.path, "%s.yaml" % section) @@ -161,7 +177,10 @@ class ConfigScope(object): if not section in self.sections: path = self.get_section_filename(section) data = _read_config_file(path) - self.sections[section] = {} if data is None else data + if data is None: + self.sections[section] = {} + else: + self.sections[section] = data return self.sections[section] @@ -171,7 +190,7 @@ class ConfigScope(object): try: mkdirp(self.path) with open(filename, 'w') as f: - yaml.dump(data, stream=f, default_flow_style=False) + syaml.dump(data, stream=f, default_flow_style=False) except (yaml.YAMLError, IOError) as e: raise ConfigFileError("Error writing to config file: '%s'" % str(e)) @@ -181,18 +200,11 @@ class ConfigScope(object): self.sections = {} -"""List of config scopes by name. - Later scopes in the list will override earlier scopes. -""" -config_scopes = [ - ConfigScope('site', os.path.join(spack.etc_path, 'spack')), - ConfigScope('user', os.path.expanduser('~/.spack'))] - -"""List of valid scopes, for convenience.""" -valid_scopes = (s.name for s in config_scopes) +ConfigScope('site', os.path.join(spack.etc_path, 'spack')), +ConfigScope('user', os.path.expanduser('~/.spack')) -def check_scope(scope): +def validate_scope(scope): """Ensure that scope is valid, and return a valid scope if it is None. This should be used by routines in ``config.py`` to validate @@ -202,16 +214,14 @@ def check_scope(scope): """ if scope is None: # default to the scope with highest precedence. - return config_scopes[-1] - elif scope not in valid_scopes: - raise ValueError("Invalid config scope: '%s'. Must be one of %s." - % (scope, valid_scopes)) - return scope + return config_scopes.values()[-1] + elif scope in config_scopes: + return config_scopes[scope] -def get_scope(scope): - scope = check_scope(scope) - return next(s for s in config_scopes if s.name == scope) + else: + raise ValueError("Invalid config scope: '%s'. Must be one of %s." + % (scope, config_scopes.keys())) def _read_config_file(filename): @@ -229,7 +239,7 @@ def _read_config_file(filename): try: with open(filename) as f: - return yaml.load(f) + return syaml.load(f) except MarkedYAMLError, e: raise ConfigFileError( @@ -243,7 +253,7 @@ def _read_config_file(filename): def clear_config_caches(): """Clears the caches for configuration files, which will cause them to be re-read upon the next request""" - for scope in config_scopes: + for scope in config_scopes.values(): scope.clear() @@ -306,10 +316,10 @@ def get_config(section): Strips off the top-level section name from the YAML dict. """ - check_section(section) + validate_section(section) merged_section = {} - for scope in config_scopes: + for scope in config_scopes.values(): # read potentially cached data from the scope. data = scope.get_section(section) if not data or not section in data: @@ -345,7 +355,7 @@ def get_repos_config(): def get_config_filename(scope, section): """For some scope and section, get the name of the configuration file""" - scope = get_scope(scope) + scope = validate_scope(scope) return scope.get_section_filename(section) @@ -363,8 +373,8 @@ def update_config(section, update_data, scope=None): # read in the config to ensure we've got current data get_config(section) - check_section(section) # validate section name - scope = get_scope(scope) # get ConfigScope object from string. + validate_section(section) # validate section name + scope = validate_scope(scope) # get ConfigScope object from string. # read only the requested section's data. data = scope.get_section(section) @@ -382,7 +392,7 @@ def remove_from_config(section, key_to_rm, scope=None): get_config(section) # check args and get the objects we need. - scope = get_scope(scope) + scope = validate_scope(scope) data = scope.get_section(section) filename = scope.get_section_filename(section) @@ -411,3 +421,4 @@ def print_section(section): class ConfigError(SpackError): pass class ConfigFileError(ConfigError): pass +class ConfigFormatError(ConfigError): pass diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py index 081e6c7b06..9609fd2f36 100644 --- a/lib/spack/spack/test/__init__.py +++ b/lib/spack/spack/test/__init__.py @@ -60,7 +60,8 @@ test_names = ['versions', 'unit_install', 'lock', 'database', - 'namespace_trie'] + 'namespace_trie', + 'yaml'] def list_tests(): diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index f56287aa98..5f99dcb903 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -26,6 +26,7 @@ import unittest import shutil import os from tempfile import mkdtemp +from ordereddict_backport import OrderedDict import spack import spack.config from spack.test.mock_packages_test import * @@ -72,10 +73,9 @@ class ConfigTest(MockPackagesTest): def setUp(self): super(ConfigTest, self).setUp() self.tmp_dir = mkdtemp('.tmp', 'spack-config-test-') - spack.config.config_scopes = [ - spack.config.ConfigScope('test_low_priority', os.path.join(self.tmp_dir, 'low')), - spack.config.ConfigScope('test_high_priority', os.path.join(self.tmp_dir, 'high'))] - spack.config.valid_scopes = ('test_low_priority', 'test_high_priority') + spack.config.config_scopes = OrderedDict() + spack.config.ConfigScope('test_low_priority', os.path.join(self.tmp_dir, 'low')) + spack.config.ConfigScope('test_high_priority', os.path.join(self.tmp_dir, 'high')) def tearDown(self): super(ConfigTest, self).tearDown() diff --git a/lib/spack/spack/test/mock_packages_test.py b/lib/spack/spack/test/mock_packages_test.py index d000271960..320c2921b0 100644 --- a/lib/spack/spack/test/mock_packages_test.py +++ b/lib/spack/spack/test/mock_packages_test.py @@ -24,6 +24,7 @@ ############################################################################## import sys import unittest +from ordereddict_backport import OrderedDict import spack import spack.config @@ -41,10 +42,12 @@ class MockPackagesTest(unittest.TestCase): spack.config.clear_config_caches() self.real_scopes = spack.config.config_scopes - self.real_valid_scopes = spack.config.valid_scopes - spack.config.config_scopes = [ - spack.config.ConfigScope('site', spack.mock_site_config), - spack.config.ConfigScope('user', spack.mock_user_config)] + + # TODO: Mocking this up is kind of brittle b/c ConfigScope + # TODO: constructor modifies config_scopes. Make it cleaner. + spack.config.config_scopes = OrderedDict() + spack.config.ConfigScope('site', spack.mock_site_config) + spack.config.ConfigScope('user', spack.mock_user_config) # Store changes to the package's dependencies so we can # restore later. @@ -72,7 +75,6 @@ class MockPackagesTest(unittest.TestCase): """Restore the real packages path after any test.""" spack.repo.swap(self.db) spack.config.config_scopes = self.real_scopes - spack.config.valid_scopes = self.real_valid_scopes spack.config.clear_config_caches() # Restore dependency changes that happened during the test diff --git a/lib/spack/spack/test/yaml.py b/lib/spack/spack/test/yaml.py new file mode 100644 index 0000000000..5a357b8e69 --- /dev/null +++ b/lib/spack/spack/test/yaml.py @@ -0,0 +1,93 @@ +############################################################################## +# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program 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 terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +""" +Test Spack's custom YAML format. +""" +import unittest +import spack.util.spack_yaml as syaml + +test_file = """\ +config_file: + x86_64: + foo: /path/to/foo + bar: /path/to/bar + baz: /path/to/baz + some_list: + - item 1 + - item 2 + - item 3 + another_list: + [ 1, 2, 3 ] + some_key: some_string +""" + +test_data = { + 'config_file' : syaml.syaml_dict([ + ('x86_64', syaml.syaml_dict([ + ('foo', '/path/to/foo'), + ('bar', '/path/to/bar'), + ('baz', '/path/to/baz' )])), + ('some_list', [ 'item 1', 'item 2', 'item 3' ]), + ('another_list', [ 1, 2, 3 ]), + ('some_key', 'some_string') + ])} + +class YamlTest(unittest.TestCase): + + def setUp(self): + self.data = syaml.load(test_file) + + + def test_parse(self): + self.assertEqual(test_data, self.data) + + + def test_dict_order(self): + self.assertEqual( + ['x86_64', 'some_list', 'another_list', 'some_key'], + self.data['config_file'].keys()) + + self.assertEqual( + ['foo', 'bar', 'baz'], + self.data['config_file']['x86_64'].keys()) + + + def test_line_numbers(self): + def check(obj, start_line, end_line): + self.assertEqual(obj._start_mark.line, start_line) + self.assertEqual(obj._end_mark.line, end_line) + + check(self.data, 0, 12) + check(self.data['config_file'], 1, 12) + check(self.data['config_file']['x86_64'], 2, 5) + check(self.data['config_file']['x86_64']['foo'], 2, 2) + check(self.data['config_file']['x86_64']['bar'], 3, 3) + check(self.data['config_file']['x86_64']['baz'], 4, 4) + check(self.data['config_file']['some_list'], 6, 9) + check(self.data['config_file']['some_list'][0], 6, 6) + check(self.data['config_file']['some_list'][1], 7, 7) + check(self.data['config_file']['some_list'][2], 8, 8) + check(self.data['config_file']['another_list'], 10, 10) + check(self.data['config_file']['some_key'], 11, 11) diff --git a/lib/spack/spack/util/spack_yaml.py b/lib/spack/spack/util/spack_yaml.py new file mode 100644 index 0000000000..728e86b8ee --- /dev/null +++ b/lib/spack/spack/util/spack_yaml.py @@ -0,0 +1,201 @@ +############################################################################## +# Copyright (c) 2013-2015, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# This file is part of Spack. +# Written by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. +# LLNL-CODE-647188 +# +# For details, see https://github.com/llnl/spack +# Please also see the LICENSE file for our notice and the LGPL. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License (as published by +# the Free Software Foundation) version 2.1 dated February 1999. +# +# This program 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 terms and +# conditions of the GNU General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public License +# along with this program; if not, write to the Free Software Foundation, +# Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +############################################################################## +"""Enhanced YAML parsing for Spack. + +- ``load()`` preserves YAML Marks on returned objects -- this allows + us to access file and line information later. + +- ``Our load methods use ``OrderedDict`` class instead of YAML's + default unorderd dict. + +""" +import yaml +from yaml.nodes import * +from yaml.constructor import ConstructorError +from yaml.representer import SafeRepresenter +from ordereddict_backport import OrderedDict + +# Only export load and dump +__all__ = ['load', 'dump'] + +# Make new classes so we can add custom attributes. +# Also, use OrderedDict instead of just dict. +class syaml_dict(OrderedDict): + def __repr__(self): + mappings = ('%r: %r' % (k,v) for k,v in self.items()) + return '{%s}' % ', '.join(mappings) +class syaml_list(list): + __repr__ = list.__repr__ +class syaml_str(str): + __repr__ = str.__repr__ + +def mark(obj, node): + """Add start and end markers to an object.""" + obj._start_mark = node.start_mark + obj._end_mark = node.end_mark + + +class OrderedLineLoader(yaml.Loader): + """YAML loader that preserves order and line numbers. + + Mappings read in by this loader behave like an ordered dict. + Sequences, mappings, and strings also have new attributes, + ``_start_mark`` and ``_end_mark``, that preserve YAML line + information in the output data. + + """ + # + # Override construct_yaml_* so that they build our derived types, + # which allows us to add new attributes to them. + # + # The standard YAML constructors return empty instances and fill + # in with mappings later. We preserve this behavior. + # + def construct_yaml_str(self, node): + value = self.construct_scalar(node) + try: + value = value.encode('ascii') + except UnicodeEncodeError: + pass + value = syaml_str(value) + mark(value, node) + return value + + + def construct_yaml_seq(self, node): + data = syaml_list() + mark(data, node) + yield data + data.extend(self.construct_sequence(node)) + + + def construct_yaml_map(self, node): + data = syaml_dict() + mark(data, node) + yield data + value = self.construct_mapping(node) + data.update(value) + + # + # Override the ``construct_*`` routines. These fill in empty + # objects after yielded by the above ``construct_yaml_*`` methods. + # + def construct_sequence(self, node, deep=False): + if not isinstance(node, SequenceNode): + raise ConstructorError(None, None, + "expected a sequence node, but found %s" % node.id, + node.start_mark) + value = syaml_list(self.construct_object(child, deep=deep) + for child in node.value) + mark(value, node) + return value + + + def construct_mapping(self, node, deep=False): + """Store mappings as OrderedDicts instead of as regular python + dictionaries to preserve file ordering.""" + if not isinstance(node, MappingNode): + raise ConstructorError(None, None, + "expected a mapping node, but found %s" % node.id, + node.start_mark) + + mapping = syaml_dict() + for key_node, value_node in node.value: + key = self.construct_object(key_node, deep=deep) + try: + hash(key) + except TypeError, exc: + raise ConstructorError("while constructing a mapping", node.start_mark, + "found unacceptable key (%s)" % exc, key_node.start_mark) + value = self.construct_object(value_node, deep=deep) + if key in mapping: + raise ConstructorError("while constructing a mapping", node.start_mark, + "found already in-use key (%s)" % key, key_node.start_mark) + mapping[key] = value + + mark(mapping, node) + return mapping + +# register above new constructors +OrderedLineLoader.add_constructor(u'tag:yaml.org,2002:map', OrderedLineLoader.construct_yaml_map) +OrderedLineLoader.add_constructor(u'tag:yaml.org,2002:seq', OrderedLineLoader.construct_yaml_seq) +OrderedLineLoader.add_constructor(u'tag:yaml.org,2002:str', OrderedLineLoader.construct_yaml_str) + + + +class OrderedLineDumper(yaml.Dumper): + """Dumper that preserves ordering and formats ``syaml_*`` objects. + + This dumper preserves insertion ordering ``syaml_dict`` objects + when they're written out. It also has some custom formatters + for ``syaml_*`` objects so that they are formatted like their + regular Python equivalents, instead of ugly YAML pyobjects. + + """ + def represent_mapping(self, tag, mapping, flow_style=None): + value = [] + node = MappingNode(tag, value, flow_style=flow_style) + if self.alias_key is not None: + self.represented_objects[self.alias_key] = node + best_style = True + if hasattr(mapping, 'items'): + # if it's a syaml_dict, preserve OrderedDict order. + # Otherwise do the default thing. + sort = not isinstance(mapping, syaml_dict) + mapping = mapping.items() + if sort: + mapping.sort() + + for item_key, item_value in mapping: + node_key = self.represent_data(item_key) + node_value = self.represent_data(item_value) + if not (isinstance(node_key, ScalarNode) and not node_key.style): + best_style = False + if not (isinstance(node_value, ScalarNode) and not node_value.style): + best_style = False + value.append((node_key, node_value)) + if flow_style is None: + if self.default_flow_style is not None: + node.flow_style = self.default_flow_style + else: + node.flow_style = best_style + return node + +# Make our special objects look like normal YAML ones. +OrderedLineDumper.add_representer(syaml_dict, OrderedLineDumper.represent_dict) +OrderedLineDumper.add_representer(syaml_list, OrderedLineDumper.represent_list) +OrderedLineDumper.add_representer(syaml_str, OrderedLineDumper.represent_str) + + +def load(*args, **kwargs): + """Load but modify the loader instance so that it will add __line__ + atrributes to the returned object.""" + kwargs['Loader'] = OrderedLineLoader + return yaml.load(*args, **kwargs) + + +def dump(*args, **kwargs): + kwargs['Dumper'] = OrderedLineDumper + return yaml.dump(*args, **kwargs) -- cgit v1.2.3-60-g2f50