environment : simplified modification of the environment

lib/spack/spack/build_environment.py

@@ -35,7 +35,7 @@
 
 import spack
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, apply_environment_modifications, concatenate_paths
+from spack.environment import EnvironmentModifications, concatenate_paths
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -283,10 +283,12 @@ def setup_package(pkg):
         set_module_variables_for_package(pkg, mod)
 
     # Allow dependencies to set up environment as well.
-    for dep_spec in pkg.spec.traverse(root=False):
+    for dependency_spec in pkg.spec.traverse(root=False):
-        dep_spec.package.module_modifications(pkg.module, dep_spec, pkg.spec)
+        dependency_spec.package.module_modifications(pkg.module, dependency_spec, pkg.spec)
-        env.extend(dep_spec.package.environment_modifications(pkg.spec))
+        env.extend(dependency_spec.package.environment_modifications(pkg.spec))
-    apply_environment_modifications(env)
+    # TODO : implement validation
+    #validate(env)
+    env.apply_modifications()
 
 
 def fork(pkg, function):

lib/spack/spack/cmd/module.py

@@ -80,7 +80,7 @@ def module_find(mtype, spec_array):
     if not os.path.isfile(mod.file_name):
         tty.die("No %s module is installed for %s" % (mtype, spec))
 
-    print mod.use_name
+    print(mod.use_name)
 
 
 def module_refresh():

lib/spack/spack/environment.py

@@ -158,41 +158,43 @@ def remove_path(self, name, path, **kwargs):
         item = RemovePath(name, path, **kwargs)
         self.env_modifications.append(item)
 
+    def group_by_name(self):
+        """
+        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
+        """
+        modifications = collections.defaultdict(list)
+        for item in self:
+            modifications[item.name].append(item)
+        return modifications
+
+    def clear(self):
+        """
+        Clears the current list of modifications
+        """
+        self.env_modifications.clear()
+
+    def apply_modifications(self):
+        """
+        Applies the modifications and clears the list
+        """
+        modifications = self.group_by_name()
+        # 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()
+
 
 def concatenate_paths(paths):
     """
-    Concatenates an iterable of paths into a column separated string
+    Concatenates an iterable of paths into a  string of column separated paths
 
     Args:
         paths: iterable of paths
 
     Returns:
-        column separated string
+        string
     """
     return ':'.join(str(item) for item in paths)
-
-
-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
-
-
-def apply_environment_modifications(env):
-    """
-    Modifies the current environment according to the request in env
-
-    Args:
-        env: object storing modifications to the environment
-    """
-    modifications = validate_environment_modifications(env)
-
-    # Cycle over the environment variables that will be modified
-    for variable, actions in modifications.items():
-        # Execute all the actions in the order they were issued
-        for x in actions:
-            x.execute()

lib/spack/spack/test/environment.py

@@ -1,6 +1,6 @@
 import unittest
 import os
-from spack.environment import EnvironmentModifications, apply_environment_modifications
+from spack.environment import EnvironmentModifications
 
 
 class EnvironmentTest(unittest.TestCase):
@@ -15,7 +15,7 @@ def test_set_env(self):
         env = EnvironmentModifications()
         env.set_env('A', 'dummy value')
         env.set_env('B', 3)
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
         self.assertEqual(str(3), os.environ['B'])
 
@@ -23,7 +23,7 @@ def test_unset_env(self):
         env = EnvironmentModifications()
         self.assertEqual('foo', os.environ['UNSET_ME'])
         env.unset_env('UNSET_ME')
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertRaises(KeyError, os.environ.__getitem__, 'UNSET_ME')
 
     def test_path_manipulation(self):
@@ -43,7 +43,7 @@ def test_path_manipulation(self):
         env.remove_path('REMOVE_PATH_LIST', '/remove/this')
         env.remove_path('REMOVE_PATH_LIST', '/duplicate/')
 
-        apply_environment_modifications(env)
+        env.apply_modifications()
         self.assertEqual('/path/first:/path/second:/path/third:/path/last', os.environ['PATH_LIST'])
         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'])
@@ -52,7 +52,9 @@ def test_path_manipulation(self):
     def test_extra_arguments(self):
         env = EnvironmentModifications()
         env.set_env('A', 'dummy value', who='Pkg1')
-        apply_environment_modifications(env)
+        for x in env:
+            assert hasattr(x, 'who')
+        env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
 
     def test_extend(self):

var/spack/repos/builtin/packages/python/package.py

@@ -98,6 +98,7 @@ def environment_modifications(self, extension_spec):
             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.