From e727f56d89ccd2aa9aabc14cf4efb471f32341dc Mon Sep 17 00:00:00 2001 From: scheibelp Date: Fri, 10 Mar 2017 13:58:48 -0800 Subject: [PATCH] Features/compiler config consistency (#2999) * default scope for config command is made consistent with cmd/__init__ default * dont specify a scope when looking for compilers with a matching spec (since compiler concretization is scope-independent) * config edit should default to platform-specific file only for compilers * when duplicate compiler specs are detected, the exception raised now points the user to the files where the duplicates appear * updated error message to emphasize that a spec is duplicated (since multiple specs can reference the same compiler) * 'spack compilers' is now also broken down into sections by os and target * Added tests for new compiler methods --- lib/spack/spack/cmd/compiler.py | 17 +- lib/spack/spack/cmd/config.py | 5 +- lib/spack/spack/compilers/__init__.py | 173 ++++++++++++------ lib/spack/spack/concretize.py | 2 +- lib/spack/spack/test/cmd/test_compiler_cmd.py | 6 +- lib/spack/spack/test/compilers.py | 48 +++++ 6 files changed, 181 insertions(+), 70 deletions(-) create mode 100644 lib/spack/spack/test/compilers.py diff --git a/lib/spack/spack/cmd/compiler.py b/lib/spack/spack/cmd/compiler.py index c609794185..444e658a92 100644 --- a/lib/spack/spack/cmd/compiler.py +++ b/lib/spack/spack/cmd/compiler.py @@ -96,8 +96,7 @@ def compiler_find(args): for c in compilers: arch_spec = ArchSpec(None, c.operating_system, c.target) same_specs = spack.compilers.compilers_for_spec(c.spec, - arch_spec, - args.scope) + arch_spec) if not same_specs: new_compilers.append(c) @@ -165,14 +164,18 @@ def compiler_info(args): def compiler_list(args): tty.msg("Available compilers") - index = index_by(spack.compilers.all_compilers(scope=args.scope), 'name') - for i, (name, compilers) in enumerate(index.items()): + index = index_by(spack.compilers.all_compilers(scope=args.scope), + lambda c: (c.spec.name, c.operating_system, c.target)) + for i, (key, compilers) in enumerate(index.items()): if i >= 1: print - - cname = "%s{%s}" % (spack.spec.compiler_color, name) + name, os, target = key + os_str = os + if target: + os_str += "-%s" % target + cname = "%s{%s} %s" % (spack.spec.compiler_color, name, os_str) tty.hline(colorize(cname), char='-') - colify(reversed(sorted(compilers))) + colify(reversed(sorted(c.spec for c in compilers))) def compiler(parser, args): diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index 1a9e44a8b9..a647e3ed6e 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -55,7 +55,10 @@ def config_get(args): def config_edit(args): if not args.scope: - args.scope = 'user' + if args.section == 'compilers': + args.scope = spack.cmd.default_modify_scope + else: + args.scope = 'user' if not args.section: args.section = None config_file = spack.config.get_config_filename(args.scope, args.section) diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py index 731acaf9c2..be19841539 100644 --- a/lib/spack/spack/compilers/__init__.py +++ b/lib/spack/spack/compilers/__init__.py @@ -164,7 +164,7 @@ def all_compilers_config(scope=None, init_config=True): return _cache_config_file -def all_compilers(scope=None, init_config=True): +def all_compiler_specs(scope=None, init_config=True): # Return compiler specs from the merged config. return [spack.spec.CompilerSpec(s['compiler']['spec']) for s in all_compilers_config(scope, init_config)] @@ -203,72 +203,93 @@ def supported(compiler_spec): def find(compiler_spec, scope=None): """Return specs of available compilers that match the supplied compiler spec. Return an empty list if nothing found.""" - return [c for c in all_compilers(scope) if c.satisfies(compiler_spec)] + return [c for c in all_compiler_specs(scope) if c.satisfies(compiler_spec)] + + +def all_compilers(scope=None): + config = get_compiler_config(scope) + compilers = list() + for items in config: + items = items['compiler'] + compilers.append(compiler_from_config_entry(items)) + return compilers @_auto_compiler_spec -def compilers_for_spec(compiler_spec, arch_spec=None, scope=None): +def compilers_for_spec(compiler_spec, arch_spec=None, scope=None, + use_cache=True): """This gets all compilers that satisfy the supplied CompilerSpec. Returns an empty list if none are found. """ - config = all_compilers_config(scope) - - def get_compilers(cspec): - compilers = [] - - for items in config: - items = items['compiler'] - if items['spec'] != str(cspec): - continue - - # If an arch spec is given, confirm that this compiler - # is for the given operating system - os = items.get('operating_system', None) - if arch_spec and os != arch_spec.platform_os: - continue - - # If an arch spec is given, confirm that this compiler - # is for the given target. If the target is 'any', match - # any given arch spec. If the compiler has no assigned - # target this is an old compiler config file, skip this logic. - target = items.get('target', None) - if arch_spec and target and (target != arch_spec.target and - target != 'any'): - continue - - if not ('paths' in items and - all(n in items['paths'] for n in _path_instance_vars)): - raise InvalidCompilerConfigurationError(cspec) - - cls = class_for_compiler_name(cspec.name) - - compiler_paths = [] - for c in _path_instance_vars: - compiler_path = items['paths'][c] - if compiler_path != 'None': - compiler_paths.append(compiler_path) - else: - compiler_paths.append(None) - - mods = items.get('modules') - if mods == 'None': - mods = [] - - alias = items.get('alias', None) - compiler_flags = items.get('flags', {}) - environment = items.get('environment', {}) - extra_rpaths = items.get('extra_rpaths', []) - - compilers.append( - cls(cspec, os, target, compiler_paths, mods, alias, - environment, extra_rpaths, **compiler_flags)) - - return compilers + if use_cache: + config = all_compilers_config(scope) + else: + config = get_compiler_config(scope) matches = set(find(compiler_spec, scope)) compilers = [] for cspec in matches: - compilers.extend(get_compilers(cspec)) + compilers.extend(get_compilers(cspec, config, arch_spec)) + return compilers + + +def compiler_from_config_entry(items): + cspec = spack.spec.CompilerSpec(items['spec']) + os = items.get('operating_system', None) + target = items.get('target', None) + + if not ('paths' in items and + all(n in items['paths'] for n in _path_instance_vars)): + raise InvalidCompilerConfigurationError(cspec) + + cls = class_for_compiler_name(cspec.name) + + compiler_paths = [] + for c in _path_instance_vars: + compiler_path = items['paths'][c] + if compiler_path != 'None': + compiler_paths.append(compiler_path) + else: + compiler_paths.append(None) + + mods = items.get('modules') + if mods == 'None': + mods = [] + + alias = items.get('alias', None) + compiler_flags = items.get('flags', {}) + environment = items.get('environment', {}) + extra_rpaths = items.get('extra_rpaths', []) + + return cls(cspec, os, target, compiler_paths, mods, alias, + environment, extra_rpaths, **compiler_flags) + + +def get_compilers(cspec, config, arch_spec=None): + compilers = [] + + for items in config: + items = items['compiler'] + if items['spec'] != str(cspec): + continue + + # If an arch spec is given, confirm that this compiler + # is for the given operating system + os = items.get('operating_system', None) + if arch_spec and os != arch_spec.platform_os: + continue + + # If an arch spec is given, confirm that this compiler + # is for the given target. If the target is 'any', match + # any given arch spec. If the compiler has no assigned + # target this is an old compiler config file, skip this logic. + target = items.get('target', None) + if arch_spec and target and (target != arch_spec.target and + target != 'any'): + continue + + compilers.append(compiler_from_config_entry(items)) + return compilers @@ -283,10 +304,28 @@ def compiler_for_spec(compiler_spec, arch_spec): if len(compilers) < 1: raise NoCompilerForSpecError(compiler_spec, arch_spec.platform_os) if len(compilers) > 1: - raise CompilerSpecInsufficientlySpecificError(compiler_spec) + raise CompilerDuplicateError(compiler_spec, arch_spec) return compilers[0] +@_auto_compiler_spec +def get_compiler_duplicates(compiler_spec, arch_spec): + config_scopes = spack.config.config_scopes + scope_to_compilers = dict() + for scope in config_scopes: + compilers = compilers_for_spec(compiler_spec, arch_spec=arch_spec, + scope=scope, use_cache=False) + if compilers: + scope_to_compilers[scope] = compilers + + cfg_file_to_duplicates = dict() + for scope, compilers in scope_to_compilers.iteritems(): + config_file = config_scopes[scope].get_section_filename('compilers') + cfg_file_to_duplicates[config_file] = compilers + + return cfg_file_to_duplicates + + def class_for_compiler_name(compiler_name): """Given a compiler module name, get the corresponding Compiler class.""" assert(supported(compiler_name)) @@ -341,6 +380,24 @@ def __init__(self, compiler_spec, target): % (target, compiler_spec)) +class CompilerDuplicateError(spack.error.SpackError): + def __init__(self, compiler_spec, arch_spec): + config_file_to_duplicates = get_compiler_duplicates( + compiler_spec, arch_spec) + duplicate_table = list( + (x, len(y)) for x, y in config_file_to_duplicates.iteritems()) + descriptor = lambda num: 'time' if num == 1 else 'times' + duplicate_msg = ( + lambda cfgfile, count: "{0}: {1} {2}".format( + cfgfile, str(count), descriptor(count))) + msg = ( + "Compiler configuration contains entries with duplicate" + + " specification ({0}, {1})".format(compiler_spec, arch_spec) + + " in the following files:\n\t" + + '\n\t'.join(duplicate_msg(x, y) for x, y in duplicate_table)) + super(CompilerDuplicateError, self).__init__(msg) + + class CompilerSpecInsufficientlySpecificError(spack.error.SpackError): def __init__(self, compiler_spec): super(CompilerSpecInsufficientlySpecificError, self).__init__( diff --git a/lib/spack/spack/concretize.py b/lib/spack/spack/concretize.py index bc3675ad84..1be0a7a81e 100644 --- a/lib/spack/spack/concretize.py +++ b/lib/spack/spack/concretize.py @@ -315,7 +315,7 @@ def concretize_compiler(self, spec): def _proper_compiler_style(cspec, aspec): return spack.compilers.compilers_for_spec(cspec, arch_spec=aspec) - all_compilers = spack.compilers.all_compilers() + all_compilers = spack.compilers.all_compiler_specs() if (spec.compiler and spec.compiler.concrete and diff --git a/lib/spack/spack/test/cmd/test_compiler_cmd.py b/lib/spack/spack/test/cmd/test_compiler_cmd.py index 647404e6da..f0160e274a 100644 --- a/lib/spack/spack/test/cmd/test_compiler_cmd.py +++ b/lib/spack/spack/test/cmd/test_compiler_cmd.py @@ -71,12 +71,12 @@ def test_compiler_remove(self): all=True, compiler_spec='gcc@4.5.0', add_paths=[], scope=None ) spack.cmd.compiler.compiler_remove(args) - compilers = spack.compilers.all_compilers() + compilers = spack.compilers.all_compiler_specs() assert spack.spec.CompilerSpec("gcc@4.5.0") not in compilers def test_compiler_add(self, mock_compiler_dir): # Compilers available by default. - old_compilers = set(spack.compilers.all_compilers()) + old_compilers = set(spack.compilers.all_compiler_specs()) args = spack.util.pattern.Bunch( all=None, @@ -87,7 +87,7 @@ def test_compiler_add(self, mock_compiler_dir): spack.cmd.compiler.compiler_find(args) # Ensure new compiler is in there - new_compilers = set(spack.compilers.all_compilers()) + new_compilers = set(spack.compilers.all_compiler_specs()) new_compiler = new_compilers - old_compilers assert new_compiler c = new_compiler.pop() diff --git a/lib/spack/spack/test/compilers.py b/lib/spack/spack/test/compilers.py new file mode 100644 index 0000000000..d0fc506f40 --- /dev/null +++ b/lib/spack/spack/test/compilers.py @@ -0,0 +1,48 @@ +############################################################################## +# Copyright (c) 2013-2016, Lawrence Livermore National Security, LLC. +# Produced at the Lawrence Livermore National Laboratory. +# +# 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/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 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 +############################################################################## +import pytest + +import spack.spec +import spack.compilers as compilers + + +@pytest.mark.usefixtures('config') +class TestCompilers(object): + + def test_get_compiler_duplicates(self): + # In this case there is only one instance of the specified compiler in + # the test configuration (so it is not actually a duplicate), but the + # method behaves the same. + cfg_file_to_duplicates = compilers.get_compiler_duplicates( + 'gcc@4.5.0', spack.spec.ArchSpec('cray-CNL-xeon')) + assert len(cfg_file_to_duplicates) == 1 + cfg_file, duplicates = cfg_file_to_duplicates.iteritems().next() + assert len(duplicates) == 1 + + def test_all_compilers(self): + all_compilers = compilers.all_compilers() + filtered = list(x for x in all_compilers if str(x.spec) == 'clang@3.3') + filtered = list(x for x in filtered if x.operating_system == 'SuSE11') + assert len(filtered) == 1