Made module handling much saner and safer

lib/spack/spack/build_environment.py

@@ -171,7 +171,7 @@ def set_compiler_environment_variables(pkg):
 
     os.environ['SPACK_COMPILER_SPEC']  = str(pkg.spec.compiler)
 
-    if compiler.modules:
+    if compiler.strategy == 'MODULES':
         for mod in compiler.modules:
             load_module(mod)
 

lib/spack/spack/compiler.py

@@ -106,26 +106,28 @@ class Compiler(object):
     PrgEnv_compiler = None
 
 
-    def __init__(self, cspec, paths, modules=None):
+    def __init__(self, cspec, strategy, paths, modules=None):
         def check(exe):
             if exe is None:
                 return None
             _verify_executables(exe)
             return exe
 
+        self.strategy = strategy
+
         self.cc  = check(paths[0])
         self.cxx = check(paths[1])
-        self.f77 = check(paths[2])
+        if len(paths) > 2:
-        if len(paths) == 3:
+            self.f77 = check(paths[2])
-            self.fc = self.f77
+            if len(paths) == 3:
-        else:
+                self.fc = self.f77
-            self.fc  = check(paths[3])
+            else:
+                self.fc  = check(paths[3])
 
         self.spec = cspec
         self.modules = modules
 
 
-
     @property
     def version(self):
         return self.spec.version
@@ -267,7 +269,7 @@ def find_in_path(cls, *path):
                 if newcount <= prevcount:
                     continue
 
-            compilers[ver] = cls(spec, paths)
+            compilers[ver] = cls(spec, 'PATH', paths)
 
         return list(compilers.values())
 
@@ -287,7 +289,7 @@ def find_in_modules(cls):
 
             for name, version in matches:
                 v = version
-                comp = cls(spack.spec.CompilerSpec(name + '@' + v),
+                comp = cls(spack.spec.CompilerSpec(name + '@' + v), 'MODULES',
                            ['cc', 'CC', 'ftn'], [cls.PrgEnv, name +'/' + v])
 
                 compilers.append(comp)
@@ -302,12 +304,12 @@ def __repr__(self):
 
     def __str__(self):
         """Return a string represntation of the compiler toolchain."""
-        if self.modules:
+        if self.strategy is 'MODULES':
             return "%s(%s)" % (
-                self.name, '\n     '.join((str(s) for s in (self.cc, self.cxx, self.f77, self.fc, self.modules))))
+                self.name, '\n     '.join((str(s) for s in (self.strategy, self.cc, self.cxx, self.f77, self.fc, self.modules))))
         else:
             return "%s(%s)" % (
-                self.name, '\n     '.join((str(s) for s in (self.cc, self.cxx, self.f77, self.fc))))
+                self.name, '\n     '.join((str(s) for s in (self.strategy, self.cc, self.cxx, self.f77, self.fc))))
 
 
 class CompilerAccessError(spack.error.SpackError):

lib/spack/spack/compilers/__init__.py

@@ -129,6 +129,11 @@ def add_compilers_to_config(scope, *compilers):
     for compiler in compilers:
         compiler_entry = {}
 
+        val = getattr(compiler, 'strategy')
+        if not val:
+            val = 'None'
+        compiler_entry[c] = val
+
         for c in _required_instance_vars:
             val = getattr(compiler, c)
             if not val:
@@ -190,6 +195,11 @@ def get_compiler(cspec):
             raise InvalidCompilerConfigurationError(cspec)
 
         cls  = class_for_compiler_name(cspec.name)
+        
+        strategy = items['strategy']
+        if not strategy:
+            raise InvalidCompilerConfigurationError(cspec)
+
         compiler_paths = []
         for c in _required_instance_vars:
             compiler_path = items[c]
@@ -203,22 +213,19 @@ def get_compiler(cspec):
                 items[m] = None
             mods = items[m]
 
-        return cls(cspec, compiler_paths, mods)
+        return cls(cspec, strategy, compiler_paths, mods)
 
     matches = find(compiler_spec)
     return [get_compiler(cspec) for cspec in matches]
 
 
 @_auto_compiler_spec
-def compiler_for_spec(compiler_spec, target):
+def compiler_for_spec(compiler_spec, operating_system):
     """Get the compiler that satisfies compiler_spec.  compiler_spec must
        be concrete."""
     assert(compiler_spec.concrete)
-    compilers = compilers_for_spec(compiler_spec)
+    compilers = [c for c in compilers_for_spec(compiler_spec)
-    if target.compiler_strategy == "PATH":
+                if c.strategy == operating_system.strategy]
-        compilers = [c for c in compilers if c.modules is None]
-    elif target.compiler_strategy == "MODULES":
-        compilers = [c for c in compilers if c.modules is not None]
     if len(compilers) < 1:
         raise NoCompilerForSpecError(compiler_spec, target)
     if len(compilers) > 1:

lib/spack/spack/concretize.py

@@ -354,10 +354,11 @@ def concretize_compiler(self, spec):
         # Should think whether this can be more efficient
         def _proper_compiler_style(cspec, architecture):
             compilers = spack.compilers.compilers_for_spec(cspec)
-            if architecture.platform_os.compiler_strategy == 'PATH':
+            filter(lambda c: c.strategy == architecture.platform_os.compiler_strategy, compilers)
-                filter(lambda c: not c.modules, compilers)
+#if architecture.platform_os.compiler_strategy == 'PATH':
-            if architecture.platform_os.compiler_strategy == 'MODULES':
+            #    filter(lambda c: not c.modules, compilers)
-                filter(lambda c: c.modules, compilers)
+            #if architecture.platform_os.compiler_strategy == 'MODULES':
+            #    filter(lambda c: c.modules, compilers)
             return compilers
 
         

lib/spack/spack/test/architecture.py

@@ -34,10 +34,10 @@ def test_from_dict_function_with_architecture(self):
              'target' : {'name':'haswell', 'module_name': 'craype-haswell'}}
                 
         arch = spack.architecture.arch_from_dict(d)                                                
-        self.assertIsInstance(arch, Arch)
+        self.assertTrue( isinstance(arch, Arch) )
-        self.assertIsInstance(arch.platform, Platform)
+        self.assertTrue( isinstance(arch.platform, Platform) )
-        self.assertIsInstance(arch.platform_os, OperatingSystem)
+        self.assertTrue( isinstance(arch.platform_os, OperatingSystem) )
-        self.assertIsInstance(arch.target, Target)
+        self.assertTrue( isinstance(arch.target, Target) )
         
 
     def test_platform_class_and_compiler_strategies(self):

var/spack/mock_configs/site_spackconfig/compilers.yaml

@@ -6,12 +6,14 @@ compilers:
       f77: None
       fc: None
       modules: None
+      strategy: PATH
     gcc@4.5.0:
       cc: /path/to/gcc
       cxx: /path/to/g++
       f77: /path/to/gfortran
       fc: /path/to/gfortran
       modules: None
+      strategy: PATH
     gcc@5.2.0:
       cc: cc
       cxx: CC
@@ -20,6 +22,7 @@ compilers:
       modules:
         - PrgEnv-gnu
         - gcc/5.2.0
+      strategy: MODULES
     intel@15.0.1:
       cc: cc
       ccx: CC
@@ -28,8 +31,10 @@ compilers:
       modules:
       - PrgEnv-intel
       - intel/15.0.1
+      strategy: MODULES
     intel@15.1.2:
       cc: /path/to/icc
       cxx: /path/to/ic++
       f77: /path/to/ifort
       fc: /path/to/ifort
+      strategy: PATH