Rename some environment methods to be less repetitive, add set_path.

8 files changed, 73 insertions, 47 deletions

diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 5688d47e2d..fc5b7d6207 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -36,7 +36,7 @@ import sys
 import spack
 import llnl.util.tty as tty
 from llnl.util.filesystem import *
-from spack.environment import EnvironmentModifications, concatenate_paths, validate
+from spack.environment import EnvironmentModifications, validate
 from spack.util.environment import *
 from spack.util.executable import Executable, which
 
@@ -93,22 +93,23 @@ def set_compiler_environment_variables(pkg, env):
     # and return it
     # TODO : add additional kwargs for better diagnostics, like requestor, ttyout, ttyerr, etc.
     link_dir = spack.build_env_path
-    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']))
+    env.set('CC', join_path(link_dir, pkg.compiler.link_paths['cc']))
+    env.set('CXX', join_path(link_dir, pkg.compiler.link_paths['cxx']))
+    env.set('F77', join_path(link_dir, pkg.compiler.link_paths['f77']))
+    env.set('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:
-        env.set_env('SPACK_CC', compiler.cc)
+        env.set('SPACK_CC', compiler.cc)
     if compiler.cxx:
-        env.set_env('SPACK_CXX', compiler.cxx)
+        env.set('SPACK_CXX', compiler.cxx)
     if compiler.f77:
-        env.set_env('SPACK_F77', compiler.f77)
+        env.set('SPACK_F77', compiler.f77)
     if compiler.fc:
-        env.set_env('SPACK_FC', compiler.fc)
+        env.set('SPACK_FC', compiler.fc)
 
-    env.set_env('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
+    env.set('SPACK_COMPILER_SPEC', str(pkg.spec.compiler))
     return env
 
 
@@ -135,25 +136,25 @@ def set_build_environment_variables(pkg, env):
 
     for item in reversed(env_paths):
         env.prepend_path('PATH', item)
-    env.set_env(SPACK_ENV_PATH, concatenate_paths(env_paths))
+    env.set_path(SPACK_ENV_PATH, env_paths)
 
     # Prefixes of all of the package's dependencies go in SPACK_DEPENDENCIES
     dep_prefixes = [d.prefix for d in pkg.spec.traverse(root=False)]
-    env.set_env(SPACK_DEPENDENCIES, concatenate_paths(dep_prefixes))
-    env.set_env('CMAKE_PREFIX_PATH', concatenate_paths(dep_prefixes))  # Add dependencies to CMAKE_PREFIX_PATH
+    env.set_path(SPACK_DEPENDENCIES, dep_prefixes)
+    env.set_path('CMAKE_PREFIX_PATH', dep_prefixes)  # Add dependencies to CMAKE_PREFIX_PATH
 
     # Install prefix
-    env.set_env(SPACK_PREFIX, pkg.prefix)
+    env.set(SPACK_PREFIX, pkg.prefix)
 
     # Install root prefix
-    env.set_env(SPACK_INSTALL, spack.install_path)
+    env.set(SPACK_INSTALL, spack.install_path)
 
     # Remove these vars from the environment during build because they
     # can affect how some packages find libraries.  We want to make
     # sure that builds never pull in unintended external dependencies.
-    env.unset_env('LD_LIBRARY_PATH')
-    env.unset_env('LD_RUN_PATH')
-    env.unset_env('DYLD_LIBRARY_PATH')
+    env.unset('LD_LIBRARY_PATH')
+    env.unset('LD_RUN_PATH')
+    env.unset('DYLD_LIBRARY_PATH')
 
     # Add bin directories from dependencies to the PATH for the build.
     bin_dirs = reversed(filter(os.path.isdir, ['%s/bin' % prefix for prefix in dep_prefixes]))
@@ -162,9 +163,9 @@ def set_build_environment_variables(pkg, env):
 
     # Working directory for the spack command itself, for debug logs.
     if spack.debug:
-        env.set_env(SPACK_DEBUG, 'TRUE')
-    env.set_env(SPACK_SHORT_SPEC, pkg.spec.short_spec)
-    env.set_env(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir)
+        env.set(SPACK_DEBUG, 'TRUE')
+    env.set(SPACK_SHORT_SPEC, pkg.spec.short_spec)
+    env.set(SPACK_DEBUG_LOG_DIR, spack.spack_working_dir)
 
     # Add any pkgconfig directories to PKG_CONFIG_PATH
     pkg_config_dirs = []
@@ -173,7 +174,7 @@ def set_build_environment_variables(pkg, env):
             pcdir = join_path(p, libdir, 'pkgconfig')
             if os.path.isdir(pcdir):
                 pkg_config_dirs.append(pcdir)
-    env.set_env('PKG_CONFIG_PATH', concatenate_paths(pkg_config_dirs))
+    env.set_path('PKG_CONFIG_PATH', pkg_config_dirs)
 
     return env
 
diff --git a/lib/spack/spack/environment.py b/lib/spack/spack/environment.py
index 74aef57fe8..72aafa4e2d 100644
--- a/lib/spack/spack/environment.py
+++ b/lib/spack/spack/environment.py
@@ -29,6 +29,12 @@ class UnsetEnv(NameModifier):
         os.environ.pop(self.name, None)  # Avoid throwing if the variable was not set
 
 
+class SetPath(NameValueModifier):
+    def execute(self):
+        string_path = concatenate_paths(self.value)
+        os.environ[self.name] = string_path
+
+
 class AppendPath(NameValueModifier):
     def execute(self):
         environment_value = os.environ.get(self.name, '')
@@ -103,7 +109,7 @@ class EnvironmentModifications(object):
         }
         return args
 
-    def set_env(self, name, value, **kwargs):
+    def set(self, name, value, **kwargs):
         """
         Stores in the current object a request to set an environment variable
 
@@ -115,7 +121,7 @@ class EnvironmentModifications(object):
         item = SetEnv(name, value, **kwargs)
         self.env_modifications.append(item)
 
-    def unset_env(self, name, **kwargs):
+    def unset(self, name, **kwargs):
         """
         Stores in the current object a request to unset an environment variable
 
@@ -126,6 +132,18 @@ class EnvironmentModifications(object):
         item = UnsetEnv(name, **kwargs)
         self.env_modifications.append(item)
 
+    def set_path(self, name, elts, **kwargs):
+        """
+        Stores a request to set a path generated from a list.
+
+        Args:
+            name: name o the environment variable to be set.
+            elts: elements of the path to set.
+        """
+        kwargs.update(self._get_outside_caller_attributes())
+        item = SetPath(name, elts, **kwargs)
+        self.env_modifications.append(item)
+
     def append_path(self, name, path, **kwargs):
         """
         Stores in the current object a request to append a path to a path list
diff --git a/lib/spack/spack/test/environment.py b/lib/spack/spack/test/environment.py
index 3e03760c01..6c8f5ea43c 100644
--- a/lib/spack/spack/test/environment.py
+++ b/lib/spack/spack/test/environment.py
@@ -11,21 +11,27 @@ class EnvironmentTest(unittest.TestCase):
         os.environ['PATH_LIST'] = '/path/second:/path/third'
         os.environ['REMOVE_PATH_LIST'] = '/a/b:/duplicate:/a/c:/remove/this:/a/d:/duplicate/:/f/g'
 
-    def test_set_env(self):
+    def test_set(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value')
-        env.set_env('B', 3)
+        env.set('A', 'dummy value')
+        env.set('B', 3)
         env.apply_modifications()
         self.assertEqual('dummy value', os.environ['A'])
         self.assertEqual(str(3), os.environ['B'])
 
-    def test_unset_env(self):
+    def test_unset(self):
         env = EnvironmentModifications()
         self.assertEqual('foo', os.environ['UNSET_ME'])
-        env.unset_env('UNSET_ME')
+        env.unset('UNSET_ME')
         env.apply_modifications()
         self.assertRaises(KeyError, os.environ.__getitem__, 'UNSET_ME')
 
+    def test_set_path(self):
+        env = EnvironmentModifications()
+        env.set_path('A', ['foo', 'bar', 'baz'])
+        env.apply_modifications()
+        self.assertEqual('foo:bar:baz', os.environ['A'])
+
     def test_path_manipulation(self):
         env = EnvironmentModifications()
 
@@ -51,7 +57,7 @@ class EnvironmentTest(unittest.TestCase):
 
     def test_extra_arguments(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value', who='Pkg1')
+        env.set('A', 'dummy value', who='Pkg1')
         for x in env:
             assert 'who' in x.args
         env.apply_modifications()
@@ -59,8 +65,8 @@ class EnvironmentTest(unittest.TestCase):
 
     def test_extend(self):
         env = EnvironmentModifications()
-        env.set_env('A', 'dummy value')
-        env.set_env('B', 3)
+        env.set('A', 'dummy value')
+        env.set('B', 3)
         copy_construct = EnvironmentModifications(env)
         self.assertEqual(len(copy_construct), 2)
         for x, y in zip(env, copy_construct):
diff --git a/var/spack/repos/builtin/packages/mpich/package.py b/var/spack/repos/builtin/packages/mpich/package.py
index 90b5d42eab..c4d9940bb7 100644
--- a/var/spack/repos/builtin/packages/mpich/package.py
+++ b/var/spack/repos/builtin/packages/mpich/package.py
@@ -48,11 +48,11 @@ class Mpich(Package):
     provides('mpi@:1.3', when='@1:')
 
     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)
+        env.set('MPICH_CC', spack_cc)
+        env.set('MPICH_CXX', spack_cxx)
+        env.set('MPICH_F77', spack_f77)
+        env.set('MPICH_F90', spack_f90)
+        env.set('MPICH_FC', spack_fc)
 
     def setup_dependent_python_module(self, module, spec, dep_spec):
         """For dependencies, make mpicc's use spack wrapper."""
diff --git a/var/spack/repos/builtin/packages/openmpi/package.py b/var/spack/repos/builtin/packages/openmpi/package.py
index c91a13e376..9a127f1812 100644
--- a/var/spack/repos/builtin/packages/openmpi/package.py
+++ b/var/spack/repos/builtin/packages/openmpi/package.py
@@ -43,10 +43,10 @@ class Openmpi(Package):
 
 
     def setup_dependent_environment(self, spack_env, run_env, dependent_spec):
-        spack_env.set_env('OMPI_CC', spack_cc)
-        spack_env.set_env('OMPI_CXX', spack_cxx)
-        spack_env.set_env('OMPI_FC', spack_fc)
-        spack_env.set_env('OMPI_F77', spack_f77)
+        spack_env.set('OMPI_CC', spack_cc)
+        spack_env.set('OMPI_CXX', spack_cxx)
+        spack_env.set('OMPI_FC', spack_fc)
+        spack_env.set('OMPI_F77', spack_f77)
 
 
     def install(self, spec, prefix):
diff --git a/var/spack/repos/builtin/packages/python/package.py b/var/spack/repos/builtin/packages/python/package.py
index 593a27708c..4f55bc803e 100644
--- a/var/spack/repos/builtin/packages/python/package.py
+++ b/var/spack/repos/builtin/packages/python/package.py
@@ -104,8 +104,8 @@ class Python(Package):
                 python_paths.append(os.path.join(d.prefix, self.site_packages_dir))
 
         pythonpath = ':'.join(python_paths)
-        spack_env.set_env('PYTHONPATH', pythonpath)
-        run_env.set_env('PYTHONPATH', pythonpath)
+        spack_env.set('PYTHONPATH', pythonpath)
+        run_env.set('PYTHONPATH', pythonpath)
 
 
     def modify_module(self, module, spec, ext_spec):
diff --git a/var/spack/repos/builtin/packages/qt/package.py b/var/spack/repos/builtin/packages/qt/package.py
index 039aeb3c31..d08e8e81e1 100644
--- a/var/spack/repos/builtin/packages/qt/package.py
+++ b/var/spack/repos/builtin/packages/qt/package.py
@@ -57,11 +57,11 @@ class Qt(Package):
 
 
     def setup_environment(self, spack_env, env):
-        env.set_env('QTDIR', self.prefix)
+        env.set('QTDIR', self.prefix)
 
 
     def setup_dependent_environment(self, spack_env, run_env, dspec):
-        spack_env.set_env('QTDIR', self.prefix)
+        spack_env.set('QTDIR', self.prefix)
 
 
     def patch(self):
diff --git a/var/spack/repos/builtin/packages/ruby/package.py b/var/spack/repos/builtin/packages/ruby/package.py
index 39f65f51d2..7ff1898ce9 100644
--- a/var/spack/repos/builtin/packages/ruby/package.py
+++ b/var/spack/repos/builtin/packages/ruby/package.py
@@ -25,9 +25,10 @@ class Ruby(Package):
             if d.package.extends(self.spec):
                 ruby_paths.append(d.prefix)
 
-        spack_env.set_env('GEM_PATH', concatenate_paths(ruby_paths))
+        spack_env.set_path('GEM_PATH', ruby_paths)
+
         # The actual installation path for this gem
-        spack_env.set_env('GEM_HOME', extension_spec.prefix)
+        spack_env.set('GEM_HOME', extension_spec.prefix)
 
     def modify_module(self, module, spec, ext_spec):
         """Called before ruby modules' install() methods.  Sets GEM_HOME