environment : refactoreded set_compiler_environment_variables

4 files changed, 47 insertions, 18 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 392ba7ea4d..e91a8a1997 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -35,6 +35,7 @@ import sys
 
 import spack
 from llnl.util.filesystem import *
+from spack.environment import EnvironmentModifications, apply_environment_modifications
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -83,30 +84,32 @@ class MakeExecutable(Executable):
 
 
 def set_compiler_environment_variables(pkg):
-    assert(pkg.spec.concrete)
-    compiler = pkg.compiler
-
+    assert pkg.spec.concrete
     # Set compiler variables used by CMake and autotools
-    assert all(key in pkg.compiler.link_paths
-               for key in ('cc', 'cxx', 'f77', 'fc'))
+    assert all(key in pkg.compiler.link_paths for key in ('cc', 'cxx', 'f77', 'fc'))
 
+    # Populate an object with the list of environment modifications
+    # and return it
+    # TODO : add additional kwargs for better diagnostics, like requestor, ttyout, ttyerr, etc.
+    env = EnvironmentModifications()
     link_dir = spack.build_env_path
-    os.environ['CC']  = join_path(link_dir, pkg.compiler.link_paths['cc'])
-    os.environ['CXX'] = join_path(link_dir, pkg.compiler.link_paths['cxx'])
-    os.environ['F77'] = join_path(link_dir, pkg.compiler.link_paths['f77'])
-    os.environ['FC']  = join_path(link_dir, pkg.compiler.link_paths['fc'])
-
+    env.set_env('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
+    env.set_env('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
+    env.set_env('F77', join_path(link_dir, pkg.compiler.link_paths['f77']))
+    env.set_env('FC', join_path(link_dir, pkg.compiler.link_paths['fc']))
     # Set SPACK compiler variables so that our wrapper knows what to call
+    compiler = pkg.compiler
     if compiler.cc:
-        os.environ['SPACK_CC']  = compiler.cc
+        env.set_env('SPACK_CC', compiler.cc)
     if compiler.cxx:
-        os.environ['SPACK_CXX'] = compiler.cxx
+        env.set_env('SPACK_CXX', compiler.cxx)
     if compiler.f77:
-        os.environ['SPACK_F77'] = compiler.f77
+        env.set_env('SPACK_F77', compiler.f77)
     if compiler.fc:
-        os.environ['SPACK_FC']  = compiler.fc
+        env.set_env('SPACK_FC', compiler.fc)
 
-    os.environ['SPACK_COMPILER_SPEC'] = str(pkg.spec.compiler)
+    env.set_env('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
+    return env
 
 
 def set_build_environment_variables(pkg):
@@ -264,9 +267,10 @@ def parent_class_modules(cls):
 
 def setup_package(pkg):
     """Execute all environment setup routines."""
-    set_compiler_environment_variables(pkg)
+    env = EnvironmentModifications()
+    env.extend(set_compiler_environment_variables(pkg))
+    apply_environment_modifications(env)
     set_build_environment_variables(pkg)
-
     # If a user makes their own package repo, e.g.
     # spack.repos.mystuff.libelf.Libelf, and they inherit from
     # an existing class like spack.repos.original.libelf.Libelf,
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 7d3d7af0de..3b73f1c7a2 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -73,12 +73,24 @@ class EnvironmentModifications(object):
     Keeps track of requests to modify the current environment
     """
 
-    def __init__(self):
+    def __init__(self, other=None):
         self.env_modifications = []
+        if other is not None:
+            self._check_other(other)
+            self.env_modifications.extend(other.env_modifications)
 
     def __iter__(self):
         return iter(self.env_modifications)
 
+    def extend(self, other):
+        self._check_other(other)
+        self.env_modifications.extend(other.env_modifications)
+
+    @staticmethod
+    def _check_other(other):
+        if not isinstance(other, EnvironmentModifications):
+            raise TypeError('other must be an instance of EnvironmentModifications')
+
     def set_env(self, name, value, **kwargs):
         """
         Stores in the current object a request to set an environment variable
@@ -138,6 +150,9 @@ def validate_environment_modifications(env):
     modifications = collections.defaultdict(list)
     for item in env:
         modifications[item.name].append(item)
+    # TODO : once we organized the modifications into a dictionary that maps an environment variable
+    # TODO : to a list of action to be done on it, we may easily spot inconsistencies and warn the user if
+    # TODO : something suspicious is happening
     return modifications
 
 
diff --git a/lib/spack/spack/test/database.py b/lib/spack/spack/test/database.py
index 9a57e1f03e..ce6e8a0552 100644
--- a/lib/spack/spack/test/database.py
+++ b/lib/spack/spack/test/database.py
@@ -26,6 +26,7 @@
 These tests check the database is functioning properly,
 both in memory and in its file
 """
+import os.path
 import multiprocessing
 import shutil
 import tempfile
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index dff3863d32..17061c8fd0 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -48,3 +48,12 @@ class EnvironmentTest(unittest.TestCase):
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['EMPTY_PATH_LIST'])
         self.assertEqual('/path/first:/path/middle:/path/last', os.environ['NEWLY_CREATED_PATH_LIST'])
         self.assertEqual('/a/b:/a/c:/a/d:/f/g', os.environ['REMOVE_PATH_LIST'])
+
+    def test_extra_arguments(self):
+        env = EnvironmentModifications()
+        env.set_env('A', 'dummy value', who='Pkg1')
+        apply_environment_modifications(env)
+        self.assertEqual('dummy value', os.environ['A'])
+
+    def test_copy(self):
+        pass