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

6 files changed, 181 insertions, 70 deletions

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 @@ class NoCompilerForSpecError(spack.error.SpackError):
             % (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 @@ class DefaultConcretizer(object):
         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 @@ class TestCompilerCommand(object):
             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 @@ class TestCompilerCommand(object):
         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