package : split `environment_modifications` into `setup_environment` and `setup_dependent_environment`.

package : renamed `module_modifications` to `modify_module` for consistency

9 files changed, 52 insertions, 63 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index e5d256a2e0..68477145fe 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -284,8 +284,8 @@ def setup_package(pkg):
 
     # Allow dependencies to set up environment as well.
     for dependency_spec in pkg.spec.traverse(root=False):
-        dependency_spec.package.module_modifications(pkg.module, dependency_spec, pkg.spec)
-        env.extend(dependency_spec.package.environment_modifications(pkg.spec))
+        dependency_spec.package.modify_module(pkg.module, dependency_spec, pkg.spec)
+        dependency_spec.package.setup_dependent_environment(env, pkg.spec)
     # TODO : implement validation
     #validate(env)
     env.apply_modifications()
diff --git a/lib/spack/spack/modules.py b/lib/spack/spack/modules.py
index 7e395736e4..09895d8c44 100644
--- a/lib/spack/spack/modules.py
+++ b/lib/spack/spack/modules.py
@@ -162,9 +162,8 @@ class EnvModule(object):
         # Environment modifications guessed by inspecting the installation prefix
         env = inspect_path(self.spec.prefix)
         # Package-specific environment modifications
-        # FIXME : decide how to distinguish between calls done in the installation and elsewhere
-        env.extend(self.spec.package.environment_modifications(None))
-        # site_specific = ...`
+        self.spec.package.setup_environment(env)
+        # TODO : implement site-specific modifications and filters
         if not env:
             return
 
diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py
index 042833964a..8e56dd1f97 100644
--- a/lib/spack/spack/package.py
+++ b/lib/spack/spack/package.py
@@ -977,7 +977,7 @@ class Package(object):
                           fromlist=[self.__class__.__name__])
 
 
-    def environment_modifications(self, dependent_spec):
+    def setup_environment(self, env):
         """
         Called before the install() method of dependents.
 
@@ -998,9 +998,12 @@ class Package(object):
         Returns:
             instance of environment modifications
         """
-        return EnvironmentModifications()
+        pass
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        self.setup_environment(env)
 
