From d546d828d322d54bb61c10dbf1fa42831049001e Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 13:20:28 +0200
Subject: module file : added filtering based on environment variable name

---
 lib/spack/spack/config.py      | 44 +++++++++++++++++++++++++++++++++++++++---
 lib/spack/spack/environment.py | 16 +++++++++++++++
 lib/spack/spack/modules.py     | 10 +++++++++-
 3 files changed, 66 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 14e5aaf4fb..4fca735fc9 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -146,7 +146,7 @@ section_schemas = {
         'type': 'object',
         'additionalProperties': False,
         'patternProperties': {
-            'compilers:?': { # optional colon for overriding site config.
+            'compilers:?': {  # optional colon for overriding site config.
                 'type': 'object',
                 'default': {},
                 'additionalProperties': False,
@@ -195,6 +195,7 @@ section_schemas = {
                 'default': [],
                 'items': {
                     'type': 'string'},},},},
+
     'packages': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack package configuration file schema',
@@ -238,11 +239,35 @@ section_schemas = {
                                 'default' : {},
                             }
                         },},},},},},
+
     'modules': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack module file configuration file schema',
         'type': 'object',
         'additionalProperties': False,
+        'definitions': {
+            'module_type_configuration': {
+                'type': 'object',
+                'default': {},
+                'additionalProperties': False,
+                'properties': {
+                    'filter': {
+                        'type': 'object',
+                        'default': {},
+                        'additionalProperties': False,
+                        'properties': {
+                            'environment_modifications': {
+                                'type': 'array',
+                                'default': [],
+                                'items': {
+                                    'type': 'string'
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        },
         'patternProperties': {
             r'modules:?': {
                 'type': 'object',
@@ -253,9 +278,22 @@ section_schemas = {
                         'type': 'array',
                         'default': [],
                         'items': {
-                            'type': 'string'
+                            'type': 'string',
+                            'enum': ['tcl', 'dotkit']
                         }
-                    }
+                    },
+                    'tcl': {
+                        'allOf': [
+                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
+                            {}  # Specific tcl extensions
+                        ]
+                    },
+                    'dotkit': {
+                        'allOf': [
+                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
+                            {}  # Specific dotkit extensions
+                        ]
+                    },
                 }
             },
         },
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 72aafa4e2d..3d18d3a63f 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -250,3 +250,19 @@ def validate(env, errstream):
     modifications = env.group_by_name()
     for variable, list_of_changes in sorted(modifications.items()):
         set_or_unset_not_first(variable, list_of_changes, errstream)
+
+
+def filter_environment_modifications(env, variables):
+    """
+    Generator that filters out any change to environment variables present in the input list
+
+    Args:
+        env: list of environment modifications
+        variables: list of variable names to be filtered
+
+    Yields:
+        items in env if they are not in variables
+    """
+    for item in env:
+        if item.name not in variables:
+            yield item
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index d797af287d..aca37ae14b 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -179,9 +179,17 @@ class EnvModule(object):
         if not env:
             return
 
+        # Filter modifications to the environment according to configuration files
+        try:
+            filter_list = CONFIGURATION[self.name]['filter']['environment_modifications']
+        except KeyError:
+            filter_list = []
+
         with open(self.file_name, 'w') as f:
             self.write_header(f)
-            for line in self.process_environment_command(env):
+            for line in self.process_environment_command(
+                    filter_environment_modifications(env, filter_list)
+            ):
                 f.write(line)
 
     def write_header(self, stream):
-- 
cgit v1.2.3-70-g09d2


From c352249e305c00d951586bc8c19a13ceadaa1887 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 14:55:05 +0200
Subject: EnvModules : narrowing the type of exceptions being "handled"

---
 lib/spack/spack/modules.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index aca37ae14b..0d486f6430 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -167,10 +167,11 @@ class EnvModule(object):
                 package = self.spec[item].package
                 package.setup_dependent_package(self.pkg.module, self.spec)
                 package.setup_dependent_environment(spack_env, env, self.spec)
-            except:
+            except KeyError as e:
                 # The extends was conditional, so it doesn't count here
                 # eg: extends('python', when='+python')
-                pass
+                tty.debug(str(e))
+
 
         # Package-specific environment modifications
         self.spec.package.setup_environment(spack_env, env)
-- 
cgit v1.2.3-70-g09d2


From ca7d7010076a520fa20f02ab0609bbc3e6216d2f Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 16:10:04 +0200
Subject: module file : all the dependencies can affect the run_time
 environment, not only extendees

---
 lib/spack/spack/modules.py | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 0d486f6430..04317f53b1 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -50,6 +50,7 @@ import llnl.util.tty as tty
 import spack
 import spack.config
 from llnl.util.filesystem import join_path, mkdirp
+from spack.build_environment import parent_class_modules, set_module_variables_for_package
 from spack.environment import *
 
 __all__ = ['EnvModule', 'Dotkit', 'TclModule']
@@ -159,12 +160,25 @@ class EnvModule(object):
         # installation prefix
         env = inspect_path(self.spec.prefix)
 
-        # Let the extendee modify their extensions before asking for
+        # Let the extendee/dependency modify their extensions/dependencies before asking for
         # package-specific modifications
         spack_env = EnvironmentModifications()
-        for item in self.pkg.extendees:
+
+        def dependencies():
+            # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
+            # FIXME : is given. This work around permits to get a unique list of spec anyhow.
+            # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
+            l = [x for x in sorted(self.spec.traverse(order='post', depth=True, cover='nodes'),reverse=True)]
+            seen = set()
+            return [x for ii, x in l if not (x in seen or seen.add(x))]
+
+        for item in dependencies():
             try:
-                package = self.spec[item].package
+                package = self.spec[item.name].package
+                modules = parent_class_modules(package.__class__)
+                for mod in modules:
+                    set_module_variables_for_package(package, mod)
+                set_module_variables_for_package(package, package.module)
                 package.setup_dependent_package(self.pkg.module, self.spec)
                 package.setup_dependent_environment(spack_env, env, self.spec)
             except KeyError as e:
@@ -172,8 +186,8 @@ class EnvModule(object):
                 # eg: extends('python', when='+python')
                 tty.debug(str(e))
 
-
         # Package-specific environment modifications
+        set_module_variables_for_package(self.pkg, self.pkg.module)
         self.spec.package.setup_environment(spack_env, env)
 
         # TODO : implement site-specific modifications and filters
-- 
cgit v1.2.3-70-g09d2


From 2968b60ee0b6b8d649e3d0c1a091857bfde28b89 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 18:03:09 +0200
Subject: module file : added autoload and prereq

---
 lib/spack/spack/config.py  |  8 +++++-
 lib/spack/spack/modules.py | 65 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 12 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 4fca735fc9..ff5fba24f8 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -246,6 +246,10 @@ section_schemas = {
         'type': 'object',
         'additionalProperties': False,
         'definitions': {
+            'dependency_selection': {
+                'type': 'string',
+                'enum': ['None', 'Direct', 'All']
+            },
             'module_type_configuration': {
                 'type': 'object',
                 'default': {},
@@ -264,7 +268,9 @@ section_schemas = {
                                 }
                             }
                         }
-                    }
+                    },
+                    'autoload': {'$ref': '#/definitions/dependency_selection'},
+                    'prerequisites': {'$ref': '#/definitions/dependency_selection'}
                 }
             }
         },
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 04317f53b1..d9a846f281 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -164,15 +164,25 @@ class EnvModule(object):
         # package-specific modifications
         spack_env = EnvironmentModifications()
 
-        def dependencies():
+        def dependencies(request='All'):
+            if request == 'None':
+                return []
+
+            l = [x for x in sorted(self.spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
+
+            if request == 'Direct':
+                return [x for ii, x in l if ii == 1]
+
             # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
             # FIXME : is given. This work around permits to get a unique list of spec anyhow.
             # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
-            l = [x for x in sorted(self.spec.traverse(order='post', depth=True, cover='nodes'),reverse=True)]
             seen = set()
-            return [x for ii, x in l if not (x in seen or seen.add(x))]
+            seen_add = seen.add
+            return [x for ii, x in l if not (x in seen or seen_add(x))]
 
-        for item in dependencies():
+        # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
+        # TODO : factored out to a single place
+        for item in dependencies('All'):
             try:
                 package = self.spec[item.name].package
                 modules = parent_class_modules(package.__class__)
@@ -190,11 +200,18 @@ class EnvModule(object):
         set_module_variables_for_package(self.pkg, self.pkg.module)
         self.spec.package.setup_environment(spack_env, env)
 
-        # TODO : implement site-specific modifications and filters
-        if not env:
-            return
+        # Get list of modules that will be loaded automatically
+        try:
+            autoload_list = dependencies(CONFIGURATION[self.name]['autoload'])
+        except KeyError:
+            autoload_list = []
 
-        # Filter modifications to the environment according to configuration files
+        try:
+            prerequisites_list = dependencies(CONFIGURATION[self.name]['prerequisites'])
+        except KeyError:
+            prerequisites_list = []
+
+        # Filter modifications to environment variables
         try:
             filter_list = CONFIGURATION[self.name]['filter']['environment_modifications']
         except KeyError:
@@ -202,14 +219,26 @@ class EnvModule(object):
 
         with open(self.file_name, 'w') as f:
             self.write_header(f)
-            for line in self.process_environment_command(
-                    filter_environment_modifications(env, filter_list)
-            ):
+            # Automatic loads
+            for x in autoload_list:
+                self.write_autoload(f, x)
+            # Prerequisites
+            for x in prerequisites_list:
+                self.write_prerequisite(f, x)
+            # Modifications to the environment
+            iterable = self.process_environment_command( filter_environment_modifications(env, filter_list))
+            for line in iterable:
                 f.write(line)
 
     def write_header(self, stream):
         raise NotImplementedError()
 
+    def write_autoload(self, stream, spec):
+        raise NotImplementedError()
+
+    def write_prerequisite(self, stream, spec):
+        raise NotImplementedError()
+
     def process_environment_command(self, env):
         for command in env:
             try:
@@ -309,3 +338,17 @@ class TclModule(EnvModule):
             for line in textwrap.wrap(self.long_description, 72):
                 module_file.write("puts stderr \"%s\"\n" % line)
             module_file.write('}\n\n')
+
+    def write_autoload(self, module_file, spec):
+        autoload_format = '''
+if ![ is-loaded {module_file} ] {{
+    puts stderr "Autoloading {module_file}"
+    module load {module_file}
+}}
+'''''
+        m = TclModule(spec)
+        module_file.write(autoload_format.format(module_file=m.use_name))
+
+    def write_prerequisite(self, module_file, spec):
+        m = TclModule(spec)
+        module_file.write('prereq {module_file}\n'.format(module_file=m.use_name))
-- 
cgit v1.2.3-70-g09d2


From 5da37c573f13f4c16021f1314a87e4f0c995ba0f Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 18:41:38 +0200
Subject: modules : don't pass stream around

---
 lib/spack/spack/modules.py | 50 ++++++++++++++++++++++++++--------------------
 1 file changed, 28 insertions(+), 22 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index d9a846f281..9a021a7c49 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -149,7 +149,6 @@ class EnvModule(object):
         # Not very descriptive fallback
         return 'spack installed package'
 
-
     def write(self):
         """Write out a module file for this object."""
         module_dir = os.path.dirname(self.file_name)
@@ -218,25 +217,27 @@ class EnvModule(object):
             filter_list = []
 
         with open(self.file_name, 'w') as f:
-            self.write_header(f)
+            # Header
+            f.write(self.header)
             # Automatic loads
             for x in autoload_list:
-                self.write_autoload(f, x)
+                f.write(self.autoload(x))
             # Prerequisites
             for x in prerequisites_list:
-                self.write_prerequisite(f, x)
+                f.write(self.prerequisite(x))
             # Modifications to the environment
-            iterable = self.process_environment_command( filter_environment_modifications(env, filter_list))
+            iterable = self.process_environment_command(filter_environment_modifications(env, filter_list))
             for line in iterable:
                 f.write(line)
 
-    def write_header(self, stream):
+    @property
+    def header(self):
         raise NotImplementedError()
 
-    def write_autoload(self, stream, spec):
+    def autoload(self, spec):
         raise NotImplementedError()
 
-    def write_prerequisite(self, stream, spec):
+    def prerequisite(self, spec):
         raise NotImplementedError()
 
     def process_environment_command(self, env):
@@ -286,19 +287,22 @@ class Dotkit(EnvModule):
                                  self.spec.compiler.version,
                                  self.spec.dag_hash())
 
-    def write_header(self, dk_file):
+    @property
+    def header(self):
         # Category
+        header = ''
         if self.category:
-            dk_file.write('#c %s\n' % self.category)
+            header += '#c %s\n' % self.category
 
         # Short description
         if self.short_description:
-            dk_file.write('#d %s\n' % self.short_description)
+            header += '#d %s\n' % self.short_description
 
         # Long description
         if self.long_description:
             for line in textwrap.wrap(self.long_description, 72):
-                dk_file.write("#h %s\n" % line)
+                header += '#h %s\n' % line
+        return header
 
 
 class TclModule(EnvModule):
@@ -324,22 +328,24 @@ class TclModule(EnvModule):
                                  self.spec.compiler.version,
                                  self.spec.dag_hash())
 
-    def write_header(self, module_file):
+    @property
+    def header(self):
         # TCL Modulefile header
-        module_file.write('#%Module1.0\n')
+        header = '#%Module1.0\n'
         # TODO : category ?
         # Short description
         if self.short_description:
-            module_file.write('module-whatis \"%s\"\n\n' % self.short_description)
+            header += 'module-whatis \"%s\"\n\n' % self.short_description
 
         # Long description
         if self.long_description:
-            module_file.write('proc ModulesHelp { } {\n')
+            header += 'proc ModulesHelp { } {\n'
             for line in textwrap.wrap(self.long_description, 72):
-                module_file.write("puts stderr \"%s\"\n" % line)
-            module_file.write('}\n\n')
+                header += 'puts stderr "%s"\n' % line
+            header += '}\n\n'
+        return header
 
-    def write_autoload(self, module_file, spec):
+    def autoload(self, spec):
         autoload_format = '''
 if ![ is-loaded {module_file} ] {{
     puts stderr "Autoloading {module_file}"
@@ -347,8 +353,8 @@ if ![ is-loaded {module_file} ] {{
 }}
 '''''
         m = TclModule(spec)
-        module_file.write(autoload_format.format(module_file=m.use_name))
+        return autoload_format.format(module_file=m.use_name)
 
-    def write_prerequisite(self, module_file, spec):
+    def prerequisite(self, spec):
         m = TclModule(spec)
-        module_file.write('prereq {module_file}\n'.format(module_file=m.use_name))
+        return 'prereq {module_file}\n'.format(module_file=m.use_name)
-- 
cgit v1.2.3-70-g09d2


From 670cb423f91b17b9757b387a9579be4f248d208b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 5 Apr 2016 18:44:40 +0200
Subject: modules : iterating on all the dependencies doesn't reuire try/except
 KeyError

---
 lib/spack/spack/modules.py | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 9a021a7c49..115d4d9a37 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -182,18 +182,13 @@ class EnvModule(object):
         # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
         # TODO : factored out to a single place
         for item in dependencies('All'):
-            try:
-                package = self.spec[item.name].package
-                modules = parent_class_modules(package.__class__)
-                for mod in modules:
-                    set_module_variables_for_package(package, mod)
-                set_module_variables_for_package(package, package.module)
-                package.setup_dependent_package(self.pkg.module, self.spec)
-                package.setup_dependent_environment(spack_env, env, self.spec)
-            except KeyError as e:
-                # The extends was conditional, so it doesn't count here
-                # eg: extends('python', when='+python')
-                tty.debug(str(e))
+            package = self.spec[item.name].package
+            modules = parent_class_modules(package.__class__)
+            for mod in modules:
+                set_module_variables_for_package(package, mod)
+            set_module_variables_for_package(package, package.module)
+            package.setup_dependent_package(self.pkg.module, self.spec)
+            package.setup_dependent_environment(spack_env, env, self.spec)
 
         # Package-specific environment modifications
         set_module_variables_for_package(self.pkg, self.pkg.module)
-- 
cgit v1.2.3-70-g09d2


From d636b4fdde5b9a37dff9a354368d36fa6969cec9 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 6 Apr 2016 13:42:47 +0200
Subject: modules : more sensible name to blacklist environment variables
 modules : added skeleton to permit modifications based on specs

---
 lib/spack/spack/config.py      | 20 ++++++++--
 lib/spack/spack/environment.py |  2 +-
 lib/spack/spack/modules.py     | 90 +++++++++++++++++++++---------------------
 3 files changed, 62 insertions(+), 50 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index ff5fba24f8..2067c6146e 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -250,7 +250,7 @@ section_schemas = {
                 'type': 'string',
                 'enum': ['None', 'Direct', 'All']
             },
