summaryrefslogtreecommitdiff
path: root/lib
diff options
context:
space:
mode:
Diffstat (limited to 'lib')
-rw-r--r--lib/spack/spack/cmd/module.py5
-rw-r--r--lib/spack/spack/hooks/module_file_generation.py7
-rw-r--r--lib/spack/spack/modules/common.py53
-rw-r--r--lib/spack/spack/spec.py74
-rw-r--r--lib/spack/spack/test/data/modules/tcl/alter_environment.yaml1
-rw-r--r--lib/spack/spack/test/data/modules/tcl/invalid_naming_scheme.yaml5
-rw-r--r--lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml21
-rw-r--r--lib/spack/spack/test/modules/tcl.py26
8 files changed, 170 insertions, 22 deletions
diff --git a/lib/spack/spack/cmd/module.py b/lib/spack/spack/cmd/module.py
index e3e559794b..422ccdc196 100644
--- a/lib/spack/spack/cmd/module.py
+++ b/lib/spack/spack/cmd/module.py
@@ -309,8 +309,9 @@ def refresh(module_types, specs, args):
try:
x.write(overwrite=True)
except Exception as e:
- msg = 'Could not write module file because of {0}: [{1}]'
- tty.warn(msg.format(str(e), x.layout.filename))
+ msg = 'Could not write module file [{0}]'
+ tty.warn(msg.format(x.layout.filename))
+ tty.warn('\t--> {0} <--'.format(str(e)))
def module(parser, args):
diff --git a/lib/spack/spack/hooks/module_file_generation.py b/lib/spack/spack/hooks/module_file_generation.py
index 292c195e7b..1e7eb6d6e7 100644
--- a/lib/spack/spack/hooks/module_file_generation.py
+++ b/lib/spack/spack/hooks/module_file_generation.py
@@ -37,7 +37,12 @@ def _for_each_enabled(spec, method_name):
"""Calls a method for each enabled module"""
for name in enabled:
generator = spack.modules.module_types[name](spec)
- getattr(generator, method_name)()
+ try:
+ getattr(generator, method_name)()
+ except RuntimeError as e:
+ msg = 'cannot perform the requested {0} operation on module files'
+ msg += ' [{1}]'
+ tty.warn(msg.format(method_name, str(e)))
post_install = lambda spec: _for_each_enabled(spec, 'write')
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 89f6234705..3907e5e654 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -73,6 +73,37 @@ configuration = spack.config.get_config('modules')
#: Inspections that needs to be done on spec prefixes
prefix_inspections = configuration.get('prefix_inspections', {})
+#: Valid tokens for naming scheme and env variable names
+_valid_tokens = (
+ 'PACKAGE',
+ 'VERSION',
+ 'COMPILER',
+ 'COMPILERNAME',
+ 'COMPILERVER',
+ 'ARCHITECTURE'
+)
+
+
+def _check_tokens_are_valid(format_string, message):
+ """Checks that the tokens used in the format string are valid in
+ the context of module file and environment variable naming.
+
+ Args:
+ format_string (str): string containing the format to be checked. This
+ is supposed to be a 'template' for ``Spec.format``
+
+ message (str): first sentence of the error message in case invalid
+ tokens are found
+
+ """
+ named_tokens = re.findall(r'\${(\w*)}', format_string)
+ invalid_tokens = [x for x in named_tokens if x not in _valid_tokens]
+ if invalid_tokens:
+ msg = message
+ msg += ' [{0}]. '.format(', '.join(invalid_tokens))
+ msg += 'Did you check your "modules.yaml" configuration?'
+ raise RuntimeError(msg)
+
def update_dictionary_extending_lists(target, update):
"""Updates a dictionary, but extends lists instead of overriding them.
@@ -223,6 +254,12 @@ class BaseConfiguration(object):
'naming_scheme',
'${PACKAGE}-${VERSION}-${COMPILERNAME}-${COMPILERVER}'
)
+
+ # Ensure the named tokens we are expanding are allowed, see
+ # issue #2884 for reference
+ msg = 'some tokens cannot be part of the module naming scheme'
+ _check_tokens_are_valid(scheme, message=msg)
+
return scheme
@property
@@ -521,14 +558,26 @@ class BaseContext(tengine.Context):
blacklist = self.conf.environment_blacklist
# We may have tokens to substitute in environment commands
+
+ # Prepare a suitable transformation dictionary for the names
+ # of the environment variables. This means turn the valid
+ # tokens uppercase.
+ transform = {}
+ for token in _valid_tokens:
+ transform[token] = str.upper
+
for x in env:
- x.name = self.spec.format(x.name)
+ # Ensure all the tokens are valid in this context
+ msg = 'some tokens cannot be expanded in an environment variable name' # noqa: E501
+ _check_tokens_are_valid(x.name, message=msg)
+ # Transform them
+ x.name = self.spec.format(x.name, transform=transform)
try:
# Not every command has a value
x.value = self.spec.format(x.value)
except AttributeError:
pass
- x.name = str(x.name).replace('-', '_').upper()
+ x.name = str(x.name).replace('-', '_')
return [(type(x).__name__, x) for x in env if x.name not in blacklist]
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index 2e144fc80f..624c28285a 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -2826,9 +2826,10 @@ class Spec(object):
return colorize_spec(self)
def format(self, format_string='$_$@$%@+$+$=', **kwargs):
- """
- Prints out particular pieces of a spec, depending on what is
- in the format string. The format strings you can provide are::
+ """Prints out particular pieces of a spec, depending on what is
+ in the format string.
+
+ The format strings you can provide are::
$_ Package name
$. Full package name (with namespace)
@@ -2870,13 +2871,36 @@ class Spec(object):
Anything else is copied verbatim into the output stream.
- *Example:* ``$_$@$+`` translates to the name, version, and options
- of the package, but no dependencies, arch, or compiler.
+ Args:
+ format_string (str): string containing the format to be expanded
+
+ **kwargs (dict): the following list of keywords is supported
+
+ - color (bool): True if returned string is colored
+
+ - transform (dict): maps full-string formats to a callable \
+ that accepts a string and returns another one
+
+ Examples:
+
+ The following line:
+
+ .. code-block:: python
+
+ s = spec.format('$_$@$+')
+
+ translates to the name, version, and options of the package, but no
+ dependencies, arch, or compiler.
TODO: allow, e.g., ``$6#`` to customize short hash length
TODO: allow, e.g., ``$//`` for full hash.
"""
color = kwargs.get('color', False)
+
+ # Dictionary of transformations for named tokens
+ token_transforms = {}
+ token_transforms.update(kwargs.get('transform', {}))
+
length = len(format_string)
out = StringIO()
named = escape = compiler = False
@@ -2953,39 +2977,55 @@ class Spec(object):
named_str += c
continue
named_str = named_str.upper()
+
+ # Retrieve the token transformation from the dictionary.
+ #
+ # The default behavior is to leave the string unchanged
+ # (`lambda x: x` is the identity function)
+ token_transform = token_transforms.get(named_str, lambda x: x)
+
if named_str == 'PACKAGE':
name = self.name if self.name else ''
- write(fmt % self.name, '@')
+ write(fmt % token_transform(name), '@')
if named_str == 'VERSION':
if self.versions and self.versions != _any_version:
- write(fmt % str(self.versions), '@')
+ write(fmt % token_transform(str(self.versions)), '@')
elif named_str == 'COMPILER':
if self.compiler:
- write(fmt % self.compiler, '%')
+ write(fmt % token_transform(self.compiler), '%')
elif named_str == 'COMPILERNAME':
if self.compiler:
- write(fmt % self.compiler.name, '%')
+ write(fmt % token_transform(self.compiler.name), '%')
elif named_str in ['COMPILERVER', 'COMPILERVERSION']:
if self.compiler:
- write(fmt % self.compiler.versions, '%')
+ write(
+ fmt % token_transform(self.compiler.versions),
+ '%'
+ )
elif named_str == 'COMPILERFLAGS':
if self.compiler:
- write(fmt % str(self.compiler_flags), '%')
+ write(
+ fmt % token_transform(str(self.compiler_flags)),
+ '%'
+ )
elif named_str == 'OPTIONS':
if self.variants:
- write(fmt % str(self.variants), '+')
+ write(fmt % token_transform(str(self.variants)), '+')
elif named_str == 'ARCHITECTURE':
if self.architecture and str(self.architecture):
- write(fmt % str(self.architecture), '=')
+ write(
+ fmt % token_transform(str(self.architecture)),
+ '='
+ )
elif named_str == 'SHA1':
if self.dependencies:
- out.write(fmt % str(self.dag_hash(7)))
+ out.write(fmt % token_transform(str(self.dag_hash(7))))
elif named_str == 'SPACK_ROOT':
- out.write(fmt % spack.prefix)
+ out.write(fmt % token_transform(spack.prefix))
elif named_str == 'SPACK_INSTALL':
- out.write(fmt % spack.store.root)
+ out.write(fmt % token_transform(spack.store.root))
elif named_str == 'PREFIX':
- out.write(fmt % self.prefix)
+ out.write(fmt % token_transform(self.prefix))
elif named_str.startswith('HASH'):
if named_str.startswith('HASH:'):
_, hashlen = named_str.split(':')
diff --git a/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml b/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml
index 3a8736a640..c9f00c6360 100644
--- a/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml
+++ b/lib/spack/spack/test/data/modules/tcl/alter_environment.yaml
@@ -13,6 +13,7 @@ tcl:
environment:
set:
FOO: 'foo'
+ OMPI_MCA_mpi_leave_pinned: '1'
unset:
- BAR
diff --git a/lib/spack/spack/test/data/modules/tcl/invalid_naming_scheme.yaml b/lib/spack/spack/test/data/modules/tcl/invalid_naming_scheme.yaml
new file mode 100644
index 0000000000..2f72ba4aab
--- /dev/null
+++ b/lib/spack/spack/test/data/modules/tcl/invalid_naming_scheme.yaml
@@ -0,0 +1,5 @@
+enable:
+ - tcl
+tcl:
+ # ${OPTIONS} is not allowed in the naming scheme, see #2884
+ naming_scheme: '${PACKAGE}/${VERSION}-${COMPILERNAME}-${OPTIONS}'
diff --git a/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml b/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml
new file mode 100644
index 0000000000..297028531b
--- /dev/null
+++ b/lib/spack/spack/test/data/modules/tcl/invalid_token_in_env_var_name.yaml
@@ -0,0 +1,21 @@
+enable:
+ - tcl
+tcl:
+ all:
+ filter:
+ environment_blacklist':
+ - CMAKE_PREFIX_PATH
+ environment:
+ set:
+ '${PACKAGE}_ROOT_${PREFIX}': '${PREFIX}'
+
+ 'platform=test target=x86_64':
+ environment:
+ set:
+ FOO_${OPTIONS}: 'foo'
+ unset:
+ - BAR
+
+ 'platform=test target=x86_32':
+ load:
+ - 'foo/bar'
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index a0a6b69340..61000efcb8 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -121,6 +121,12 @@ class TestTcl(object):
if x.startswith('prepend-path CMAKE_PREFIX_PATH')
]) == 0
assert len([x for x in content if 'setenv FOO "foo"' in x]) == 1
+ assert len([
+ x for x in content if 'setenv OMPI_MCA_mpi_leave_pinned "1"' in x
+ ]) == 1
+ assert len([
+ x for x in content if 'setenv OMPI_MCA_MPI_LEAVE_PINNED "1"' in x
+ ]) == 0
assert len([x for x in content if 'unsetenv BAR' in x]) == 1
assert len([x for x in content if 'setenv MPILEAKS_ROOT' in x]) == 1
@@ -168,6 +174,26 @@ class TestTcl(object):
assert writer.conf.naming_scheme == expected
+ def test_invalid_naming_scheme(self, factory, patch_configuration):
+ """Tests the evaluation of an invalid naming scheme."""
+
+ patch_configuration('invalid_naming_scheme')
+
+ # Test that having invalid tokens in the naming scheme raises
+ # a RuntimeError
+ writer, _ = factory('mpileaks')
+ with pytest.raises(RuntimeError):
+ writer.layout.use_name
+
+ def test_invalid_token_in_env_name(self, factory, patch_configuration):
+ """Tests setting environment variables with an invalid name."""
+
+ patch_configuration('invalid_token_in_env_var_name')
+
+ writer, _ = factory('mpileaks')
+ with pytest.raises(RuntimeError):
+ writer.write()
+
def test_conflicts(self, modulefile_content, patch_configuration):
"""Tests adding conflicts to the module."""