Bug Fix : Apply Compiler Flags Specified by Manual Compiler Configuration (#1532)

* Fixed a bug causing config-specified compiler flags to be ignored.
Updated the compiler config so all flags are in a separate section.

* Updated the documentation for the `compilers.yaml` file spec.

* Implemented basic testing for the 'flags' section of compiler config.

* Fixed a few minor problems with the manual compiler config documentation.

7 files changed, 126 insertions, 53 deletions

diff --git a/lib/spack/docs/getting_started.rst b/lib/spack/docs/getting_started.rst
index 9267a9b756..5f1b95a0b6 100644
--- a/lib/spack/docs/getting_started.rst
+++ b/lib/spack/docs/getting_started.rst
@@ -218,12 +218,14 @@ If you want to see specifics on a particular compiler, you can run
 
    $ spack compiler info intel@15
    intel@15.0.0:
-           cc  = /usr/local/bin/icc-15.0.090
-           cxx = /usr/local/bin/icpc-15.0.090
-           f77 = /usr/local/bin/ifort-15.0.090
-           fc  = /usr/local/bin/ifort-15.0.090
-           modules  = []
-           operating system  = centos6
+     paths:
+       cc  = /usr/local/bin/icc-15.0.090
+       cxx = /usr/local/bin/icpc-15.0.090
+       f77 = /usr/local/bin/ifort-15.0.090
+       fc  = /usr/local/bin/ifort-15.0.090
+     modules = []
+     operating system = centos6
+   ...
 
 This shows which C, C++, and Fortran compilers were detected by Spack.
 Notice also that we didn't have to be too specific about the
@@ -244,7 +246,7 @@ Each compiler configuration in the file looks like this:
 
    compilers:
    - compiler:
-       modules = []
+       modules: []
        operating_system: centos6
        paths:
          cc: /usr/local/bin/icc-15.0.024-beta
@@ -253,39 +255,46 @@ Each compiler configuration in the file looks like this:
          fc: /usr/local/bin/ifort-15.0.024-beta
        spec: intel@15.0.0:
 
-For compilers, like ``clang``, that do not support Fortran, put
+For compilers that do not support Fortran (like ``clang``), put
 ``None`` for ``f77`` and ``fc``:
 
 .. code-block:: yaml
 
+   compilers:
+   - compiler:
+       modules: []
+       operating_system: centos6
        paths:
          cc: /usr/bin/clang
          cxx: /usr/bin/clang++
          f77: None
          fc: None
-       spec: clang@3.3svn:
+       spec: clang@3.3svn
 
 Once you save the file, the configured compilers will show up in the
 list displayed by ``spack compilers``.
 
-You can also add compiler flags to manually configured compilers. The
-valid flags are ``cflags``, ``cxxflags``, ``fflags``, ``cppflags``,
-``ldflags``, and ``ldlibs``. For example:
+You can also add compiler flags to manually configured compilers. These
+flags should be specified in the ``flags`` section of the compiler
+specification. The valid flags are ``cflags``, ``cxxflags``, ``fflags``,
+``cppflags``, ``ldflags``, and ``ldlibs``. For example:
 
 .. code-block:: yaml
 
    compilers:
    - compiler:
-       modules = []
-       operating_system: OS
+       modules: []
+       operating_system: centos6
        paths:
-         cc: /usr/local/bin/icc-15.0.024-beta
-         cxx: /usr/local/bin/icpc-15.0.024-beta
-         f77: /usr/local/bin/ifort-15.0.024-beta
-         fc: /usr/local/bin/ifort-15.0.024-beta
+         cc: /usr/bin/gcc
+         cxx: /usr/bin/g++
+         f77: /usr/bin/gfortran
+         fc: /usr/bin/gfortran
        flags:
+         cflags: -O3 -fPIC
+         cxxflags: -O3 -fPIC
          cppflags: -O3 -fPIC
-       spec: intel@15.0.0:
+       spec: gcc@4.7.2
 
 These flags will be treated by spack as if they were entered from
 the command line each time this compiler is used. The compiler wrappers
@@ -527,7 +536,7 @@ configuration in ``compilers.yaml`` illustrates this technique:
 
    compilers:
    - compiler:
-       modules = [gcc-4.9.3, intel-15.0.24]
+       modules: [gcc-4.9.3, intel-15.0.24]
        operating_system: centos7
        paths:
          cc: /opt/intel-15.0.24/bin/icc-15.0.24-beta
@@ -568,7 +577,7 @@ flags to the ``icc`` command:
 
        compilers:
        - compiler:
-           modules = [intel-15.0.24]
+           modules: [intel-15.0.24]
            operating_system: centos7
            paths:
              cc: /opt/intel-15.0.24/bin/icc-15.0.24-beta
diff --git a/lib/spack/spack/cmd/compiler.py b/lib/spack/spack/cmd/compiler.py
index ea91c71479..522530a6f7 100644
--- a/lib/spack/spack/cmd/compiler.py
+++ b/lib/spack/spack/cmd/compiler.py
@@ -133,10 +133,13 @@ def compiler_info(args):
     else:
         for c in compilers:
             print str(c.spec) + ":"
-            print "\tcc  = %s" % c.cc
-            print "\tcxx = %s" % c.cxx
-            print "\tf77 = %s" % c.f77
-            print "\tfc  = %s" % c.fc
+            print "\tpaths:"
+            for cpath in ['cc', 'cxx', 'f77', 'fc']:
+                print "\t\t%s = %s" % (cpath, getattr(c, cpath, None))
+            if c.flags:
+                print "\tflags:"
+                for flag, flag_value in c.flags.iteritems():
+                    print "\t\t%s = %s" % (flag, flag_value)
             print "\tmodules  = %s" % c.modules
             print "\toperating system  = %s" % c.operating_system
 
diff --git a/lib/spack/spack/compilers/__init__.py b/lib/spack/spack/compilers/__init__.py
index eb866c8bbb..0db632a880 100644
--- a/lib/spack/spack/compilers/__init__.py
+++ b/lib/spack/spack/compilers/__init__.py
@@ -65,6 +65,7 @@ def _to_dict(compiler):
     d['spec'] = str(compiler.spec)
     d['paths'] = dict((attr, getattr(compiler, attr, None))
                       for attr in _path_instance_vars)
+    d['flags'] = dict((fname, fvals) for fname, fvals in compiler.flags)
     d['operating_system'] = str(compiler.operating_system)
     d['modules'] = compiler.modules if compiler.modules else []
 
@@ -212,7 +213,7 @@ def supported(compiler_spec):
 @_auto_compiler_spec
 def find(compiler_spec, scope=None):
     """Return specs of available compilers that match the supplied
-       compiler spec.  Return an list if nothing found."""
+       compiler spec.  Return an empty list if nothing found."""
     return [c for c in all_compilers(scope) if c.satisfies(compiler_spec)]
 
 
@@ -221,7 +222,7 @@ def compilers_for_spec(compiler_spec, scope=None, **kwargs):
     """This gets all compilers that satisfy the supplied CompilerSpec.
        Returns an empty list if none are found.
     """
-    platform = kwargs.get("platform", None)
+    platform = kwargs.get('platform', None)
     config = all_compilers_config(scope)
 
     def get_compilers(cspec):
@@ -241,7 +242,7 @@ def compilers_for_spec(compiler_spec, scope=None, **kwargs):
             compiler_paths = []
             for c in _path_instance_vars:
                 compiler_path = items['paths'][c]
-                if compiler_path != "None":
+                if compiler_path != 'None':
                     compiler_paths.append(compiler_path)
                 else:
                     compiler_paths.append(None)
@@ -250,21 +251,17 @@ def compilers_for_spec(compiler_spec, scope=None, **kwargs):
             if mods == 'None':
                 mods = []
 
+            os = None
             if 'operating_system' in items:
                 os = spack.architecture._operating_system_from_dict(
                     items['operating_system'], platform)
-            else:
-                os = None
 
-            alias = items['alias'] if 'alias' in items else None
+            alias = items.get('alias', None)
 
-            flags = {}
-            for f in spack.spec.FlagMap.valid_compiler_flags():
-                if f in items:
-                    flags[f] = items[f]
+            compiler_flags = items.get('flags', {})
 
             compilers.append(
-                cls(cspec, os, compiler_paths, mods, alias, **flags))
+                cls(cspec, os, compiler_paths, mods, alias, **compiler_flags))
 
         return compilers
 
diff --git a/lib/spack/spack/schema/compilers.py b/lib/spack/spack/schema/compilers.py
index 2ffac03a66..72aa8e0d8b 100644
--- a/lib/spack/spack/schema/compilers.py
+++ b/lib/spack/spack/schema/compilers.py
@@ -52,7 +52,11 @@ schema = {
                                 'f77': {'anyOf': [{'type': 'string'},
                                                   {'type': 'null'}]},
                                 'fc':  {'anyOf': [{'type': 'string'},
-                                                  {'type': 'null'}]},
+                                                  {'type': 'null'}]}}},
+                        'flags': {
+                            'type': 'object',
+                            'additionalProperties': False,
+                            'properties': {
                                 'cflags': {'anyOf': [{'type': 'string'},
                                                      {'type': 'null'}]},
                                 'cxxflags': {'anyOf': [{'type': 'string'},
diff --git a/lib/spack/spack/test/cc.py b/lib/spack/spack/test/cc.py
index 73c711724c..74b6b31654 100644
--- a/lib/spack/spack/test/cc.py
+++ b/lib/spack/spack/test/cc.py
@@ -51,7 +51,7 @@ test_command = [
     'arg5', 'arg6']
 
 
-class CompilerTest(unittest.TestCase):
+class CompilerWrapperTest(unittest.TestCase):
 
     def setUp(self):
         self.cc = Executable(join_path(spack.build_env_path, "cc"))
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index 0822e44db8..02f53a5261 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -57,6 +57,21 @@ a_comps = [
     }},
     {'compiler': {
         'paths': {
+            "cc": "/gcc422",
+            "cxx": "/g++422",
+            "f77": 'gfortran',
+            "fc": 'gfortran'
+        },
+        'flags': {
+            "cppflags": "-O0 -fpic",
+            "fflags": "-f77",
+        },
+        'modules': None,
+        'spec': 'gcc@4.2.2',
+        'operating_system': 'CNL10'
+    }},
+    {'compiler': {
+        'paths': {
             "cc": "<overwritten>",
             "cxx": "<overwritten>",
             "f77": '<overwritten>',
@@ -92,6 +107,21 @@ b_comps = [
     }},
     {'compiler': {
         'paths': {
+            "cc": "/icc123",
+            "cxx": "/icp123",
+            "f77": 'ifort',
+            "fc": 'ifort'
+        },
+        'flags': {
+            "cppflags": "-O3",
+            "fflags": "-f77rtl",
+        },
+        'modules': None,
+        'spec': 'icc@12.3',
+        'operating_system': 'CNL10'
+    }},
+    {'compiler': {
+        'paths': {
             "cc": "<overwritten>",
             "cxx": "<overwritten>",
             "f77": '<overwritten>',
@@ -112,11 +142,13 @@ class ConfigTest(MockPackagesTest):
     def setUp(self):
         super(ConfigTest, self).setUp()
         self.tmp_dir = mkdtemp('.tmp', 'spack-config-test-')
+        self.a_comp_specs = [ac['compiler']['spec'] for ac in a_comps]
+        self.b_comp_specs = [bc['compiler']['spec'] for bc in b_comps]
+
         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'))
+        for priority in ['low', 'high']:
+            spack.config.ConfigScope('test_{0}_priority'.format(priority),
+                                     os.path.join(self.tmp_dir, priority))
 
     def tearDown(self):
         super(ConfigTest, self).tearDown()
@@ -126,19 +158,22 @@ class ConfigTest(MockPackagesTest):
         """Check that named compilers in comps match Spack's config."""
         config = spack.config.get_config('compilers')
         compiler_list = ['cc', 'cxx', 'f77', 'fc']
+        flag_list = ['cflags', 'cxxflags', 'fflags', 'cppflags',
+                     'ldflags', 'ldlibs']
         param_list = ['modules', 'paths', 'spec', 'operating_system']
         for compiler in config:
             conf = compiler['compiler']
             if conf['spec'] in compiler_names:
-                comp = None
-                for c in comps:
-                    if c['compiler']['spec'] == conf['spec']:
-                        comp = c['compiler']
-                        break
+                comp = next((c['compiler'] for c in comps if
+                             c['compiler']['spec'] == conf['spec']), None)
                 if not comp:
                     self.fail('Bad config spec')
                 for p in param_list:
                     self.assertEqual(conf[p], comp[p])
+                for f in flag_list:
+                    expected = comp.get('flags', {}).get(f, None)
+                    actual = conf.get('flags', {}).get(f, None)
+                    self.assertEqual(expected, actual)
                 for c in compiler_list:
                     expected = comp['paths'][c]
                     actual = conf['paths'][c]
@@ -156,8 +191,8 @@ class ConfigTest(MockPackagesTest):
         spack.config.update_config('compilers', b_comps, 'test_high_priority')
 
         # Make sure the config looks how we expect.
-        self.check_config(a_comps, 'gcc@4.7.3', 'gcc@4.5.0')
-        self.check_config(b_comps, 'icc@10.0', 'icc@11.1', 'clang@3.3')
+        self.check_config(a_comps, *self.a_comp_specs)
+        self.check_config(b_comps, *self.b_comp_specs)
 
     def test_write_key_to_disk(self):
         # Write b_comps "on top of" a_comps.
@@ -168,8 +203,8 @@ class ConfigTest(MockPackagesTest):
         spack.config.clear_config_caches()
 
         # Same check again, to ensure consistency.
-        self.check_config(a_comps, 'gcc@4.7.3', 'gcc@4.5.0')
-        self.check_config(b_comps, 'icc@10.0', 'icc@11.1', 'clang@3.3')
+        self.check_config(a_comps, *self.a_comp_specs)
+        self.check_config(b_comps, *self.b_comp_specs)
 
     def test_write_to_same_priority_file(self):
         # Write b_comps in the same file as a_comps.
@@ -180,5 +215,5 @@ class ConfigTest(MockPackagesTest):
         spack.config.clear_config_caches()
 
         # Same check again, to ensure consistency.
-        self.check_config(a_comps, 'gcc@4.7.3', 'gcc@4.5.0')
-        self.check_config(b_comps, 'icc@10.0', 'icc@11.1', 'clang@3.3')
+        self.check_config(a_comps, *self.a_comp_specs)
+        self.check_config(b_comps, *self.b_comp_specs)
diff --git a/lib/spack/spack/test/mock_packages_test.py b/lib/spack/spack/test/mock_packages_test.py
index 82c2712b0e..cba9c81e9d 100644
--- a/lib/spack/spack/test/mock_packages_test.py
+++ b/lib/spack/spack/test/mock_packages_test.py
@@ -136,6 +136,31 @@ compilers:
       f77: None
       fc: None
     modules: 'None'
+- compiler:
+    spec: gcc@4.7.2
+    operating_system: redhat6
+    paths:
+      cc: /path/to/gcc472
+      cxx: /path/to/g++472
+      f77: /path/to/gfortran472
+      fc: /path/to/gfortran472
+    flags:
+      cflags: -O0
+      cxxflags: -O0
+      fflags: -O0
+    modules: 'None'
+- compiler:
+    spec: clang@3.5
+    operating_system: redhat6
+    paths:
+      cc: /path/to/clang35
+      cxx: /path/to/clang++35
+      f77: None
+      fc: None
+    flags:
+      cflags: -O3
+      cxxflags: -O3
+    modules: 'None'
 """.format(linux_os_name, linux_os_version)
 
 mock_packages_config = """\