-            'module_type_configuration': {
+            'module_file_configuration': {
                 'type': 'object',
                 'default': {},
                 'additionalProperties': False,
@@ -260,7 +260,7 @@ section_schemas = {
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
-                            'environment_modifications': {
+                            'environment_blacklist': {
                                 'type': 'array',
                                 'default': [],
                                 'items': {
@@ -270,7 +270,21 @@ section_schemas = {
                         }
                     },
                     'autoload': {'$ref': '#/definitions/dependency_selection'},
-                    'prerequisites': {'$ref': '#/definitions/dependency_selection'}
+                    'prerequisites': {'$ref': '#/definitions/dependency_selection'},
+                    'environment': {
+                        'type': 'object',
+                        'default': {}
+                    }
+                }
+            },
+            'module_type_configuration': {
+                'type': 'object',
+                'default': {},
+                'properties': {
+                    'all': {'$ref': '#/definitions/module_file_configuration'}
+                },
+                'patternProperties': {
+                    r'\w[\w-]*': {'$ref': '#/definitions/module_file_configuration'}
                 }
             }
         },
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 3d18d3a63f..92ab4e6bea 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -252,7 +252,7 @@ def validate(env, errstream):
         set_or_unset_not_first(variable, list_of_changes, errstream)
 
 
-def filter_environment_modifications(env, variables):
+def filter_environment_blacklist(env, variables):
     """
     Generator that filters out any change to environment variables present in the input list
 
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 115d4d9a37..62cd2a8a8b 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -49,6 +49,7 @@ import textwrap
 import llnl.util.tty as tty
 import spack
 import spack.config
+
 from llnl.util.filesystem import join_path, mkdirp
 from spack.build_environment import parent_class_modules, set_module_variables_for_package
 from spack.environment import *
@@ -137,7 +138,6 @@ class EnvModule(object):
         if self.spec.package.__doc__:
             self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
-
     @property
     def category(self):
         # Anything defined at the package level takes precedence
@@ -150,35 +150,40 @@ class EnvModule(object):
         return 'spack installed package'
 
     def write(self):
-        """Write out a module file for this object."""
+        """
+        Writes out a module file for this object.
+
+        This method employs a template pattern and expects derived classes to:
+        - override the header property
+        - provide formats for autoload, prerequisites and environment changes
+        """
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
             mkdirp(module_dir)
 
-        # Environment modifications guessed by inspecting the
-        # installation prefix
-        env = inspect_path(self.spec.prefix)
-
-        # Let the extendee/dependency modify their extensions/dependencies before asking for
-        # package-specific modifications
-        spack_env = EnvironmentModifications()
-
         def dependencies(request='All'):
             if request == 'None':
                 return []
 
-            l = [x for x in sorted(self.spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
+            l = [xx for xx in sorted(self.spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
 
             if request == 'Direct':
-                return [x for ii, x in l if ii == 1]
+                return [xx for ii, xx in l if ii == 1]
 
             # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
             # FIXME : is given. This work around permits to get a unique list of spec anyhow.
             # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
             seen = set()
             seen_add = seen.add
-            return [x for ii, x in l if not (x in seen or seen_add(x))]
+            return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
+
+        # Environment modifications guessed by inspecting the
+        # installation prefix
+        env = inspect_path(self.spec.prefix)
 
+        # Let the extendee/dependency modify their extensions/dependencies before asking for
+        # package-specific modifications
+        spack_env = EnvironmentModifications()
         # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
         # TODO : factored out to a single place
         for item in dependencies('All'):
@@ -207,43 +212,43 @@ class EnvModule(object):
 
         # Filter modifications to environment variables
         try:
-            filter_list = CONFIGURATION[self.name]['filter']['environment_modifications']
+            filter_list = CONFIGURATION[self.name]['filter']['environment_blacklist']
         except KeyError:
             filter_list = []
 
+        # Build up the module file content
+        module_file_content = self.header
+        for x in autoload_list:
+            module_file_content += self.autoload(x)
+        for x in prerequisites_list:
+            module_file_content += self.prerequisite(x)
+        for line in self.process_environment_command(filter_environment_blacklist(env, filter_list)):
+            module_file_content += line
+
+        # Dump to file
         with open(self.file_name, 'w') as f:
-            # Header
-            f.write(self.header)
-            # Automatic loads
-            for x in autoload_list:
-                f.write(self.autoload(x))
-            # Prerequisites
-            for x in prerequisites_list:
-                f.write(self.prerequisite(x))
-            # Modifications to the environment
-            iterable = self.process_environment_command(filter_environment_modifications(env, filter_list))
-            for line in iterable:
-                f.write(line)
+            f.write(module_file_content)
 
     @property
     def header(self):
         raise NotImplementedError()
 
     def autoload(self, spec):
-        raise NotImplementedError()
+        m = TclModule(spec)
+        return self.autoload_format.format(module_file=m.use_name)
 
     def prerequisite(self, spec):
-        raise NotImplementedError()
+        m = TclModule(spec)
+        return self.prerequisite_format.format(module_file=m.use_name)
 
     def process_environment_command(self, env):
         for command in env:
             try:
-                yield self.formats[type(command)].format(**command.args)
+                yield self.environment_modifications_formats[type(command)].format(**command.args)
             except KeyError:
                 tty.warn('Cannot handle command of type {command} : skipping request'.format(command=type(command)))
                 tty.warn('{context} at {filename}:{lineno}'.format(**command.args))
 
-
     @property
     def file_name(self):
         """Subclasses should implement this to return the name of the file
@@ -266,7 +271,7 @@ class Dotkit(EnvModule):
     name = 'dotkit'
     path = join_path(spack.share_path, "dotkit")
 
-    formats = {
+    environment_modifications_formats = {
         PrependPath: 'dk_alter {name} {value}\n',
         SetEnv: 'dk_setenv {name} {value}\n'
     }
@@ -304,7 +309,7 @@ class TclModule(EnvModule):
     name = 'tcl'
     path = join_path(spack.share_path, "modules")
 
-    formats = {
+    environment_modifications_formats = {
         PrependPath: 'prepend-path {name} \"{value}\"\n',
         AppendPath: 'append-path {name} \"{value}\"\n',
         RemovePath: 'remove-path {name} \"{value}\"\n',
@@ -312,6 +317,13 @@ class TclModule(EnvModule):
         UnsetEnv: 'unsetenv {name}\n'
     }
 
+    autoload_format = ('if ![ is-loaded {module_file} ] {{'
+                       '    puts stderr "Autoloading {module_file}"'
+                       '    module load {module_file}'
+                       '}}')
+
+    prerequisite_format = 'prereq {module_file}\n'
+
     @property
     def file_name(self):
         return join_path(TclModule.path, self.spec.architecture, self.use_name)
@@ -339,17 +351,3 @@ class TclModule(EnvModule):
                 header += 'puts stderr "%s"\n' % line
             header += '}\n\n'
         return header
-
-    def autoload(self, spec):
-        autoload_format = '''
-if ![ is-loaded {module_file} ] {{
-    puts stderr "Autoloading {module_file}"
-    module load {module_file}
-}}
-'''''
-        m = TclModule(spec)
-        return autoload_format.format(module_file=m.use_name)
-
-    def prerequisite(self, spec):
-        m = TclModule(spec)
-        return 'prereq {module_file}\n'.format(module_file=m.use_name)
-- 
cgit v1.2.3-70-g09d2


From e993c17f89fd44dbf22b19b6669b79b5c79d85a4 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 6 Apr 2016 14:02:27 +0200
Subject: modules : added environment modifications from configuration file

---
 lib/spack/spack/config.py | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 2067c6146e..8babb356b4 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -246,6 +246,13 @@ section_schemas = {
         'type': 'object',
         'additionalProperties': False,
         'definitions': {
+            'array_of_strings': {
+                'type': 'array',
+                'default': [],
+                'items': {
+                    'type': 'string'
+                }
+            },
             'dependency_selection': {
                 'type': 'string',
                 'enum': ['None', 'Direct', 'All']
@@ -273,7 +280,14 @@ section_schemas = {
                     'prerequisites': {'$ref': '#/definitions/dependency_selection'},
                     'environment': {
                         'type': 'object',
-                        'default': {}
+                        'default': {},
+                        'additionalProperties': False,
+                        'properties': {
+                            'set-env': {'$ref': '#/definitions/array_of_strings'},
+                            'unset-env': {'$ref': '#/definitions/array_of_strings'},
+                            'prepend-path': {'$ref': '#/definitions/array_of_strings'},
+                            'append-path': {'$ref': '#/definitions/array_of_strings'}
+                        }
                     }
                 }
             },
-- 
cgit v1.2.3-70-g09d2


From e4cfe7488717eb94b824e9846ebf571c62e625fb Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 6 Apr 2016 16:42:56 +0200
Subject: modules : customization based on specs

---
 lib/spack/spack/config.py  |   8 +--
 lib/spack/spack/modules.py | 122 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 90 insertions(+), 40 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 8babb356b4..3565f17a77 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -283,10 +283,10 @@ section_schemas = {
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
-                            'set-env': {'$ref': '#/definitions/array_of_strings'},
-                            'unset-env': {'$ref': '#/definitions/array_of_strings'},
-                            'prepend-path': {'$ref': '#/definitions/array_of_strings'},
-                            'append-path': {'$ref': '#/definitions/array_of_strings'}
+                            'set': {'$ref': '#/definitions/array_of_strings'},
+                            'unset': {'$ref': '#/definitions/array_of_strings'},
+                            'prepend_path': {'$ref': '#/definitions/array_of_strings'},
+                            'append_path': {'$ref': '#/definitions/array_of_strings'}
                         }
                     }
                 }
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 62cd2a8a8b..3d5775d03f 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -45,6 +45,7 @@ import os.path
 import re
 import shutil
 import textwrap
+import copy
 
 import llnl.util.tty as tty
 import spack
@@ -113,6 +114,84 @@ def inspect_path(prefix):
     return env
 
 
+def dependencies(spec, request='All'):
+    if request == 'None':
+        return []
+
+    l = [xx for xx in
+         sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
+
+    if request == 'Direct':
+        return [xx for ii, xx in l if ii == 1]
+
+    # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
+    # FIXME : is given. This work around permits to get a unique list of spec anyhow.
+    # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
+    seen = set()
+    seen_add = seen.add
+    return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
+
+
+def parse_config_options(module_generator):
+    autoloads, prerequisites, filters = [], [], []
+    env = EnvironmentModifications()
+    # Get the configuration for this kind of generator
+    try:
+        module_configuration = copy.copy(CONFIGURATION[module_generator.name])
+    except KeyError:
+        return autoloads, prerequisites, filters, env
+
+    # Get the defaults for all packages
+    all_conf = module_configuration.pop('all', {})
+
+    update_single(module_generator.spec, all_conf, autoloads, prerequisites, filters, env)
+
+    for spec, conf in module_configuration.items():
+        override = False
+        if spec.endswith(':'):
+            spec = spec.strip(':')
+            override = True
+        if module_generator.spec.satisfies(spec):
+            if override:
+                autoloads, prerequisites, filters = [], [], []
+                env = EnvironmentModifications()
+            update_single(module_generator.spec, conf, autoloads, prerequisites, filters, env)
+            tty.msg('{request}, {spec}'.format(request=spec, spec=module_generator.spec))
+
+    return autoloads, prerequisites, filters, env
+
+
+def update_single(spec, configuration, autoloads, prerequisites, filters, env):
+    # Get list of modules that will be loaded automatically
+    try:
+        autoloads.extend(dependencies(spec, configuration['autoload']))
+    except KeyError:
+        pass
+    try:
+        prerequisites.extend(dependencies(spec, configuration['prerequisites']))
+    except KeyError:
+        pass
+
+    # Filter modifications to environment variables
+    try:
+        filters.extend(configuration['filter']['environment_blacklist'])
+    except KeyError:
+        pass
+
+    try:
+        for method, arglist in configuration['environment'].items():
+            for item in arglist:
+                if method == 'unset':
+                    args = [item]
+                else:
+                    args = item.split(',')
+                getattr(env, method)(*args)
+    except KeyError:
+        pass
+
+
+
+
 class EnvModule(object):
     name = 'env_module'
     formats = {}
@@ -161,22 +240,6 @@ class EnvModule(object):
         if not os.path.exists(module_dir):
             mkdirp(module_dir)
 
-        def dependencies(request='All'):
-            if request == 'None':
-                return []
-
-            l = [xx for xx in sorted(self.spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
-
-            if request == 'Direct':
-                return [xx for ii, xx in l if ii == 1]
-
-            # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
-            # FIXME : is given. This work around permits to get a unique list of spec anyhow.
-            # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
-            seen = set()
-            seen_add = seen.add
-            return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
-
         # Environment modifications guessed by inspecting the
         # installation prefix
         env = inspect_path(self.spec.prefix)
@@ -186,7 +249,7 @@ class EnvModule(object):
         spack_env = EnvironmentModifications()
         # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
         # TODO : factored out to a single place
-        for item in dependencies('All'):
+        for item in dependencies(self.spec, 'All'):
             package = self.spec[item.name].package
             modules = parent_class_modules(package.__class__)
             for mod in modules:
@@ -199,30 +262,17 @@ class EnvModule(object):
         set_module_variables_for_package(self.pkg, self.pkg.module)
         self.spec.package.setup_environment(spack_env, env)
 
-        # Get list of modules that will be loaded automatically
-        try:
-            autoload_list = dependencies(CONFIGURATION[self.name]['autoload'])
-        except KeyError:
-            autoload_list = []
-
-        try:
-            prerequisites_list = dependencies(CONFIGURATION[self.name]['prerequisites'])
-        except KeyError:
-            prerequisites_list = []
-
-        # Filter modifications to environment variables
-        try:
-            filter_list = CONFIGURATION[self.name]['filter']['environment_blacklist']
-        except KeyError:
-            filter_list = []
+        # Parse configuration file
+        autoloads, prerequisites, filters, conf_env = parse_config_options(self)
+        env.extend(conf_env)
 
         # Build up the module file content
         module_file_content = self.header
-        for x in autoload_list:
+        for x in autoloads:
             module_file_content += self.autoload(x)
-        for x in prerequisites_list:
+        for x in prerequisites:
             module_file_content += self.prerequisite(x)
-        for line in self.process_environment_command(filter_environment_blacklist(env, filter_list)):
+        for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
             module_file_content += line
 
         # Dump to file
-- 
cgit v1.2.3-70-g09d2


From 7904f75782a3ba65c6096b22e61f8f789d7fc001 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 6 Apr 2016 17:05:47 +0200
Subject: modules : all the other strings are lower case

---
 lib/spack/spack/config.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 3565f17a77..6b5b3dfd62 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -255,7 +255,7 @@ section_schemas = {
             },
             'dependency_selection': {
                 'type': 'string',
-                'enum': ['None', 'Direct', 'All']
+                'enum': ['none', 'direct', 'all']
             },
             'module_file_configuration': {
                 'type': 'object',
-- 
cgit v1.2.3-70-g09d2


From 21b7734d92d210f3287588ef2fd49062b3c727c7 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 6 Apr 2016 17:08:31 +0200
Subject: modules : removed debug leftover

---
 lib/spack/spack/modules.py | 1 -
 1 file changed, 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 3d5775d03f..b12b93e32d 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -156,7 +156,6 @@ def parse_config_options(module_generator):
                 autoloads, prerequisites, filters = [], [], []
                 env = EnvironmentModifications()
             update_single(module_generator.spec, conf, autoloads, prerequisites, filters, env)
-            tty.msg('{request}, {spec}'.format(request=spec, spec=module_generator.spec))
 
     return autoloads, prerequisites, filters, env
 
-- 
cgit v1.2.3-70-g09d2


From 7dc1524c0807ea8cb6cf990d1811e0ca2218444a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Thu, 7 Apr 2016 10:48:05 +0200
Subject: modules : added stub for dotkit (to be verified)

---
 lib/spack/spack/modules.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index b12b93e32d..4f7f67e910 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -189,8 +189,6 @@ def update_single(spec, configuration, autoloads, prerequisites, filters, env):
         pass
 
 
-
-
 class EnvModule(object):
     name = 'env_module'
     formats = {}
@@ -325,6 +323,10 @@ class Dotkit(EnvModule):
         SetEnv: 'dk_setenv {name} {value}\n'
     }
 
+    autoload_format = 'dk_op {module_file}\n'  # TODO : Check this line
+
+    prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
+
     @property
     def file_name(self):
         return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
-- 
cgit v1.2.3-70-g09d2


From a5a5dbc4081ca13f579dce111394fe77d2c54a53 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@googlemail.com>
Date: Sun, 10 Apr 2016 13:21:41 +0200
Subject: tcl module file : added new-lines in autoload (per @glennpj bug
 report)

---
 lib/spack/spack/modules.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 747c0d3be5..291dd775c5 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -372,10 +372,10 @@ class TclModule(EnvModule):
         UnsetEnv: 'unsetenv {name}\n'
     }
 
-    autoload_format = ('if ![ is-loaded {module_file} ] {{'
-                       '    puts stderr "Autoloading {module_file}"'
-                       '    module load {module_file}'
-                       '}}')
+    autoload_format = ('if ![ is-loaded {module_file} ] {{\n'
+                       '    puts stderr "Autoloading {module_file}"\n'
+                       '    module load {module_file}\n'
+                       '}}\n')
 
     prerequisite_format = 'prereq {module_file}\n'
 
-- 
cgit v1.2.3-70-g09d2


From 0e2b1359e32e2948dc3ef831b83ae34483909fd0 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 11 Apr 2016 11:01:47 +0200
Subject: modules : fixed bug preventing the creation of modulefiles
 autoloading only direct dependencies

---
 lib/spack/spack/config.py  | 16 +++++++---------
 lib/spack/spack/modules.py |  6 +++---
 2 files changed, 10 insertions(+), 12 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 6b5b3dfd62..e696a62e96 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -117,22 +117,20 @@ Will make Spack take compilers *only* from the user configuration, and
 the site configuration will be ignored.
 
 """
+import copy
 import os
 import re
 import sys
-import copy
-import jsonschema
-from jsonschema import Draft4Validator, validators
-import yaml
-from yaml.error import MarkedYAMLError
-from ordereddict_backport import OrderedDict
 
+import jsonschema
 import llnl.util.tty as tty
-from llnl.util.filesystem import mkdirp
-import copy
-
 import spack
+import yaml
+from jsonschema import Draft4Validator, validators
+from llnl.util.filesystem import mkdirp
+from ordereddict_backport import OrderedDict
 from spack.error import SpackError
+from yaml.error import MarkedYAMLError
 
 # Hacked yaml for configuration files preserves line numbers.
 import spack.util.spack_yaml as syaml
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 291dd775c5..8a96d49144 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -114,14 +114,14 @@ def inspect_path(prefix):
     return env
 
 
-def dependencies(spec, request='All'):
-    if request == 'None':
+def dependencies(spec, request='all'):
+    if request == 'none':
         return []
 
     l = [xx for xx in
          sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
 
-    if request == 'Direct':
+    if request == 'direct':
         return [xx for ii, xx in l if ii == 1]
 
     # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
-- 
cgit v1.2.3-70-g09d2


From 3959ca627046ff7d6519da9c5375a0ae6da50b74 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 11 Apr 2016 18:10:06 +0200
Subject: modules : added possibility to blacklist or whitelist module files

---
 lib/spack/spack/cmd/module.py |  1 -
 lib/spack/spack/config.py     | 17 +++++++++++------
 lib/spack/spack/modules.py    | 32 +++++++++++++++++++++++++++++---
 3 files changed, 40 insertions(+), 10 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index a67f5c0c13..f996f4eb84 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -89,7 +89,6 @@ def module_refresh():
             shutil.rmtree(cls.path, ignore_errors=False)
         mkdirp(cls.path)
         for spec in specs:
-            tty.debug("   Writing file for %s" % spec)
             cls(spec).write()
 
 
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index e696a62e96..da4e787f18 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -292,12 +292,17 @@ section_schemas = {
             'module_type_configuration': {
                 'type': 'object',
                 'default': {},
-                'properties': {
-                    'all': {'$ref': '#/definitions/module_file_configuration'}
-                },
-                'patternProperties': {
-                    r'\w[\w-]*': {'$ref': '#/definitions/module_file_configuration'}
-                }
+                'oneOf': [
+                    {
+                        'properties': {
+                            'whitelist': {'$ref': '#/definitions/array_of_strings'},
+                            'blacklist': {'$ref': '#/definitions/array_of_strings'},
+                        }
+                    },
+                    {
+                        'patternProperties': {r'\w[\w-]*': {'$ref': '#/definitions/module_file_configuration'}}
+                    }
+                ]
             }
         },
         'patternProperties': {
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 8a96d49144..04f43b9605 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -40,17 +40,15 @@ various shell-support files in $SPACK/share/spack/setup-env.*.
 
 Each hook in hooks/ implements the logic for writing its specific type of module file.
 """
+import copy
 import os
 import os.path
 import re
-import shutil
 import textwrap
-import copy
 
 import llnl.util.tty as tty
 import spack
 import spack.config
-
 from llnl.util.filesystem import join_path, mkdirp
 from spack.build_environment import parent_class_modules, set_module_variables_for_package
 from spack.environment import *
@@ -225,6 +223,30 @@ class EnvModule(object):
         # Not very descriptive fallback
         return 'spack installed package'
 
+    @property
+    def blacklisted(self):
+        configuration = CONFIGURATION.get(self.name, {})
+        whitelist_matches = [x for x in configuration.get('whitelist', []) if self.spec.satisfies(x)]
+        blacklist_matches = [x for x in configuration.get('blacklist', []) if self.spec.satisfies(x)]
+        if whitelist_matches:
+            message = '\t%s is whitelisted [matches : ' % self.spec.cshort_spec
+            for rule in whitelist_matches:
+                message += '%s ' % rule
+            message += ' ]'
+            tty.debug(message)
+
+        if blacklist_matches:
+            message = '\t%s is blacklisted [matches : ' % self.spec.cshort_spec
+            for rule in blacklist_matches:
+                message += '%s ' % rule
+            message += ' ]'
+            tty.debug(message)
+
+        if not whitelist_matches and blacklist_matches:
+            return True
+
+        return False
+
     def write(self):
         """
         Writes out a module file for this object.
@@ -233,6 +255,10 @@ class EnvModule(object):
         - override the header property
         - provide formats for autoload, prerequisites and environment changes
         """
+        if self.blacklisted:
+            return
+        tty.debug("\t%s : writing module file" % self.spec.cshort_spec)
+
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
             mkdirp(module_dir)
-- 
cgit v1.2.3-70-g09d2


From 41f365112c07dc6e144da92611018ee1fe6bf30b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 12 Apr 2016 09:13:50 +0200
Subject: modules : added provenance comment in tcl header

---
 lib/spack/spack/modules.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 04f43b9605..0f921d7bf2 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -45,6 +45,7 @@ import os
 import os.path
 import re
 import textwrap
+import datetime
 
 import llnl.util.tty as tty
 import spack
@@ -401,7 +402,7 @@ class TclModule(EnvModule):
     autoload_format = ('if ![ is-loaded {module_file} ] {{\n'
                        '    puts stderr "Autoloading {module_file}"\n'
                        '    module load {module_file}\n'
-                       '}}\n')
+                       '}}\n\n')
 
     prerequisite_format = 'prereq {module_file}\n'
 
@@ -420,6 +421,12 @@ class TclModule(EnvModule):
     def header(self):
         # TCL Modulefile header
         header = '#%Module1.0\n'
+        header += '## Module file created by spack (https://github.com/LLNL/spack)'
+        header += ' on %s\n' % datetime.datetime.now()
+        header += '##\n'
+        header += '## %s\n' % self.spec.short_spec
+        header += '##\n'
+
         # TODO : category ?
         # Short description
         if self.short_description:
-- 
cgit v1.2.3-70-g09d2


From c422ce7c1d85e4f934e0ce6e2ffd90d742723a15 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 13 Apr 2016 11:26:22 +0200
Subject: modules : added doc strings, fixed minor style issues, filtered from
 dependencies blacklisted modules

---
 lib/spack/spack/config.py  |  2 +-
 lib/spack/spack/modules.py | 85 ++++++++++++++++++++++++++++++++++++----------
 2 files changed, 68 insertions(+), 19 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index da4e787f18..cf3b885ae6 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -488,7 +488,7 @@ def _read_config_file(filename, schema):
 
     elif not os.path.isfile(filename):
         raise ConfigFileError(
-            "Invlaid configuration. %s exists but is not a file." % filename)
+            "Invalid configuration. %s exists but is not a file." % filename)
 
     elif not os.access(filename, os.R_OK):
         raise ConfigFileError("Config file is not readable: %s" % filename)
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 0f921d7bf2..2ec20afa69 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -63,8 +63,9 @@ CONFIGURATION = spack.config.get_config('modules')
 
 
 def print_help():
-    """For use by commands to tell user how to activate shell support."""
-
+    """
+    For use by commands to tell user how to activate shell support.
+    """
     tty.msg("This command requires spack's shell integration.",
             "",
             "To initialize spack's shell commands, you must run one of",
@@ -114,6 +115,20 @@ def inspect_path(prefix):
 
 
 def dependencies(spec, request='all'):
+    """
+    Returns the list of dependent specs for a given spec, according to the given request
+
+    Args:
+        spec: target spec
+        request: either 'none', 'direct' or 'all'
+
+    Returns:
+        empty list if 'none', direct dependency list if 'direct', all dependencies if 'all'
+    """
+    if request not in ('none', 'direct', 'all'):
+        raise tty.error("Wrong value for argument 'request' : should be one of ('none', 'direct', 'all') "
+                        " [current value is '%s']" % request)
+
     if request == 'none':
         return []
 
@@ -132,6 +147,18 @@ def dependencies(spec, request='all'):
 
 
 def parse_config_options(module_generator):
+    """
+    Parse the configuration file and returns a bunch of items that will be needed during module file generation
+
+    Args:
+        module_generator: module generator for a given spec
+
+    Returns:
+        autoloads: list of specs to be autoloaded
+        prerequisites: list of specs to be marked as prerequisite
+        filters: list of environment variables whose modification is blacklisted in module files
+        env: list of custom environment modifications to be applied in the module file
+    """
     autoloads, prerequisites, filters = [], [], []
     env = EnvironmentModifications()
     # Get the configuration for this kind of generator
@@ -160,15 +187,20 @@ def parse_config_options(module_generator):
 
 
 def update_single(spec, configuration, autoloads, prerequisites, filters, env):
+    """
+    Updates the entries in the arguments according to the configuration
+
+    Args:
+        spec: [in] target spec
+        configuration: [in] configuration file for the current type of module file generator
+        autoloads: [inout] list of dependencies to be automatically loaded
+        prerequisites: [inout] list of prerequisites
+        filters: [inout] list of environment variables whose modification is to be blacklisted
+        env: [inout] list of modifications to the environment
+    """
     # Get list of modules that will be loaded automatically
-    try:
-        autoloads.extend(dependencies(spec, configuration['autoload']))
-    except KeyError:
-        pass
-    try:
-        prerequisites.extend(dependencies(spec, configuration['prerequisites']))
-    except KeyError:
-        pass
+    autoloads.extend(dependencies(spec, configuration.get('autoload', 'none')))
+    prerequisites.extend(dependencies(spec, configuration.get('prerequisites', 'none')))
 
     # Filter modifications to environment variables
     try:
@@ -188,6 +220,24 @@ def update_single(spec, configuration, autoloads, prerequisites, filters, env):
         pass
 
 
+def filter_blacklisted(specs, module_name):
+    """
+    Given a sequence of specs, filters the ones that are blacklisted in the module configuration file.
+
+    Args:
+        specs: sequence of spec instances
+        module_name: type of module file objects
+
+    Yields:
+        non blacklisted specs
+    """
+    for x in specs:
+        if module_types[module_name](x).blacklisted:
+            tty.debug('\tFILTER : %s' % x)
+            continue
+        yield x
+
+
 class EnvModule(object):
     name = 'env_module'
     formats = {}
@@ -230,14 +280,14 @@ class EnvModule(object):
         whitelist_matches = [x for x in configuration.get('whitelist', []) if self.spec.satisfies(x)]
         blacklist_matches = [x for x in configuration.get('blacklist', []) if self.spec.satisfies(x)]
         if whitelist_matches:
-            message = '\t%s is whitelisted [matches : ' % self.spec.cshort_spec
+            message = '\tWHITELIST : %s [matches : ' % self.spec.cshort_spec
             for rule in whitelist_matches:
                 message += '%s ' % rule
             message += ' ]'
             tty.debug(message)
 
         if blacklist_matches:
-            message = '\t%s is blacklisted [matches : ' % self.spec.cshort_spec
+            message = '\tBLACKLIST : %s [matches : ' % self.spec.cshort_spec
             for rule in blacklist_matches:
                 message += '%s ' % rule
             message += ' ]'
@@ -258,7 +308,7 @@ class EnvModule(object):
         """
         if self.blacklisted:
             return
-        tty.debug("\t%s : writing module file" % self.spec.cshort_spec)
+        tty.debug("\tWRITE : %s [%s]" % (self.spec.cshort_spec, self.file_name))
 
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
@@ -273,7 +323,7 @@ class EnvModule(object):
         spack_env = EnvironmentModifications()
         # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
         # TODO : factored out to a single place
-        for item in dependencies(self.spec, 'All'):
+        for item in dependencies(self.spec, 'all'):
             package = self.spec[item.name].package
             modules = parent_class_modules(package.__class__)
             for mod in modules:
@@ -292,9 +342,9 @@ class EnvModule(object):
 
         # Build up the module file content
         module_file_content = self.header
-        for x in autoloads:
+        for x in filter_blacklisted(autoloads, self.name):
             module_file_content += self.autoload(x)
-        for x in prerequisites:
+        for x in filter_blacklisted(prerequisites, self.name):
             module_file_content += self.prerequisite(x)
         for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
             module_file_content += line
@@ -421,8 +471,7 @@ class TclModule(EnvModule):
     def header(self):
         # TCL Modulefile header
         header = '#%Module1.0\n'
-        header += '## Module file created by spack (https://github.com/LLNL/spack)'
-        header += ' on %s\n' % datetime.datetime.now()
+        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % datetime.datetime.now()
         header += '##\n'
         header += '## %s\n' % self.spec.short_spec
         header += '##\n'
-- 
cgit v1.2.3-70-g09d2


From 1b4c4be151650f28e0e7d796cbfe85d05ea91f49 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 13 Apr 2016 12:44:51 +0200
Subject: modules : category is a single word (as I am not sure how dotkit will
 react to spaces)

---
 lib/spack/spack/modules.py | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 2ec20afa69..2c68c0c170 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -270,9 +270,9 @@ class EnvModule(object):
             return self.pkg.category
         # Extensions
         for extendee in self.pkg.extendees:
-            return '{extendee} extension'.format(extendee=extendee)
+            return '{extendee}_extension'.format(extendee=extendee)
         # Not very descriptive fallback
-        return 'spack installed package'
+        return 'spack'
 
     @property
     def blacklisted(self):
@@ -404,7 +404,7 @@ class Dotkit(EnvModule):
         SetEnv: 'dk_setenv {name} {value}\n'
     }
 
-    autoload_format = 'dk_op {module_file}\n'  # TODO : Check this line
+    autoload_format = 'dk_op {module_file}\n'
 
     prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
 
-- 
cgit v1.2.3-70-g09d2


From 00f44d558ab3b3ee4b9d04534a01ad7decbe0d7b Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 12 Apr 2016 15:42:55 +0200
Subject: modules : started working on naming schemes and conflict

---
 lib/spack/spack/config.py  |  9 +++++++-
 lib/spack/spack/modules.py | 56 ++++++++++++++++++++++++++++------------------
 2 files changed, 42 insertions(+), 23 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index cf3b885ae6..f9a680d109 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -297,6 +297,9 @@ section_schemas = {
                         'properties': {
                             'whitelist': {'$ref': '#/definitions/array_of_strings'},
                             'blacklist': {'$ref': '#/definitions/array_of_strings'},
+                            'naming_scheme': {
+                                'type': 'string'  # Can we be more specific here?
+                            }
                         }
                     },
                     {
@@ -322,7 +325,11 @@ section_schemas = {
                     'tcl': {
                         'allOf': [
                             {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
-                            {}  # Specific tcl extensions
+                            {
+                                'properties': {
+                                    'conflict': {'type': 'string'}
+                                }
+                            }  # Specific tcl extensions
                         ]
                     },
                     'dotkit': {
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 2c68c0c170..e6e9be1eff 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -41,11 +41,11 @@ various shell-support files in $SPACK/share/spack/setup-env.*.
 Each hook in hooks/ implements the logic for writing its specific type of module file.
 """
 import copy
+import datetime
 import os
 import os.path
 import re
 import textwrap
-import datetime
 
 import llnl.util.tty as tty
 import spack
@@ -75,7 +75,7 @@ def print_help():
             "    . %s/setup-env.sh" % spack.share_path,
             "",
             "For csh and tcsh:",
-            "    setenv SPACK_ROOT %s"    % spack.prefix,
+            "    setenv SPACK_ROOT %s" % spack.prefix,
             "    source %s/setup-env.csh" % spack.share_path,
             "")
 
@@ -263,6 +263,34 @@ class EnvModule(object):
         if self.spec.package.__doc__:
             self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
+    @property
+    def naming_scheme(self):
+        try:
+            naming_scheme = CONFIGURATION[self.name]['naming_scheme']
+        except KeyError:
+            naming_scheme = self.default_naming_format
+        return naming_scheme
+
+    @property
+    def tokens(self):
+        tokens = {
+            'name': self.spec.name,
+            'version': self.spec.version,
+            'compiler': self.spec.compiler,
+            'hash': self.spec.dag_hash()
+        }
+        return tokens
+
+    @property
+    def use_name(self):
+        """
+        Subclasses should implement this to return the name the module command uses to refer to the package.
+        """
+        naming_tokens = self.tokens
+        naming_scheme = self.naming_scheme
+        name = naming_scheme.format(**naming_tokens)
+        return name
+
     @property
     def category(self):
         # Anything defined at the package level takes precedence
@@ -379,12 +407,6 @@ class EnvModule(object):
            where this module lives."""
         raise NotImplementedError()
 
-    @property
-    def use_name(self):
-        """Subclasses should implement this to return the name the
-           module command uses to refer to the package."""
-        raise NotImplementedError()
-
     def remove(self):
         mod_file = self.file_name
         if os.path.exists(mod_file):
@@ -408,17 +430,12 @@ class Dotkit(EnvModule):
 
     prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
 
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}-{hash}'
+
     @property
     def file_name(self):
         return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
 
-    @property
-    def use_name(self):
-      return "%s-%s-%s-%s-%s" % (self.spec.name, self.spec.version,
-                                 self.spec.compiler.name,
-                                 self.spec.compiler.version,
-                                 self.spec.dag_hash())
-
     @property
     def header(self):
         # Category
@@ -456,17 +473,12 @@ class TclModule(EnvModule):
 
     prerequisite_format = 'prereq {module_file}\n'
 
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}-{hash}'
+
     @property
     def file_name(self):
         return join_path(TclModule.path, self.spec.architecture, self.use_name)
 
-    @property
-    def use_name(self):
-      return "%s-%s-%s-%s-%s" % (self.spec.name, self.spec.version,
-                                 self.spec.compiler.name,
-                                 self.spec.compiler.version,
-                                 self.spec.dag_hash())
-
     @property
     def header(self):
         # TCL Modulefile header
-- 
cgit v1.2.3-70-g09d2


From c69acfa5c80fc4ea93787c19f9f4a6f3cf833f9c Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 15 Apr 2016 12:21:32 +0200
Subject: naming work correctly

---
 lib/spack/spack/modules.py | 11 +++++++----
 share/spack/setup-env.sh   |  6 +++---
 2 files changed, 10 insertions(+), 7 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index e6e9be1eff..a24d5e5d09 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -276,8 +276,7 @@ class EnvModule(object):
         tokens = {
             'name': self.spec.name,
             'version': self.spec.version,
-            'compiler': self.spec.compiler,
-            'hash': self.spec.dag_hash()
+            'compiler': self.spec.compiler
         }
         return tokens
 
@@ -289,6 +288,10 @@ class EnvModule(object):
         naming_tokens = self.tokens
         naming_scheme = self.naming_scheme
         name = naming_scheme.format(**naming_tokens)
+        name += '-' + self.spec.dag_hash()  # Always append the hash to make the module file unique
+        # Not everybody is working on linux...
+        parts = name.split('/')
+        name = join_path(*parts)
         return name
 
     @property
@@ -430,7 +433,7 @@ class Dotkit(EnvModule):
 
     prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}-{hash}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
 
     @property
     def file_name(self):
@@ -473,7 +476,7 @@ class TclModule(EnvModule):
 
     prerequisite_format = 'prereq {module_file}\n'
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}-{hash}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
 
     @property
     def file_name(self):
diff --git a/share/spack/setup-env.sh b/share/spack/setup-env.sh
index 764af68400..0c9e5b2409 100755
--- a/share/spack/setup-env.sh
+++ b/share/spack/setup-env.sh
@@ -41,7 +41,7 @@
 # commands.  This allows the user to use packages without knowing all
 # their installation details.
 #
-# e.g., rather than requring a full spec for libelf, the user can type:
+# e.g., rather than requiring a full spec for libelf, the user can type:
 #
 #     spack use libelf
 #
@@ -110,11 +110,11 @@ function spack {
                         unuse $_sp_module_args $_sp_full_spec
                     fi ;;
                 "load")
-                    if _sp_full_spec=$(command spack $_sp_flags module find dotkit $_sp_spec); then
+                    if _sp_full_spec=$(command spack $_sp_flags module find tcl $_sp_spec); then
                         module load $_sp_module_args $_sp_full_spec
                     fi ;;
                 "unload")
-                    if _sp_full_spec=$(command spack $_sp_flags module find dotkit $_sp_spec); then
+                    if _sp_full_spec=$(command spack $_sp_flags module find tcl $_sp_spec); then
                         module unload $_sp_module_args $_sp_full_spec
                     fi ;;
             esac
-- 
cgit v1.2.3-70-g09d2


From 18a241fe213618a0390a8071480854a5c06e3681 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 15 Apr 2016 15:19:02 +0200
Subject: modules : added hook for module specific extensions

---
 lib/spack/spack/config.py  |   7 +---
 lib/spack/spack/modules.py | 100 +++++++++++++++++++++------------------------
 2 files changed, 49 insertions(+), 58 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index f9a680d109..8e4cbba96e 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -276,6 +276,7 @@ section_schemas = {
                     },
                     'autoload': {'$ref': '#/definitions/dependency_selection'},
                     'prerequisites': {'$ref': '#/definitions/dependency_selection'},
+                    'conflict': {'type': 'string'},
                     'environment': {
                         'type': 'object',
                         'default': {},
@@ -325,11 +326,7 @@ section_schemas = {
                     'tcl': {
                         'allOf': [
                             {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
-                            {
-                                'properties': {
-                                    'conflict': {'type': 'string'}
-                                }
-                            }  # Specific tcl extensions
+                            {}  # Specific tcl extensions
                         ]
                     },
                     'dotkit': {
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index a24d5e5d09..a81132bcac 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -146,6 +146,17 @@ def dependencies(spec, request='all'):
     return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
 
 
+def update_dictionary_extending_lists(target, update):
+    for key in update:
+        value = target.get(key, None)
+        if isinstance(value, list):
+            target[key].extend(update[key])
+        elif isinstance(value, dict):
+            update_dictionary_extending_lists(target[key], update[key])
+        else:
+            target[key] = update[key]
+
+
 def parse_config_options(module_generator):
     """
     Parse the configuration file and returns a bunch of items that will be needed during module file generation
@@ -159,19 +170,13 @@ def parse_config_options(module_generator):
         filters: list of environment variables whose modification is blacklisted in module files
         env: list of custom environment modifications to be applied in the module file
     """
-    autoloads, prerequisites, filters = [], [], []
-    env = EnvironmentModifications()
     # Get the configuration for this kind of generator
-    try:
-        module_configuration = copy.copy(CONFIGURATION[module_generator.name])
-    except KeyError:
-        return autoloads, prerequisites, filters, env
-
-    # Get the defaults for all packages
-    all_conf = module_configuration.pop('all', {})
-
-    update_single(module_generator.spec, all_conf, autoloads, prerequisites, filters, env)
+    module_configuration = copy.copy(CONFIGURATION.get(module_generator.name, {}))
 
+    #####
+    # Merge all the rules
+    #####
+    module_file_actions = module_configuration.pop('all', {})
     for spec, conf in module_configuration.items():
         override = False
         if spec.endswith(':'):
@@ -179,45 +184,29 @@ def parse_config_options(module_generator):
             override = True
         if module_generator.spec.satisfies(spec):
             if override:
-                autoloads, prerequisites, filters = [], [], []
-                env = EnvironmentModifications()
-            update_single(module_generator.spec, conf, autoloads, prerequisites, filters, env)
-
-    return autoloads, prerequisites, filters, env
-
-
-def update_single(spec, configuration, autoloads, prerequisites, filters, env):
-    """
-    Updates the entries in the arguments according to the configuration
+                module_file_actions = {}
+            update_dictionary_extending_lists(module_file_actions, conf)
+
+    #####
+    # Process the common rules
+    #####
+
+    # Automatic loading loads
+    module_file_actions['autoload'] = dependencies(module_generator.spec, module_file_actions.get('autoload', 'none'))
+    # Prerequisites
+    module_file_actions['prerequisites'] = dependencies(module_generator.spec, module_file_actions.get('prerequisites', 'none'))
+    # Environment modifications
+    environment_actions = module_file_actions.pop('environment', {})
+    env = EnvironmentModifications()
+    for method, arglist in environment_actions.items():
+        for item in arglist:
+            if method == 'unset':
+                args = [item]
+            else:
+                args = item.split(',')
+            getattr(env, method)(*args)
 
-    Args:
-        spec: [in] target spec
-        configuration: [in] configuration file for the current type of module file generator
-        autoloads: [inout] list of dependencies to be automatically loaded
-        prerequisites: [inout] list of prerequisites
-        filters: [inout] list of environment variables whose modification is to be blacklisted
-        env: [inout] list of modifications to the environment
-    """
-    # Get list of modules that will be loaded automatically
-    autoloads.extend(dependencies(spec, configuration.get('autoload', 'none')))
-    prerequisites.extend(dependencies(spec, configuration.get('prerequisites', 'none')))
-
-    # Filter modifications to environment variables
-    try:
-        filters.extend(configuration['filter']['environment_blacklist'])
-    except KeyError:
-        pass
-
-    try:
-        for method, arglist in configuration['environment'].items():
-            for item in arglist:
-                if method == 'unset':
-                    args = [item]
-                else:
-                    args = item.split(',')
-                getattr(env, method)(*args)
-    except KeyError:
-        pass
+    return module_file_actions, env
 
 
 def filter_blacklisted(specs, module_name):
@@ -368,17 +357,19 @@ class EnvModule(object):
         self.spec.package.setup_environment(spack_env, env)
 
         # Parse configuration file
-        autoloads, prerequisites, filters, conf_env = parse_config_options(self)
+        module_configuration, conf_env = parse_config_options(self)
         env.extend(conf_env)
-
+        filters = module_configuration.get('filter', {}).get('environment_blacklist',{})
         # Build up the module file content
         module_file_content = self.header
-        for x in filter_blacklisted(autoloads, self.name):
+        for x in filter_blacklisted(module_configuration.pop('autoload', []), self.name):
             module_file_content += self.autoload(x)
-        for x in filter_blacklisted(prerequisites, self.name):
+        for x in filter_blacklisted(module_configuration.pop('prerequisites', []), self.name):
             module_file_content += self.prerequisite(x)
         for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
             module_file_content += line
+        for line in self.module_specific_content(module_configuration):
+            module_file_content += line
 
         # Dump to file
         with open(self.file_name, 'w') as f:
@@ -388,6 +379,9 @@ class EnvModule(object):
     def header(self):
         raise NotImplementedError()
 
+    def module_specific_content(self, configuration):
+        return tuple()
+
     def autoload(self, spec):
         m = TclModule(spec)
         return self.autoload_format.format(module_file=m.use_name)
-- 
cgit v1.2.3-70-g09d2


From 50b148ca22b8bc9daeb1cb427eadeefd06e84800 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 18 Apr 2016 13:09:39 +0200
Subject: modules : tcl modules handle 'conflict' directive. This should
 completely cover the functionality in #498

---
 lib/spack/spack/modules.py | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index a81132bcac..61ecdc10c3 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -497,3 +497,19 @@ class TclModule(EnvModule):
                 header += 'puts stderr "%s"\n' % line
             header += '}\n\n'
         return header
+
+    def module_specific_content(self, configuration):
+        naming_tokens = self.tokens
+        # Conflict
+        conflict_format = configuration.get('conflict', '')
+        if conflict_format:
+            for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), conflict_format.split('/')):
+                if naming_dir != conflict_dir:
+                    message = 'Conflict scheme does not match naming scheme [{spec}]\n\n'
+                    message += 'naming scheme   : "{nformat}"\n'
+                    message += 'conflict scheme : "{cformat}"\n'
+                    raise tty.error(
+                        message.format(spec=self.spec, nformat=self.naming_scheme, cformat=conflict_format)
+                    )
+            conflict_format = 'conflict ' + conflict_format
+            yield conflict_format.format(**naming_tokens)
-- 
cgit v1.2.3-70-g09d2


From bce276d57301e97f7d09a902f4caa8a76735e22d Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Mon, 18 Apr 2016 17:51:53 +0200
Subject: fix : missing autoload, failing validation

---
 lib/spack/spack/config.py  | 2 +-
 lib/spack/spack/modules.py | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 8e4cbba96e..1ac8a01619 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -293,7 +293,7 @@ section_schemas = {
             'module_type_configuration': {
                 'type': 'object',
                 'default': {},
-                'oneOf': [
+                'anyOf': [
                     {
                         'properties': {
                             'whitelist': {'$ref': '#/definitions/array_of_strings'},
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 61ecdc10c3..5bf003f9ad 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -171,7 +171,7 @@ def parse_config_options(module_generator):
         env: list of custom environment modifications to be applied in the module file
     """
     # Get the configuration for this kind of generator
-    module_configuration = copy.copy(CONFIGURATION.get(module_generator.name, {}))
+    module_configuration = copy.deepcopy(CONFIGURATION.get(module_generator.name, {}))
 
     #####
     # Merge all the rules
-- 
cgit v1.2.3-70-g09d2


From 5deaaa278ce011e2a143dd017a494d97378959c4 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 19 Apr 2016 14:25:12 +0200
Subject: modules : added a few unit tests

---
 lib/spack/spack/modules.py       |   6 +-
 lib/spack/spack/test/__init__.py |   1 +
 lib/spack/spack/test/modules.py  | 126 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+), 4 deletions(-)
 create mode 100644 lib/spack/spack/test/modules.py

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 5bf003f9ad..bcccf2f9b8 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -132,17 +132,15 @@ def dependencies(spec, request='all'):
     if request == 'none':
         return []
 
-    l = [xx for xx in
-         sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
-
     if request == 'direct':
-        return [xx for ii, xx in l if ii == 1]
+        return [xx for _, xx in spec.dependencies.items()]
 
     # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
     # FIXME : is given. This work around permits to get a unique list of spec anyhow.
     # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
     seen = set()
     seen_add = seen.add
+    l = [xx for xx in sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
     return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
 
 
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 175a49428c..b1c91c7903 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -54,6 +54,7 @@ test_names = ['versions',
               'svn_fetch',
               'hg_fetch',
               'mirror',
+              'modules',
               'url_extrapolate',
               'cc',
               'link_tree',
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
new file mode 100644
index 0000000000..21afa355e7
--- /dev/null
+++ b/lib/spack/spack/test/modules.py
@@ -0,0 +1,126 @@
+import collections
+from contextlib import contextmanager
+
+import StringIO
+
+FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
+
+# Monkey-patch open to write module files to a StringIO instance
+@contextmanager
+def mock_open(filename, mode):
+    if not mode == 'w':
+        raise RuntimeError('test.modules : unexpected opening mode for monkey-patched open')
+
+    FILE_REGISTRY[filename] = StringIO.StringIO()
+
+    try:
+        yield FILE_REGISTRY[filename]
+    finally:
+        handle = FILE_REGISTRY[filename]
+        FILE_REGISTRY[filename] = handle.getvalue()
+        handle.close()
+
+import spack.modules
+
+configuration_autoload_direct = {
+    'enable': ['tcl'],
+    'tcl': {
+        'all': {
+            'autoload': 'direct'
+        }
+    }
+}
+
+configuration_autoload_all = {
+    'enable': ['tcl'],
+    'tcl': {
+        'all': {
+            'autoload': 'all'
+        }
+    }
+}
+
+configuration_alter_environment = {
+    'enable': ['tcl'],
+    'tcl': {
+        'all': {
+            'filter': {'environment_blacklist': ['CMAKE_PREFIX_PATH']}
+        },
+        '=x86-linux': {
+            'environment': {'set': ['FOO,foo'], 'unset': ['BAR']}
+        }
+    }
+}
+
+configuration_blacklist = {
+    'enable': ['tcl'],
+    'tcl': {
+        'blacklist': ['callpath'],
+        'all': {
+            'autoload': 'direct'
+        }
+    }
+}
+
+from spack.test.mock_packages_test import MockPackagesTest
+
+
+class TclTests(MockPackagesTest):
+    def setUp(self):
+        super(TclTests, self).setUp()
+        self.configuration_obj = spack.modules.CONFIGURATION
+        spack.modules.open = mock_open
+        spack.modules.CONFIGURATION = None  # Make sure that a non-mocked configuration will trigger an error
+
+    def tearDown(self):
+        del spack.modules.open
+        spack.modules.CONFIGURATION = self.configuration_obj
+        super(TclTests, self).tearDown()
+
+    def get_modulefile_content(self, spec):
+        spec.concretize()
+        generator = spack.modules.TclModule(spec)
+        generator.write()
+        content = FILE_REGISTRY[generator.file_name].split('\n')
+        return content
+
+    def test_simple_case(self):
+        spack.modules.CONFIGURATION = configuration_autoload_direct
+        spec = spack.spec.Spec('mpich@3.0.4=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertTrue('module-whatis "mpich @3.0.4"' in content )
+        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 1)
+
+    def test_autoload(self):
+        spack.modules.CONFIGURATION = configuration_autoload_direct
+        spec = spack.spec.Spec('mpileaks=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if 'is-loaded' in x]), 2)
+        self.assertEqual(len([x for x in content if 'module load ' in x]), 2)
+
+        spack.modules.CONFIGURATION = configuration_autoload_all
+        spec = spack.spec.Spec('mpileaks=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if 'is-loaded' in x]), 5)
+        self.assertEqual(len([x for x in content if 'module load ' in x]), 5)
+
+    def test_alter_environment(self):
+        spack.modules.CONFIGURATION = configuration_alter_environment
+        spec = spack.spec.Spec('mpileaks=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 1)
+        self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 1)
+
+        spec = spack.spec.Spec('libdwarf=x64-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 0)
+        self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 0)
+
+    def test_blacklist(self):
+        spack.modules.CONFIGURATION = configuration_blacklist
+        spec = spack.spec.Spec('mpileaks=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if 'is-loaded' in x]), 1)
+        self.assertEqual(len([x for x in content if 'module load ' in x]), 1)
-- 
cgit v1.2.3-70-g09d2


From 67a01ef2ee182697df49f808654cf17f0e164b4a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Fri, 22 Apr 2016 10:50:24 +0200
Subject: tcl : extended conflict to be an array of strings

---
 lib/spack/spack/config.py       |  2 +-
 lib/spack/spack/modules.py      | 30 ++++++++++++++++++------------
 lib/spack/spack/test/modules.py | 18 ++++++++++++++++++
 3 files changed, 37 insertions(+), 13 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 1ac8a01619..71cdff09ea 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -276,7 +276,7 @@ section_schemas = {
                     },
                     'autoload': {'$ref': '#/definitions/dependency_selection'},
                     'prerequisites': {'$ref': '#/definitions/dependency_selection'},
-                    'conflict': {'type': 'string'},
+                    'conflict': {'$ref': '#/definitions/array_of_strings'},
                     'environment': {
                         'type': 'object',
                         'default': {},
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index bcccf2f9b8..57a4a2c754 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -46,6 +46,7 @@ import os
 import os.path
 import re
 import textwrap
+import string
 
 import llnl.util.tty as tty
 import spack
@@ -499,15 +500,20 @@ class TclModule(EnvModule):
     def module_specific_content(self, configuration):
         naming_tokens = self.tokens
         # Conflict
-        conflict_format = configuration.get('conflict', '')
-        if conflict_format:
-            for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), conflict_format.split('/')):
-                if naming_dir != conflict_dir:
-                    message = 'Conflict scheme does not match naming scheme [{spec}]\n\n'
-                    message += 'naming scheme   : "{nformat}"\n'
-                    message += 'conflict scheme : "{cformat}"\n'
-                    raise tty.error(
-                        message.format(spec=self.spec, nformat=self.naming_scheme, cformat=conflict_format)
-                    )
-            conflict_format = 'conflict ' + conflict_format
-            yield conflict_format.format(**naming_tokens)
+        conflict_format = configuration.get('conflict', [])
+        f = string.Formatter()
+        for item in conflict_format:
+            line = 'conflict ' + item + '\n'
+            if len([x for x in f.parse(line)]) > 1:  # We do have placeholder to substitute
+                for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), item.split('/')):
+                    if naming_dir != conflict_dir:
+                        message = 'conflict scheme does not match naming scheme [{spec}]\n\n'
+                        message += 'naming scheme   : "{nformat}"\n'
+                        message += 'conflict scheme : "{cformat}"\n\n'
+                        message += '** You may want to check your `modules.yaml` configuration file **\n'
+                        tty.error(
+                            message.format(spec=self.spec, nformat=self.naming_scheme, cformat=item)
+                        )
+                        raise SystemExit('Module generation aborted.')
+                line = line.format(**naming_tokens)
+            yield line
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index 21afa355e7..704700417b 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -62,6 +62,16 @@ configuration_blacklist = {
     }
 }
 
+configuration_conflicts = {
+    'enable': ['tcl'],
+    'tcl': {
+        'naming_scheme': '{name}/{version}-{compiler.name}',
+        'all': {
+            'conflict': ['{name}', 'intel/14.0.1']
+        }
+    }
+}
+
 from spack.test.mock_packages_test import MockPackagesTest
 
 
@@ -124,3 +134,11 @@ class TclTests(MockPackagesTest):
         content = self.get_modulefile_content(spec)
         self.assertEqual(len([x for x in content if 'is-loaded' in x]), 1)
         self.assertEqual(len([x for x in content if 'module load ' in x]), 1)
+
+    def test_conflicts(self):
+        spack.modules.CONFIGURATION = configuration_conflicts
+        spec = spack.spec.Spec('mpileaks=x86-linux')
+        content = self.get_modulefile_content(spec)
+        self.assertEqual(len([x for x in content if x.startswith('conflict')]), 2)
+        self.assertEqual(len([x for x in content if x == 'conflict mpileaks']), 1)
+        self.assertEqual(len([x for x in content if x == 'conflict intel/14.0.1']), 1)
-- 
cgit v1.2.3-70-g09d2


From f8f71b1c2c456210b64a90eb5b21484a8265cfa6 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 10 May 2016 13:37:03 +0200
Subject: modules : prefix_inspections moved to modules.yaml

---
 etc/spack/modules.yaml     | 10 ++++++++++
 lib/spack/spack/config.py  |  8 ++++++++
 lib/spack/spack/modules.py | 18 +++---------------
 3 files changed, 21 insertions(+), 15 deletions(-)

(limited to 'lib')

diff --git a/etc/spack/modules.yaml b/etc/spack/modules.yaml
index aa2a2c3fe2..8f8f88e908 100644
--- a/etc/spack/modules.yaml
+++ b/etc/spack/modules.yaml
@@ -5,4 +5,14 @@
 # although users can override these settings in their ~/.spack/modules.yaml.
 # -------------------------------------------------------------------------
 modules:
+  prefix_inspections: {
+        bin: ['PATH'],
+        man: ['MANPATH'],
+        lib: ['LIBRARY_PATH', 'LD_LIBRARY_PATH'],
+        lib64: ['LIBRARY_PATH', 'LD_LIBRARY_PATH'],
+        include: ['CPATH'],
+        lib/pkgconfig: ['PKGCONFIG'],
+        lib64/pkgconfig: ['PKGCONFIG'],
+        '': ['CMAKE_PREFIX_PATH']
+  }
   enable: ['tcl', 'dotkit']
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 64809462f9..d008a513e7 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -315,6 +315,14 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'properties': {
+                    'prefix_inspections': {
+                        'type': 'object',
+                        'patternProperties': {
+                            r'\w[\w-]*': {  # path to be inspected for existence (relative to prefix)
+                                '$ref': '#/definitions/array_of_strings'
+                            }
+                        }
+                    },
                     'enable': {
                         'type': 'array',
                         'default': [],
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 57a4a2c754..f1c0bd87de 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -94,24 +94,12 @@ def inspect_path(prefix):
     """
     env = EnvironmentModifications()
     # Inspect the prefix to check for the existence of common directories
-    prefix_inspections = {
-        'bin': ('PATH',),
-        'man': ('MANPATH',),
-        'lib': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-        'lib64': ('LIBRARY_PATH', 'LD_LIBRARY_PATH'),
-        'include': ('CPATH',)
-    }
-    for attribute, variables in prefix_inspections.items():
-        expected = getattr(prefix, attribute)
+    prefix_inspections = CONFIGURATION.get('prefix_inspections', {})
+    for relative_path, variables in prefix_inspections.items():
+        expected = join_path(prefix, relative_path)
         if os.path.isdir(expected):
             for variable in variables:
                 env.prepend_path(variable, expected)
-    # PKGCONFIG
-    for expected in (join_path(prefix.lib, 'pkgconfig'), join_path(prefix.lib64, 'pkgconfig')):
-        if os.path.isdir(expected):
-            env.prepend_path('PKG_CONFIG_PATH', expected)
-    # CMake related variables
-    env.prepend_path('CMAKE_PREFIX_PATH', prefix)
     return env
 
 
-- 
cgit v1.2.3-70-g09d2


From 0b7c673205049565a95707fd55d2c086dd601b35 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 10 May 2016 15:48:37 +0200
Subject: modules : changed syntax for environment modifications

---
 lib/spack/spack/config.py       | 14 +++++++++++---
 lib/spack/spack/modules.py      | 22 +++++++++++++++++-----
 lib/spack/spack/test/modules.py |  3 +--
 3 files changed, 29 insertions(+), 10 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index d008a513e7..6ddf07776b 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -251,6 +251,14 @@ section_schemas = {
                     'type': 'string'
                 }
             },
+            'dictionary_of_strings': {
+                'type': 'object',
+                'patternProperties': {
+                    r'\w[\w-]*': {  # key
+                        'type': 'string'
+                    }
+                }
+            },
             'dependency_selection': {
                 'type': 'string',
                 'enum': ['none', 'direct', 'all']
@@ -282,10 +290,10 @@ section_schemas = {
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
-                            'set': {'$ref': '#/definitions/array_of_strings'},
+                            'set': {'$ref': '#/definitions/dictionary_of_strings'},
                             'unset': {'$ref': '#/definitions/array_of_strings'},
-                            'prepend_path': {'$ref': '#/definitions/array_of_strings'},
-                            'append_path': {'$ref': '#/definitions/array_of_strings'}
+                            'prepend_path': {'$ref': '#/definitions/dictionary_of_strings'},
+                            'append_path': {'$ref': '#/definitions/dictionary_of_strings'}
                         }
                     }
                 }
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index f1c0bd87de..19bd1993a7 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -185,14 +185,26 @@ def parse_config_options(module_generator):
     # Environment modifications
     environment_actions = module_file_actions.pop('environment', {})
     env = EnvironmentModifications()
+
+    def process_arglist(arglist):
+        if method == 'unset':
+            for x in arglist:
+                yield (x,)
+        else:
+            for x in arglist.iteritems():
+                yield x
+
     for method, arglist in environment_actions.items():
-        for item in arglist:
-            if method == 'unset':
-                args = [item]
-            else:
-                args = item.split(',')
+        for args in process_arglist(arglist):
             getattr(env, method)(*args)
 
+        # for item in arglist:
+        #     if method == 'unset':
+        #         args = [item]
+        #     else:
+        #         args = item.split(',')
+        #     getattr(env, method)(*args)
+
     return module_file_actions, env
 
 
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index 704700417b..b8b0d6fc6a 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -47,7 +47,7 @@ configuration_alter_environment = {
             'filter': {'environment_blacklist': ['CMAKE_PREFIX_PATH']}
         },
         '=x86-linux': {
-            'environment': {'set': ['FOO,foo'], 'unset': ['BAR']}
+            'environment': {'set': {'FOO': 'foo'}, 'unset': ['BAR']}
         }
     }
 }
@@ -99,7 +99,6 @@ class TclTests(MockPackagesTest):
         spec = spack.spec.Spec('mpich@3.0.4=x86-linux')
         content = self.get_modulefile_content(spec)
         self.assertTrue('module-whatis "mpich @3.0.4"' in content )
-        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 1)
 
     def test_autoload(self):
         spack.modules.CONFIGURATION = configuration_autoload_direct
-- 
cgit v1.2.3-70-g09d2


From c3f3f26632ff0450683c99bebcce3acfad8a5ebc Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 10 May 2016 16:16:50 +0200
Subject: modules : added warning if a user tries to add prerequisite with
 dotkit

---
 lib/spack/spack/modules.py | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 19bd1993a7..ffed469b20 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -382,11 +382,11 @@ class EnvModule(object):
         return tuple()
 
     def autoload(self, spec):
-        m = TclModule(spec)
+        m = type(self)(spec)
         return self.autoload_format.format(module_file=m.use_name)
 
     def prerequisite(self, spec):
-        m = TclModule(spec)
+        m = type(self)(spec)
         return self.prerequisite_format.format(module_file=m.use_name)
 
     def process_environment_command(self, env):
@@ -449,6 +449,11 @@ class Dotkit(EnvModule):
                 header += '#h %s\n' % line
         return header
 
+    def prerequisite(self, spec):
+        tty.warn('prerequisites:  not supported by dotkit module files')
+        tty.warn('\tYou may want to check  ~/.spack/modules.yaml')
+        return ''
+
 
 class TclModule(EnvModule):
     name = 'tcl'
-- 
cgit v1.2.3-70-g09d2


From 71e49e289a849b8aaa4f0d9a195d07569051ca88 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Tue, 10 May 2016 17:19:22 +0200
Subject: flake8 : fixed all issues?

---
 lib/spack/spack/cmd/module.py    |  16 +--
 lib/spack/spack/config.py        | 204 ++++++++++++++++++++++++---------------
 lib/spack/spack/environment.py   |  51 ++++++----
 lib/spack/spack/modules.py       | 202 ++++++++++++++++++++++----------------
 lib/spack/spack/test/__init__.py |  69 ++++---------
 lib/spack/spack/test/modules.py  |  42 +++++---
 6 files changed, 330 insertions(+), 254 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index f996f4eb84..cfe59c8d98 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -32,18 +32,21 @@ from llnl.util.filesystem import mkdirp
 from spack.modules import module_types
 from spack.util.string import *
 
-description ="Manipulate modules and dotkits."
+description = "Manipulate modules and dotkits."
 
 
 def setup_parser(subparser):
     sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command')
 
-    refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.')
+    sp.add_parser('refresh', help='Regenerate all module files.')
 
     find_parser = sp.add_parser('find', help='Find module files for packages.')
-    find_parser.add_argument(
-        'module_type', help="Type of module to find file for. [" + '|'.join(module_types) + "]")
-    find_parser.add_argument('spec', nargs='+', help='spec to find a module file for.')
+    find_parser.add_argument('module_type',
+                             help="Type of module to find file for. [" +
+                             '|'.join(module_types) + "]")
+    find_parser.add_argument('spec',
+                             nargs='+',
+                             help='spec to find a module file for.')
 
 
 def module_find(mtype, spec_array):
@@ -53,7 +56,8 @@ def module_find(mtype, spec_array):
        should type to use that package's module.
     """
     if mtype not in module_types:
-        tty.die("Invalid module type: '%s'.  Options are %s" % (mtype, comma_or(module_types)))
+        tty.die("Invalid module type: '%s'.  Options are %s" %
+                (mtype, comma_or(module_types)))
 
     specs = spack.cmd.parse_specs(spec_array)
     if len(specs) > 1:
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 6ddf07776b..684a420b3b 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -134,8 +134,6 @@ from yaml.error import MarkedYAMLError
 
 # Hacked yaml for configuration files preserves line numbers.
 import spack.util.spack_yaml as syaml
-
-
 """Dict from section names -> schema for that section."""
 section_schemas = {
     'compilers': {
@@ -149,25 +147,31 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'patternProperties': {
-                    r'\w[\w-]*': {           # architecture
+                    r'\w[\w-]*': {  # architecture
                         'type': 'object',
                         'additionalProperties': False,
                         'patternProperties': {
-                            r'\w[\w-]*@\w[\w-]*': {   # compiler spec
+                            r'\w[\w-]*@\w[\w-]*': {  # compiler spec
                                 'type': 'object',
                                 'additionalProperties': False,
                                 'required': ['cc', 'cxx', 'f77', 'fc'],
                                 'properties': {
-                                    'cc':  { 'anyOf': [ {'type' : 'string' },
-                                                        {'type' : 'null' }]},
-                                    'cxx': { 'anyOf': [ {'type' : 'string' },
-                                                        {'type' : 'null' }]},
-                                    'f77': { 'anyOf': [ {'type' : 'string' },
-                                                        {'type' : 'null' }]},
-                                    'fc':  { 'anyOf': [ {'type' : 'string' },
-                                                        {'type' : 'null' }]},
-                                },},},},},},},},
-
+                                    'cc': {'anyOf': [{'type': 'string'},
+                                                     {'type': 'null'}]},
+                                    'cxx': {'anyOf': [{'type': 'string'},
+                                                      {'type': 'null'}]},
+                                    'f77': {'anyOf': [{'type': 'string'},
+                                                      {'type': 'null'}]},
+                                    'fc': {'anyOf': [{'type': 'string'},
+                                                     {'type': 'null'}]},
+                                },
+                            },
+                        },
+                    },
+                },
+            },
+        },
+    },
     'mirrors': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack mirror configuration file schema',
