From f9617b2ad8bbb12bbd3bda0d2cd8d1856e2c7fa8 Mon Sep 17 00:00:00 2001 From: Sergey Kosukhin Date: Wed, 5 Sep 2018 19:56:45 +0200 Subject: [PATCH] Extended set of environment modification commands. (#8996) --- lib/spack/docs/basic_usage.rst | 30 +++- lib/spack/env/cc | 12 -- lib/spack/spack/build_environment.py | 22 +-- lib/spack/spack/schema/compilers.py | 158 +++++++++++++--------- lib/spack/spack/test/build_environment.py | 83 ++++++++++++ 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 @@ -202,18 +202,6 @@ if [[ -z $command ]]; then die "ERROR: Compiler '$SPACK_COMPILER_SPEC' does not support compiling $language programs." 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_ -# -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 @@ '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 @@ 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 _set_wrong_cc(x): 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)