-    def module_modifications(self, module, spec, dependent_spec):
+    def modify_module(self, module, spec, dependent_spec):
         """
         Called before the install() method of dependents.
 
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 4c34d0308f..5af9b585ea 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -47,30 +47,21 @@ class Mpich(Package):
     provides('mpi@:3.0', when='@3:')
     provides('mpi@:1.3', when='@1:')
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Mpich, self).environment_modifications(dependent_spec)
-
-        if dependent_spec is None:
-            # We are not using compiler wrappers
-            cc = self.compiler.cc
-            cxx = self.compiler.cxx
-            f77 = self.compiler.f77
-            f90 = fc = self.compiler.fc
-        else:
-            # Spack compiler wrappers
-            cc = os.environ['CC']
-            cxx = os.environ['CXX']
-            f77 = os.environ['F77']
-            f90 = fc = os.environ['FC']
-
-        env.set_env('MPICH_CC', cc)
-        env.set_env('MPICH_CXX', cxx)
-        env.set_env('MPICH_F77', f77)
-        env.set_env('MPICH_F90', f90)
-        env.set_env('MPICH_FC', fc)
-        return env
-
-    def module_modifications(self, module, spec, dep_spec):
+    def setup_environment(self, env):
+        env.set_env('MPICH_CC', self.compiler.cc)
+        env.set_env('MPICH_CXX', self.compiler.cxx)
+        env.set_env('MPICH_F77', self.compiler.f77)
+        env.set_env('MPICH_F90', self.compiler.fc)
+        env.set_env('MPICH_FC', self.compiler.fc)
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        env.set_env('MPICH_CC', spack_cc)
+        env.set_env('MPICH_CXX', spack_cxx)
+        env.set_env('MPICH_F77', spack_f77)
+        env.set_env('MPICH_F90', spack_f90)
+        env.set_env('MPICH_FC', spack_fc)
+
+    def modify_module(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
         # FIXME : is this necessary ? Shouldn't this be part of a contract with MPI providers?
         module.mpicc = join_path(self.prefix.bin, 'mpicc')
diff --git a/var/spack/repos/builtin/packages/netlib-scalapack/package.py b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
index ecdea46442..7e7e5b2e2e 100644
--- a/var/spack/repos/builtin/packages/netlib-scalapack/package.py
+++ b/var/spack/repos/builtin/packages/netlib-scalapack/package.py
@@ -40,7 +40,7 @@ class NetlibScalapack(Package):
             make()
             make("install")
 
-    def module_modifications(self, module, spec, dependent_spec):
+    def modify_module(self, module, spec, dependent_spec):
         # TODO treat OS that are not Linux...
         lib_suffix = '.so' if '+shared' in spec['scalapack'] else '.a'
 
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index 3a14170457..7783ca8766 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -41,14 +41,17 @@ class Openmpi(Package):
     def url_for_version(self, version):
         return "http://www.open-mpi.org/software/ompi/v%s/downloads/openmpi-%s.tar.bz2" % (version.up_to(2), version)
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Openmpi, self).environment_modifications(dependent_spec)
-        # FIXME : the compilers should point to the current wrappers, not to generic cc etc.
-        env.set_env('OMPI_CC', 'cc')
-        env.set_env('OMPI_CXX', 'c++')
-        env.set_env('OMPI_FC', 'f90')
-        env.set_env('OMPI_F77', 'f77')
-        return env
+    def setup_environment(self, env):
+        env.set_env('OMPI_CC', self.compiler.cc)
+        env.set_env('OMPI_CXX', self.compiler.cxx)
+        env.set_env('OMPI_FC', self.compiler.fc)
+        env.set_env('OMPI_F77', self.compiler.f77)
+
+    def setup_dependent_environment(self, env, dependent_spec):
+        env.set_env('OMPI_CC', spack_cc)
+        env.set_env('OMPI_CXX', spack_cxx)
+        env.set_env('OMPI_FC', spack_fc)
+        env.set_env('OMPI_F77', spack_f77)
 
     def install(self, spec, prefix):
         config_args = ["--prefix=%s" % prefix,
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index acb3651726..d47c1d1b2f 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -89,24 +89,21 @@ class Python(Package):
     def site_packages_dir(self):
         return os.path.join(self.python_lib_dir, 'site-packages')
 
-    def environment_modifications(self, extension_spec):
-        env = super(Python, self).environment_modifications(extension_spec)
-        if extension_spec is not None:
-            # Set PYTHONPATH to include site-packages dir for the
-            # extension and any other python extensions it depends on.
-            python_paths = []
-            for d in extension_spec.traverse():
-                if d.package.extends(self.spec):
-                    python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
-            env.set_env['PYTHONPATH'] = ':'.join(python_paths)
-        return env
-
-    def module_modifications(self, module, spec, ext_spec):
-        """Called before python modules' install() methods.
+    def setup_dependent_environment(self, env, extension_spec):
+        # Set PYTHONPATH to include site-packages dir for the extension and any other python extensions it depends on.
+        python_paths = []
+        for d in extension_spec.traverse():
+            if d.package.extends(self.spec):
+                python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
+        env.set_env['PYTHONPATH'] = ':'.join(python_paths)
+
+    def modify_module(self, module, spec, ext_spec):
+        """
+        Called before python modules' install() methods.
 
         In most cases, extensions will only need to have one line::
 
-            python('setup.py', 'install', '--prefix=%s' % prefix)
+        python('setup.py', 'install', '--prefix=%s' % prefix)
         """
         # Python extension builds can have a global python executable function
         if self.version >= Version("3.0.0") and self.version < Version("4.0.0"):
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 0adf352be2..35b9d68462 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -55,10 +55,8 @@ class Qt(Package):
     depends_on("mesa", when='@4:+mesa')
     depends_on("libxcb")
 
-    def environment_modifications(self, dependent_spec):
-        env = super(Qt, self).environment_modifications(dependent_spec)
+    def setup_environment(self, env):
         env.set_env['QTDIR'] = self.prefix
-        return env
 
     def patch(self):
         if self.spec.satisfies('@4'):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 9caea30ef4..2d1da8c9af 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -17,8 +17,7 @@ class Ruby(Package):
         make()
         make("install")
 
-    def environment_modifications(self, extension_spec):
-        env = super(Ruby, self).environment_modifications(extension_spec)
+    def setup_dependent_environment(self, env, extension_spec):
         # Set GEM_PATH to include dependent gem directories
         ruby_paths = []
         for d in extension_spec.traverse():
@@ -27,9 +26,8 @@ class Ruby(Package):
         env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
         # The actual installation path for this gem
         env.set_env('GEM_HOME', extension_spec.prefix)
-        return env
 
-    def module_modifications(self, module, spec, ext_spec):
+    def modify_module(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME
         and GEM_PATH to values appropriate for the package being built.