@@ -180,8 +184,12 @@ section_schemas = {
                 'additionalProperties': False,
                 'patternProperties': {
                     r'\w[\w-]*': {
-                        'type': 'string'},},},},},
-
+                        'type': 'string'
+                    },
+                },
+            },
+        },
+    },
     'repos': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack repository configuration file schema',
@@ -192,8 +200,11 @@ section_schemas = {
                 'type': 'array',
                 'default': [],
                 'items': {
-                    'type': 'string'},},},},
-
+                    'type': 'string'
+                },
+            },
+        },
+    },
     'packages': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack package configuration file schema',
@@ -205,39 +216,48 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'patternProperties': {
-                    r'\w[\w-]*': { # package name
+                    r'\w[\w-]*': {  # package name
                         'type': 'object',
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
                             'version': {
-                                'type' : 'array',
-                                'default' : [],
-                                'items' : { 'anyOf' : [ { 'type' : 'string' },
-                                                        { 'type' : 'number'}]}}, #version strings
+                                'type': 'array',
+                                'default': [],
+                                'items': {'anyOf': [{'type': 'string'},
+                                                    {'type': 'number'}]}
+                            },  # version strings
                             'compiler': {
-                                'type' : 'array',
-                                'default' : [],
-                                'items' : { 'type' : 'string' } }, #compiler specs
+                                'type': 'array',
+                                'default': [],
+                                'items': {'type': 'string'}
+                            },  # compiler specs
                             'buildable': {
-                                'type':  'boolean',
+                                'type': 'boolean',
                                 'default': True,
-                             },
+                            },
                             'providers': {
-                                'type':  'object',
+                                'type': 'object',
                                 'default': {},
                                 'additionalProperties': False,
                                 'patternProperties': {
                                     r'\w[\w-]*': {
-                                        'type' : 'array',
-                                        'default' : [],
-                                        'items' : { 'type' : 'string' },},},},
+                                        'type': 'array',
+                                        'default': [],
+                                        'items': {'type': 'string'},
+                                    },
+                                },
+                            },
                             'paths': {
-                                'type' : 'object',
-                                'default' : {},
+                                'type': 'object',
+                                'default': {},
                             }
-                        },},},},},},
-
+                        },
+                    },
+                },
+            },
+        },
+    },
     'modules': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack module file configuration file schema',
@@ -283,17 +303,22 @@ section_schemas = {
                         }
                     },
                     'autoload': {'$ref': '#/definitions/dependency_selection'},
-                    'prerequisites': {'$ref': '#/definitions/dependency_selection'},
+                    'prerequisites':
+                    {'$ref': '#/definitions/dependency_selection'},
                     'conflict': {'$ref': '#/definitions/array_of_strings'},
                     'environment': {
                         'type': 'object',
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
-                            'set': {'$ref': '#/definitions/dictionary_of_strings'},
-                            'unset': {'$ref': '#/definitions/array_of_strings'},
-                            'prepend_path': {'$ref': '#/definitions/dictionary_of_strings'},
-                            'append_path': {'$ref': '#/definitions/dictionary_of_strings'}
+                            'set':
+                            {'$ref': '#/definitions/dictionary_of_strings'},
+                            'unset':
+                            {'$ref': '#/definitions/array_of_strings'},
+                            'prepend_path':
+                            {'$ref': '#/definitions/dictionary_of_strings'},
+                            'append_path':
+                            {'$ref': '#/definitions/dictionary_of_strings'}
                         }
                     }
                 }
@@ -304,15 +329,20 @@ section_schemas = {
                 'anyOf': [
                     {
                         'properties': {
-                            'whitelist': {'$ref': '#/definitions/array_of_strings'},
-                            'blacklist': {'$ref': '#/definitions/array_of_strings'},
+                            'whitelist':
+                            {'$ref': '#/definitions/array_of_strings'},
+                            'blacklist':
+                            {'$ref': '#/definitions/array_of_strings'},
                             'naming_scheme': {
-                                'type': 'string'  # Can we be more specific here?
+                                'type':
+                                'string'  # Can we be more specific here?
                             }
                         }
                     },
                     {
-                        'patternProperties': {r'\w[\w-]*': {'$ref': '#/definitions/module_file_configuration'}}
+                        'patternProperties':
+                        {r'\w[\w-]*':
+                         {'$ref': '#/definitions/module_file_configuration'}}
                     }
                 ]
             }
