environment : added test, modified docs

lib/spack/spack/environment.py

@@ -3,33 +3,40 @@
 import collections
 
 
-class SetEnv(object):
+class AttributeHolder(object):
-    def __init__(self, name, value, **kwargs):
+    """
-        self.name = name
+    Policy that permits to store any kind of attribute on self. The attributes must be passed as key/value pairs
-        self.value = value
+    during the initialization of the instance.
+    """
+    def __init__(self, **kwargs):
         for key, value in kwargs.items():
             setattr(self, key, value)
 
+
+class SetEnv(AttributeHolder):
+    def __init__(self, name, value, **kwargs):
+        super(SetEnv, self).__init__(**kwargs)
+        self.name = name
+        self.value = value
+
     def execute(self):
         os.environ[self.name] = str(self.value)
 
 
-class UnsetEnv(object):
+class UnsetEnv(AttributeHolder):
     def __init__(self, name, **kwargs):
+        super(UnsetEnv, self).__init__(**kwargs)
         self.name = name
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
-class AppendPath(object):
+class AppendPath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(AppendPath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -39,12 +46,11 @@ def execute(self):
         os.environ[self.name] = ':'.join(directories)
 
 
-class PrependPath(object):
+class PrependPath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(PrependPath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -54,12 +60,11 @@ def execute(self):
         os.environ[self.name] = ':'.join(directories)
 
 
-class RemovePath(object):
+class RemovePath(AttributeHolder):
     def __init__(self, name, path, **kwargs):
+        super(RemovePath, self).__init__(**kwargs)
         self.name = name
         self.path = path
-        for key, value in kwargs.items():
-            setattr(self, key, value)
 
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -70,18 +75,26 @@ def execute(self):
 
 class EnvironmentModifications(object):
     """
-    Keeps track of requests to modify the current environment
+    Keeps track of requests to modify the current environment.
     """
 
     def __init__(self, other=None):
+        """
+        Initializes a new instance, copying commands from other if it is not None
+
+        Args:
+            other: another instance of EnvironmentModifications from which (optional)
+        """
         self.env_modifications = []
         if other is not None:
-            self._check_other(other)
+            self.extend(other)
-            self.env_modifications.extend(other.env_modifications)
 
     def __iter__(self):
         return iter(self.env_modifications)
 
+    def __len__(self):
+        return len(self.env_modifications)
+
     def extend(self, other):
         self._check_other(other)
         self.env_modifications.extend(other.env_modifications)
@@ -147,8 +160,18 @@ def remove_path(self, name, path, **kwargs):
 
 
 def concatenate_paths(paths):
+    """
+    Concatenates an iterable of paths into a column separated string
+
+    Args:
+        paths: iterable of paths
+
+    Returns:
+        column separated string
+    """
     return ':'.join(str(item) for item in paths)
 
+
 def validate_environment_modifications(env):
     modifications = collections.defaultdict(list)
     for item in env:

lib/spack/spack/package.py

@@ -985,30 +985,41 @@ def module(self):
 
 
     def environment_modifications(self, dependent_spec):
+        """
+        Called before the install() method of dependents.
+
+        Return the list of environment modifications needed by dependents (or extensions). Default implementation does
+        nothing, but this can be overridden by an extendable package to set up the install environment for its
+        extensions. This is useful if there are some common steps to installing all extensions for a certain package.
+
+        Example :
+
+        1. Installing python modules generally requires `PYTHONPATH` to point to the lib/pythonX.Y/site-packages
+        directory in the module's install prefix.  This could set that variable.
+
+        2. A lot of Qt extensions need `QTDIR` set.  This can be used to do that.
+
+        Args:
+            dependent_spec: dependent (or extension) of this spec
+
+        Returns:
+            instance of environment modifications
+        """
         return EnvironmentModifications()
 
     def module_modifications(self, module, spec, dependent_spec):
-        """Called before the install() method of dependents.
+        """
+        Called before the install() method of dependents.
 
-        Default implementation does nothing, but this can be
+        Default implementation does nothing, but this can be overridden by an extendable package to set up the module of
-        overridden by an extendable package to set up the install
+        its extensions. This is useful if there are some common steps to installing all extensions for a
-        environment for its extensions.  This is useful if there are
-        some common steps to installing all extensions for a
         certain package.
 
-        Some examples:
+        Example :
-
-        1. Installing python modules generally requires PYTHONPATH to
-           point to the lib/pythonX.Y/site-packages directory in the
-           module's install prefix.  This could set that variable.
-
-        2. Extensions often need to invoke the 'python' interpreter
-           from the Python installation being extended.  This routine can
-           put a 'python' Execuable object in the module scope for the
-           extension package to simplify extension installs.
-
-        3. A lot of Qt extensions need QTDIR set.  This can be used to do that.
 
+        1. Extensions often need to invoke the 'python' interpreter from the Python installation being extended.
+        This routine can put a 'python' Executable object in the module scope for the extension package to simplify
+        extension installs.
         """
         pass
 

lib/spack/spack/test/environment.py

@@ -55,5 +55,11 @@ def test_extra_arguments(self):
         apply_environment_modifications(env)
         self.assertEqual('dummy value', os.environ['A'])
 
-    def test_copy(self):
+    def test_extend(self):
-        pass
+        env = EnvironmentModifications()
+        env.set_env('A', 'dummy value')
+        env.set_env('B', 3)
+        copy_construct = EnvironmentModifications(env)
+        self.assertEqual(len(copy_construct), 2)
+        for x, y in zip(env, copy_construct):
+            self.assertIs(x, y)

lib/spack/spack/util/environment.py

@@ -40,11 +40,13 @@ def env_flag(name):
     return False
 
 
+# FIXME : remove this function ?
 def path_set(var_name, directories):
     path_str = ":".join(str(dir) for dir in directories)
     os.environ[var_name] = path_str
 
 
+# FIXME : remove this function ?
 def path_put_first(var_name, directories):
     """Puts the provided directories first in the path, adding them
        if they're not already there.
@@ -59,12 +61,6 @@ def path_put_first(var_name, directories):
     path_set(var_name, new_path)
 
 
-def pop_keys(dictionary, *keys):
-    for key in keys:
-        if key in dictionary:
-            dictionary.pop(key)
-
-
 def dump_environment(path):
     """Dump the current environment out to a file."""
     with open(path, 'w') as env_file: