From 6af5dfbbc2ba2ed12b8a6968907240dd50288a3b Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 3 Sep 2018 23:46:25 -0700 Subject: config: allow env.yaml to contain configuration in a single file - add `SingleFileScope` to configuration, which allows us to pull config sections from a single file. - update `env.yaml` and tests to ensure that the env.yaml schema works when pulling configurtion from the env file. --- lib/spack/llnl/util/lang.py | 20 +++++++ lib/spack/llnl/util/tty/colify.py | 4 +- lib/spack/spack/cmd/env.py | 15 +++-- lib/spack/spack/config.py | 75 ++++++++++++++++++++++- lib/spack/spack/environment.py | 27 ++------- lib/spack/spack/schema/env.py | 49 +++++++++------ lib/spack/spack/schema/merged.py | 40 +++++++++++++ lib/spack/spack/test/config.py | 122 +++++++++++++++++++++++++++++--------- 8 files changed, 271 insertions(+), 81 deletions(-) create mode 100644 lib/spack/spack/schema/merged.py (limited to 'lib') diff --git a/lib/spack/llnl/util/lang.py b/lib/spack/llnl/util/lang.py index 92a9222f27..b2d582db5a 100644 --- a/lib/spack/llnl/util/lang.py +++ b/lib/spack/llnl/util/lang.py @@ -146,6 +146,26 @@ def has_method(cls, name): return False +def union_dicts(*dicts): + """Use update() to combine all dicts into one. + + This builds a new dictionary, into which we ``update()`` each element + of ``dicts`` in order. Items from later dictionaries will override + items from earlier dictionaries. + + Args: + dicts (list): list of dictionaries + + Return: (dict): a merged dictionary containing combined keys and + values from ``dicts``. + + """ + result = {} + for d in dicts: + result.update(d) + return result + + class memoized(object): """Decorator that caches the results of a function, storing them in an attribute of that function.""" diff --git a/lib/spack/llnl/util/tty/colify.py b/lib/spack/llnl/util/tty/colify.py index c329193fba..b6cf15a574 100644 --- a/lib/spack/llnl/util/tty/colify.py +++ b/lib/spack/llnl/util/tty/colify.py @@ -10,7 +10,7 @@ from __future__ import division import os import sys -from six import StringIO +from six import StringIO, text_type from llnl.util.tty import terminal_size from llnl.util.tty.color import clen, cextra @@ -137,7 +137,7 @@ def colify(elts, **options): % next(options.iterkeys())) # elts needs to be an array of strings so we can count the elements - elts = [str(elt) for elt in elts] + elts = [text_type(elt) for elt in elts] if not elts: return (0, ()) diff --git a/lib/spack/spack/cmd/env.py b/lib/spack/spack/cmd/env.py index 391b43c52f..aa48724ff4 100644 --- a/lib/spack/spack/cmd/env.py +++ b/lib/spack/spack/cmd/env.py @@ -44,17 +44,16 @@ subcommands = [ # =============== Modifies Environment def setup_create_parser(subparser): - """create a new environment""" + """create a new environment.""" + subparser.add_argument('env', help='name of environment to create') subparser.add_argument( - '--init-file', dest='init_file', - help='File with user specs to add and configuration yaml to use') + 'envfile', nargs='?', help='optional initialization file') def environment_create(args): - if os.path.exists(ev.root(args.environment)): - raise tty.die("Environment already exists: " + args.environment) - - _environment_create(args.environment) + if os.path.exists(ev.root(args.env)): + raise tty.die("Environment already exists: " + args.env) + _environment_create(args.env) def _environment_create(name, init_config=None): @@ -310,7 +309,6 @@ def add_use_repo_argument(cmd_parser): def setup_parser(subparser): - subparser.add_argument('environment', help="name of environment") sp = subparser.add_subparsers( metavar='SUBCOMMAND', dest='environment_command') @@ -324,6 +322,7 @@ def setup_parser(subparser): def env(parser, args, **kwargs): """Look for a function called environment_ and call it.""" + function_name = 'environment_%s' % args.environment_command action = globals()[function_name] action(args) diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index 0d2d5046a9..39e3e339e3 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -177,6 +177,8 @@ class ConfigScope(object): def write_section(self, section): filename = self.get_section_filename(section) data = self.get_section(section) + _validate(data, section_schemas[section]) + try: mkdirp(self.path) with open(filename, 'w') as f: @@ -194,6 +196,77 @@ class ConfigScope(object): return '' % (self.name, self.path) +class SingleFileScope(ConfigScope): + """This class represents a configuration scope in a single YAML file.""" + def __init__(self, name, path, schema, yaml_path=None): + """Similar to ``ConfigScope`` but can be embedded in another schema. + + Arguments: + schema (dict): jsonschema for the file to read + yaml_path (list): list of dict keys in the schema where + config data can be found. + + """ + super(SingleFileScope, self).__init__(name, path) + self._raw_data = None + self.schema = schema + self.yaml_path = yaml_path or [] + + def get_section_filename(self, section): + return self.path + + def get_section(self, section): + # read raw data from the file, which looks like: + # { + # 'config': { + # ... data ... + # }, + # 'packages': { + # ... data ... + # }, + # } + if self._raw_data is None: + self._raw_data = _read_config_file(self.path, self.schema) + if self._raw_data is None: + return None + + for key in self.yaml_path: + self._raw_data = self._raw_data[key] + + # data in self.sections looks (awkwardly) like this: + # { + # 'config': { + # 'config': { + # ... data ... + # } + # }, + # 'packages': { + # 'packages': { + # ... data ... + # } + # } + # } + return self.sections.setdefault( + section, {section: self._raw_data.get(section)}) + + def write_section(self, section): + _validate(self.sections, self.schema) + try: + parent = os.path.dirname(self.path) + mkdirp(parent) + + tmp = os.path.join(parent, '.%s.tmp' % self.path) + with open(tmp, 'w') as f: + syaml.dump(self.sections, stream=f, default_flow_style=False) + os.path.move(tmp, self.path) + except (yaml.YAMLError, IOError) as e: + raise ConfigFileError( + "Error writing to config file: '%s'" % str(e)) + + def __repr__(self): + return '' % (self.name, self.path) + + class ImmutableConfigScope(ConfigScope): """A configuration scope that cannot be written to. @@ -215,7 +288,7 @@ class InternalConfigScope(ConfigScope): override settings from files. """ def __init__(self, name, data=None): - self.name = name + super(InternalConfigScope, self).__init__(name, None) self.sections = syaml.syaml_dict() if data: diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py index 5b745c6c9b..5b8d5b3a93 100644 --- a/lib/spack/spack/environment.py +++ b/lib/spack/spack/environment.py @@ -1,27 +1,8 @@ -############################################################################## -# Copyright (c) 2013-2018, Lawrence Livermore National Security, LLC. -# Produced at the Lawrence Livermore National Laboratory. +# Copyright 2013-2018 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. # -# This file is part of Spack. -# Created by Todd Gamblin, tgamblin@llnl.gov, All rights reserved. -# LLNL-CODE-647188 -# -# For details, see https://github.com/spack/spack -# Please also see the NOTICE and LICENSE files 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 Lesser General Public License (as -# published by the Free Software Foundation) version 2.1, 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 Lesser 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 -############################################################################## +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + import os import sys import shutil diff --git a/lib/spack/spack/schema/env.py b/lib/spack/spack/schema/env.py index 676d65ec62..7b6ad602f7 100644 --- a/lib/spack/spack/schema/env.py +++ b/lib/spack/spack/schema/env.py @@ -6,41 +6,52 @@ """Schema for env.yaml configuration file. .. literalinclude:: ../spack/schema/env.py - :lines: 32- + :lines: 36- """ +from llnl.util.lang import union_dicts + +import spack.schema.merged +import spack.schema.modules schema = { '$schema': 'http://json-schema.org/schema#', 'title': 'Spack environment file schema', + 'definitions': spack.schema.modules.definitions, 'type': 'object', 'additionalProperties': False, 'patternProperties': { '^env|spack$': { 'type': 'object', 'default': {}, - 'properties': { - 'include': { - 'type': 'array', - 'items': { - 'type': 'string' + 'additionalProperties': False, + 'properties': union_dicts( + # merged configuration scope schemas + spack.schema.merged.properties, + # extra environment schema properties + { + 'include': { + 'type': 'array', + 'items': { + 'type': 'string' + }, }, - }, - - - 'specs': { - 'type': 'object', - 'default': {}, - 'additionalProperties': False, - 'patternProperties': { - r'\w[\w-]*': { # user spec - 'type': 'object', - 'default': {}, - 'additionalProperties': False, + 'specs': { + # Specs is a list of specs, which can have + # optional additional properties in a sub-dict + 'type': 'array', + 'default': [], + 'additionalProperties': False, + 'items': { + 'anyOf': [ + {'type': 'string'}, + {'type': 'null'}, + {'type': 'object'}, + ] } } } - } + ) } } } diff --git a/lib/spack/spack/schema/merged.py b/lib/spack/spack/schema/merged.py new file mode 100644 index 0000000000..4505728076 --- /dev/null +++ b/lib/spack/spack/schema/merged.py @@ -0,0 +1,40 @@ +# Copyright 2013-2018 Lawrence Livermore National Security, LLC and other +# Spack Project Developers. See the top-level COPYRIGHT file for details. +# +# SPDX-License-Identifier: (Apache-2.0 OR MIT) + +"""Schema for configuration merged into one file. + +.. literalinclude:: ../spack/schema/merged.py + :lines: 40- +""" +from llnl.util.lang import union_dicts + +import spack.schema.compilers +import spack.schema.config +import spack.schema.mirrors +import spack.schema.modules +import spack.schema.packages +import spack.schema.repos + + +#: Properties for inclusion in other schemas +properties = union_dicts( + spack.schema.compilers.properties, + spack.schema.config.properties, + spack.schema.mirrors.properties, + spack.schema.modules.properties, + spack.schema.packages.properties, + spack.schema.repos.properties +) + + +#: Full schema with metadata +schema = { + '$schema': 'http://json-schema.org/schema#', + 'title': 'Spack merged configuration file schema', + 'definitions': spack.schema.modules.definitions, + 'type': 'object', + 'additionalProperties': False, + 'properties': properties, +} diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py index 3f58a9c799..1a162bd598 100644 --- a/lib/spack/spack/test/config.py +++ b/lib/spack/spack/test/config.py @@ -18,6 +18,7 @@ import spack.paths import spack.config import spack.schema.compilers import spack.schema.config +import spack.schema.env import spack.schema.packages import spack.schema.mirrors import spack.schema.repos @@ -49,7 +50,7 @@ config_override_list = { @pytest.fixture() -def config(tmpdir): +def mock_config(tmpdir): """Mocks the configuration scope.""" real_configuration = spack.config.config @@ -216,7 +217,7 @@ def compiler_specs(): return CompilerSpecs(a=a, b=b) -def test_write_key_in_memory(config, compiler_specs): +def test_write_key_in_memory(mock_config, compiler_specs): # Write b_comps "on top of" a_comps. spack.config.set('compilers', a_comps['compilers'], scope='low') spack.config.set('compilers', b_comps['compilers'], scope='high') @@ -226,7 +227,7 @@ def test_write_key_in_memory(config, compiler_specs): check_compiler_config(b_comps['compilers'], *compiler_specs.b) -def test_write_key_to_disk(config, compiler_specs): +def test_write_key_to_disk(mock_config, compiler_specs): # Write b_comps "on top of" a_comps. spack.config.set('compilers', a_comps['compilers'], scope='low') spack.config.set('compilers', b_comps['compilers'], scope='high') @@ -239,7 +240,7 @@ def test_write_key_to_disk(config, compiler_specs): check_compiler_config(b_comps['compilers'], *compiler_specs.b) -def test_write_to_same_priority_file(config, compiler_specs): +def test_write_to_same_priority_file(mock_config, compiler_specs): # Write b_comps in the same file as a_comps. spack.config.set('compilers', a_comps['compilers'], scope='low') spack.config.set('compilers', b_comps['compilers'], scope='low') @@ -260,7 +261,7 @@ repos_high = {'repos': ["/some/other/path"]} # repos -def test_write_list_in_memory(config): +def test_write_list_in_memory(mock_config): spack.config.set('repos', repos_low['repos'], scope='low') spack.config.set('repos', repos_high['repos'], scope='high') @@ -268,7 +269,7 @@ def test_write_list_in_memory(config): assert config == repos_high['repos'] + repos_low['repos'] -def test_substitute_config_variables(config): +def test_substitute_config_variables(mock_config): prefix = spack.paths.prefix.lstrip('/') assert os.path.join( @@ -328,7 +329,7 @@ packages_merge_high = { @pytest.mark.regression('7924') -def test_merge_with_defaults(config, write_config_file): +def test_merge_with_defaults(mock_config, write_config_file): """This ensures that specified preferences merge with defaults as expected. Originally all defaults were initialized with the exact same object, which led to aliasing problems. Therefore @@ -344,14 +345,14 @@ def test_merge_with_defaults(config, write_config_file): assert cfg['baz']['version'] == ['c'] -def test_substitute_user(config): +def test_substitute_user(mock_config): user = getpass.getuser() assert '/foo/bar/' + user + '/baz' == canonicalize_path( '/foo/bar/$user/baz' ) -def test_substitute_tempdir(config): +def test_substitute_tempdir(mock_config): tempdir = tempfile.gettempdir() assert tempdir == canonicalize_path('$tempdir') assert tempdir + '/foo/bar/baz' == canonicalize_path( @@ -359,12 +360,12 @@ def test_substitute_tempdir(config): ) -def test_read_config(config, write_config_file): +def test_read_config(mock_config, write_config_file): write_config_file('config', config_low, 'low') assert spack.config.get('config') == config_low['config'] -def test_read_config_override_all(config, write_config_file): +def test_read_config_override_all(mock_config, write_config_file): write_config_file('config', config_low, 'low') write_config_file('config', config_override_all, 'high') assert spack.config.get('config') == { @@ -372,7 +373,7 @@ def test_read_config_override_all(config, write_config_file): } -def test_read_config_override_key(config, write_config_file): +def test_read_config_override_key(mock_config, write_config_file): write_config_file('config', config_low, 'low') write_config_file('config', config_override_key, 'high') assert spack.config.get('config') == { @@ -381,7 +382,7 @@ def test_read_config_override_key(config, write_config_file): } -def test_read_config_merge_list(config, write_config_file): +def test_read_config_merge_list(mock_config, write_config_file): write_config_file('config', config_low, 'low') write_config_file('config', config_merge_list, 'high') assert spack.config.get('config') == { @@ -390,7 +391,7 @@ def test_read_config_merge_list(config, write_config_file): } -def test_read_config_override_list(config, write_config_file): +def test_read_config_override_list(mock_config, write_config_file): write_config_file('config', config_low, 'low') write_config_file('config', config_override_list, 'high') assert spack.config.get('config') == { @@ -399,33 +400,33 @@ def test_read_config_override_list(config, write_config_file): } -def test_internal_config_update(config, write_config_file): +def test_internal_config_update(mock_config, write_config_file): write_config_file('config', config_low, 'low') - before = config.get('config') + before = mock_config.get('config') assert before['install_tree'] == 'install_tree_path' # add an internal configuration scope scope = spack.config.InternalConfigScope('command_line') assert 'InternalConfigScope' in repr(scope) - config.push_scope(scope) + mock_config.push_scope(scope) - command_config = config.get('config', scope='command_line') + command_config = mock_config.get('config', scope='command_line') command_config['install_tree'] = 'foo/bar' - config.set('config', command_config, scope='command_line') + mock_config.set('config', command_config, scope='command_line') - after = config.get('config') + after = mock_config.get('config') assert after['install_tree'] == 'foo/bar' -def test_internal_config_filename(config, write_config_file): +def test_internal_config_filename(mock_config, write_config_file): write_config_file('config', config_low, 'low') - config.push_scope(spack.config.InternalConfigScope('command_line')) + mock_config.push_scope(spack.config.InternalConfigScope('command_line')) with pytest.raises(NotImplementedError): - config.get_config_filename('command_line', 'config') + mock_config.get_config_filename('command_line', 'config') def test_mark_internal(): @@ -597,7 +598,7 @@ mirrors: assert "mirrors.yaml:5" in str(e) -def test_bad_config_section(config): +def test_bad_config_section(mock_config): """Test that getting or setting a bad section gives an error.""" with pytest.raises(spack.config.ConfigSectionError): spack.config.set('foobar', 'foobar') @@ -606,7 +607,7 @@ def test_bad_config_section(config): spack.config.get('foobar') -def test_bad_command_line_scopes(tmpdir, config): +def test_bad_command_line_scopes(tmpdir, mock_config): cfg = spack.config.Configuration() with tmpdir.as_cwd(): @@ -631,9 +632,9 @@ def test_add_command_line_scopes(tmpdir, mutable_config): with open(config_yaml, 'w') as f: f.write("""\ config: - verify_ssh: False + verify_ssl: False dirty: False -"""'') +""") spack.config._add_command_line_scopes(mutable_config, [str(tmpdir)]) @@ -644,7 +645,7 @@ def test_immutable_scope(tmpdir): f.write("""\ config: install_tree: dummy_tree_value -"""'') +""") scope = spack.config.ImmutableConfigScope('test', str(tmpdir)) data = scope.get_section('config') @@ -654,6 +655,38 @@ config: scope.write_section('config') +def test_single_file_scope(tmpdir, config): + env_yaml = str(tmpdir.join("env.yaml")) + with open(env_yaml, 'w') as f: + f.write("""\ +env: + config: + verify_ssl: False + dirty: False + packages: + libelf: + compiler: [ 'gcc@4.5.3' ] + repos: + - /x/y/z +""") + + scope = spack.config.SingleFileScope( + 'env', env_yaml, spack.schema.env.schema, ['env']) + + with spack.config.override(scope): + # from the single-file config + assert spack.config.get('config:verify_ssl') is False + assert spack.config.get('config:dirty') is False + assert spack.config.get('packages:libelf:compiler') == ['gcc@4.5.3'] + + # from the lower config scopes + assert spack.config.get('config:checksum') is True + assert spack.config.get('config:checksum') is True + assert spack.config.get('packages:externalmodule:buildable') is False + assert spack.config.get('repos') == [ + '/x/y/z', '$spack/var/spack/repos/builtin'] + + def check_schema(name, file_contents): """Check a Spack YAML schema against some data""" f = StringIO(file_contents) @@ -661,6 +694,39 @@ def check_schema(name, file_contents): spack.config._validate(data, name) +def test_good_env_yaml(tmpdir): + check_schema(spack.schema.env.schema, """\ +spack: + config: + verify_ssl: False + dirty: False + repos: + - ~/my/repo/location + mirrors: + remote: /foo/bar/baz + compilers: + - compiler: + spec: cce@2.1 + operating_system: cnl + modules: [] + paths: + cc: /path/to/cc + cxx: /path/to/cxx + fc: /path/to/fc + f77: /path/to/f77 +""") + + +def test_bad_env_yaml(tmpdir): + with pytest.raises(spack.config.ConfigFormatError): + check_schema(spack.schema.env.schema, """\ +env: + foobar: + verify_ssl: False + dirty: False +""") + + def test_bad_config_yaml(tmpdir): with pytest.raises(spack.config.ConfigFormatError): check_schema(spack.schema.config.schema, """\ -- cgit v1.2.3-60-g2f50