@@ -326,7 +356,8 @@ section_schemas = {
                     'prefix_inspections': {
                         'type': 'object',
                         'patternProperties': {
-                            r'\w[\w-]*': {  # path to be inspected for existence (relative to prefix)
+                            r'\w[\w-]*':
+                            {  # path to be inspected (relative to prefix)
                                 '$ref': '#/definitions/array_of_strings'
                             }
                         }
@@ -341,13 +372,15 @@ section_schemas = {
                     },
                     'tcl': {
                         'allOf': [
-                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
+                            {'$ref': '#/definitions/module_type_configuration'
+                             },  # Base configuration
                             {}  # Specific tcl extensions
                         ]
                     },
                     'dotkit': {
                         'allOf': [
-                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
+                            {'$ref': '#/definitions/module_type_configuration'
+                             },  # Base configuration
                             {}  # Specific dotkit extensions
                         ]
                     },
@@ -356,7 +389,6 @@ section_schemas = {
         },
     },
 }
-
 """OrderedDict of config scopes keyed by name.
    Later scopes will override earlier scopes.
 """
@@ -366,12 +398,13 @@ config_scopes = OrderedDict()
 def validate_section_name(section):
     """Raise a ValueError if the section is not a valid section."""
     if section not in section_schemas:
-        raise ValueError("Invalid config section: '%s'.  Options are %s"
-                         % (section, section_schemas))
+        raise ValueError("Invalid config section: '%s'.  Options are %s" %
+                         (section, section_schemas))
 
 
 def extend_with_default(validator_class):
-    """Add support for the 'default' attribute for properties and patternProperties.
+    """Add support for the 'default' attribute for
+       properties and patternProperties
 
        jsonschema does not handle this out of the box -- it only
        validates.  This allows us to set default values for configs
@@ -380,13 +413,15 @@ def extend_with_default(validator_class):
 
     """
     validate_properties = validator_class.VALIDATORS["properties"]
-    validate_pattern_properties = validator_class.VALIDATORS["patternProperties"]
+    validate_pattern_properties = validator_class.VALIDATORS[
+        "patternProperties"]
 
     def set_defaults(validator, properties, instance, schema):
         for property, subschema in properties.iteritems():
             if "default" in subschema:
                 instance.setdefault(property, subschema["default"])
-        for err in validate_properties(validator, properties, instance, schema):
+        for err in validate_properties(validator, properties, instance,
+                                       schema):
             yield err
 
     def set_pp_defaults(validator, properties, instance, schema):
@@ -397,17 +432,19 @@ def extend_with_default(validator_class):
                         if re.match(property, key) and val is None:
                             instance[key] = subschema["default"]
 
-        for err in validate_pattern_properties(validator, properties, instance, schema):
+        for err in validate_pattern_properties(validator, properties, instance,
+                                               schema):
             yield err
 
     return validators.extend(validator_class, {
-        "properties" : set_defaults,
-        "patternProperties" : set_pp_defaults
+        "properties": set_defaults,
+        "patternProperties": set_pp_defaults
     })
 
 
 DefaultSettingValidator = extend_with_default(Draft4Validator)
 
+
 def validate_section(data, schema):
     """Validate data read in from a Spack YAML file.
 
@@ -429,9 +466,9 @@ class ConfigScope(object):
     """
 
     def __init__(self, name, path):
-        self.name = name           # scope name.
-        self.path = path           # path to directory containing configs.
-        self.sections = {}         # sections read from config files.
+        self.name = name  # scope name.
+        self.path = path  # path to directory containing configs.
+        self.sections = {}  # sections read from config files.
 
         # Register in a dict of all ConfigScopes
         # TODO: make this cleaner.  Mocking up for testing is brittle.
@@ -442,16 +479,14 @@ class ConfigScope(object):
         validate_section_name(section)
         return os.path.join(self.path, "%s.yaml" % section)
 
-
     def get_section(self, section):
-        if not section in self.sections:
-            path   = self.get_section_filename(section)
+        if section not in self.sections:
+            path = self.get_section_filename(section)
             schema = section_schemas[section]
-            data   = _read_config_file(path, schema)
+            data = _read_config_file(path, schema)
             self.sections[section] = data
         return self.sections[section]
 
-
     def write_section(self, section):
         filename = self.get_section_filename(section)
         data = self.get_section(section)
@@ -463,8 +498,8 @@ class ConfigScope(object):
         except jsonschema.ValidationError as e:
             raise ConfigSanityError(e, data)
         except (yaml.YAMLError, IOError) as e:
-            raise ConfigFileError("Error writing to config file: '%s'" % str(e))
-
+            raise ConfigFileError("Error writing to config file: '%s'" %
+                                  str(e))
 
     def clear(self):
         """Empty cached config information."""
@@ -496,8 +531,8 @@ def validate_scope(scope):
         return config_scopes[scope]
 
     else:
-        raise ValueError("Invalid config scope: '%s'.  Must be one of %s"
-                         % (scope, config_scopes.keys()))
+        raise ValueError("Invalid config scope: '%s'.  Must be one of %s" %
+                         (scope, config_scopes.keys()))
 
 
 def _read_config_file(filename, schema):
@@ -523,12 +558,12 @@ def _read_config_file(filename, schema):
         return data
 
     except MarkedYAMLError as e:
-        raise ConfigFileError(
-            "Error parsing yaml%s: %s" % (str(e.context_mark), e.problem))
+        raise ConfigFileError("Error parsing yaml%s: %s" %
+                              (str(e.context_mark), e.problem))
 
     except IOError as e:
-        raise ConfigFileError(
-            "Error reading configuration file %s: %s" % (filename, str(e)))
+        raise ConfigFileError("Error reading configuration file %s: %s" %
+                              (filename, str(e)))
 
 
 def clear_config_caches():
@@ -551,6 +586,7 @@ def _merge_yaml(dest, source):
     parent instead of merging.
 
     """
+
     def they_are(t):
         return isinstance(dest, t) and isinstance(source, t)
 
@@ -571,7 +607,7 @@ def _merge_yaml(dest, source):
     # Source dict is merged into dest.
     elif they_are(dict):
         for sk, sv in source.iteritems():
-            if not sk in dest:
+            if sk not in dest:
                 dest[sk] = copy.copy(sv)
             else:
                 dest[sk] = _merge_yaml(dest[sk], source[sk])
@@ -653,7 +689,7 @@ def print_section(section):
         data = syaml.syaml_dict()
         data[section] = get_config(section)
         syaml.dump(data, stream=sys.stdout, default_flow_style=False)
-    except (yaml.YAMLError, IOError) as e:
+    except (yaml.YAMLError, IOError):
         raise ConfigError("Error reading configuration: %s" % section)
 
 
@@ -683,15 +719,22 @@ def is_spec_buildable(spec):
     """Return true if the spec pkgspec is configured as buildable"""
     allpkgs = get_config('packages')
     name = spec.name
-    if not spec.name in allpkgs:
+    if name not in allpkgs:
         return True
-    if not 'buildable' in allpkgs[spec.name]:
+    if 'buildable' not in allpkgs[name]:
         return True
     return allpkgs[spec.name]['buildable']
 
 
-class ConfigError(SpackError): pass
-class ConfigFileError(ConfigError): pass
+class ConfigError(SpackError):
+
+    pass
+
+
+class ConfigFileError(ConfigError):
+
+    pass
+
 
 def get_path(path, data):
     if path:
@@ -699,8 +742,10 @@ def get_path(path, data):
     else:
         return data
 
+
 class ConfigFormatError(ConfigError):
     """Raised when a configuration format does not match its schema."""
+
     def __init__(self, validation_error, data):
         # Try to get line number from erroneous instance and its parent
         instance_mark = getattr(validation_error.instance, '_start_mark', None)
@@ -733,5 +778,6 @@ class ConfigFormatError(ConfigError):
         message = '%s: %s' % (location, validation_error.message)
         super(ConfigError, self).__init__(message)
 
+
 class ConfigSanityError(ConfigFormatError):
     """Same as ConfigFormatError, raised when config is written by Spack."""
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 92ab4e6bea..9748b4033a 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -26,7 +26,8 @@ class SetEnv(NameValueModifier):
 
 class UnsetEnv(NameModifier):
     def execute(self):
-        os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
+        # Avoid throwing if the variable was not set
+        os.environ.pop(self.name, None)
 
 
 class SetPath(NameValueModifier):
@@ -55,7 +56,9 @@ class RemovePath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.value)]
+        directories = [os.path.normpath(x)
+                       for x in directories
+                       if x != os.path.normpath(self.value)]
         os.environ[self.name] = ':'.join(directories)
 
 
@@ -63,7 +66,8 @@ class EnvironmentModifications(object):
     """
     Keeps track of requests to modify the current environment.
 
-    Each call to a method to modify the environment stores the extra information on the caller in the request:
+    Each call to a method to modify the environment stores the extra
+    information on the caller in the request:
     - 'filename' : filename of the module where the caller is defined
     - 'lineno': line number where the request occurred
     - 'context' : line of code that issued the request that failed
@@ -71,10 +75,10 @@ class EnvironmentModifications(object):
 
     def __init__(self, other=None):
         """
-        Initializes a new instance, copying commands from other if it is not None
+        Initializes a new instance, copying commands from other if not None
 
         Args:
-            other: another instance of EnvironmentModifications from which (optional)
+            other: another instance of EnvironmentModifications
         """
         self.env_modifications = []
         if other is not None:
@@ -93,7 +97,7 @@ class EnvironmentModifications(object):
     @staticmethod
     def _check_other(other):
         if not isinstance(other, EnvironmentModifications):
-            raise TypeError('other must be an instance of EnvironmentModifications')
+            raise TypeError('not an instance of EnvironmentModifications')
 
     def _get_outside_caller_attributes(self):
         stack = inspect.stack()
@@ -101,12 +105,10 @@ class EnvironmentModifications(object):
             _, filename, lineno, _, context, index = stack[2]
             context = context[index].strip()
         except Exception:
-            filename, lineno, context = 'unknown file', 'unknown line', 'unknown context'
-        args = {
-            'filename': filename,
-            'lineno': lineno,
-            'context': context
-        }
+            filename = 'unknown file'
+            lineno = 'unknown line'
+            context = 'unknown context'
+        args = {'filename': filename, 'lineno': lineno, 'context': context}
         return args
 
     def set(self, name, value, **kwargs):
@@ -170,7 +172,7 @@ class EnvironmentModifications(object):
 
     def remove_path(self, name, path, **kwargs):
         """
-        Stores in the current object a request to remove a path from a path list
+        Stores in the current object a request to remove a path from a list
 
         Args:
             name: name of the path list in the environment
@@ -185,7 +187,8 @@ class EnvironmentModifications(object):
         Returns a dict of the modifications grouped by variable name
 
         Returns:
-            dict mapping the environment variable name to the modifications to be done on it
+            dict mapping the environment variable name to the modifications
+            to be done on it
         """
         modifications = collections.defaultdict(list)
         for item in self:
@@ -203,7 +206,8 @@ class EnvironmentModifications(object):
         Applies the modifications and clears the list
         """
         modifications = self.group_by_name()
-        # Apply the modifications to the environment variables one variable at a time
+        # Apply the modifications to the environment variables one variable
+        # at a time
         for name, actions in sorted(modifications.items()):
             for x in actions:
                 x.execute()
@@ -224,13 +228,17 @@ def concatenate_paths(paths):
 
 def set_or_unset_not_first(variable, changes, errstream):
     """
-    Check if we are going to set or unset something after other modifications have already been requested
+    Check if we are going to set or unset something after other modifications
+    have already been requested
     """
-    indexes = [ii for ii, item in enumerate(changes) if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
+    indexes = [ii
+               for ii, item in enumerate(changes)
+               if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
     if indexes:
         good = '\t    \t{context} at {filename}:{lineno}'
         nogood = '\t--->\t{context} at {filename}:{lineno}'
-        errstream('Suspicious requests to set or unset the variable \'{var}\' found'.format(var=variable))
+        message = 'Suspicious requests to set or unset the variable \'{var}\' found'  # NOQA: ignore=E501
+        errstream(message.format(var=variable))
         for ii, item in enumerate(changes):
             print_format = nogood if ii in indexes else good
             errstream(print_format.format(**item.args))
@@ -238,8 +246,8 @@ def set_or_unset_not_first(variable, changes, errstream):
 
 def validate(env, errstream):
     """
-    Validates the environment modifications to check for the presence of suspicious patterns. Prompts a warning for
-    everything that was found
+    Validates the environment modifications to check for the presence of
+    suspicious patterns. Prompts a warning for everything that was found
 
     Current checks:
     - set or unset variables after other changes on the same variable
@@ -254,7 +262,8 @@ def validate(env, errstream):
 
 def filter_environment_blacklist(env, variables):
     """
-    Generator that filters out any change to environment variables present in the input list
+    Generator that filters out any change to environment variables present in
+    the input list
 
     Args:
         env: list of environment modifications
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index ffed469b20..0dc6f06f55 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -23,36 +23,34 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 """
-This module contains code for creating environment modules, which can include dotkits, tcl modules, lmod, and others.
+This module contains code for creating environment modules, which can include
+dotkits, tcl modules, lmod, and others.
 
-The various types of modules are installed by post-install hooks and removed after an uninstall by post-uninstall hooks.
-This class consolidates the logic for creating an abstract description of the information that module systems need.
-Currently that includes a number of directories to be appended to paths in the user's environment:
+The various types of modules are installed by post-install hooks and removed
+after an uninstall by post-uninstall hooks. This class consolidates the logic
+for creating an abstract description of the information that module systems
+need.
 
-  * /bin directories to be appended to PATH
-  * /lib* directories for LD_LIBRARY_PATH
-  * /include directories for CPATH
-  * /man* and /share/man* directories for MANPATH
-  * the package prefix for CMAKE_PREFIX_PATH
+This module also includes logic for coming up with unique names for the module
+files so that they can be found by the various shell-support files in
+$SPACK/share/spack/setup-env.*.
 
-This module also includes logic for coming up with unique names for the module files so that they can be found by the
-various shell-support files in $SPACK/share/spack/setup-env.*.
-
-Each hook in hooks/ implements the logic for writing its specific type of module file.
+Each hook implements the logic for writing its specific type of module.
 """
 import copy
 import datetime
 import os
 import os.path
 import re
-import textwrap
 import string
+import textwrap
 
 import llnl.util.tty as tty
 import spack
 import spack.config
 from llnl.util.filesystem import join_path, mkdirp
-from spack.build_environment import parent_class_modules, set_module_variables_for_package
+from spack.build_environment import parent_class_modules
+from spack.build_environment import set_module_variables_for_package
 from spack.environment import *
 
 __all__ = ['EnvModule', 'Dotkit', 'TclModule']
@@ -67,30 +65,26 @@ def print_help():
     """
     For use by commands to tell user how to activate shell support.
     """
-    tty.msg("This command requires spack's shell integration.",
-            "",
+    tty.msg("This command requires spack's shell integration.", "",
             "To initialize spack's shell commands, you must run one of",
             "the commands below.  Choose the right command for your shell.",
-            "",
-            "For bash and zsh:",
-            "    . %s/setup-env.sh" % spack.share_path,
-            "",
-            "For csh and tcsh:",
-            "    setenv SPACK_ROOT %s" % spack.prefix,
-            "    source %s/setup-env.csh" % spack.share_path,
-            "")
+            "", "For bash and zsh:",
+            "    . %s/setup-env.sh" % spack.share_path, "",
+            "For csh and tcsh:", "    setenv SPACK_ROOT %s" % spack.prefix,
+            "    source %s/setup-env.csh" % spack.share_path, "")
 
 
 def inspect_path(prefix):
     """
-    Inspects the prefix of an installation to search for common layouts. Issues a request to modify the environment
-    accordingly when an item is found.
+    Inspects the prefix of an installation to search for common layouts.
+    Issues a request to modify the environment when an item is found.
 
     Args:
         prefix: prefix of the installation
 
     Returns:
-        instance of EnvironmentModifications containing the requested modifications
+        instance of EnvironmentModifications containing the requested
+        modifications
     """
     env = EnvironmentModifications()
     # Inspect the prefix to check for the existence of common directories
@@ -105,18 +99,22 @@ def inspect_path(prefix):
 
 def dependencies(spec, request='all'):
     """
-    Returns the list of dependent specs for a given spec, according to the given request
+    Returns the list of dependent specs for a given spec, according to the
+    given request
 
     Args:
         spec: target spec
         request: either 'none', 'direct' or 'all'
 
     Returns:
-        empty list if 'none', direct dependency list if 'direct', all dependencies if 'all'
+        empty list if 'none', direct dependency list if 'direct', all
+        dependencies if 'all'
     """
     if request not in ('none', 'direct', 'all'):
-        raise tty.error("Wrong value for argument 'request' : should be one of ('none', 'direct', 'all') "
-                        " [current value is '%s']" % request)
+        message = "Wrong value for argument 'request' : "
+        message += "should be one of ('none', 'direct', 'all')"
+        message += " [current value is '{0}']"
+        raise tty.error(message.format(request))
 
     if request == 'none':
         return []
@@ -124,12 +122,19 @@ def dependencies(spec, request='all'):
     if request == 'direct':
         return [xx for _, xx in spec.dependencies.items()]
 
-    # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
-    # FIXME : is given. This work around permits to get a unique list of spec anyhow.
-    # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
+    # FIXME : during module file creation nodes seem to be visited
+    # FIXME : multiple times even if cover='nodes' is given. This work around
+    # FIXME : permits to get a unique list of spec anyhow. Maybe we miss a
+    # FIXME : merge step among nodes that refer to the same package?
     seen = set()
     seen_add = seen.add
-    l = [xx for xx in sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
+    l = [xx
+         for xx in sorted(
+             spec.traverse(order='post',
+                           depth=True,
+                           cover='nodes',
+                           root=False),
+             reverse=True)]
     return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
 
 
@@ -146,7 +151,8 @@ def update_dictionary_extending_lists(target, update):
 
 def parse_config_options(module_generator):
     """
-    Parse the configuration file and returns a bunch of items that will be needed during module file generation
+    Parse the configuration file and returns a bunch of items that will be
+    needed during module file generation
 
     Args:
         module_generator: module generator for a given spec
@@ -154,11 +160,14 @@ def parse_config_options(module_generator):
     Returns:
         autoloads: list of specs to be autoloaded
         prerequisites: list of specs to be marked as prerequisite
-        filters: list of environment variables whose modification is blacklisted in module files
-        env: list of custom environment modifications to be applied in the module file
+        filters: list of environment variables whose modification is
+        blacklisted in module files
+        env: list of custom environment modifications to be applied in the
+        module file
     """
     # Get the configuration for this kind of generator
-    module_configuration = copy.deepcopy(CONFIGURATION.get(module_generator.name, {}))
+    module_configuration = copy.deepcopy(CONFIGURATION.get(
+        module_generator.name, {}))
 
     #####
     # Merge all the rules
@@ -179,9 +188,12 @@ def parse_config_options(module_generator):
     #####
 
     # Automatic loading loads
-    module_file_actions['autoload'] = dependencies(module_generator.spec, module_file_actions.get('autoload', 'none'))
+    module_file_actions['autoload'] = dependencies(
+        module_generator.spec, module_file_actions.get('autoload', 'none'))
     # Prerequisites
-    module_file_actions['prerequisites'] = dependencies(module_generator.spec, module_file_actions.get('prerequisites', 'none'))
+    module_file_actions['prerequisites'] = dependencies(
+        module_generator.spec, module_file_actions.get('prerequisites',
+                                                       'none'))
     # Environment modifications
     environment_actions = module_file_actions.pop('environment', {})
     env = EnvironmentModifications()
@@ -189,7 +201,7 @@ def parse_config_options(module_generator):
     def process_arglist(arglist):
         if method == 'unset':
             for x in arglist:
-                yield (x,)
+                yield (x, )
         else:
             for x in arglist.iteritems():
                 yield x
@@ -198,19 +210,20 @@ def parse_config_options(module_generator):
         for args in process_arglist(arglist):
             getattr(env, method)(*args)
 
-        # for item in arglist:
-        #     if method == 'unset':
-        #         args = [item]
-        #     else:
-        #         args = item.split(',')
-        #     getattr(env, method)(*args)
+            # for item in arglist:
+            #     if method == 'unset':
+            #         args = [item]
+            #     else:
+            #         args = item.split(',')
+            #     getattr(env, method)(*args)
 
     return module_file_actions, env
 
 
 def filter_blacklisted(specs, module_name):
     """
-    Given a sequence of specs, filters the ones that are blacklisted in the module configuration file.
+    Given a sequence of specs, filters the ones that are blacklisted in the
+    module configuration file.
 
     Args:
         specs: sequence of spec instances
@@ -233,7 +246,8 @@ class EnvModule(object):
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
-            if cls.name != 'env_module' and cls.name in CONFIGURATION['enable']:
+            if cls.name != 'env_module' and cls.name in CONFIGURATION[
+                    'enable']:
                 module_types[cls.name] = cls
 
     def __init__(self, spec=None):
@@ -249,7 +263,8 @@ class EnvModule(object):
         # long description is the docstring with reduced whitespace.
         self.long_description = None
         if self.spec.package.__doc__:
-            self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
+            self.long_description = re.sub(r'\s+', ' ',
+                                           self.spec.package.__doc__)
 
     @property
     def naming_scheme(self):
@@ -271,12 +286,14 @@ class EnvModule(object):
     @property
     def use_name(self):
         """
-        Subclasses should implement this to return the name the module command uses to refer to the package.
+        Subclasses should implement this to return the name the module command
+        uses to refer to the package.
         """
         naming_tokens = self.tokens
         naming_scheme = self.naming_scheme
         name = naming_scheme.format(**naming_tokens)
-        name += '-' + self.spec.dag_hash()  # Always append the hash to make the module file unique
+        name += '-' + self.spec.dag_hash(
+        )  # Always append the hash to make the module file unique
         # Not everybody is working on linux...
         parts = name.split('/')
         name = join_path(*parts)
@@ -296,8 +313,12 @@ class EnvModule(object):
     @property
     def blacklisted(self):
         configuration = CONFIGURATION.get(self.name, {})
-        whitelist_matches = [x for x in configuration.get('whitelist', []) if self.spec.satisfies(x)]
-        blacklist_matches = [x for x in configuration.get('blacklist', []) if self.spec.satisfies(x)]
+        whitelist_matches = [x
+                             for x in configuration.get('whitelist', [])
+                             if self.spec.satisfies(x)]
+        blacklist_matches = [x
+                             for x in configuration.get('blacklist', [])
+                             if self.spec.satisfies(x)]
         if whitelist_matches:
             message = '\tWHITELIST : %s [matches : ' % self.spec.cshort_spec
             for rule in whitelist_matches:
@@ -327,7 +348,8 @@ class EnvModule(object):
         """
         if self.blacklisted:
             return
-        tty.debug("\tWRITE : %s [%s]" % (self.spec.cshort_spec, self.file_name))
+        tty.debug("\tWRITE : %s [%s]" %
+                  (self.spec.cshort_spec, self.file_name))
 
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
@@ -337,11 +359,12 @@ class EnvModule(object):
         # installation prefix
         env = inspect_path(self.spec.prefix)
 
-        # Let the extendee/dependency modify their extensions/dependencies before asking for
-        # package-specific modifications
+        # Let the extendee/dependency modify their extensions/dependencies
+        # before asking for package-specific modifications
         spack_env = EnvironmentModifications()
-        # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
-        # TODO : factored out to a single place
+        # TODO : the code down below is quite similar to
+        # TODO : build_environment.setup_package and needs to be factored out
+        # TODO : to a single place
         for item in dependencies(self.spec, 'all'):
             package = self.spec[item.name].package
             modules = parent_class_modules(package.__class__)
@@ -358,14 +381,18 @@ class EnvModule(object):
         # Parse configuration file
         module_configuration, conf_env = parse_config_options(self)
         env.extend(conf_env)
-        filters = module_configuration.get('filter', {}).get('environment_blacklist',{})
+        filters = module_configuration.get('filter', {}).get(
+            'environment_blacklist', {})
         # Build up the module file content
         module_file_content = self.header
-        for x in filter_blacklisted(module_configuration.pop('autoload', []), self.name):
+        for x in filter_blacklisted(
+                module_configuration.pop('autoload', []), self.name):
             module_file_content += self.autoload(x)
-        for x in filter_blacklisted(module_configuration.pop('prerequisites', []), self.name):
+        for x in filter_blacklisted(
+                module_configuration.pop('prerequisites', []), self.name):
             module_file_content += self.prerequisite(x)
-        for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
+        for line in self.process_environment_command(
+                filter_environment_blacklist(env, filters)):
             module_file_content += line
         for line in self.module_specific_content(module_configuration):
             module_file_content += line
@@ -392,10 +419,13 @@ class EnvModule(object):
     def process_environment_command(self, env):
         for command in env:
             try:
-                yield self.environment_modifications_formats[type(command)].format(**command.args)
+                yield self.environment_modifications_formats[type(
+                    command)].format(**command.args)
             except KeyError:
-                tty.warn('Cannot handle command of type {command} : skipping request'.format(command=type(command)))
-                tty.warn('{context} at {filename}:{lineno}'.format(**command.args))
+                message = 'Cannot handle command of type {command} : skipping request'  # NOQA: ignore=E501
+                tty.warn(message.format(command=type(command)))
+                context = '{context} at {filename}:{lineno}'
+                tty.warn(context.format(**command.args))
 
     @property
     def file_name(self):
@@ -408,9 +438,12 @@ class EnvModule(object):
         if os.path.exists(mod_file):
             try:
                 os.remove(mod_file)  # Remove the module file
-                os.removedirs(os.path.dirname(mod_file))  # Remove all the empty directories from the leaf up
+                os.removedirs(
+                    os.path.dirname(mod_file)
+                )  # Remove all the empty directories from the leaf up
             except OSError:
-                pass  # removedirs throws OSError on first non-empty directory found
+                # removedirs throws OSError on first non-empty directory found
+                pass
 
 
 class Dotkit(EnvModule):
@@ -424,13 +457,12 @@ class Dotkit(EnvModule):
 
     autoload_format = 'dk_op {module_file}\n'
 
-    prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
-
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
 
     @property
     def file_name(self):
-        return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
+        return join_path(Dotkit.path, self.spec.architecture,
+                         '%s.dk' % self.use_name)
 
     @property
     def header(self):
@@ -474,7 +506,7 @@ class TclModule(EnvModule):
 
     prerequisite_format = 'prereq {module_file}\n'
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
 
     @property
     def file_name(self):
@@ -482,9 +514,10 @@ class TclModule(EnvModule):
 
     @property
     def header(self):
+        timestamp = datetime.datetime.now()
         # TCL Modulefile header
         header = '#%Module1.0\n'
-        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % datetime.datetime.now()
+        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % timestamp  # NOQA: ignore=E501
         header += '##\n'
         header += '## %s\n' % self.spec.short_spec
         header += '##\n'
@@ -509,16 +542,19 @@ class TclModule(EnvModule):
         f = string.Formatter()
         for item in conflict_format:
             line = 'conflict ' + item + '\n'
-            if len([x for x in f.parse(line)]) > 1:  # We do have placeholder to substitute
-                for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), item.split('/')):
+            if len([x for x in f.parse(line)
+                    ]) > 1:  # We do have placeholder to substitute
+                for naming_dir, conflict_dir in zip(
+                        self.naming_scheme.split('/'), item.split('/')):
                     if naming_dir != conflict_dir:
-                        message = 'conflict scheme does not match naming scheme [{spec}]\n\n'
+                        message = 'conflict scheme does not match naming'
+                        message += ' [{spec}]\n\n'
                         message += 'naming scheme   : "{nformat}"\n'
                         message += 'conflict scheme : "{cformat}"\n\n'
-                        message += '** You may want to check your `modules.yaml` configuration file **\n'
-                        tty.error(
-                            message.format(spec=self.spec, nformat=self.naming_scheme, cformat=item)
-                        )
+                        message += '** You may want to check your `modules.yaml` configuration file **\n'  # NOQA: ignore=E501
+                        tty.error(message.format(spec=self.spec,
+                                                 nformat=self.naming_scheme,
+                                                 cformat=item))
                         raise SystemExit('Module generation aborted.')
                 line = line.format(**naming_tokens)
             yield line
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 05f58ab7b1..395ca0c87a 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -23,53 +23,23 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import sys
-import unittest
-import nose
 
-from spack.test.tally_plugin import Tally
-from llnl.util.filesystem import join_path
 import llnl.util.tty as tty
-from llnl.util.tty.colify import colify
-
+import nose
 import spack
-
+from llnl.util.filesystem import join_path
+from llnl.util.tty.colify import colify
+from spack.test.tally_plugin import Tally
 """Names of tests to be included in Spack's test suite"""
-test_names = ['versions',
-              'url_parse',
-              'url_substitution',
-              'packages',
-              'stage',
-              'spec_syntax',
-              'spec_semantics',
-              'spec_dag',
-              'concretize',
-              'multimethod',
-              'install',
-              'package_sanity',
-              'config',
-              'directory_layout',
-              'pattern',
-              'python_version',
-              'git_fetch',
-              'svn_fetch',
-              'hg_fetch',
-              'mirror',
-              'modules',
-              'url_extrapolate',
-              'cc',
-              'link_tree',
-              'spec_yaml',
-              'optional_deps',
-              'make_executable',
-              'configure_guess',
-              'lock',
-              'database',
-              'namespace_trie',
-              'yaml',
-              'sbang',
-              'environment',
-              'cmd.uninstall',
-              'cmd.test_install']
+test_names = ['versions', 'url_parse', 'url_substitution', 'packages', 'stage',
+              'spec_syntax', 'spec_semantics', 'spec_dag', 'concretize',
+              'multimethod', 'install', 'package_sanity', 'config',
+              'directory_layout', 'pattern', 'python_version', 'git_fetch',
+              'svn_fetch', 'hg_fetch', 'mirror', 'modules', 'url_extrapolate',
+              'cc', 'link_tree', 'spec_yaml', 'optional_deps',
+              'make_executable', 'configure_guess', 'lock', 'database',
+              'namespace_trie', 'yaml', 'sbang', 'environment',
+              'cmd.uninstall', 'cmd.test_install']
 
 
 def list_tests():
@@ -80,7 +50,6 @@ def list_tests():
 def run(names, outputDir, verbose=False):
     """Run tests with the supplied names.  Names should be a list.  If
        it's empty, run ALL of Spack's tests."""
-    verbosity = 1 if not verbose else 2
 
     if not names:
         names = test_names
@@ -95,7 +64,7 @@ def run(names, outputDir, verbose=False):
     tally = Tally()
     for test in names:
         module = 'spack.test.' + test
-        print module
+        print(module)
 
         tty.msg("Running test: %s" % test)
 
@@ -105,15 +74,13 @@ def run(names, outputDir, verbose=False):
             xmlOutputFname = "unittests-{0}.xml".format(test)
             xmlOutputPath = join_path(outputDir, xmlOutputFname)
             runOpts += ["--with-xunit",
-                "--xunit-file={0}".format(xmlOutputPath)]
+                        "--xunit-file={0}".format(xmlOutputPath)]
         argv = [""] + runOpts + [module]
-        result = nose.run(argv=argv, addplugins=[tally])
+        nose.run(argv=argv, addplugins=[tally])
 
     succeeded = not tally.failCount and not tally.errorCount
-    tty.msg("Tests Complete.",
-            "%5d tests run" % tally.numberOfTestsRun,
-            "%5d failures" % tally.failCount,
-            "%5d errors" % tally.errorCount)
+    tty.msg("Tests Complete.", "%5d tests run" % tally.numberOfTestsRun,
+            "%5d failures" % tally.failCount, "%5d errors" % tally.errorCount)
 
     if succeeded:
         tty.info("OK", format='g')
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index b8b0d6fc6a..f0ff778a10 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -2,14 +2,18 @@ import collections
 from contextlib import contextmanager
 
 import StringIO
+import spack.modules
+from spack.test.mock_packages_test import MockPackagesTest
 
 FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
 
+
 # Monkey-patch open to write module files to a StringIO instance
 @contextmanager
 def mock_open(filename, mode):
     if not mode == 'w':
-        raise RuntimeError('test.modules : unexpected opening mode for monkey-patched open')
+        message = 'test.modules : unexpected opening mode [mock_open]'
+        raise RuntimeError(message)
 
     FILE_REGISTRY[filename] = StringIO.StringIO()
 
@@ -20,7 +24,6 @@ def mock_open(filename, mode):
         FILE_REGISTRY[filename] = handle.getvalue()
         handle.close()
 
-import spack.modules
 
 configuration_autoload_direct = {
     'enable': ['tcl'],
@@ -47,7 +50,8 @@ configuration_alter_environment = {
             'filter': {'environment_blacklist': ['CMAKE_PREFIX_PATH']}
         },
         '=x86-linux': {
-            'environment': {'set': {'FOO': 'foo'}, 'unset': ['BAR']}
+            'environment': {'set': {'FOO': 'foo'},
+                            'unset': ['BAR']}
         }
     }
 }
@@ -72,15 +76,14 @@ configuration_conflicts = {
     }
 }
 
-from spack.test.mock_packages_test import MockPackagesTest
-
 
 class TclTests(MockPackagesTest):
     def setUp(self):
         super(TclTests, self).setUp()
         self.configuration_obj = spack.modules.CONFIGURATION
         spack.modules.open = mock_open
-        spack.modules.CONFIGURATION = None  # Make sure that a non-mocked configuration will trigger an error
+        # Make sure that a non-mocked configuration will trigger an error
+        spack.modules.CONFIGURATION = None
 
     def tearDown(self):
         del spack.modules.open
