Extended set of environment modification commands. (#8996)

5 files changed, 217 insertions, 88 deletions

diff --git a/lib/spack/docs/basic_usage.rst b/lib/spack/docs/basic_usage.rst
index 622860b75f..d61ca699ad 100644
--- a/lib/spack/docs/basic_usage.rst
+++ b/lib/spack/docs/basic_usage.rst
@@ -684,11 +684,12 @@ Compiler environment variables and additional RPATHs
 
 In the exceptional case a compiler requires setting special environment
 variables, like an explicit library load path. These can bet set in an
-extra section in the compiler configuration.  The user can also specify
-additional ``RPATHs`` that the compiler will add to all executables
-generated by that compiler.  This is useful for forcing certain compilers
-to RPATH their own runtime libraries, so that executables will run
-without the need to set ``LD_LIBRARY_PATH``.
+extra section in the compiler configuration (the supported environment
+modification commands are: ``set``, ``unset``, ``append-path``, and
+``prepend-path``). The user can also specify additional ``RPATHs`` that the
+compiler will add to all executables generated by that compiler.  This is
+useful for forcing certain compilers to RPATH their own runtime libraries, so
+that executables will run without the need to set ``LD_LIBRARY_PATH``.
 
 .. code-block:: yaml
 
@@ -701,12 +702,29 @@ without the need to set ``LD_LIBRARY_PATH``.
           f77: /opt/gcc/bin/gfortran
           fc: /opt/gcc/bin/gfortran
         environment:
+          unset:
+            BAD_VARIABLE: # The colon is required but the value must be empty
           set:
-            LD_LIBRARY_PATH : /opt/gcc/lib
+            GOOD_VARIABLE_NUM: 1
+            GOOD_VARIABLE_STR: good
+          prepend-path:
+            PATH: /path/to/binutils
+          append-path:
+            LD_LIBRARY_PATH: /opt/gcc/lib
         extra_rpaths:
         - /path/to/some/compiler/runtime/directory
         - /path/to/some/other/compiler/runtime/directory
 
+
+.. note::
+
+   The section `environment` is interpreted as an ordered dictionary, which
+   means two things. First, environment modification are applied in the order
+   they are specified in the configuration file. Second, you cannot express
+   environment modifications that require mixing different commands, i.e. you
+   cannot `set` one variable, than `prepend-path` to another one, and than
+   again `set` a third one.
+
 ^^^^^^^^^^^^^^^^^^^^^^^
 Architecture specifiers
 ^^^^^^^^^^^^^^^^^^^^^^^
diff --git a/lib/spack/env/cc b/lib/spack/env/cc
index 826a0fe457..bf78f2d5d0 100755
--- a/lib/spack/env/cc
+++ b/lib/spack/env/cc
@@ -203,18 +203,6 @@ if [[ -z $command ]]; then
 fi
 
 #
-# Set paths as defined in the 'environment' section of the compiler config
-#   names are stored in SPACK_ENV_TO_SET
-#   values are stored in SPACK_ENV_SET_<varname>
-#
-IFS=':' read -ra env_set_varnames <<< "$SPACK_ENV_TO_SET"
-for varname in "${env_set_varnames[@]}"; do
-    spack_varname="SPACK_ENV_SET_$varname"
-    export "$varname"="${!spack_varname}"
-    unset "$spack_varname"
-done
-
-#
 # Filter '.' and Spack environment directories out of PATH so that
 # this script doesn't just call itself
 #
diff --git a/lib/spack/spack/build_environment.py b/lib/spack/spack/build_environment.py
index 41b760c21f..f4c4d71a85 100644
--- a/lib/spack/spack/build_environment.py
+++ b/lib/spack/spack/build_environment.py
@@ -303,14 +303,20 @@ def set_build_environment_variables(pkg, env, dirty):
     # the given compiler
     compiler = pkg.compiler
     environment = compiler.environment
-    if 'set' in environment:
-        env_to_set = environment['set']
-        for key, value in iteritems(env_to_set):
-            env.set('SPACK_ENV_SET_%s' % key, value)
-            env.set('%s' % key, value)
-        # Let shell know which variables to set
-        env_variables = ":".join(env_to_set.keys())
-        env.set('SPACK_ENV_TO_SET', env_variables)
+
+    for command, variable in iteritems(environment):
+        if command == 'set':
+            for name, value in iteritems(variable):
+                env.set(name, value)
+        elif command == 'unset':
+            for name, _ in iteritems(variable):
+                env.unset(name)
+        elif command == 'prepend-path':
+            for name, value in iteritems(variable):
+                env.prepend_path(name, value)
+        elif command == 'append-path':
+            for name, value in iteritems(variable):
+                env.append_path(name, value)
 
     if compiler.extra_rpaths:
         extra_rpaths = ':'.join(compiler.extra_rpaths)
diff --git a/lib/spack/spack/schema/compilers.py b/lib/spack/spack/schema/compilers.py
index e569989d30..fb145b35f3 100644
--- a/lib/spack/spack/schema/compilers.py
+++ b/lib/spack/spack/schema/compilers.py
@@ -34,75 +34,109 @@ schema = {
     'title': 'Spack compiler configuration file schema',
     'type': 'object',
     'additionalProperties': False,
-    'patternProperties': {
+    'properties': {
         'compilers': {
             'type': 'array',
             'items': {
-                'compiler': {
-                    'type': 'object',
-                    'additionalProperties': False,
-                    'required': [
-                        'paths', 'spec', 'modules', 'operating_system'],
-                    'properties': {
-                        'paths': {
-                            'type': 'object',
-                            'required': ['cc', 'cxx', 'f77', 'fc'],
-                            'additionalProperties': False,
-                            'properties': {
-                                'cc':  {'anyOf': [{'type': 'string'},
-                                                  {'type': 'null'}]},
-                                'cxx': {'anyOf': [{'type': 'string'},
-                                                  {'type': 'null'}]},
-                                'f77': {'anyOf': [{'type': 'string'},
-                                                  {'type': 'null'}]},
-                                'fc':  {'anyOf': [{'type': 'string'},
-                                                  {'type': 'null'}]}}},
-                        'flags': {
-                            'type': 'object',
-                            'additionalProperties': False,
-                            'properties': {
-                                'cflags': {'anyOf': [{'type': 'string'},
-                                                     {'type': 'null'}]},
-                                'cxxflags': {'anyOf': [{'type': 'string'},
-                                                       {'type': 'null'}]},
-                                'fflags': {'anyOf': [{'type': 'string'},
-                                                     {'type': 'null'}]},
-                                'cppflags': {'anyOf': [{'type': 'string'},
-                                                       {'type': 'null'}]},
-                                'ldflags': {'anyOf': [{'type': 'string'},
+                'type': 'object',
+                'additionalProperties': False,
+                'properties': {
+                    'compiler': {
+                        'type': 'object',
+                        'additionalProperties': False,
+                        'required': [
+                            'paths', 'spec', 'modules', 'operating_system'],
+                        'properties': {
+                            'paths': {
+                                'type': 'object',
+                                'required': ['cc', 'cxx', 'f77', 'fc'],
+                                'additionalProperties': False,
+                                'properties': {
+                                    'cc':  {'anyOf': [{'type': 'string'},
                                                       {'type': 'null'}]},
-                                'ldlibs': {'anyOf': [{'type': 'string'},
-                                                     {'type': 'null'}]}}},
-                        'spec': {'type': 'string'},
-                        'operating_system': {'type': 'string'},
-                        'alias': {'anyOf': [{'type': 'string'},
-                                            {'type': 'null'}]},
-                        'modules': {'anyOf': [{'type': 'string'},
-                                              {'type': 'null'},
-                                              {'type': 'array'}]},
-                        'environment': {
-                            'type': 'object',
-                            'default': {},
-                            'additionalProperties': False,
-                            'properties': {
-                                'set': {
-                                    'type': 'object',
-                                    'patternProperties': {
-                                        r'\w[\w-]*': {  # variable name
-                                            'type': 'string'
+                                    'cxx': {'anyOf': [{'type': 'string'},
+                                                      {'type': 'null'}]},
+                                    'f77': {'anyOf': [{'type': 'string'},
+                                                      {'type': 'null'}]},
+                                    'fc':  {'anyOf': [{'type': 'string'},
+                                                      {'type': 'null'}]}}},
+                            'flags': {
+                                'type': 'object',
+                                'additionalProperties': False,
+                                'properties': {
+                                    'cflags': {'anyOf': [{'type': 'string'},
+                                                         {'type': 'null'}]},
+                                    'cxxflags': {'anyOf': [{'type': 'string'},
+                                                           {'type': 'null'}]},
+                                    'fflags': {'anyOf': [{'type': 'string'},
+                                                         {'type': 'null'}]},
+                                    'cppflags': {'anyOf': [{'type': 'string'},
+                                                           {'type': 'null'}]},
+                                    'ldflags': {'anyOf': [{'type': 'string'},
+                                                          {'type': 'null'}]},
+                                    'ldlibs': {'anyOf': [{'type': 'string'},
+                                                         {'type': 'null'}]}}},
+                            'spec': {'type': 'string'},
+                            'operating_system': {'type': 'string'},
+                            'target': {'type': 'string'},
+                            'alias': {'anyOf': [{'type': 'string'},
+                                                {'type': 'null'}]},
+                            'modules': {'anyOf': [{'type': 'string'},
+                                                  {'type': 'null'},
+                                                  {'type': 'array'}]},
+                            'environment': {
+                                'type': 'object',
+                                'default': {},
+                                'additionalProperties': False,
+                                'properties': {
+                                    'set': {
+                                        'type': 'object',
+                                        'patternProperties': {
+                                            # Variable name
+                                            r'\w[\w-]*': {
+                                                'anyOf': [{'type': 'string'},
+                                                          {'type': 'number'}]
+                                            }
+                                        }
+                                    },
+                                    'unset': {
+                                        'type': 'object',
+                                        'patternProperties': {
+                                            # Variable name
+                                            r'\w[\w-]*': {'type': 'null'}
+                                        }
+                                    },
+                                    'prepend-path': {
+                                        'type': 'object',
+                                        'patternProperties': {
+                                            # Variable name
+                                            r'\w[\w-]*': {
+                                                'anyOf': [{'type': 'string'},
+                                                          {'type': 'number'}]
+                                            }
+                                        }
+                                    },
+                                    'append-path': {
+                                        'type': 'object',
+                                        'patternProperties': {
+                                            # Variable name
+                                            r'\w[\w-]*': {
+                                                'anyOf': [{'type': 'string'},
+                                                          {'type': 'number'}]
+                                            }
                                         }
                                     }
                                 }
+                            },
+                            'extra_rpaths': {
+                                'type': 'array',
+                                'default': [],
+                                'items': {'type': 'string'}
                             }
-                        },
-                        'extra_rpaths': {
-                            'type': 'array',
-                            'default': [],
-                            'items': {'type': 'string'}
                         }
-                    },
-                },
-            },
-        },
-    },
+                    }
+                }
+            }
+        }
+    }
 }
diff --git a/lib/spack/spack/test/build_environment.py b/lib/spack/spack/test/build_environment.py
index f48f46cd4c..4ac63fa5dc 100644
--- a/lib/spack/spack/test/build_environment.py
+++ b/lib/spack/spack/test/build_environment.py
@@ -30,6 +30,7 @@ import spack.spec
 from spack.paths import build_env_path
 from spack.build_environment import dso_suffix, _static_to_shared_library
 from spack.util.executable import Executable
+from spack.util.spack_yaml import syaml_dict, syaml_str
 
 
 @pytest.fixture
@@ -126,3 +127,85 @@ def test_cc_not_changed_by_modules(monkeypatch):
 
     assert os.environ['CC'] != 'NOT_THIS_PLEASE'
     assert os.environ['ANOTHER_VAR'] == 'THIS_IS_SET'
+
+
+@pytest.mark.usefixtures('config', 'mock_packages')
+def test_compiler_config_modifications(monkeypatch):
+
+    s = spack.spec.Spec('cmake')
+    s.concretize()
+    pkg = s.package
+
+    os.environ['SOME_VAR_STR'] = ''
+    os.environ['SOME_VAR_NUM'] = '0'
+    os.environ['PATH_LIST'] = '/path/third:/path/forth'
+    os.environ['EMPTY_PATH_LIST'] = ''
+    os.environ.pop('NEW_PATH_LIST', None)
+
+    env_mod = syaml_dict()
+    set_cmd = syaml_dict()
+    env_mod[syaml_str('set')] = set_cmd
+
+    set_cmd[syaml_str('SOME_VAR_STR')] = syaml_str('SOME_STR')
+    set_cmd[syaml_str('SOME_VAR_NUM')] = 1
+
+    monkeypatch.setattr(pkg.compiler, 'environment', env_mod)
+    spack.build_environment.setup_package(pkg, False)
+    assert os.environ['SOME_VAR_STR'] == 'SOME_STR'
+    assert os.environ['SOME_VAR_NUM'] == str(1)
+
+    env_mod = syaml_dict()
+    unset_cmd = syaml_dict()
+    env_mod[syaml_str('unset')] = unset_cmd
+
+    unset_cmd[syaml_str('SOME_VAR_STR')] = None
+
+    monkeypatch.setattr(pkg.compiler, 'environment', env_mod)
+    assert 'SOME_VAR_STR' in os.environ
+    spack.build_environment.setup_package(pkg, False)
+    assert 'SOME_VAR_STR' not in os.environ
+
+    env_mod = syaml_dict()
+    set_cmd = syaml_dict()
+    env_mod[syaml_str('set')] = set_cmd
+    append_cmd = syaml_dict()
+    env_mod[syaml_str('append-path')] = append_cmd
+    unset_cmd = syaml_dict()
+    env_mod[syaml_str('unset')] = unset_cmd
+    prepend_cmd = syaml_dict()
+    env_mod[syaml_str('prepend-path')] = prepend_cmd
+
+    set_cmd[syaml_str('EMPTY_PATH_LIST')] = syaml_str('/path/middle')
+
+    append_cmd[syaml_str('PATH_LIST')] = syaml_str('/path/last')
+    append_cmd[syaml_str('EMPTY_PATH_LIST')] = syaml_str('/path/last')
+    append_cmd[syaml_str('NEW_PATH_LIST')] = syaml_str('/path/last')
+
+    unset_cmd[syaml_str('SOME_VAR_NUM')] = None
+
+    prepend_cmd[syaml_str('PATH_LIST')] = syaml_str('/path/first:/path/second')
+    prepend_cmd[syaml_str('EMPTY_PATH_LIST')] = syaml_str('/path/first')
+    prepend_cmd[syaml_str('NEW_PATH_LIST')] = syaml_str('/path/first')
+    prepend_cmd[syaml_str('SOME_VAR_NUM')] = syaml_str('/8')
+
+    assert 'SOME_VAR_NUM' in os.environ
+    monkeypatch.setattr(pkg.compiler, 'environment', env_mod)
+    spack.build_environment.setup_package(pkg, False)
+    # Check that the order of modifications is respected and the
+    # variable was unset before it was prepended.
+    assert os.environ['SOME_VAR_NUM'] == '/8'
+
+    expected = '/path/first:/path/second:/path/third:/path/forth:/path/last'
+    assert os.environ['PATH_LIST'] == expected
+
+    expected = '/path/first:/path/middle:/path/last'
+    assert os.environ['EMPTY_PATH_LIST'] == expected
+
+    expected = '/path/first:/path/last'
+    assert os.environ['NEW_PATH_LIST'] == expected
+
+    os.environ.pop('SOME_VAR_STR', None)
+    os.environ.pop('SOME_VAR_NUM', None)
+    os.environ.pop('PATH_LIST', None)
+    os.environ.pop('EMPTY_PATH_LIST', None)
+    os.environ.pop('NEW_PATH_LIST', None)