@@ -98,7 +101,7 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_autoload_direct
         spec = spack.spec.Spec('mpich@3.0.4=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertTrue('module-whatis "mpich @3.0.4"' in content )
+        self.assertTrue('module-whatis "mpich @3.0.4"' in content)
 
     def test_autoload(self):
         spack.modules.CONFIGURATION = configuration_autoload_direct
@@ -117,14 +120,22 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_alter_environment
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 1)
+        self.assertEqual(
+            len([x
+                 for x in content
+                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(
+            len([x for x in content if 'setenv FOO "foo"' in x]), 1)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 1)
 
         spec = spack.spec.Spec('libdwarf=x64-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 0)
+        self.assertEqual(
+            len([x
+                 for x in content
+                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(
+            len([x for x in content if 'setenv FOO "foo"' in x]), 0)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 0)
 
     def test_blacklist(self):
@@ -138,6 +149,9 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_conflicts
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('conflict')]), 2)
-        self.assertEqual(len([x for x in content if x == 'conflict mpileaks']), 1)
-        self.assertEqual(len([x for x in content if x == 'conflict intel/14.0.1']), 1)
+        self.assertEqual(
+            len([x for x in content if x.startswith('conflict')]), 2)
+        self.assertEqual(
+            len([x for x in content if x == 'conflict mpileaks']), 1)
+        self.assertEqual(
+            len([x for x in content if x == 'conflict intel/14.0.1']), 1)
-- 
cgit v1.2.3-70-g09d2


From b215b19cae1eaf665dfbec4e20c36497c57dd182 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 11 May 2016 15:42:48 +0200
Subject: modules : added docs

---
 lib/spack/docs/basic_usage.rst | 272 ++++++++++++++++++++++++++++++++++++-----
 lib/spack/spack/package.py     |   2 +-
 2 files changed, 242 insertions(+), 32 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/docs/basic_usage.rst b/lib/spack/docs/basic_usage.rst
index 29791d98c4..ce79ee0929 100644
--- a/lib/spack/docs/basic_usage.rst
+++ b/lib/spack/docs/basic_usage.rst
@@ -788,7 +788,7 @@ versions are now filtered out.
 
 .. _shell-support:
 
-Environment modules
+Integration with module systems
 -------------------------------
 
 .. note::
@@ -798,42 +798,50 @@ Environment modules
    interface and/or generated module names may change in future
    versions.
 
-Spack provides some limited integration with environment module
-systems to make it easier to use the packages it provides.
+Spack provides some integration with
+`Environment Modules <http://modules.sourceforge.net/>`_
+and `Dotkit <https://computing.llnl.gov/?set=jobs&page=dotkit>`_ to make
+it easier to use the packages it installed.
+
 
 
 Installing Environment Modules
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
 In order to use Spack's generated environment modules, you must have
 installed the *Environment Modules* package.  On many Linux
-distributions, this can be installed from the vendor's repository.
-For example: ```yum install environment-modules``
-(Fedora/RHEL/CentOS).  If your Linux distribution does not have
-Environment Modules, you can get it with Spack:
+distributions, this can be installed from the vendor's repository:
+
+.. code-block:: sh
 
-1. Install with::
+    yum install environment-modules # (Fedora/RHEL/CentOS)
+    apt-get install environment-modules # (Ubuntu/Debian)
+
+If your Linux distribution does not have
+Environment Modules, you can get it with Spack:
 
 .. code-block:: sh
 
     spack install environment-modules
 
-2. Activate with::
 
-Add the following two lines to your ``.bashrc`` profile (or similar):
+In this case to activate it automatically you need to add the following two
+lines to your ``.bashrc`` profile (or similar):
 
 .. code-block:: sh
 
    MODULES_HOME=`spack location -i environment-modules`
    source ${MODULES_HOME}/Modules/init/bash
 
-In case you use a Unix shell other than bash, substitute ``bash`` by
-the appropriate file in ``${MODULES_HOME}/Modules/init/``.
+If you use a Unix shell other than ``bash``, modify the commands above
+accordingly and source the appropriate file in
+``${MODULES_HOME}/Modules/init/``.
 
 
-Spack and Environment Modules
-~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+.. TODO : Add a similar section on how to install dotkit ?
 
+Spack and module systems
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 You can enable shell support by sourcing some files in the
 ``/share/spack`` directory.
 
@@ -841,7 +849,7 @@ For ``bash`` or ``ksh``, run:
 
 .. code-block:: sh
 
-   . $SPACK_ROOT/share/spack/setup-env.sh
+   . ${SPACK_ROOT}/share/spack/setup-env.sh
 
 For ``csh`` and ``tcsh`` run:
 
@@ -853,17 +861,19 @@ For ``csh`` and ``tcsh`` run:
 You can put the above code in your ``.bashrc`` or ``.cshrc``, and
 Spack's shell support will be available on the command line.
 
-When you install a package with Spack, it automatically generates an
-environment module that lets you add the package to your environment.
+When you install a package with Spack, it automatically generates a module file
+that lets you add the package to your environment.
 
-Currently, Spack supports the generation of `TCL Modules
+Currently, Spack supports the generation of `Environment Modules
 <http://wiki.tcl.tk/12999>`_ and `Dotkit
 <https://computing.llnl.gov/?set=jobs&page=dotkit>`_.  Generated
 module files for each of these systems can be found in these
 directories:
 
-  * ``$SPACK_ROOT/share/spack/modules``
-  * ``$SPACK_ROOT/share/spack/dotkit``
+.. code-block:: sh
+
+  ${SPACK_ROOT}/share/spack/modules
+  ${SPACK_ROOT}/share/spack/dotkit
 
 The directories are automatically added to your ``MODULEPATH`` and
 ``DK_NODE`` environment variables when you enable Spack's `shell
@@ -919,8 +929,7 @@ of installed packages.
 
 The names here should look familiar, they're the same ones from
 ``spack find``.  You *can* use the names here directly.  For example,
-you could type either of these commands to load the callpath module
-(assuming dotkit and modules are installed):
+you could type either of these commands to load the callpath module:
 
 .. code-block:: sh
 
@@ -935,7 +944,7 @@ easy to type.  Luckily, Spack has its own interface for using modules
 and dotkits.  You can use the same spec syntax you're used to:
 
   =========================  ==========================
-  Modules                    Dotkit
+  Environment Modules        Dotkit
   =========================  ==========================
   ``spack load <spec>``      ``spack use <spec>``
   ``spack unload <spec>``    ``spack unuse <spec>``
@@ -1002,15 +1011,216 @@ used ``gcc``.  You could therefore just type:
 
 To identify just the one built with the Intel compiler.
 
+Module files generation and customization
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Environment Modules and Dotkit files are generated when packages are installed,
+and are placed in the following directories under the Spack root:
+
+.. code-block:: sh
+
+  ${SPACK_ROOT}/share/spack/modules
+  ${SPACK_ROOT}/share/spack/dotkit
+
+The content that gets written in each module file can be customized in two ways:
+
+  1. overriding part of the ``spack.Package`` API within a ``package.py``
+  2. writing dedicated configuration files
+
+Override ``Package`` API
+^^^^^^^^^^^^^^^^^^^^^^^^
+There are currently two methods in ``spack.Package`` that may affect the content
+of module files:
+
+.. code-block:: python
+
+  def setup_environment(self, spack_env, run_env):
+      """Set up the compile and runtime environments for a package."""
+      pass
+
+.. code-block:: python
+
+  def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
+      """Set up the environment of packages that depend on this one"""
+      pass
+
+As briefly stated in the comments, the first method lets you customize the
+module file content for the package you are currently writing, the second
+allows for modifications to your dependees module file. In both cases one
+needs to fill ``run_env`` with the desired list of environment modifications.
+
+Example : ``builtin/packages/python/package.py``
+""""""""""""""""""""""""""""""""""""""""""""""""
+
+The ``python`` package that comes with the ``builtin`` Spack repository
+overrides ``setup_dependent_environment`` in the following way:
+
+.. code-block:: python
+
+  def setup_dependent_environment(self, spack_env, run_env, extension_spec):
+        # ...
+        if extension_spec.package.extends(self.spec):
+            run_env.prepend_path('PYTHONPATH', os.path.join(extension_spec.prefix, self.site_packages_dir))
+
+to insert the appropriate ``PYTHONPATH`` modifications in the module
+files of python packages.
+
+Configuration files
+^^^^^^^^^^^^^^^^^^^
+
+Another way of modifying the content of module files is writing a
+``modules.yaml`` configuration file. Following usual Spack conventions, this
+file can be placed either at *site* or *user* scope.
+
+The default site configuration reads:
+
+ .. literalinclude:: ../../../etc/spack/modules.yaml
+    :language: yaml
+
+It basically inspects the installation prefixes for the
+existence of a few folders and, if they exist, it prepends a path to a given
+list of environment variables.
+
+For each module system that can be enabled a finer configuration is possible:
+
+.. code-block:: yaml
+
+ modules:
+   tcl:
+     # contains environment modules specific customizations
+   dotkit:
+     # contains dotkit specific customizations
 
-Regenerating Module files
-~~~~~~~~~~~~~~~~~~~~~~~~~~~
+The structure under the ``tcl`` and ``dotkit`` keys is almost equal, and will
+be showcased in the following by some examples.
 
-Module and dotkit files are generated when packages are installed, and
-are placed in the following directories under the Spack root:
+Select module files by spec constraints
+"""""""""""""""""""""""""""""""""""""""
+Using spec syntax it's possible to have different customizations for different
+groups of module files.
 
-  * ``$SPACK_ROOT/share/spack/modules``
-  * ``$SPACK_ROOT/share/spack/dotkit``
+Considering :
+
+.. code-block:: yaml
+
+ modules:
+   tcl:
+     all: # Default addition for every package
+       environment:
+         set:
+           BAR: 'bar'
+     ^openmpi:: # A double ':' overrides previous rules
+       environment:
+         set:
+           BAR: 'baz'
+     zlib:
+       environment:
+         prepend_path:
+           LD_LIBRARY_PATH: 'foo'
+     zlib%gcc@4.8:
+       environment:
+         unset:
+         - FOOBAR
+
+what will happen is that:
+
+ - every module file will set ``BAR=bar``
+ - unless the associated spec satisfies ``^openmpi`` in which case ``BAR=baz``
+ - any spec that satisfies ``zlib`` will additionally prepend ``foo`` to ``LD_LIBRARY_PATH``
+ - any spec that satisfies ``zlib%gcc@4.8`` will additionally unset ``FOOBAR``
+
+.. note::
+  Order does matter
+    The modifications associated with the ``all`` keyword are always evaluated
+    first, no matter where they appear in the configuration file. All the other
+    spec constraints are instead evaluated top to bottom.
+
+Filter modifications out of module files
+""""""""""""""""""""""""""""""""""""""""
+
+Modifications to certain environment variables in module files are generated by
+default. Suppose you would like to avoid having ``CPATH`` and ``LIBRARY_PATH``
+modified by your dotkit modules. Then :
+
+.. code-block:: yaml
+
+  modules:
+    dotkit:
+      all:
+        filter:
+          environment_blacklist: ['CPATH', 'LIBRARY_PATH']  # Exclude changes to any of these variables
+
+will generate dotkit module files that will not contain modifications to either
+``CPATH`` or ``LIBRARY_PATH`` and environment module files that instead will
+contain those modifications.
+
+Autoload dependencies
+"""""""""""""""""""""
+
+The following lines in ``modules.yaml``:
+
+.. code-block:: yaml
+
+  modules:
+    tcl:
+      all:
+        autoload: 'direct'
+
+will produce environment module files that will automatically load their direct
+dependencies.
+
+.. note::
+  Allowed values for ``autoload`` statements
+    Allowed values for ``autoload`` statements are either ``none``, ``direct``
+    or ``all``. In ``tcl`` configuration it is possible to use the option
+    ``prerequisites`` that accepts the same values and will add ``prereq``
+    statements instead of automatically loading other modules.
+
+Blacklist or whitelist the generation of specific module files
+""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""""
+
+Sometimes it is desirable not to generate module files, a common use case being
+not providing the users with software built using the system compiler.
+
+A configuration file like:
+
+.. code-block:: yaml
+
+  modules:
+    tcl:
+      whitelist: ['gcc', 'llvm']  # Whitelist will have precedence over blacklist
+      blacklist: ['%gcc@4.4.7']  # Assuming gcc@4.4.7 is the system compiler
+
+will skip module file generation for anything that satisfies ``%gcc@4.4.7``,
+with the exception of specs that satisfy ``gcc`` or ``llvm``.
+
+Customize the naming scheme and insert conflicts
+""""""""""""""""""""""""""""""""""""""""""""""""
+
+A configuration file like:
+
+.. code-block:: yaml
+
+  modules:
+    tcl:
+      naming_scheme: '{name}/{version}-{compiler.name}-{compiler.version}'
+      all:
+        conflict: ['{name}', 'intel/14.0.1']
+
+will create module files that will conflict with ``intel/14.0.1`` and with the
+base directory of the same module, effectively preventing the possibility to
+load two or more versions of the same software at the same time.
+
+.. note::
+  Tokens available for the naming scheme
+    currently only the tokens shown in the example are available to construct
+    the naming scheme
+
+.. note::
+  The ``conflict`` option is ``tcl`` specific
+
+Regenerating module files
+^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Sometimes you may need to regenerate the modules files.  For example,
 if newer, fancier module support is added to Spack at some later date,
@@ -1020,7 +1230,7 @@ new features.
 .. _spack-module:
 
 ``spack module refresh``
-^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+""""""""""""""""""""""""
 
 Running ``spack module refresh`` will remove the
 ``share/spack/modules`` and ``share/spack/dotkit`` directories, then
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 4065553131..3626a574c8 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -1006,7 +1006,7 @@ class Package(object):
                           fromlist=[self.__class__.__name__])
 
     def setup_environment(self, spack_env, run_env):
-        """Set up the compile and runtime environemnts for a package.
+        """Set up the compile and runtime environments for a package.
 
         `spack_env` and `run_env` are `EnvironmentModifications`
         objects.  Package authors can call methods on them to alter
-- 
cgit v1.2.3-70-g09d2


From 22bb0562fea525afb329d5710970785189d3af63 Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 11 May 2016 16:09:47 +0200
Subject: Revert "flake8 : fixed all issues?"

This reverts commit 71e49e289a849b8aaa4f0d9a195d07569051ca88.
---
 lib/spack/spack/cmd/module.py    |  16 ++-
 lib/spack/spack/config.py        | 204 +++++++++++++++------------------------
 lib/spack/spack/environment.py   |  51 ++++------
 lib/spack/spack/modules.py       | 202 ++++++++++++++++----------------------
 lib/spack/spack/test/__init__.py |  69 +++++++++----
 lib/spack/spack/test/modules.py  |  42 +++-----
 6 files changed, 254 insertions(+), 330 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index cfe59c8d98..f996f4eb84 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -32,21 +32,18 @@ from llnl.util.filesystem import mkdirp
 from spack.modules import module_types
 from spack.util.string import *
 
-description = "Manipulate modules and dotkits."
+description ="Manipulate modules and dotkits."
 
 
 def setup_parser(subparser):
     sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command')
 
-    sp.add_parser('refresh', help='Regenerate all module files.')
+    refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.')
 
     find_parser = sp.add_parser('find', help='Find module files for packages.')
-    find_parser.add_argument('module_type',
-                             help="Type of module to find file for. [" +
-                             '|'.join(module_types) + "]")
-    find_parser.add_argument('spec',
-                             nargs='+',
-                             help='spec to find a module file for.')
+    find_parser.add_argument(
+        'module_type', help="Type of module to find file for. [" + '|'.join(module_types) + "]")
+    find_parser.add_argument('spec', nargs='+', help='spec to find a module file for.')
 
 
 def module_find(mtype, spec_array):
@@ -56,8 +53,7 @@ def module_find(mtype, spec_array):
        should type to use that package's module.
     """
     if mtype not in module_types:
-        tty.die("Invalid module type: '%s'.  Options are %s" %
-                (mtype, comma_or(module_types)))
+        tty.die("Invalid module type: '%s'.  Options are %s" % (mtype, comma_or(module_types)))
 
     specs = spack.cmd.parse_specs(spec_array)
     if len(specs) > 1:
diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py
index 684a420b3b..6ddf07776b 100644
--- a/lib/spack/spack/config.py
+++ b/lib/spack/spack/config.py
@@ -134,6 +134,8 @@ from yaml.error import MarkedYAMLError
 
 # Hacked yaml for configuration files preserves line numbers.
 import spack.util.spack_yaml as syaml
+
+
 """Dict from section names -> schema for that section."""
 section_schemas = {
     'compilers': {
@@ -147,31 +149,25 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'patternProperties': {
-                    r'\w[\w-]*': {  # architecture
+                    r'\w[\w-]*': {           # architecture
                         'type': 'object',
                         'additionalProperties': False,
                         'patternProperties': {
-                            r'\w[\w-]*@\w[\w-]*': {  # compiler spec
+                            r'\w[\w-]*@\w[\w-]*': {   # compiler spec
                                 'type': 'object',
                                 'additionalProperties': False,
                                 'required': ['cc', 'cxx', 'f77', 'fc'],
                                 'properties': {
-                                    'cc': {'anyOf': [{'type': 'string'},
-                                                     {'type': 'null'}]},
-                                    'cxx': {'anyOf': [{'type': 'string'},
-                                                      {'type': 'null'}]},
-                                    'f77': {'anyOf': [{'type': 'string'},
-                                                      {'type': 'null'}]},
-                                    'fc': {'anyOf': [{'type': 'string'},
-                                                     {'type': 'null'}]},
-                                },
-                            },
-                        },
-                    },
-                },
-            },
-        },
-    },
+                                    'cc':  { 'anyOf': [ {'type' : 'string' },
+                                                        {'type' : 'null' }]},
+                                    'cxx': { 'anyOf': [ {'type' : 'string' },
+                                                        {'type' : 'null' }]},
+                                    'f77': { 'anyOf': [ {'type' : 'string' },
+                                                        {'type' : 'null' }]},
+                                    'fc':  { 'anyOf': [ {'type' : 'string' },
+                                                        {'type' : 'null' }]},
+                                },},},},},},},},
+
     'mirrors': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack mirror configuration file schema',
@@ -184,12 +180,8 @@ section_schemas = {
                 'additionalProperties': False,
                 'patternProperties': {
                     r'\w[\w-]*': {
-                        'type': 'string'
-                    },
-                },
-            },
-        },
-    },
+                        'type': 'string'},},},},},
+
     'repos': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack repository configuration file schema',
@@ -200,11 +192,8 @@ section_schemas = {
                 'type': 'array',
                 'default': [],
                 'items': {
-                    'type': 'string'
-                },
-            },
-        },
-    },
+                    'type': 'string'},},},},
+
     'packages': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack package configuration file schema',
@@ -216,48 +205,39 @@ section_schemas = {
                 'default': {},
                 'additionalProperties': False,
                 'patternProperties': {
-                    r'\w[\w-]*': {  # package name
+                    r'\w[\w-]*': { # package name
                         'type': 'object',
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
                             'version': {
-                                'type': 'array',
-                                'default': [],
-                                'items': {'anyOf': [{'type': 'string'},
-                                                    {'type': 'number'}]}
-                            },  # version strings
+                                'type' : 'array',
+                                'default' : [],
+                                'items' : { 'anyOf' : [ { 'type' : 'string' },
+                                                        { 'type' : 'number'}]}}, #version strings
                             'compiler': {
-                                'type': 'array',
-                                'default': [],
-                                'items': {'type': 'string'}
-                            },  # compiler specs
+                                'type' : 'array',
+                                'default' : [],
+                                'items' : { 'type' : 'string' } }, #compiler specs
                             'buildable': {
-                                'type': 'boolean',
+                                'type':  'boolean',
                                 'default': True,
-                            },
+                             },
                             'providers': {
-                                'type': 'object',
+                                'type':  'object',
                                 'default': {},
                                 'additionalProperties': False,
                                 'patternProperties': {
                                     r'\w[\w-]*': {
-                                        'type': 'array',
-                                        'default': [],
-                                        'items': {'type': 'string'},
-                                    },
-                                },
-                            },
+                                        'type' : 'array',
+                                        'default' : [],
+                                        'items' : { 'type' : 'string' },},},},
                             'paths': {
-                                'type': 'object',
-                                'default': {},
+                                'type' : 'object',
+                                'default' : {},
                             }
-                        },
-                    },
-                },
-            },
-        },
-    },
+                        },},},},},},
+
     'modules': {
         '$schema': 'http://json-schema.org/schema#',
         'title': 'Spack module file configuration file schema',
@@ -303,22 +283,17 @@ section_schemas = {
                         }
                     },
                     'autoload': {'$ref': '#/definitions/dependency_selection'},
-                    'prerequisites':
-                    {'$ref': '#/definitions/dependency_selection'},
+                    'prerequisites': {'$ref': '#/definitions/dependency_selection'},
                     'conflict': {'$ref': '#/definitions/array_of_strings'},
                     'environment': {
                         'type': 'object',
                         'default': {},
                         'additionalProperties': False,
                         'properties': {
-                            'set':
-                            {'$ref': '#/definitions/dictionary_of_strings'},
-                            'unset':
-                            {'$ref': '#/definitions/array_of_strings'},
-                            'prepend_path':
-                            {'$ref': '#/definitions/dictionary_of_strings'},
-                            'append_path':
-                            {'$ref': '#/definitions/dictionary_of_strings'}
+                            'set': {'$ref': '#/definitions/dictionary_of_strings'},
+                            'unset': {'$ref': '#/definitions/array_of_strings'},
+                            'prepend_path': {'$ref': '#/definitions/dictionary_of_strings'},
+                            'append_path': {'$ref': '#/definitions/dictionary_of_strings'}
                         }
                     }
                 }
@@ -329,20 +304,15 @@ section_schemas = {
                 'anyOf': [
                     {
                         'properties': {
-                            'whitelist':
-                            {'$ref': '#/definitions/array_of_strings'},
-                            'blacklist':
-                            {'$ref': '#/definitions/array_of_strings'},
+                            'whitelist': {'$ref': '#/definitions/array_of_strings'},
+                            'blacklist': {'$ref': '#/definitions/array_of_strings'},
                             'naming_scheme': {
-                                'type':
-                                'string'  # Can we be more specific here?
+                                'type': 'string'  # Can we be more specific here?
                             }
                         }
                     },
                     {
-                        'patternProperties':
-                        {r'\w[\w-]*':
-                         {'$ref': '#/definitions/module_file_configuration'}}
+                        'patternProperties': {r'\w[\w-]*': {'$ref': '#/definitions/module_file_configuration'}}
                     }
                 ]
             }
@@ -356,8 +326,7 @@ section_schemas = {
                     'prefix_inspections': {
                         'type': 'object',
                         'patternProperties': {
-                            r'\w[\w-]*':
-                            {  # path to be inspected (relative to prefix)
+                            r'\w[\w-]*': {  # path to be inspected for existence (relative to prefix)
                                 '$ref': '#/definitions/array_of_strings'
                             }
                         }
@@ -372,15 +341,13 @@ section_schemas = {
                     },
                     'tcl': {
                         'allOf': [
-                            {'$ref': '#/definitions/module_type_configuration'
-                             },  # Base configuration
+                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
                             {}  # Specific tcl extensions
                         ]
                     },
                     'dotkit': {
                         'allOf': [
-                            {'$ref': '#/definitions/module_type_configuration'
-                             },  # Base configuration
+                            {'$ref': '#/definitions/module_type_configuration'},  # Base configuration
                             {}  # Specific dotkit extensions
                         ]
                     },
@@ -389,6 +356,7 @@ section_schemas = {
         },
     },
 }
+
 """OrderedDict of config scopes keyed by name.
    Later scopes will override earlier scopes.
 """
@@ -398,13 +366,12 @@ config_scopes = OrderedDict()
 def validate_section_name(section):
     """Raise a ValueError if the section is not a valid section."""
     if section not in section_schemas:
-        raise ValueError("Invalid config section: '%s'.  Options are %s" %
-                         (section, section_schemas))
+        raise ValueError("Invalid config section: '%s'.  Options are %s"
+                         % (section, section_schemas))
 
 
 def extend_with_default(validator_class):
-    """Add support for the 'default' attribute for
-       properties and patternProperties
+    """Add support for the 'default' attribute for properties and patternProperties.
 
        jsonschema does not handle this out of the box -- it only
        validates.  This allows us to set default values for configs
@@ -413,15 +380,13 @@ def extend_with_default(validator_class):
 
     """
     validate_properties = validator_class.VALIDATORS["properties"]
-    validate_pattern_properties = validator_class.VALIDATORS[
-        "patternProperties"]
+    validate_pattern_properties = validator_class.VALIDATORS["patternProperties"]
 
     def set_defaults(validator, properties, instance, schema):
         for property, subschema in properties.iteritems():
             if "default" in subschema:
                 instance.setdefault(property, subschema["default"])
-        for err in validate_properties(validator, properties, instance,
-                                       schema):
+        for err in validate_properties(validator, properties, instance, schema):
             yield err
 
     def set_pp_defaults(validator, properties, instance, schema):
@@ -432,19 +397,17 @@ def extend_with_default(validator_class):
                         if re.match(property, key) and val is None:
                             instance[key] = subschema["default"]
 
-        for err in validate_pattern_properties(validator, properties, instance,
-                                               schema):
+        for err in validate_pattern_properties(validator, properties, instance, schema):
             yield err
 
     return validators.extend(validator_class, {
-        "properties": set_defaults,
-        "patternProperties": set_pp_defaults
+        "properties" : set_defaults,
+        "patternProperties" : set_pp_defaults
     })
 
 
 DefaultSettingValidator = extend_with_default(Draft4Validator)
 
-
 def validate_section(data, schema):
     """Validate data read in from a Spack YAML file.
 
@@ -466,9 +429,9 @@ class ConfigScope(object):
     """
 
     def __init__(self, name, path):
-        self.name = name  # scope name.
-        self.path = path  # path to directory containing configs.
-        self.sections = {}  # sections read from config files.
+        self.name = name           # scope name.
+        self.path = path           # path to directory containing configs.
+        self.sections = {}         # sections read from config files.
 
         # Register in a dict of all ConfigScopes
         # TODO: make this cleaner.  Mocking up for testing is brittle.
@@ -479,14 +442,16 @@ class ConfigScope(object):
         validate_section_name(section)
         return os.path.join(self.path, "%s.yaml" % section)
 
+
     def get_section(self, section):
-        if section not in self.sections:
-            path = self.get_section_filename(section)
+        if not section in self.sections:
+            path   = self.get_section_filename(section)
             schema = section_schemas[section]
-            data = _read_config_file(path, schema)
+            data   = _read_config_file(path, schema)
             self.sections[section] = data
         return self.sections[section]
 
+
     def write_section(self, section):
         filename = self.get_section_filename(section)
         data = self.get_section(section)
@@ -498,8 +463,8 @@ class ConfigScope(object):
         except jsonschema.ValidationError as e:
             raise ConfigSanityError(e, data)
         except (yaml.YAMLError, IOError) as e:
-            raise ConfigFileError("Error writing to config file: '%s'" %
-                                  str(e))
+            raise ConfigFileError("Error writing to config file: '%s'" % str(e))
+
 
     def clear(self):
         """Empty cached config information."""
@@ -531,8 +496,8 @@ def validate_scope(scope):
         return config_scopes[scope]
 
     else:
-        raise ValueError("Invalid config scope: '%s'.  Must be one of %s" %
-                         (scope, config_scopes.keys()))
+        raise ValueError("Invalid config scope: '%s'.  Must be one of %s"
+                         % (scope, config_scopes.keys()))
 
 
 def _read_config_file(filename, schema):
@@ -558,12 +523,12 @@ def _read_config_file(filename, schema):
         return data
 
     except MarkedYAMLError as e:
-        raise ConfigFileError("Error parsing yaml%s: %s" %
-                              (str(e.context_mark), e.problem))
+        raise ConfigFileError(
+            "Error parsing yaml%s: %s" % (str(e.context_mark), e.problem))
 
     except IOError as e:
-        raise ConfigFileError("Error reading configuration file %s: %s" %
-                              (filename, str(e)))
+        raise ConfigFileError(
+            "Error reading configuration file %s: %s" % (filename, str(e)))
 
 
 def clear_config_caches():
@@ -586,7 +551,6 @@ def _merge_yaml(dest, source):
     parent instead of merging.
 
     """
-
     def they_are(t):
         return isinstance(dest, t) and isinstance(source, t)
 
@@ -607,7 +571,7 @@ def _merge_yaml(dest, source):
     # Source dict is merged into dest.
     elif they_are(dict):
         for sk, sv in source.iteritems():
-            if sk not in dest:
+            if not sk in dest:
                 dest[sk] = copy.copy(sv)
             else:
                 dest[sk] = _merge_yaml(dest[sk], source[sk])
@@ -689,7 +653,7 @@ def print_section(section):
         data = syaml.syaml_dict()
         data[section] = get_config(section)
         syaml.dump(data, stream=sys.stdout, default_flow_style=False)
-    except (yaml.YAMLError, IOError):
+    except (yaml.YAMLError, IOError) as e:
         raise ConfigError("Error reading configuration: %s" % section)
 
 
@@ -719,22 +683,15 @@ def is_spec_buildable(spec):
     """Return true if the spec pkgspec is configured as buildable"""
     allpkgs = get_config('packages')
     name = spec.name
-    if name not in allpkgs:
+    if not spec.name in allpkgs:
         return True
-    if 'buildable' not in allpkgs[name]:
+    if not 'buildable' in allpkgs[spec.name]:
         return True
     return allpkgs[spec.name]['buildable']
 
 
-class ConfigError(SpackError):
-
-    pass
-
-
-class ConfigFileError(ConfigError):
-
-    pass
-
+class ConfigError(SpackError): pass
+class ConfigFileError(ConfigError): pass
 
 def get_path(path, data):
     if path:
@@ -742,10 +699,8 @@ def get_path(path, data):
     else:
         return data
 
-
 class ConfigFormatError(ConfigError):
     """Raised when a configuration format does not match its schema."""
-
     def __init__(self, validation_error, data):
         # Try to get line number from erroneous instance and its parent
         instance_mark = getattr(validation_error.instance, '_start_mark', None)
@@ -778,6 +733,5 @@ class ConfigFormatError(ConfigError):
         message = '%s: %s' % (location, validation_error.message)
         super(ConfigError, self).__init__(message)
 
-
 class ConfigSanityError(ConfigFormatError):
     """Same as ConfigFormatError, raised when config is written by Spack."""
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 9748b4033a..92ab4e6bea 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -26,8 +26,7 @@ class SetEnv(NameValueModifier):
 
 class UnsetEnv(NameModifier):
     def execute(self):
-        # Avoid throwing if the variable was not set
-        os.environ.pop(self.name, None)
+        os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
 class SetPath(NameValueModifier):
@@ -56,9 +55,7 @@ class RemovePath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        directories = [os.path.normpath(x)
-                       for x in directories
-                       if x != os.path.normpath(self.value)]
+        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.value)]
         os.environ[self.name] = ':'.join(directories)
 
 
@@ -66,8 +63,7 @@ class EnvironmentModifications(object):
     """
     Keeps track of requests to modify the current environment.
 
-    Each call to a method to modify the environment stores the extra
-    information on the caller in the request:
+    Each call to a method to modify the environment stores the extra information on the caller in the request:
     - 'filename' : filename of the module where the caller is defined
     - 'lineno': line number where the request occurred
     - 'context' : line of code that issued the request that failed
@@ -75,10 +71,10 @@ class EnvironmentModifications(object):
 
     def __init__(self, other=None):
         """
-        Initializes a new instance, copying commands from other if not None
+        Initializes a new instance, copying commands from other if it is not None
 
         Args:
-            other: another instance of EnvironmentModifications
+            other: another instance of EnvironmentModifications from which (optional)
         """
         self.env_modifications = []
         if other is not None:
@@ -97,7 +93,7 @@ class EnvironmentModifications(object):
     @staticmethod
     def _check_other(other):
         if not isinstance(other, EnvironmentModifications):
-            raise TypeError('not an instance of EnvironmentModifications')
+            raise TypeError('other must be an instance of EnvironmentModifications')
 
     def _get_outside_caller_attributes(self):
         stack = inspect.stack()
@@ -105,10 +101,12 @@ class EnvironmentModifications(object):
             _, filename, lineno, _, context, index = stack[2]
             context = context[index].strip()
         except Exception:
-            filename = 'unknown file'
-            lineno = 'unknown line'
-            context = 'unknown context'
-        args = {'filename': filename, 'lineno': lineno, 'context': context}
+            filename, lineno, context = 'unknown file', 'unknown line', 'unknown context'
+        args = {
+            'filename': filename,
+            'lineno': lineno,
+            'context': context
+        }
         return args
 
     def set(self, name, value, **kwargs):
@@ -172,7 +170,7 @@ class EnvironmentModifications(object):
 
     def remove_path(self, name, path, **kwargs):
         """
-        Stores in the current object a request to remove a path from a list
+        Stores in the current object a request to remove a path from a path list
 
         Args:
             name: name of the path list in the environment
@@ -187,8 +185,7 @@ class EnvironmentModifications(object):
         Returns a dict of the modifications grouped by variable name
 
         Returns:
-            dict mapping the environment variable name to the modifications
-            to be done on it
+            dict mapping the environment variable name to the modifications to be done on it
         """
         modifications = collections.defaultdict(list)
         for item in self:
@@ -206,8 +203,7 @@ class EnvironmentModifications(object):
         Applies the modifications and clears the list
         """
         modifications = self.group_by_name()
-        # Apply the modifications to the environment variables one variable
-        # at a time
+        # Apply the modifications to the environment variables one variable at a time
         for name, actions in sorted(modifications.items()):
             for x in actions:
                 x.execute()
@@ -228,17 +224,13 @@ def concatenate_paths(paths):
 
 def set_or_unset_not_first(variable, changes, errstream):
     """
-    Check if we are going to set or unset something after other modifications
-    have already been requested
+    Check if we are going to set or unset something after other modifications have already been requested
     """
-    indexes = [ii
-               for ii, item in enumerate(changes)
-               if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
+    indexes = [ii for ii, item in enumerate(changes) if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
     if indexes:
         good = '\t    \t{context} at {filename}:{lineno}'
         nogood = '\t--->\t{context} at {filename}:{lineno}'
-        message = 'Suspicious requests to set or unset the variable \'{var}\' found'  # NOQA: ignore=E501
-        errstream(message.format(var=variable))
+        errstream('Suspicious requests to set or unset the variable \'{var}\' found'.format(var=variable))
         for ii, item in enumerate(changes):
             print_format = nogood if ii in indexes else good
             errstream(print_format.format(**item.args))
@@ -246,8 +238,8 @@ def set_or_unset_not_first(variable, changes, errstream):
 
 def validate(env, errstream):
     """
-    Validates the environment modifications to check for the presence of
-    suspicious patterns. Prompts a warning for everything that was found
+    Validates the environment modifications to check for the presence of suspicious patterns. Prompts a warning for
+    everything that was found
 
     Current checks:
     - set or unset variables after other changes on the same variable
@@ -262,8 +254,7 @@ def validate(env, errstream):
 
 def filter_environment_blacklist(env, variables):
     """
-    Generator that filters out any change to environment variables present in
-    the input list
+    Generator that filters out any change to environment variables present in the input list
 
     Args:
         env: list of environment modifications
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 0dc6f06f55..ffed469b20 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -23,34 +23,36 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 """
-This module contains code for creating environment modules, which can include
-dotkits, tcl modules, lmod, and others.
+This module contains code for creating environment modules, which can include dotkits, tcl modules, lmod, and others.
 
-The various types of modules are installed by post-install hooks and removed
-after an uninstall by post-uninstall hooks. This class consolidates the logic
-for creating an abstract description of the information that module systems
-need.
+The various types of modules are installed by post-install hooks and removed after an uninstall by post-uninstall hooks.
+This class consolidates the logic for creating an abstract description of the information that module systems need.
+Currently that includes a number of directories to be appended to paths in the user's environment:
 
-This module also includes logic for coming up with unique names for the module
-files so that they can be found by the various shell-support files in
-$SPACK/share/spack/setup-env.*.
+  * /bin directories to be appended to PATH
+  * /lib* directories for LD_LIBRARY_PATH
+  * /include directories for CPATH
+  * /man* and /share/man* directories for MANPATH
+  * the package prefix for CMAKE_PREFIX_PATH
 
-Each hook implements the logic for writing its specific type of module.
+This module also includes logic for coming up with unique names for the module files so that they can be found by the
+various shell-support files in $SPACK/share/spack/setup-env.*.
+
+Each hook in hooks/ implements the logic for writing its specific type of module file.
 """
 import copy
 import datetime
 import os
 import os.path
 import re
-import string
 import textwrap
+import string
 
 import llnl.util.tty as tty
 import spack
 import spack.config
 from llnl.util.filesystem import join_path, mkdirp
-from spack.build_environment import parent_class_modules
-from spack.build_environment import set_module_variables_for_package
+from spack.build_environment import parent_class_modules, set_module_variables_for_package
 from spack.environment import *
 
 __all__ = ['EnvModule', 'Dotkit', 'TclModule']
@@ -65,26 +67,30 @@ def print_help():
     """
     For use by commands to tell user how to activate shell support.
     """
-    tty.msg("This command requires spack's shell integration.", "",
+    tty.msg("This command requires spack's shell integration.",
+            "",
             "To initialize spack's shell commands, you must run one of",
             "the commands below.  Choose the right command for your shell.",
-            "", "For bash and zsh:",
-            "    . %s/setup-env.sh" % spack.share_path, "",
-            "For csh and tcsh:", "    setenv SPACK_ROOT %s" % spack.prefix,
-            "    source %s/setup-env.csh" % spack.share_path, "")
+            "",
+            "For bash and zsh:",
+            "    . %s/setup-env.sh" % spack.share_path,
+            "",
+            "For csh and tcsh:",
+            "    setenv SPACK_ROOT %s" % spack.prefix,
+            "    source %s/setup-env.csh" % spack.share_path,
+            "")
 
 
 def inspect_path(prefix):
     """
-    Inspects the prefix of an installation to search for common layouts.
-    Issues a request to modify the environment when an item is found.
+    Inspects the prefix of an installation to search for common layouts. Issues a request to modify the environment
+    accordingly when an item is found.
 
     Args:
         prefix: prefix of the installation
 
     Returns:
-        instance of EnvironmentModifications containing the requested
-        modifications
+        instance of EnvironmentModifications containing the requested modifications
     """
     env = EnvironmentModifications()
     # Inspect the prefix to check for the existence of common directories
@@ -99,22 +105,18 @@ def inspect_path(prefix):
 
 def dependencies(spec, request='all'):
     """
-    Returns the list of dependent specs for a given spec, according to the
-    given request
+    Returns the list of dependent specs for a given spec, according to the given request
 
     Args:
         spec: target spec
         request: either 'none', 'direct' or 'all'
 
     Returns:
-        empty list if 'none', direct dependency list if 'direct', all
-        dependencies if 'all'
+        empty list if 'none', direct dependency list if 'direct', all dependencies if 'all'
     """
     if request not in ('none', 'direct', 'all'):
-        message = "Wrong value for argument 'request' : "
-        message += "should be one of ('none', 'direct', 'all')"
-        message += " [current value is '{0}']"
-        raise tty.error(message.format(request))
+        raise tty.error("Wrong value for argument 'request' : should be one of ('none', 'direct', 'all') "
+                        " [current value is '%s']" % request)
 
     if request == 'none':
         return []
@@ -122,19 +124,12 @@ def dependencies(spec, request='all'):
     if request == 'direct':
         return [xx for _, xx in spec.dependencies.items()]
 
-    # FIXME : during module file creation nodes seem to be visited
-    # FIXME : multiple times even if cover='nodes' is given. This work around
-    # FIXME : permits to get a unique list of spec anyhow. Maybe we miss a
-    # FIXME : merge step among nodes that refer to the same package?
+    # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
+    # FIXME : is given. This work around permits to get a unique list of spec anyhow.
+    # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
     seen = set()
     seen_add = seen.add
-    l = [xx
-         for xx in sorted(
-             spec.traverse(order='post',
-                           depth=True,
-                           cover='nodes',
-                           root=False),
-             reverse=True)]
+    l = [xx for xx in sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
     return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
 
 
@@ -151,8 +146,7 @@ def update_dictionary_extending_lists(target, update):
 
 def parse_config_options(module_generator):
     """
-    Parse the configuration file and returns a bunch of items that will be
-    needed during module file generation
+    Parse the configuration file and returns a bunch of items that will be needed during module file generation
 
     Args:
         module_generator: module generator for a given spec
@@ -160,14 +154,11 @@ def parse_config_options(module_generator):
     Returns:
         autoloads: list of specs to be autoloaded
         prerequisites: list of specs to be marked as prerequisite
-        filters: list of environment variables whose modification is
-        blacklisted in module files
-        env: list of custom environment modifications to be applied in the
-        module file
+        filters: list of environment variables whose modification is blacklisted in module files
+        env: list of custom environment modifications to be applied in the module file
     """
     # Get the configuration for this kind of generator
-    module_configuration = copy.deepcopy(CONFIGURATION.get(
-        module_generator.name, {}))
+    module_configuration = copy.deepcopy(CONFIGURATION.get(module_generator.name, {}))
 
     #####
     # Merge all the rules
@@ -188,12 +179,9 @@ def parse_config_options(module_generator):
     #####
 
     # Automatic loading loads
-    module_file_actions['autoload'] = dependencies(
-        module_generator.spec, module_file_actions.get('autoload', 'none'))
+    module_file_actions['autoload'] = dependencies(module_generator.spec, module_file_actions.get('autoload', 'none'))
     # Prerequisites
-    module_file_actions['prerequisites'] = dependencies(
-        module_generator.spec, module_file_actions.get('prerequisites',
-                                                       'none'))
+    module_file_actions['prerequisites'] = dependencies(module_generator.spec, module_file_actions.get('prerequisites', 'none'))
     # Environment modifications
     environment_actions = module_file_actions.pop('environment', {})
     env = EnvironmentModifications()
@@ -201,7 +189,7 @@ def parse_config_options(module_generator):
     def process_arglist(arglist):
         if method == 'unset':
             for x in arglist:
-                yield (x, )
+                yield (x,)
         else:
             for x in arglist.iteritems():
                 yield x
@@ -210,20 +198,19 @@ def parse_config_options(module_generator):
         for args in process_arglist(arglist):
             getattr(env, method)(*args)
 
-            # for item in arglist:
-            #     if method == 'unset':
-            #         args = [item]
-            #     else:
-            #         args = item.split(',')
-            #     getattr(env, method)(*args)
+        # for item in arglist:
+        #     if method == 'unset':
+        #         args = [item]
+        #     else:
+        #         args = item.split(',')
+        #     getattr(env, method)(*args)
 
     return module_file_actions, env
 
 
 def filter_blacklisted(specs, module_name):
     """
-    Given a sequence of specs, filters the ones that are blacklisted in the
-    module configuration file.
+    Given a sequence of specs, filters the ones that are blacklisted in the module configuration file.
 
     Args:
         specs: sequence of spec instances
@@ -246,8 +233,7 @@ class EnvModule(object):
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
-            if cls.name != 'env_module' and cls.name in CONFIGURATION[
-                    'enable']:
+            if cls.name != 'env_module' and cls.name in CONFIGURATION['enable']:
                 module_types[cls.name] = cls
 
     def __init__(self, spec=None):
@@ -263,8 +249,7 @@ class EnvModule(object):
         # long description is the docstring with reduced whitespace.
         self.long_description = None
         if self.spec.package.__doc__:
-            self.long_description = re.sub(r'\s+', ' ',
-                                           self.spec.package.__doc__)
+            self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
 
     @property
     def naming_scheme(self):
@@ -286,14 +271,12 @@ class EnvModule(object):
     @property
     def use_name(self):
         """
-        Subclasses should implement this to return the name the module command
-        uses to refer to the package.
+        Subclasses should implement this to return the name the module command uses to refer to the package.
         """
         naming_tokens = self.tokens
         naming_scheme = self.naming_scheme
         name = naming_scheme.format(**naming_tokens)
-        name += '-' + self.spec.dag_hash(
-        )  # Always append the hash to make the module file unique
+        name += '-' + self.spec.dag_hash()  # Always append the hash to make the module file unique
         # Not everybody is working on linux...
         parts = name.split('/')
         name = join_path(*parts)
@@ -313,12 +296,8 @@ class EnvModule(object):
     @property
     def blacklisted(self):
         configuration = CONFIGURATION.get(self.name, {})
-        whitelist_matches = [x
-                             for x in configuration.get('whitelist', [])
-                             if self.spec.satisfies(x)]
-        blacklist_matches = [x
-                             for x in configuration.get('blacklist', [])
-                             if self.spec.satisfies(x)]
+        whitelist_matches = [x for x in configuration.get('whitelist', []) if self.spec.satisfies(x)]
+        blacklist_matches = [x for x in configuration.get('blacklist', []) if self.spec.satisfies(x)]
         if whitelist_matches:
             message = '\tWHITELIST : %s [matches : ' % self.spec.cshort_spec
             for rule in whitelist_matches:
@@ -348,8 +327,7 @@ class EnvModule(object):
         """
         if self.blacklisted:
             return
-        tty.debug("\tWRITE : %s [%s]" %
-                  (self.spec.cshort_spec, self.file_name))
+        tty.debug("\tWRITE : %s [%s]" % (self.spec.cshort_spec, self.file_name))
 
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
@@ -359,12 +337,11 @@ class EnvModule(object):
         # installation prefix
         env = inspect_path(self.spec.prefix)
 
-        # Let the extendee/dependency modify their extensions/dependencies
-        # before asking for package-specific modifications
+        # Let the extendee/dependency modify their extensions/dependencies before asking for
+        # package-specific modifications
         spack_env = EnvironmentModifications()
-        # TODO : the code down below is quite similar to
-        # TODO : build_environment.setup_package and needs to be factored out
-        # TODO : to a single place
+        # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
+        # TODO : factored out to a single place
         for item in dependencies(self.spec, 'all'):
             package = self.spec[item.name].package
             modules = parent_class_modules(package.__class__)
@@ -381,18 +358,14 @@ class EnvModule(object):
         # Parse configuration file
         module_configuration, conf_env = parse_config_options(self)
         env.extend(conf_env)
-        filters = module_configuration.get('filter', {}).get(
-            'environment_blacklist', {})
+        filters = module_configuration.get('filter', {}).get('environment_blacklist',{})
         # Build up the module file content
         module_file_content = self.header
-        for x in filter_blacklisted(
-                module_configuration.pop('autoload', []), self.name):
+        for x in filter_blacklisted(module_configuration.pop('autoload', []), self.name):
             module_file_content += self.autoload(x)
-        for x in filter_blacklisted(
-                module_configuration.pop('prerequisites', []), self.name):
+        for x in filter_blacklisted(module_configuration.pop('prerequisites', []), self.name):
             module_file_content += self.prerequisite(x)
-        for line in self.process_environment_command(
-                filter_environment_blacklist(env, filters)):
+        for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
             module_file_content += line
         for line in self.module_specific_content(module_configuration):
             module_file_content += line
@@ -419,13 +392,10 @@ class EnvModule(object):
     def process_environment_command(self, env):
         for command in env:
             try:
-                yield self.environment_modifications_formats[type(
-                    command)].format(**command.args)
+                yield self.environment_modifications_formats[type(command)].format(**command.args)
             except KeyError:
-                message = 'Cannot handle command of type {command} : skipping request'  # NOQA: ignore=E501
-                tty.warn(message.format(command=type(command)))
-                context = '{context} at {filename}:{lineno}'
-                tty.warn(context.format(**command.args))
+                tty.warn('Cannot handle command of type {command} : skipping request'.format(command=type(command)))
+                tty.warn('{context} at {filename}:{lineno}'.format(**command.args))
 
     @property
     def file_name(self):
@@ -438,12 +408,9 @@ class EnvModule(object):
         if os.path.exists(mod_file):
             try:
                 os.remove(mod_file)  # Remove the module file
-                os.removedirs(
-                    os.path.dirname(mod_file)
-                )  # Remove all the empty directories from the leaf up
+                os.removedirs(os.path.dirname(mod_file))  # Remove all the empty directories from the leaf up
             except OSError:
-                # removedirs throws OSError on first non-empty directory found
-                pass
+                pass  # removedirs throws OSError on first non-empty directory found
 
 
 class Dotkit(EnvModule):
@@ -457,12 +424,13 @@ class Dotkit(EnvModule):
 
     autoload_format = 'dk_op {module_file}\n'
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
+    prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
+
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
 
     @property
     def file_name(self):
-        return join_path(Dotkit.path, self.spec.architecture,
-                         '%s.dk' % self.use_name)
+        return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
 
     @property
     def header(self):
@@ -506,7 +474,7 @@ class TclModule(EnvModule):
 
     prerequisite_format = 'prereq {module_file}\n'
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
 
     @property
     def file_name(self):
@@ -514,10 +482,9 @@ class TclModule(EnvModule):
 
     @property
     def header(self):
-        timestamp = datetime.datetime.now()
         # TCL Modulefile header
         header = '#%Module1.0\n'
-        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % timestamp  # NOQA: ignore=E501
+        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % datetime.datetime.now()
         header += '##\n'
         header += '## %s\n' % self.spec.short_spec
         header += '##\n'
@@ -542,19 +509,16 @@ class TclModule(EnvModule):
         f = string.Formatter()
         for item in conflict_format:
             line = 'conflict ' + item + '\n'
-            if len([x for x in f.parse(line)
-                    ]) > 1:  # We do have placeholder to substitute
-                for naming_dir, conflict_dir in zip(
-                        self.naming_scheme.split('/'), item.split('/')):
+            if len([x for x in f.parse(line)]) > 1:  # We do have placeholder to substitute
+                for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), item.split('/')):
                     if naming_dir != conflict_dir:
-                        message = 'conflict scheme does not match naming'
-                        message += ' [{spec}]\n\n'
+                        message = 'conflict scheme does not match naming scheme [{spec}]\n\n'
                         message += 'naming scheme   : "{nformat}"\n'
                         message += 'conflict scheme : "{cformat}"\n\n'
-                        message += '** You may want to check your `modules.yaml` configuration file **\n'  # NOQA: ignore=E501
-                        tty.error(message.format(spec=self.spec,
-                                                 nformat=self.naming_scheme,
-                                                 cformat=item))
+                        message += '** You may want to check your `modules.yaml` configuration file **\n'
+                        tty.error(
+                            message.format(spec=self.spec, nformat=self.naming_scheme, cformat=item)
+                        )
                         raise SystemExit('Module generation aborted.')
                 line = line.format(**naming_tokens)
             yield line
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 395ca0c87a..05f58ab7b1 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -23,23 +23,53 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import sys
-
-import llnl.util.tty as tty
+import unittest
 import nose
-import spack
+
+from spack.test.tally_plugin import Tally
 from llnl.util.filesystem import join_path
+import llnl.util.tty as tty
 from llnl.util.tty.colify import colify
-from spack.test.tally_plugin import Tally
+
+import spack
+
 """Names of tests to be included in Spack's test suite"""
-test_names = ['versions', 'url_parse', 'url_substitution', 'packages', 'stage',
-              'spec_syntax', 'spec_semantics', 'spec_dag', 'concretize',
-              'multimethod', 'install', 'package_sanity', 'config',
-              'directory_layout', 'pattern', 'python_version', 'git_fetch',
-              'svn_fetch', 'hg_fetch', 'mirror', 'modules', 'url_extrapolate',
-              'cc', 'link_tree', 'spec_yaml', 'optional_deps',
-              'make_executable', 'configure_guess', 'lock', 'database',
-              'namespace_trie', 'yaml', 'sbang', 'environment',
-              'cmd.uninstall', 'cmd.test_install']
+test_names = ['versions',
+              'url_parse',
+              'url_substitution',
+              'packages',
+              'stage',
+              'spec_syntax',
+              'spec_semantics',
+              'spec_dag',
+              'concretize',
+              'multimethod',
+              'install',
+              'package_sanity',
+              'config',
+              'directory_layout',
+              'pattern',
+              'python_version',
+              'git_fetch',
+              'svn_fetch',
+              'hg_fetch',
+              'mirror',
+              'modules',
+              'url_extrapolate',
+              'cc',
+              'link_tree',
+              'spec_yaml',
+              'optional_deps',
+              'make_executable',
+              'configure_guess',
+              'lock',
+              'database',
+              'namespace_trie',
+              'yaml',
+              'sbang',
+              'environment',
+              'cmd.uninstall',
+              'cmd.test_install']
 
 
 def list_tests():
@@ -50,6 +80,7 @@ def list_tests():
 def run(names, outputDir, verbose=False):
     """Run tests with the supplied names.  Names should be a list.  If
        it's empty, run ALL of Spack's tests."""
+    verbosity = 1 if not verbose else 2
 
     if not names:
         names = test_names
@@ -64,7 +95,7 @@ def run(names, outputDir, verbose=False):
     tally = Tally()
     for test in names:
         module = 'spack.test.' + test
-        print(module)
+        print module
 
         tty.msg("Running test: %s" % test)
 
@@ -74,13 +105,15 @@ def run(names, outputDir, verbose=False):
             xmlOutputFname = "unittests-{0}.xml".format(test)
             xmlOutputPath = join_path(outputDir, xmlOutputFname)
             runOpts += ["--with-xunit",
-                        "--xunit-file={0}".format(xmlOutputPath)]
+                "--xunit-file={0}".format(xmlOutputPath)]
         argv = [""] + runOpts + [module]
-        nose.run(argv=argv, addplugins=[tally])
+        result = nose.run(argv=argv, addplugins=[tally])
 
     succeeded = not tally.failCount and not tally.errorCount
-    tty.msg("Tests Complete.", "%5d tests run" % tally.numberOfTestsRun,
-            "%5d failures" % tally.failCount, "%5d errors" % tally.errorCount)
+    tty.msg("Tests Complete.",
+            "%5d tests run" % tally.numberOfTestsRun,
+            "%5d failures" % tally.failCount,
+            "%5d errors" % tally.errorCount)
 
     if succeeded:
         tty.info("OK", format='g')
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index f0ff778a10..b8b0d6fc6a 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -2,18 +2,14 @@ import collections
 from contextlib import contextmanager
 
 import StringIO
-import spack.modules
-from spack.test.mock_packages_test import MockPackagesTest
 
 FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
 
-
 # Monkey-patch open to write module files to a StringIO instance
 @contextmanager
 def mock_open(filename, mode):
     if not mode == 'w':
-        message = 'test.modules : unexpected opening mode [mock_open]'
-        raise RuntimeError(message)
+        raise RuntimeError('test.modules : unexpected opening mode for monkey-patched open')
 
     FILE_REGISTRY[filename] = StringIO.StringIO()
 
@@ -24,6 +20,7 @@ def mock_open(filename, mode):
         FILE_REGISTRY[filename] = handle.getvalue()
         handle.close()
 
+import spack.modules
 
 configuration_autoload_direct = {
     'enable': ['tcl'],
@@ -50,8 +47,7 @@ configuration_alter_environment = {
             'filter': {'environment_blacklist': ['CMAKE_PREFIX_PATH']}
         },
         '=x86-linux': {
-            'environment': {'set': {'FOO': 'foo'},
-                            'unset': ['BAR']}
+            'environment': {'set': {'FOO': 'foo'}, 'unset': ['BAR']}
         }
     }
 }
@@ -76,14 +72,15 @@ configuration_conflicts = {
     }
 }
 
+from spack.test.mock_packages_test import MockPackagesTest
+
 
 class TclTests(MockPackagesTest):
     def setUp(self):
         super(TclTests, self).setUp()
         self.configuration_obj = spack.modules.CONFIGURATION
         spack.modules.open = mock_open
-        # Make sure that a non-mocked configuration will trigger an error
-        spack.modules.CONFIGURATION = None
+        spack.modules.CONFIGURATION = None  # Make sure that a non-mocked configuration will trigger an error
 
     def tearDown(self):
         del spack.modules.open
@@ -101,7 +98,7 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_autoload_direct
         spec = spack.spec.Spec('mpich@3.0.4=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertTrue('module-whatis "mpich @3.0.4"' in content)
+        self.assertTrue('module-whatis "mpich @3.0.4"' in content )
 
     def test_autoload(self):
         spack.modules.CONFIGURATION = configuration_autoload_direct
@@ -120,22 +117,14 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_alter_environment
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(
-            len([x
-                 for x in content
-                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(
-            len([x for x in content if 'setenv FOO "foo"' in x]), 1)
+        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 1)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 1)
 
         spec = spack.spec.Spec('libdwarf=x64-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(
-            len([x
-                 for x in content
-                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(
-            len([x for x in content if 'setenv FOO "foo"' in x]), 0)
+        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 0)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 0)
 
     def test_blacklist(self):
@@ -149,9 +138,6 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_conflicts
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(
-            len([x for x in content if x.startswith('conflict')]), 2)
-        self.assertEqual(
-            len([x for x in content if x == 'conflict mpileaks']), 1)
-        self.assertEqual(
-            len([x for x in content if x == 'conflict intel/14.0.1']), 1)
+        self.assertEqual(len([x for x in content if x.startswith('conflict')]), 2)
+        self.assertEqual(len([x for x in content if x == 'conflict mpileaks']), 1)
+        self.assertEqual(len([x for x in content if x == 'conflict intel/14.0.1']), 1)
-- 
cgit v1.2.3-70-g09d2


From bb4b6c8ee24aa81af6b6ee3a62692a624d52d65a Mon Sep 17 00:00:00 2001
From: alalazo <massimiliano.culpo@googlemail.com>
Date: Wed, 11 May 2016 17:02:36 +0200
Subject: flake 8 : fixed checks

---
 lib/spack/spack/cmd/module.py    |  16 +-
 lib/spack/spack/environment.py   |  58 ++++---
 lib/spack/spack/modules.py       | 196 ++++++++++++----------
 lib/spack/spack/package.py       | 353 +++++++++++++++++++++------------------
 lib/spack/spack/test/__init__.py |  70 ++------
 lib/spack/spack/test/modules.py  |  42 +++--
 6 files changed, 391 insertions(+), 344 deletions(-)

(limited to 'lib')

diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index f996f4eb84..cfe59c8d98 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -32,18 +32,21 @@ from llnl.util.filesystem import mkdirp
 from spack.modules import module_types
 from spack.util.string import *
 
-description ="Manipulate modules and dotkits."
+description = "Manipulate modules and dotkits."
 
 
 def setup_parser(subparser):
     sp = subparser.add_subparsers(metavar='SUBCOMMAND', dest='module_command')
 
-    refresh_parser = sp.add_parser('refresh', help='Regenerate all module files.')
+    sp.add_parser('refresh', help='Regenerate all module files.')
 
     find_parser = sp.add_parser('find', help='Find module files for packages.')
-    find_parser.add_argument(
-        'module_type', help="Type of module to find file for. [" + '|'.join(module_types) + "]")
-    find_parser.add_argument('spec', nargs='+', help='spec to find a module file for.')
+    find_parser.add_argument('module_type',
+                             help="Type of module to find file for. [" +
+                             '|'.join(module_types) + "]")
+    find_parser.add_argument('spec',
+                             nargs='+',
+                             help='spec to find a module file for.')
 
 
 def module_find(mtype, spec_array):
@@ -53,7 +56,8 @@ def module_find(mtype, spec_array):
        should type to use that package's module.
     """
     if mtype not in module_types:
-        tty.die("Invalid module type: '%s'.  Options are %s" % (mtype, comma_or(module_types)))
+        tty.die("Invalid module type: '%s'.  Options are %s" %
+                (mtype, comma_or(module_types)))
 
     specs = spack.cmd.parse_specs(spec_array)
     if len(specs) > 1:
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 92ab4e6bea..3fbe2531c1 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -1,7 +1,7 @@
-import os
-import os.path
 import collections
 import inspect
+import os
+import os.path
 
 
 class NameModifier(object):
@@ -26,7 +26,8 @@ class SetEnv(NameValueModifier):
 
 class UnsetEnv(NameModifier):
     def execute(self):
-        os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
+        # Avoid throwing if the variable was not set
+        os.environ.pop(self.name, None)
 
 
 class SetPath(NameValueModifier):
@@ -55,7 +56,9 @@ class RemovePath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
         directories = environment_value.split(':') if environment_value else []
-        directories = [os.path.normpath(x) for x in directories if x != os.path.normpath(self.value)]
+        directories = [os.path.normpath(x)
+                       for x in directories
+                       if x != os.path.normpath(self.value)]
         os.environ[self.name] = ':'.join(directories)
 
 
@@ -63,7 +66,8 @@ class EnvironmentModifications(object):
     """
     Keeps track of requests to modify the current environment.
 
-    Each call to a method to modify the environment stores the extra information on the caller in the request:
+    Each call to a method to modify the environment stores the extra
+    information on the caller in the request:
     - 'filename' : filename of the module where the caller is defined
     - 'lineno': line number where the request occurred
     - 'context' : line of code that issued the request that failed
@@ -71,10 +75,10 @@ class EnvironmentModifications(object):
 
     def __init__(self, other=None):
         """
-        Initializes a new instance, copying commands from other if it is not None
+        Initializes a new instance, copying commands from other if not None
 
         Args:
-            other: another instance of EnvironmentModifications from which (optional)
+            other: another instance of EnvironmentModifications (optional)
         """
         self.env_modifications = []
         if other is not None:
@@ -93,7 +97,8 @@ class EnvironmentModifications(object):
     @staticmethod
     def _check_other(other):
         if not isinstance(other, EnvironmentModifications):
-            raise TypeError('other must be an instance of EnvironmentModifications')
+            raise TypeError(
+                'other must be an instance of EnvironmentModifications')
 
     def _get_outside_caller_attributes(self):
         stack = inspect.stack()
@@ -101,12 +106,10 @@ class EnvironmentModifications(object):
             _, filename, lineno, _, context, index = stack[2]
             context = context[index].strip()
         except Exception:
-            filename, lineno, context = 'unknown file', 'unknown line', 'unknown context'
-        args = {
-            'filename': filename,
-            'lineno': lineno,
-            'context': context
-        }
+            filename = 'unknown file'
+            lineno = 'unknown line'
+            context = 'unknown context'
+        args = {'filename': filename, 'lineno': lineno, 'context': context}
         return args
 
     def set(self, name, value, **kwargs):
@@ -170,7 +173,8 @@ class EnvironmentModifications(object):
 
     def remove_path(self, name, path, **kwargs):
         """
-        Stores in the current object a request to remove a path from a path list
+        Stores in the current object a request to remove a path from a path
+        list
 
         Args:
             name: name of the path list in the environment
@@ -185,7 +189,8 @@ class EnvironmentModifications(object):
         Returns a dict of the modifications grouped by variable name
 
         Returns:
-            dict mapping the environment variable name to the modifications to be done on it
+            dict mapping the environment variable name to the modifications to
+            be done on it
         """
         modifications = collections.defaultdict(list)
         for item in self:
@@ -203,7 +208,7 @@ class EnvironmentModifications(object):
         Applies the modifications and clears the list
         """
         modifications = self.group_by_name()
-        # Apply the modifications to the environment variables one variable at a time
+        # Apply modifications one variable at a time
         for name, actions in sorted(modifications.items()):
             for x in actions:
                 x.execute()
@@ -224,13 +229,19 @@ def concatenate_paths(paths):
 
 def set_or_unset_not_first(variable, changes, errstream):
     """
-    Check if we are going to set or unset something after other modifications have already been requested
+    Check if we are going to set or unset something after other modifications
+    have already been requested
     """
-    indexes = [ii for ii, item in enumerate(changes) if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
+    indexes = [ii
+               for ii, item in enumerate(changes)
+               if ii != 0 and type(item) in [SetEnv, UnsetEnv]]
     if indexes:
         good = '\t    \t{context} at {filename}:{lineno}'
         nogood = '\t--->\t{context} at {filename}:{lineno}'
-        errstream('Suspicious requests to set or unset the variable \'{var}\' found'.format(var=variable))
+        message = 'Suspicious requests to set or unset the variable \'{var}\' found'  # NOQA: ignore=E501
+        errstream(
+            message.format(
+                var=variable))
         for ii, item in enumerate(changes):
             print_format = nogood if ii in indexes else good
             errstream(print_format.format(**item.args))
@@ -238,8 +249,8 @@ def set_or_unset_not_first(variable, changes, errstream):
 
 def validate(env, errstream):
     """
-    Validates the environment modifications to check for the presence of suspicious patterns. Prompts a warning for
-    everything that was found
+    Validates the environment modifications to check for the presence of
+    suspicious patterns. Prompts a warning for everything that was found
 
     Current checks:
     - set or unset variables after other changes on the same variable
@@ -254,7 +265,8 @@ def validate(env, errstream):
 
 def filter_environment_blacklist(env, variables):
     """
-    Generator that filters out any change to environment variables present in the input list
+    Generator that filters out any change to environment variables present in
+    the input list
 
     Args:
         env: list of environment modifications
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index ffed469b20..53f054094a 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -23,36 +23,35 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 """
-This module contains code for creating environment modules, which can include dotkits, tcl modules, lmod, and others.
+This module contains code for creating environment modules, which can include
+dotkits, tcl modules, lmod, and others.
 
-The various types of modules are installed by post-install hooks and removed after an uninstall by post-uninstall hooks.
-This class consolidates the logic for creating an abstract description of the information that module systems need.
-Currently that includes a number of directories to be appended to paths in the user's environment:
+The various types of modules are installed by post-install hooks and removed
+after an uninstall by post-uninstall hooks. This class consolidates the logic
+for creating an abstract description of the information that module systems
+need.
 
-  * /bin directories to be appended to PATH
-  * /lib* directories for LD_LIBRARY_PATH
-  * /include directories for CPATH
-  * /man* and /share/man* directories for MANPATH
-  * the package prefix for CMAKE_PREFIX_PATH
+This module also includes logic for coming up with unique names for the module
+files so that they can be found by the various shell-support files in
+$SPACK/share/spack/setup-env.*.
 
-This module also includes logic for coming up with unique names for the module files so that they can be found by the
-various shell-support files in $SPACK/share/spack/setup-env.*.
-
-Each hook in hooks/ implements the logic for writing its specific type of module file.
+Each hook in hooks/ implements the logic for writing its specific type of
+module file.
 """
 import copy
 import datetime
 import os
 import os.path
 import re
-import textwrap
 import string
+import textwrap
 
 import llnl.util.tty as tty
 import spack
 import spack.config
 from llnl.util.filesystem import join_path, mkdirp
-from spack.build_environment import parent_class_modules, set_module_variables_for_package
+from spack.build_environment import parent_class_modules
+from spack.build_environment import set_module_variables_for_package
 from spack.environment import *
 
 __all__ = ['EnvModule', 'Dotkit', 'TclModule']
@@ -67,30 +66,26 @@ def print_help():
     """
     For use by commands to tell user how to activate shell support.
     """
-    tty.msg("This command requires spack's shell integration.",
-            "",
+    tty.msg("This command requires spack's shell integration.", "",
             "To initialize spack's shell commands, you must run one of",
             "the commands below.  Choose the right command for your shell.",
-            "",
-            "For bash and zsh:",
-            "    . %s/setup-env.sh" % spack.share_path,
-            "",
-            "For csh and tcsh:",
-            "    setenv SPACK_ROOT %s" % spack.prefix,
-            "    source %s/setup-env.csh" % spack.share_path,
-            "")
+            "", "For bash and zsh:",
+            "    . %s/setup-env.sh" % spack.share_path, "",
+            "For csh and tcsh:", "    setenv SPACK_ROOT %s" % spack.prefix,
+            "    source %s/setup-env.csh" % spack.share_path, "")
 
 
 def inspect_path(prefix):
     """
-    Inspects the prefix of an installation to search for common layouts. Issues a request to modify the environment
-    accordingly when an item is found.
+    Inspects the prefix of an installation to search for common layouts. Issues
+    a request to modify the environment accordingly when an item is found.
 
     Args:
         prefix: prefix of the installation
 
     Returns:
-        instance of EnvironmentModifications containing the requested modifications
+        instance of EnvironmentModifications containing the requested
+        modifications
     """
     env = EnvironmentModifications()
     # Inspect the prefix to check for the existence of common directories
@@ -105,18 +100,21 @@ def inspect_path(prefix):
 
 def dependencies(spec, request='all'):
     """
-    Returns the list of dependent specs for a given spec, according to the given request
+    Returns the list of dependent specs for a given spec, according to the
+    given request
 
     Args:
         spec: target spec
         request: either 'none', 'direct' or 'all'
 
     Returns:
-        empty list if 'none', direct dependency list if 'direct', all dependencies if 'all'
+        empty list if 'none', direct dependency list if 'direct', all
+        dependencies if 'all'
     """
     if request not in ('none', 'direct', 'all'):
-        raise tty.error("Wrong value for argument 'request' : should be one of ('none', 'direct', 'all') "
-                        " [current value is '%s']" % request)
+        message = "Wrong value for argument 'request' : "
+        message += "should be one of ('none', 'direct', 'all')"
+        raise tty.error(message + " [current value is '%s']" % request)
 
     if request == 'none':
         return []
@@ -124,12 +122,19 @@ def dependencies(spec, request='all'):
     if request == 'direct':
         return [xx for _, xx in spec.dependencies.items()]
 
-    # FIXME : during module file creation nodes seem to be visited multiple times even if cover='nodes'
-    # FIXME : is given. This work around permits to get a unique list of spec anyhow.
-    # FIXME : Possibly we miss a merge step among nodes that refer to the same package.
+    # FIXME : during module file creation nodes seem to be visited multiple
+    # FIXME : times even if cover='nodes' is given. This work around permits
+    # FIXME : to get a unique list of spec anyhow. Do we miss a merge
+    # FIXME : step among nodes that refer to the same package?
     seen = set()
     seen_add = seen.add
-    l = [xx for xx in sorted(spec.traverse(order='post', depth=True, cover='nodes', root=False), reverse=True)]
+    l = [xx
+         for xx in sorted(
+             spec.traverse(order='post',
+                           depth=True,
+                           cover='nodes',
+                           root=False),
+             reverse=True)]
     return [xx for ii, xx in l if not (xx in seen or seen_add(xx))]
 
 
@@ -146,7 +151,8 @@ def update_dictionary_extending_lists(target, update):
 
 def parse_config_options(module_generator):
     """
-    Parse the configuration file and returns a bunch of items that will be needed during module file generation
+    Parse the configuration file and returns a bunch of items that will be
+    needed during module file generation
 
     Args:
         module_generator: module generator for a given spec
@@ -154,11 +160,14 @@ def parse_config_options(module_generator):
     Returns:
         autoloads: list of specs to be autoloaded
         prerequisites: list of specs to be marked as prerequisite
-        filters: list of environment variables whose modification is blacklisted in module files
-        env: list of custom environment modifications to be applied in the module file
+        filters: list of environment variables whose modification is
+        blacklisted in module files
+        env: list of custom environment modifications to be applied in the
+        module file
     """
     # Get the configuration for this kind of generator
-    module_configuration = copy.deepcopy(CONFIGURATION.get(module_generator.name, {}))
+    module_configuration = copy.deepcopy(CONFIGURATION.get(
+        module_generator.name, {}))
 
     #####
     # Merge all the rules
@@ -179,9 +188,12 @@ def parse_config_options(module_generator):
     #####
 
     # Automatic loading loads
-    module_file_actions['autoload'] = dependencies(module_generator.spec, module_file_actions.get('autoload', 'none'))
+    module_file_actions['autoload'] = dependencies(
+        module_generator.spec, module_file_actions.get('autoload', 'none'))
     # Prerequisites
-    module_file_actions['prerequisites'] = dependencies(module_generator.spec, module_file_actions.get('prerequisites', 'none'))
+    module_file_actions['prerequisites'] = dependencies(
+        module_generator.spec, module_file_actions.get('prerequisites',
+                                                       'none'))
     # Environment modifications
     environment_actions = module_file_actions.pop('environment', {})
     env = EnvironmentModifications()
@@ -189,7 +201,7 @@ def parse_config_options(module_generator):
     def process_arglist(arglist):
         if method == 'unset':
             for x in arglist:
-                yield (x,)
+                yield (x, )
         else:
             for x in arglist.iteritems():
                 yield x
@@ -198,19 +210,13 @@ def parse_config_options(module_generator):
         for args in process_arglist(arglist):
             getattr(env, method)(*args)
 
-        # for item in arglist:
-        #     if method == 'unset':
-        #         args = [item]
-        #     else:
-        #         args = item.split(',')
-        #     getattr(env, method)(*args)
-
     return module_file_actions, env
 
 
 def filter_blacklisted(specs, module_name):
     """
-    Given a sequence of specs, filters the ones that are blacklisted in the module configuration file.
+    Given a sequence of specs, filters the ones that are blacklisted in the
+    module configuration file.
 
     Args:
         specs: sequence of spec instances
@@ -233,7 +239,8 @@ class EnvModule(object):
     class __metaclass__(type):
         def __init__(cls, name, bases, dict):
             type.__init__(cls, name, bases, dict)
-            if cls.name != 'env_module' and cls.name in CONFIGURATION['enable']:
+            if cls.name != 'env_module' and cls.name in CONFIGURATION[
+                    'enable']:
                 module_types[cls.name] = cls
 
     def __init__(self, spec=None):
@@ -249,7 +256,8 @@ class EnvModule(object):
         # long description is the docstring with reduced whitespace.
         self.long_description = None
         if self.spec.package.__doc__:
-            self.long_description = re.sub(r'\s+', ' ', self.spec.package.__doc__)
+            self.long_description = re.sub(r'\s+', ' ',
+                                           self.spec.package.__doc__)
 
     @property
     def naming_scheme(self):
@@ -271,12 +279,14 @@ class EnvModule(object):
     @property
     def use_name(self):
         """
-        Subclasses should implement this to return the name the module command uses to refer to the package.
+        Subclasses should implement this to return the name the module command
+        uses to refer to the package.
         """
         naming_tokens = self.tokens
         naming_scheme = self.naming_scheme
         name = naming_scheme.format(**naming_tokens)
-        name += '-' + self.spec.dag_hash()  # Always append the hash to make the module file unique
+        name += '-' + self.spec.dag_hash(
+        )  # Always append the hash to make the module file unique
         # Not everybody is working on linux...
         parts = name.split('/')
         name = join_path(*parts)
@@ -296,8 +306,12 @@ class EnvModule(object):
     @property
     def blacklisted(self):
         configuration = CONFIGURATION.get(self.name, {})
-        whitelist_matches = [x for x in configuration.get('whitelist', []) if self.spec.satisfies(x)]
-        blacklist_matches = [x for x in configuration.get('blacklist', []) if self.spec.satisfies(x)]
+        whitelist_matches = [x
+                             for x in configuration.get('whitelist', [])
+                             if self.spec.satisfies(x)]
+        blacklist_matches = [x
+                             for x in configuration.get('blacklist', [])
+                             if self.spec.satisfies(x)]
         if whitelist_matches:
             message = '\tWHITELIST : %s [matches : ' % self.spec.cshort_spec
             for rule in whitelist_matches:
@@ -327,7 +341,8 @@ class EnvModule(object):
         """
         if self.blacklisted:
             return
-        tty.debug("\tWRITE : %s [%s]" % (self.spec.cshort_spec, self.file_name))
+        tty.debug("\tWRITE : %s [%s]" %
+                  (self.spec.cshort_spec, self.file_name))
 
         module_dir = os.path.dirname(self.file_name)
         if not os.path.exists(module_dir):
@@ -337,11 +352,12 @@ class EnvModule(object):
         # installation prefix
         env = inspect_path(self.spec.prefix)
 
-        # Let the extendee/dependency modify their extensions/dependencies before asking for
-        # package-specific modifications
+        # Let the extendee/dependency modify their extensions/dependencies
+        # before asking for package-specific modifications
         spack_env = EnvironmentModifications()
-        # TODO : the code down below is quite similar to build_environment.setup_package and needs to be
-        # TODO : factored out to a single place
+        # TODO : the code down below is quite similar to
+        # TODO : build_environment.setup_package and needs to be factored out
+        # TODO : to a single place
         for item in dependencies(self.spec, 'all'):
             package = self.spec[item.name].package
             modules = parent_class_modules(package.__class__)
@@ -358,14 +374,18 @@ class EnvModule(object):
         # Parse configuration file
         module_configuration, conf_env = parse_config_options(self)
         env.extend(conf_env)
-        filters = module_configuration.get('filter', {}).get('environment_blacklist',{})
+        filters = module_configuration.get('filter', {}).get(
+            'environment_blacklist', {})
         # Build up the module file content
         module_file_content = self.header
-        for x in filter_blacklisted(module_configuration.pop('autoload', []), self.name):
+        for x in filter_blacklisted(
+                module_configuration.pop('autoload', []), self.name):
             module_file_content += self.autoload(x)
-        for x in filter_blacklisted(module_configuration.pop('prerequisites', []), self.name):
+        for x in filter_blacklisted(
+                module_configuration.pop('prerequisites', []), self.name):
             module_file_content += self.prerequisite(x)
-        for line in self.process_environment_command(filter_environment_blacklist(env, filters)):
+        for line in self.process_environment_command(
+                filter_environment_blacklist(env, filters)):
             module_file_content += line
         for line in self.module_specific_content(module_configuration):
             module_file_content += line
@@ -392,10 +412,13 @@ class EnvModule(object):
     def process_environment_command(self, env):
         for command in env:
             try:
-                yield self.environment_modifications_formats[type(command)].format(**command.args)
+                yield self.environment_modifications_formats[type(
+                    command)].format(**command.args)
             except KeyError:
-                tty.warn('Cannot handle command of type {command} : skipping request'.format(command=type(command)))
-                tty.warn('{context} at {filename}:{lineno}'.format(**command.args))
+                message = 'Cannot handle command of type {command} : skipping request'  # NOQA: ignore=E501
+                details = '{context} at {filename}:{lineno}'
+                tty.warn(message.format(command=type(command)))
+                tty.warn(details.format(**command.args))
 
     @property
     def file_name(self):
@@ -408,9 +431,12 @@ class EnvModule(object):
         if os.path.exists(mod_file):
             try:
                 os.remove(mod_file)  # Remove the module file
-                os.removedirs(os.path.dirname(mod_file))  # Remove all the empty directories from the leaf up
+                os.removedirs(
+                    os.path.dirname(mod_file)
+                )  # Remove all the empty directories from the leaf up
             except OSError:
-                pass  # removedirs throws OSError on first non-empty directory found
+                # removedirs throws OSError on first non-empty directory found
+                pass
 
 
 class Dotkit(EnvModule):
@@ -424,13 +450,12 @@ class Dotkit(EnvModule):
 
     autoload_format = 'dk_op {module_file}\n'
 
-    prerequisite_format = None  # TODO : does something like prerequisite exist for dotkit?
-
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
 
     @property
     def file_name(self):
-        return join_path(Dotkit.path, self.spec.architecture, '%s.dk' % self.use_name)
+        return join_path(Dotkit.path, self.spec.architecture,
+                         '%s.dk' % self.use_name)
 
     @property
     def header(self):
@@ -474,7 +499,7 @@ class TclModule(EnvModule):
 
     prerequisite_format = 'prereq {module_file}\n'
 
-    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'
+    default_naming_format = '{name}-{version}-{compiler.name}-{compiler.version}'  # NOQA: ignore=E501
 
     @property
     def file_name(self):
@@ -482,9 +507,10 @@ class TclModule(EnvModule):
 
     @property
     def header(self):
+        timestamp = datetime.datetime.now()
         # TCL Modulefile header
         header = '#%Module1.0\n'
-        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % datetime.datetime.now()
+        header += '## Module file created by spack (https://github.com/LLNL/spack) on %s\n' % timestamp  # NOQA: ignore=E501
         header += '##\n'
         header += '## %s\n' % self.spec.short_spec
         header += '##\n'
@@ -509,16 +535,18 @@ class TclModule(EnvModule):
         f = string.Formatter()
         for item in conflict_format:
             line = 'conflict ' + item + '\n'
-            if len([x for x in f.parse(line)]) > 1:  # We do have placeholder to substitute
-                for naming_dir, conflict_dir in zip(self.naming_scheme.split('/'), item.split('/')):
+            if len([x for x in f.parse(line)
+                    ]) > 1:  # We do have placeholder to substitute
+                for naming_dir, conflict_dir in zip(
+                        self.naming_scheme.split('/'), item.split('/')):
                     if naming_dir != conflict_dir:
-                        message = 'conflict scheme does not match naming scheme [{spec}]\n\n'
+                        message = 'conflict scheme does not match naming scheme [{spec}]\n\n'  # NOQA: ignore=E501
                         message += 'naming scheme   : "{nformat}"\n'
                         message += 'conflict scheme : "{cformat}"\n\n'
-                        message += '** You may want to check your `modules.yaml` configuration file **\n'
-                        tty.error(
-                            message.format(spec=self.spec, nformat=self.naming_scheme, cformat=item)
-                        )
+                        message += '** You may want to check your `modules.yaml` configuration file **\n'  # NOQA: ignore=E501
+                        tty.error(message.format(spec=self.spec,
+                                                 nformat=self.naming_scheme,
+                                                 cformat=item))
                         raise SystemExit('Module generation aborted.')
                 line = line.format(**naming_tokens)
             yield line
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 3626a574c8..8e6cf32954 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -37,7 +37,6 @@ import os
 import re
 import textwrap
 import time
-import glob
 
 import llnl.util.tty as tty
 import spack
@@ -62,7 +61,6 @@ from spack.util.environment import dump_environment
 from spack.util.executable import ProcessError
 from spack.version import *
 from urlparse import urlparse
-
 """Allowed URL schemes for spack packages."""
 _ALLOWED_URL_SCHEMES = ["http", "https", "ftp", "file", "git"]
 
@@ -305,26 +303,21 @@ class Package(object):
     #
     """By default we build in parallel.  Subclasses can override this."""
     parallel = True
-
     """# jobs to use for parallel make. If set, overrides default of ncpus."""
     make_jobs = None
-
-    """Most packages are NOT extendable.  Set to True if you want extensions."""
+    """Most packages are NOT extendable. Set to True if you want extensions."""
     extendable = False
-
     """List of prefix-relative file paths (or a single path). If these do
        not exist after install, or if they exist but are not files,
        sanity checks fail.
     """
     sanity_check_is_file = []
-
     """List of prefix-relative directory paths (or a single path). If
        these do not exist after install, or if they exist but are not
        directories, sanity checks will fail.
     """
     sanity_check_is_dir = []
 
-
     def __init__(self, spec):
         # this determines how the package should be built.
         self.spec = spec
@@ -336,23 +329,24 @@ class Package(object):
             self.name = self.name[self.name.rindex('.') + 1:]
 
         # Allow custom staging paths for packages
-        self.path=None
+        self.path = None
 
         # Sanity check attributes required by Spack directives.
         spack.directives.ensure_dicts(type(self))
 
         # Check versions in the versions dict.
         for v in self.versions:
-            assert(isinstance(v, Version))
+            assert (isinstance(v, Version))
 
         # Check version descriptors
         for v in sorted(self.versions):
-            assert(isinstance(self.versions[v], dict))
+            assert (isinstance(self.versions[v], dict))
 
         # Version-ize the keys in versions dict
         try:
-            self.versions = dict((Version(v), h) for v,h in self.versions.items())
-        except ValueError, e:
+            self.versions = dict((Version(v), h)
+                                 for v, h in self.versions.items())
+        except ValueError as e:
             raise ValueError("In package %s: %s" % (self.name, e.message))
 
         # stage used to build this package.
@@ -366,9 +360,9 @@ class Package(object):
         # This makes self.url behave sanely.
         if self.spec.versions.concrete:
             # TODO: this is a really roundabout way of determining the type
-            # TODO: of fetch to do. figure out a more sane fetch strategy/package
-            # TODO: init order (right now it's conflated with stage, package, and
-            # TODO: the tests make assumptions)
+            # TODO: of fetch to do. figure out a more sane fetch
+            # TODO: strategy/package init order (right now it's conflated with
+            # TODO: stage, package, and the tests make assumptions)
             f = fs.for_package_version(self, self.version)
             if isinstance(f, fs.URLFetchStrategy):
                 self.url = self.url_for_version(self.spec.version)
@@ -387,14 +381,12 @@ class Package(object):
         if self.is_extension:
             spack.repo.get(self.extendee_spec)._check_extendable()
 
-
     @property
     def version(self):
         if not self.spec.versions.concrete:
             raise ValueError("Can only get of package with concrete version.")
         return self.spec.versions[0]
 
-
     @memoized
     def version_urls(self):
         """Return a list of URLs for different versions of this
@@ -407,7 +399,6 @@ class Package(object):
                 version_urls[v] = args['url']
         return version_urls
 
-
     def nearest_url(self, version):
         """Finds the URL for the next lowest version with a URL.
            If there is no lower version with a URL, uses the
@@ -424,10 +415,11 @@ class Package(object):
                 url = version_urls[v]
         return url
 
-
     # TODO: move this out of here and into some URL extrapolation module?
     def url_for_version(self, version):
-        """Returns a URL that you can download a new version of this package from."""
+        """
+        Returns a URL that you can download a new version of this package from.
+        """
         if not isinstance(version, Version):
             version = Version(version)
 
@@ -441,14 +433,17 @@ class Package(object):
             return version_urls[version]
 
         # If we have no idea, try to substitute the version.
-        return spack.url.substitute_version(self.nearest_url(version),
-                                            self.url_version(version))
+        return spack.url.substitute_version(
+            self.nearest_url(version), self.url_version(version))
 
     def _make_resource_stage(self, root_stage, fetcher, resource):
         resource_stage_folder = self._resource_stage(resource)
         resource_mirror = join_path(self.name, os.path.basename(fetcher.url))
-        stage = ResourceStage(resource.fetcher, root=root_stage, resource=resource,
-                              name=resource_stage_folder, mirror_path=resource_mirror,
+        stage = ResourceStage(resource.fetcher,
+                              root=root_stage,
+                              resource=resource,
+                              name=resource_stage_folder,
+                              mirror_path=resource_mirror,
                               path=self.path)
         return stage
 
@@ -474,7 +469,8 @@ class Package(object):
             else:
                 # Construct resource stage
                 resource = resources[ii - 1]  # ii == 0 is root!
-                stage = self._make_resource_stage(composite_stage[0], fetcher, resource)
+                stage = self._make_resource_stage(composite_stage[0], fetcher,
+                                                  resource)
             # Append the item to the composite
             composite_stage.append(stage)
 
@@ -492,13 +488,11 @@ class Package(object):
             self._stage = self._make_stage()
         return self._stage
 
-
     @stage.setter
     def stage(self, stage):
         """Allow a stage object to be set to override the default."""
         self._stage = stage
 
-
     def _make_fetcher(self):
         # Construct a composite fetcher that always contains at least
         # one element (the root package). In case there are resources
@@ -515,7 +509,8 @@ class Package(object):
     @property
     def fetcher(self):
         if not self.spec.versions.concrete:
-            raise ValueError("Can only get a fetcher for a package with concrete versions.")
+            raise ValueError(
+                "Can only get a fetcher for a package with concrete versions.")
         if not self._fetcher:
             self._fetcher = self._make_fetcher()
         return self._fetcher
@@ -524,10 +519,11 @@ class Package(object):
     def fetcher(self, f):
         self._fetcher = f
 
-
     @property
     def extendee_spec(self):
-        """Spec of the extendee of this package, or None if it is not an extension."""
+        """
+        Spec of the extendee of this package, or None if it is not an extension
+        """
         if not self.extendees:
             return None
 
@@ -549,10 +545,11 @@ class Package(object):
             spec, kwargs = self.extendees[name]
             return spec
 
-
     @property
     def extendee_args(self):
-        """Spec of the extendee of this package, or None if it is not an extension."""
+        """
+        Spec of the extendee of this package, or None if it is not an extension
+        """
         if not self.extendees:
             return None
 
@@ -560,7 +557,6 @@ class Package(object):
         name = next(iter(self.extendees))
         return self.extendees[name][1]
 
-
     @property
     def is_extension(self):
         # if it is concrete, it's only an extension if it actually
@@ -571,22 +567,20 @@ class Package(object):
             # If not, then it's an extension if it *could* be an extension
             return bool(self.extendees)
 
-
     def extends(self, spec):
-        if not spec.name in self.extendees:
+        if spec.name not in self.extendees:
             return False
         s = self.extendee_spec
         return s and s.satisfies(spec)
 
-
     @property
     def activated(self):
         if not self.is_extension:
-            raise ValueError("is_extension called on package that is not an extension.")
+            raise ValueError(
+                "is_extension called on package that is not an extension.")
         exts = spack.install_layout.extension_map(self.extendee_spec)
         return (self.name in exts) and (exts[self.name] == self.spec)
 
-
     def preorder_traversal(self, visited=None, **kwargs):
         """This does a preorder traversal of the package's dependence DAG."""
         virtual = kwargs.get("virtual", False)
@@ -605,36 +599,35 @@ class Package(object):
             spec = self.dependencies[name]
 
             # currently, we do not descend into virtual dependencies, as this
-            # makes doing a sensible traversal much harder.  We just assume that
-            # ANY of the virtual deps will work, which might not be true (due to
-            # conflicts or unsatisfiable specs).  For now this is ok but we might
-            # want to reinvestigate if we start using a lot of complicated virtual
-            # dependencies
+            # makes doing a sensible traversal much harder.  We just assume
+            # that ANY of the virtual deps will work, which might not be true
+            # (due to conflicts or unsatisfiable specs).  For now this is ok
+            # but we might want to reinvestigate if we start using a lot of
+            # complicated virtual dependencies
             # TODO: reinvestigate this.
             if spec.virtual:
                 if virtual:
                     yield spec
                 continue
 
-            for pkg in spack.repo.get(name).preorder_traversal(visited, **kwargs):
+            for pkg in spack.repo.get(name).preorder_traversal(visited,
+                                                               **kwargs):
                 yield pkg
 
-
     def provides(self, vpkg_name):
-        """True if this package provides a virtual package with the specified name."""
+        """
+        True if this package provides a virtual package with the specified name
+        """
         return any(s.name == vpkg_name for s in self.provided)
 
-
     def virtual_dependencies(self, visited=None):
         for spec in sorted(set(self.preorder_traversal(virtual=True))):
             yield spec
 
-
     @property
     def installed(self):
         return os.path.isdir(self.prefix)
 
-
     @property
     def installed_dependents(self):
         """Return a list of the specs of all installed packages that depend
@@ -651,60 +644,62 @@ class Package(object):
                     dependents.append(spec)
         return dependents
 
-
     @property
     def prefix(self):
         """Get the prefix into which this package should be installed."""
         return self.spec.prefix
 
-
     @property
     def compiler(self):
-        """Get the spack.compiler.Compiler object used to build this package."""
+        """Get the spack.compiler.Compiler object used to build this package"""
         if not self.spec.concrete:
             raise ValueError("Can only get a compiler for a concrete package.")
         return spack.compilers.compiler_for_spec(self.spec.compiler)
 
-
     def url_version(self, version):
-        """Given a version, this returns a string that should be substituted into the
-           package's URL to download that version.
-           By default, this just returns the version string. Subclasses may need to
-           override this, e.g. for boost versions where you need to ensure that there
-           are _'s in the download URL.
         """
-        return str(version)
+        Given a version, this returns a string that should be substituted
+        into the package's URL to download that version.
 
+        By default, this just returns the version string. Subclasses may need
+        to override this, e.g. for boost versions where you need to ensure that
+        there are _'s in the download URL.
+        """
+        return str(version)
 
     def remove_prefix(self):
-        """Removes the prefix for a package along with any empty parent directories."""
+        """
+        Removes the prefix for a package along with any empty parent
+        directories
+        """
         spack.install_layout.remove_install_directory(self.spec)
 
-
     def do_fetch(self, mirror_only=False):
-        """Creates a stage directory and downloads the tarball for this package.
-           Working directory will be set to the stage directory.
+        """
+        Creates a stage directory and downloads the tarball for this package.
+        Working directory will be set to the stage directory.
         """
         if not self.spec.concrete:
             raise ValueError("Can only fetch concrete packages.")
 
         start_time = time.time()
-        if spack.do_checksum and not self.version in self.versions:
-            tty.warn("There is no checksum on file to fetch %s safely."
-                     % self.spec.format('$_$@'))
+        if spack.do_checksum and self.version not in self.versions:
+            tty.warn("There is no checksum on file to fetch %s safely." %
+                     self.spec.format('$_$@'))
 
             # Ask the user whether to skip the checksum if we're
             # interactive, but just fail if non-interactive.
-            checksum_msg = "Add a checksum or use --no-checksum to skip this check."
+            checksum_msg = "Add a checksum or use --no-checksum to skip this check."  # NOQA: ignore=E501
             ignore_checksum = False
             if sys.stdout.isatty():
-                ignore_checksum = tty.get_yes_or_no("  Fetch anyway?", default=False)
+                ignore_checksum = tty.get_yes_or_no("  Fetch anyway?",
+                                                    default=False)
                 if ignore_checksum:
                     tty.msg("Fetching with no checksum.", checksum_msg)
 
             if not ignore_checksum:
-                raise FetchError(
-                    "Will not fetch %s" % self.spec.format('$_$@'), checksum_msg)
+                raise FetchError("Will not fetch %s" %
+                                 self.spec.format('$_$@'), checksum_msg)
 
         self.stage.fetch(mirror_only)
 
@@ -723,7 +718,6 @@ class Package(object):
         self.stage.expand_archive()
         self.stage.chdir_to_source()
 
-
     def do_patch(self):
         """Calls do_stage(), then applied patches to the expanded tarball if they
            haven't been applied already."""
@@ -743,10 +737,10 @@ class Package(object):
 
         # Construct paths to special files in the archive dir used to
         # keep track of whether patches were successfully applied.
-        archive_dir     = self.stage.source_path
-        good_file       = join_path(archive_dir, '.spack_patched')
+        archive_dir = self.stage.source_path
+        good_file = join_path(archive_dir, '.spack_patched')
         no_patches_file = join_path(archive_dir, '.spack_no_patches')
-        bad_file        = join_path(archive_dir, '.spack_patch_failed')
+        bad_file = join_path(archive_dir, '.spack_patch_failed')
 
         # If we encounter an archive that failed to patch, restage it
         # so that we can apply all the patches again.
@@ -801,13 +795,11 @@ class Package(object):
         else:
             touch(no_patches_file)
 
-
     @property
     def namespace(self):
         namespace, dot, module = self.__module__.rpartition('.')
         return namespace
 
-
     def do_fake_install(self):
         """Make a fake install directory contaiing a 'fake' file in bin."""
         mkdirp(self.prefix.bin)
@@ -815,15 +807,15 @@ class Package(object):
         mkdirp(self.prefix.lib)
         mkdirp(self.prefix.man1)
 
-
     def _get_needed_resources(self):
         resources = []
         # Select the resources that are needed for this build
         for when_spec, resource_list in self.resources.items():
             if when_spec in self.spec:
                 resources.extend(resource_list)
-        # Sorts the resources by the length of the string representing their destination. Since any nested resource
-        # must contain another resource's name in its path, it seems that should work
+        # Sorts the resources by the length of the string representing their
+        # destination. Since any nested resource must contain another
+        # resource's name in its path, it seems that should work
         resources = sorted(resources, key=lambda res: len(res.destination))
         return resources
 
@@ -832,10 +824,14 @@ class Package(object):
         resource_stage_folder = '-'.join(pieces)
         return resource_stage_folder
 
-
     def do_install(self,
-                   keep_prefix=False,  keep_stage=False, ignore_deps=False,
-                   skip_patch=False, verbose=False, make_jobs=None, fake=False):
+                   keep_prefix=False,
+                   keep_stage=False,
+                   ignore_deps=False,
+                   skip_patch=False,
+                   verbose=False,
+                   make_jobs=None,
+                   fake=False):
         """Called by commands to install a package and its dependencies.
 
         Package implementations should override install() to describe
@@ -846,18 +842,20 @@ class Package(object):
         keep_stage  -- By default, stage is destroyed only if there are no
                        exceptions during build. Set to True to keep the stage
                        even with exceptions.
-        ignore_deps -- Do not install dependencies before installing this package.
+        ignore_deps -- Don't install dependencies before installing this
+                       package
         fake        -- Don't really build -- install fake stub files instead.
         skip_patch  -- Skip patch stage of build if True.
         verbose     -- Display verbose build output (by default, suppresses it)
-        make_jobs   -- Number of make jobs to use for install.  Default is ncpus.
+        make_jobs   -- Number of make jobs to use for install. Default is ncpus
         """
         if not self.spec.concrete:
             raise ValueError("Can only install concrete packages.")
 
         # No installation needed if package is external
         if self.spec.external:
-            tty.msg("%s is externally installed in %s" % (self.name, self.spec.external))
+            tty.msg("%s is externally installed in %s" %
+                    (self.name, self.spec.external))
             return
 
         # Ensure package is not already installed
@@ -869,9 +867,13 @@ class Package(object):
 
         # First, install dependencies recursively.
         if not ignore_deps:
-            self.do_install_dependencies(
-                keep_prefix=keep_prefix, keep_stage=keep_stage, ignore_deps=ignore_deps,
-                fake=fake, skip_patch=skip_patch, verbose=verbose, make_jobs=make_jobs)
+            self.do_install_dependencies(keep_prefix=keep_prefix,
+                                         keep_stage=keep_stage,
+                                         ignore_deps=ignore_deps,
+                                         fake=fake,
+                                         skip_patch=skip_patch,
+                                         verbose=verbose,
+                                         make_jobs=make_jobs)
 
         # Set parallelism before starting build.
         self.make_jobs = make_jobs
@@ -899,35 +901,41 @@ class Package(object):
                     self.do_fake_install()
                 else:
                     # Do the real install in the source directory.
-                     self.stage.chdir_to_source()
+                    self.stage.chdir_to_source()
 
-                     # Save the build environment in a file before building.
-                     env_path = join_path(os.getcwd(), 'spack-build.env')
+                    # Save the build environment in a file before building.
+                    env_path = join_path(os.getcwd(), 'spack-build.env')
 
-                     try:
-                        # Redirect I/O to a build log (and optionally to the terminal)
+                    try:
+                        # Redirect I/O to a build log (and optionally to
+                        # the terminal)
                         log_path = join_path(os.getcwd(), 'spack-build.out')
                         log_file = open(log_path, 'w')
-                        with log_output(log_file, verbose, sys.stdout.isatty(), True):
+                        with log_output(log_file, verbose, sys.stdout.isatty(),
+                                        True):
                             dump_environment(env_path)
                             self.install(self.spec, self.prefix)
 
-                     except ProcessError as e:
-                         # Annotate ProcessErrors with the location of the build log.
-                         e.build_log = log_path
-                         raise e
+                    except ProcessError as e:
+                        # Annotate ProcessErrors with the location of
+                        # the build log
+                        e.build_log = log_path
+                        raise e
 
-                     # Ensure that something was actually installed.
-                     self.sanity_check_prefix()
+                    # Ensure that something was actually installed.
+                    self.sanity_check_prefix()
 
-                     # Copy provenance into the install directory on success
-                     log_install_path = spack.install_layout.build_log_path(self.spec)
-                     env_install_path = spack.install_layout.build_env_path(self.spec)
-                     packages_dir = spack.install_layout.build_packages_path(self.spec)
+                    # Copy provenance into the install directory on success
+                    log_install_path = spack.install_layout.build_log_path(
+                        self.spec)
+                    env_install_path = spack.install_layout.build_env_path(
+                        self.spec)
+                    packages_dir = spack.install_layout.build_packages_path(
+                        self.spec)
 
-                     install(log_path, log_install_path)
-                     install(env_path, env_install_path)
-                     dump_packages(self.spec, packages_dir)
+                    install(log_path, log_install_path)
+                    install(env_path, env_install_path)
+                    dump_packages(self.spec, packages_dir)
 
                 # Run post install hooks before build stage is removed.
                 spack.hooks.post_install(self)
@@ -937,8 +945,9 @@ class Package(object):
             build_time = self._total_time - self._fetch_time
 
             tty.msg("Successfully installed %s" % self.name,
-                    "Fetch: %s.  Build: %s.  Total: %s."
-                    % (_hms(self._fetch_time), _hms(build_time), _hms(self._total_time)))
+                    "Fetch: %s.  Build: %s.  Total: %s." %
+                    (_hms(self._fetch_time), _hms(build_time),
+                     _hms(self._total_time)))
             print_pkg(self.prefix)
 
         try:
@@ -953,16 +962,17 @@ class Package(object):
                 tty.warn("Keeping install prefix in place despite error.",
                          "Spack will think this package is installed. " +
                          "Manually remove this directory to fix:",
-                         self.prefix, wrap=True)
+                         self.prefix,
+                         wrap=True)
             raise
 
         # note: PARENT of the build process adds the new package to
         # the database, so that we don't need to re-read from file.
         spack.installed_db.add(self.spec, self.prefix)
 
-
     def sanity_check_prefix(self):
         """This function checks whether install succeeded."""
+
         def check_paths(path_list, filetype, predicate):
             if isinstance(path_list, basestring):
                 path_list = [path_list]
@@ -970,8 +980,9 @@ class Package(object):
             for path in path_list:
                 abs_path = os.path.join(self.prefix, path)
                 if not predicate(abs_path):
-                    raise InstallError("Install failed for %s. No such %s in prefix: %s"
-                                       % (self.name, filetype, path))
+                    raise InstallError(
+                        "Install failed for %s. No such %s in prefix: %s" %
+                        (self.name, filetype, path))
 
         check_paths(self.sanity_check_is_file, 'file', os.path.isfile)
         check_paths(self.sanity_check_is_dir, 'directory', os.path.isdir)
@@ -982,13 +993,11 @@ class Package(object):
             raise InstallError(
                 "Install failed for %s.  Nothing was installed!" % self.name)
 
-
     def do_install_dependencies(self, **kwargs):
         # Pass along paths of dependencies here
         for dep in self.spec.dependencies.values():
             dep.package.do_install(**kwargs)
 
-
     @property
     def build_log_path(self):
         if self.installed:
@@ -996,7 +1005,6 @@ class Package(object):
         else:
             return join_path(self.stage.source_path, 'spack-build.out')
 
-
     @property
     def module(self):
         """Use this to add variables to the class's module's scope.
@@ -1037,7 +1045,6 @@ class Package(object):
         """
         pass
 
-
     def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
         """Set up the environment of packages that depend on this one.
 
@@ -1077,7 +1084,6 @@ class Package(object):
         """
         self.setup_environment(spack_env, run_env)
 
-
     def setup_dependent_package(self, module, dependent_spec):
         """Set up Python module-scope variables for dependent packages.
 
@@ -1123,8 +1129,11 @@ class Package(object):
         pass
 
     def install(self, spec, prefix):
-        """Package implementations override this with their own build configuration."""
-        raise InstallError("Package %s provides no install method!" % self.name)
+        """
+        Package implementations override this with their own configuration
+        """
+        raise InstallError("Package %s provides no install method!" %
+                           self.name)
 
     def do_uninstall(self, force=False):
         if not self.installed:
@@ -1146,12 +1155,10 @@ class Package(object):
         # Once everything else is done, run post install hooks
         spack.hooks.post_uninstall(self)
 
-
     def _check_extendable(self):
         if not self.extendable:
             raise ValueError("Package %s is not extendable!" % self.name)
 
-
     def _sanity_check_extension(self):
         if not self.is_extension:
             raise ActivationError("This package is not an extension.")
@@ -1160,12 +1167,13 @@ class Package(object):
         extendee_package._check_extendable()
 
         if not extendee_package.installed:
-            raise ActivationError("Can only (de)activate extensions for installed packages.")
+            raise ActivationError(
+                "Can only (de)activate extensions for installed packages.")
         if not self.installed:
             raise ActivationError("Extensions must first be installed.")
-        if not self.extendee_spec.name in self.extendees:
-            raise ActivationError("%s does not extend %s!" % (self.name, self.extendee.name))
-
+        if self.extendee_spec.name not in self.extendees:
+            raise ActivationError("%s does not extend %s!" %
+                                  (self.name, self.extendee.name))
 
     def do_activate(self, force=False):
         """Called on an etension to invoke the extendee's activate method.
@@ -1175,8 +1183,8 @@ class Package(object):
         """
         self._sanity_check_extension()
 
-        spack.install_layout.check_extension_conflict(
-            self.extendee_spec, self.spec)
+        spack.install_layout.check_extension_conflict(self.extendee_spec,
+                                                      self.spec)
 
         # Activate any package dependencies that are also extensions.
         if not force:
@@ -1188,9 +1196,8 @@ class Package(object):
         self.extendee_spec.package.activate(self, **self.extendee_args)
 
         spack.install_layout.add_extension(self.extendee_spec, self.spec)
-        tty.msg("Activated extension %s for %s"
-                % (self.spec.short_spec, self.extendee_spec.format("$_$@$+$%@")))
-
+        tty.msg("Activated extension %s for %s" %
+                (self.spec.short_spec, self.extendee_spec.format("$_$@$+$%@")))
 
     def activate(self, extension, **kwargs):
         """Symlinks all files from the extension into extendee's install dir.
@@ -1201,6 +1208,7 @@ class Package(object):
         always executed.
 
         """
+
         def ignore(filename):
             return (filename in spack.install_layout.hidden_file_paths or
                     kwargs.get('ignore', lambda f: False)(filename))
@@ -1212,7 +1220,6 @@ class Package(object):
 
         tree.merge(self.prefix, ignore=ignore)
 
-
     def do_deactivate(self, **kwargs):
         """Called on the extension to invoke extendee's deactivate() method."""
         self._sanity_check_extension()
@@ -1230,7 +1237,7 @@ class Package(object):
                 for dep in aspec.traverse():
                     if self.spec == dep:
                         raise ActivationError(
-                            "Cannot deactivate %s beacuse %s is activated and depends on it."
+                            "Cannot deactivate %s because %s is activated and depends on it."  # NOQA: ignore=E501
                             % (self.spec.short_spec, aspec.short_spec))
 
         self.extendee_spec.package.deactivate(self, **self.extendee_args)
@@ -1238,11 +1245,11 @@ class Package(object):
         # redundant activation check -- makes SURE the spec is not
         # still activated even if something was wrong above.
         if self.activated:
-            spack.install_layout.remove_extension(self.extendee_spec, self.spec)
-
-        tty.msg("Deactivated extension %s for %s"
-                % (self.spec.short_spec, self.extendee_spec.format("$_$@$+$%@")))
+            spack.install_layout.remove_extension(self.extendee_spec,
+                                                  self.spec)
 
+        tty.msg("Deactivated extension %s for %s" %
+                (self.spec.short_spec, self.extendee_spec.format("$_$@$+$%@")))
 
     def deactivate(self, extension, **kwargs):
         """Unlinks all files from extension out of this package's install dir.
@@ -1253,6 +1260,7 @@ class Package(object):
         always executed.
 
         """
+
         def ignore(filename):
             return (filename in spack.install_layout.hidden_file_paths or
                     kwargs.get('ignore', lambda f: False)(filename))
@@ -1260,17 +1268,14 @@ class Package(object):
         tree = LinkTree(extension.prefix)
         tree.unmerge(self.prefix, ignore=ignore)
 
-
     def do_restage(self):
         """Reverts expanded/checked out source to a pristine state."""
         self.stage.restage()
 
-
     def do_clean(self):
         """Removes the package's build stage and source tarball."""
         self.stage.destroy()
 
-
     def format_doc(self, **kwargs):
         """Wrap doc string at 72 characters and format nicely"""
         indent = kwargs.get('indent', 0)
@@ -1285,7 +1290,6 @@ class Package(object):
             results.write((" " * indent) + line + "\n")
         return results.getvalue()
 
-
     @property
     def all_urls(self):
         urls = []
@@ -1297,7 +1301,6 @@ class Package(object):
                 urls.append(args['url'])
         return urls
 
-
     def fetch_remote_versions(self):
         """Try to find remote versions of this package using the
            list_url and any other URLs described in the package file."""
@@ -1306,26 +1309,30 @@ class Package(object):
 
         try:
             return spack.util.web.find_versions_of_archive(
-                *self.all_urls, list_url=self.list_url, list_depth=self.list_depth)
+                *self.all_urls,
+                list_url=self.list_url,
+                list_depth=self.list_depth)
         except spack.error.NoNetworkConnectionError as e:
-            tty.die("Package.fetch_versions couldn't connect to:",
-                    e.url, e.message)
-
+            tty.die("Package.fetch_versions couldn't connect to:", e.url,
+                    e.message)
 
     @property
     def rpath(self):
         """Get the rpath this package links with, as a list of paths."""
         rpaths = [self.prefix.lib, self.prefix.lib64]
-        rpaths.extend(d.prefix.lib for d in self.spec.traverse(root=False)
+        rpaths.extend(d.prefix.lib
+                      for d in self.spec.traverse(root=False)
                       if os.path.isdir(d.prefix.lib))
-        rpaths.extend(d.prefix.lib64 for d in self.spec.traverse(root=False)
+        rpaths.extend(d.prefix.lib64
+                      for d in self.spec.traverse(root=False)
                       if os.path.isdir(d.prefix.lib64))
         return rpaths
 
-
     @property
     def rpath_args(self):
-        """Get the rpath args as a string, with -Wl,-rpath, for each element."""
+        """
+        Get the rpath args as a string, with -Wl,-rpath, for each element
+        """
         return " ".join("-Wl,-rpath,%s" % p for p in self.rpath)
 
 
@@ -1333,6 +1340,7 @@ def install_dependency_symlinks(pkg, spec, prefix):
     """Execute a dummy install and flatten dependencies"""
     flatten_dependencies(spec, prefix)
 
+
 def flatten_dependencies(spec, flat_dir):
     """Make each dependency of spec present in dir via symlink."""
     for dep in spec.traverse(root=False):
@@ -1341,13 +1349,13 @@ def flatten_dependencies(spec, flat_dir):
         dep_path = spack.install_layout.path_for_spec(dep)
         dep_files = LinkTree(dep_path)
 
-        os.mkdir(flat_dir+'/'+name)
+        os.mkdir(flat_dir + '/' + name)
 
-        conflict = dep_files.find_conflict(flat_dir+'/'+name)
+        conflict = dep_files.find_conflict(flat_dir + '/' + name)
         if conflict:
             raise DependencyConflictError(conflict)
 
-        dep_files.merge(flat_dir+'/'+name)
+        dep_files.merge(flat_dir + '/' + name)
 
 
 def validate_package_url(url_string):
@@ -1388,9 +1396,11 @@ def dump_packages(spec, path):
             # Create a source repo and get the pkg directory out of it.
             try:
                 source_repo = spack.repository.Repo(source_repo_root)
-                source_pkg_dir = source_repo.dirname_for_package_name(node.name)
-            except RepoError as e:
-                tty.warn("Warning: Couldn't copy in provenance for %s" % node.name)
+                source_pkg_dir = source_repo.dirname_for_package_name(
+                    node.name)
+            except RepoError:
+                tty.warn("Warning: Couldn't copy in provenance for %s" %
+                         node.name)
 
         # Create a destination repository
         dest_repo_root = join_path(path, node.namespace)
@@ -1410,7 +1420,7 @@ def print_pkg(message):
     """Outputs a message with a package icon."""
     from llnl.util.tty.color import cwrite
     cwrite('@*g{[+]} ')
-    print message
+    print(message)
 
 
 def _hms(seconds):
@@ -1419,20 +1429,25 @@ def _hms(seconds):
     h, m = divmod(m, 60)
 
     parts = []
-    if h: parts.append("%dh" % h)
-    if m: parts.append("%dm" % m)
-    if s: parts.append("%.2fs" % s)
+    if h:
+        parts.append("%dh" % h)
+    if m:
+        parts.append("%dm" % m)
+    if s:
+        parts.append("%.2fs" % s)
     return ' '.join(parts)
 
 
 class FetchError(spack.error.SpackError):
     """Raised when something goes wrong during fetch."""
+
     def __init__(self, message, long_msg=None):
         super(FetchError, self).__init__(message, long_msg)
 
 
 class InstallError(spack.error.SpackError):
     """Raised when something goes wrong during install or uninstall."""
+
     def __init__(self, message, long_msg=None):
         super(InstallError, self).__init__(message, long_msg)
 
@@ -1443,21 +1458,24 @@ class ExternalPackageError(InstallError):
 
 class PackageStillNeededError(InstallError):
     """Raised when package is still needed by another on uninstall."""
+
     def __init__(self, spec, dependents):
-        super(PackageStillNeededError, self).__init__(
-            "Cannot uninstall %s" % spec)
+        super(PackageStillNeededError, self).__init__("Cannot uninstall %s" %
+                                                      spec)
         self.spec = spec
         self.dependents = dependents
 
 
 class PackageError(spack.error.SpackError):
     """Raised when something is wrong with a package definition."""
+
     def __init__(self, message, long_msg=None):
         super(PackageError, self).__init__(message, long_msg)
 
 
 class PackageVersionError(PackageError):
     """Raised when a version URL cannot automatically be determined."""
+
     def __init__(self, version):
         super(PackageVersionError, self).__init__(
             "Cannot determine a URL automatically for version %s" % version,
@@ -1466,6 +1484,7 @@ class PackageVersionError(PackageError):
 
 class VersionFetchError(PackageError):
     """Raised when a version URL cannot automatically be determined."""
+
     def __init__(self, cls):
         super(VersionFetchError, self).__init__(
             "Cannot fetch versions for package %s " % cls.__name__ +
@@ -1474,12 +1493,15 @@ class VersionFetchError(PackageError):
 
 class NoURLError(PackageError):
     """Raised when someone tries to build a URL for a package with no URLs."""
+
     def __init__(self, cls):
         super(NoURLError, self).__init__(
             "Package %s has no version with a URL." % cls.__name__)
 
 
-class ExtensionError(PackageError): pass
+class ExtensionError(PackageError):
+
+    pass
 
 
 class ExtensionConflictError(ExtensionError):
@@ -1495,7 +1517,8 @@ class ActivationError(ExtensionError):
 
 class DependencyConflictError(spack.error.SpackError):
     """Raised when the dependencies cannot be flattened as asked for."""
+
     def __init__(self, conflict):
         super(DependencyConflictError, self).__init__(
-            "%s conflicts with another file in the flattened directory." %(
+            "%s conflicts with another file in the flattened directory." % (
                 conflict))
diff --git a/lib/spack/spack/test/__init__.py b/lib/spack/spack/test/__init__.py
index 05f58ab7b1..10eaac1344 100644
--- a/lib/spack/spack/test/__init__.py
+++ b/lib/spack/spack/test/__init__.py
@@ -23,53 +23,23 @@
 # Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 ##############################################################################
 import sys
-import unittest
-import nose
 
-from spack.test.tally_plugin import Tally
-from llnl.util.filesystem import join_path
 import llnl.util.tty as tty
-from llnl.util.tty.colify import colify
-
+import nose
 import spack
-
+from llnl.util.filesystem import join_path
+from llnl.util.tty.colify import colify
+from spack.test.tally_plugin import Tally
 """Names of tests to be included in Spack's test suite"""
-test_names = ['versions',
-              'url_parse',
-              'url_substitution',
-              'packages',
-              'stage',
-              'spec_syntax',
-              'spec_semantics',
-              'spec_dag',
-              'concretize',
-              'multimethod',
-              'install',
-              'package_sanity',
-              'config',
-              'directory_layout',
-              'pattern',
-              'python_version',
-              'git_fetch',
-              'svn_fetch',
-              'hg_fetch',
-              'mirror',
-              'modules',
-              'url_extrapolate',
-              'cc',
-              'link_tree',
-              'spec_yaml',
-              'optional_deps',
-              'make_executable',
-              'configure_guess',
-              'lock',
-              'database',
-              'namespace_trie',
-              'yaml',
-              'sbang',
-              'environment',
-              'cmd.uninstall',
-              'cmd.test_install']
+test_names = ['versions', 'url_parse', 'url_substitution', 'packages', 'stage',
+              'spec_syntax', 'spec_semantics', 'spec_dag', 'concretize',
+              'multimethod', 'install', 'package_sanity', 'config',
+              'directory_layout', 'pattern', 'python_version', 'git_fetch',
+              'svn_fetch', 'hg_fetch', 'mirror', 'modules', 'url_extrapolate',
+              'cc', 'link_tree', 'spec_yaml', 'optional_deps',
+              'make_executable', 'configure_guess', 'lock', 'database',
+              'namespace_trie', 'yaml', 'sbang', 'environment',
+              'cmd.uninstall', 'cmd.test_install']
 
 
 def list_tests():
@@ -80,8 +50,6 @@ def list_tests():
 def run(names, outputDir, verbose=False):
     """Run tests with the supplied names.  Names should be a list.  If
        it's empty, run ALL of Spack's tests."""
-    verbosity = 1 if not verbose else 2
-
     if not names:
         names = test_names
     else:
@@ -95,7 +63,7 @@ def run(names, outputDir, verbose=False):
     tally = Tally()
     for test in names:
         module = 'spack.test.' + test
-        print module
+        print(module)
 
         tty.msg("Running test: %s" % test)
 
@@ -105,15 +73,13 @@ def run(names, outputDir, verbose=False):
             xmlOutputFname = "unittests-{0}.xml".format(test)
             xmlOutputPath = join_path(outputDir, xmlOutputFname)
             runOpts += ["--with-xunit",
-                "--xunit-file={0}".format(xmlOutputPath)]
+                        "--xunit-file={0}".format(xmlOutputPath)]
         argv = [""] + runOpts + [module]
-        result = nose.run(argv=argv, addplugins=[tally])
+        nose.run(argv=argv, addplugins=[tally])
 
     succeeded = not tally.failCount and not tally.errorCount
-    tty.msg("Tests Complete.",
-            "%5d tests run" % tally.numberOfTestsRun,
-            "%5d failures" % tally.failCount,
-            "%5d errors" % tally.errorCount)
+    tty.msg("Tests Complete.", "%5d tests run" % tally.numberOfTestsRun,
+            "%5d failures" % tally.failCount, "%5d errors" % tally.errorCount)
 
     if succeeded:
         tty.info("OK", format='g')
diff --git a/lib/spack/spack/test/modules.py b/lib/spack/spack/test/modules.py
index b8b0d6fc6a..c65d663250 100644
--- a/lib/spack/spack/test/modules.py
+++ b/lib/spack/spack/test/modules.py
@@ -2,14 +2,18 @@ import collections
 from contextlib import contextmanager
 
 import StringIO
+import spack.modules
+from spack.test.mock_packages_test import MockPackagesTest
 
 FILE_REGISTRY = collections.defaultdict(StringIO.StringIO)
 
+
 # Monkey-patch open to write module files to a StringIO instance
 @contextmanager
 def mock_open(filename, mode):
     if not mode == 'w':
-        raise RuntimeError('test.modules : unexpected opening mode for monkey-patched open')
+        raise RuntimeError(
+            'test.modules : unexpected opening mode for monkey-patched open')
 
     FILE_REGISTRY[filename] = StringIO.StringIO()
 
@@ -20,7 +24,6 @@ def mock_open(filename, mode):
         FILE_REGISTRY[filename] = handle.getvalue()
         handle.close()
 
-import spack.modules
 
 configuration_autoload_direct = {
     'enable': ['tcl'],
@@ -47,7 +50,8 @@ configuration_alter_environment = {
             'filter': {'environment_blacklist': ['CMAKE_PREFIX_PATH']}
         },
         '=x86-linux': {
-            'environment': {'set': {'FOO': 'foo'}, 'unset': ['BAR']}
+            'environment': {'set': {'FOO': 'foo'},
+                            'unset': ['BAR']}
         }
     }
 }
@@ -72,15 +76,14 @@ configuration_conflicts = {
     }
 }
 
-from spack.test.mock_packages_test import MockPackagesTest
-
 
 class TclTests(MockPackagesTest):
     def setUp(self):
         super(TclTests, self).setUp()
         self.configuration_obj = spack.modules.CONFIGURATION
         spack.modules.open = mock_open
-        spack.modules.CONFIGURATION = None  # Make sure that a non-mocked configuration will trigger an error
+        # Make sure that a non-mocked configuration will trigger an error
+        spack.modules.CONFIGURATION = None
 
     def tearDown(self):
         del spack.modules.open
@@ -98,7 +101,7 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_autoload_direct
         spec = spack.spec.Spec('mpich@3.0.4=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertTrue('module-whatis "mpich @3.0.4"' in content )
+        self.assertTrue('module-whatis "mpich @3.0.4"' in content)
 
     def test_autoload(self):
         spack.modules.CONFIGURATION = configuration_autoload_direct
@@ -117,14 +120,22 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_alter_environment
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 1)
+        self.assertEqual(
+            len([x
+                 for x in content
+                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(
+            len([x for x in content if 'setenv FOO "foo"' in x]), 1)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 1)
 
         spec = spack.spec.Spec('libdwarf=x64-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
-        self.assertEqual(len([x for x in content if 'setenv FOO "foo"' in x]), 0)
+        self.assertEqual(
+            len([x
+                 for x in content
+                 if x.startswith('prepend-path CMAKE_PREFIX_PATH')]), 0)
+        self.assertEqual(
+            len([x for x in content if 'setenv FOO "foo"' in x]), 0)
         self.assertEqual(len([x for x in content if 'unsetenv BAR' in x]), 0)
 
     def test_blacklist(self):
@@ -138,6 +149,9 @@ class TclTests(MockPackagesTest):
         spack.modules.CONFIGURATION = configuration_conflicts
         spec = spack.spec.Spec('mpileaks=x86-linux')
         content = self.get_modulefile_content(spec)
-        self.assertEqual(len([x for x in content if x.startswith('conflict')]), 2)
-        self.assertEqual(len([x for x in content if x == 'conflict mpileaks']), 1)
-        self.assertEqual(len([x for x in content if x == 'conflict intel/14.0.1']), 1)
+        self.assertEqual(
+            len([x for x in content if x.startswith('conflict')]), 2)
+        self.assertEqual(
+            len([x for x in content if x == 'conflict mpileaks']), 1)
+        self.assertEqual(
+            len([x for x in content if x == 'conflict intel/14.0.1']), 1)
-- 
cgit v1.2.3-70-g09d2