summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorHarmen Stoppels <harmenstoppels@gmail.com>2022-04-08 21:00:35 +0200
committerGitHub <noreply@github.com>2022-04-08 19:00:35 +0000
commit99083f170681a2d965bd027b08951f1ffa98c993 (patch)
treed852508f2aa397ab90e31e5fe0ca1f284a284cf4
parent13f3bd533dfac3c226fd644bfbe8243070c3f7a9 (diff)
downloadspack-99083f170681a2d965bd027b08951f1ffa98c993.tar.gz
spack-99083f170681a2d965bd027b08951f1ffa98c993.tar.bz2
spack-99083f170681a2d965bd027b08951f1ffa98c993.tar.xz
spack-99083f170681a2d965bd027b08951f1ffa98c993.zip
Deprecate top-level module config (#28659)
* Ignore top-level module config; add auto-update In Spack 0.17 we got module sets (modules:[name]:[prop]), and for backwards compat modules:[prop] was short for modules:default:[prop]. But this makes it awkward to define default config for the "default" module set. Since 0.17 is branched off, we can now deprecate top-level module config (that is, just ignore it with a warning). This PR does that, and it implements `spack config update modules` to make upgrading easy (we should have added that to 0.17 already...) It also removes references to `dotkit` stuff which was already deprecated in 0.13 and could have been removed in 0.14. Prefix inspections are the only exception, since the top-level prefix inspections used for `spack load` and `spack env activate`.
-rw-r--r--etc/spack/defaults/modules.yaml9
-rw-r--r--lib/spack/docs/config_yaml.rst15
-rw-r--r--lib/spack/docs/configuration.rst20
-rw-r--r--lib/spack/docs/module_file_support.rst5
-rw-r--r--lib/spack/spack/modules/common.py26
-rw-r--r--lib/spack/spack/schema/config.py22
-rw-r--r--lib/spack/spack/schema/modules.py117
-rw-r--r--lib/spack/spack/test/bootstrap.py14
-rw-r--r--lib/spack/spack/test/config.py30
-rw-r--r--lib/spack/spack/test/modules/lmod.py17
-rw-r--r--lib/spack/spack/test/modules/tcl.py19
11 files changed, 103 insertions, 191 deletions
diff --git a/etc/spack/defaults/modules.yaml b/etc/spack/defaults/modules.yaml
index 35be259d4d..8914a6a733 100644
--- a/etc/spack/defaults/modules.yaml
+++ b/etc/spack/defaults/modules.yaml
@@ -35,13 +35,10 @@ modules:
# These are configurations for the module set named "default"
default:
- # These values are defaulted in the code. They are not defaulted here so
- # that we can enable backwards compatibility with the old syntax more
- # easily (old value is in the config yaml, config:module_roots)
# Where to install modules
- # roots:
- # tcl: $spack/share/spack/modules
- # lmod: $spack/share/spack/lmod
+ roots:
+ tcl: $spack/share/spack/modules
+ lmod: $spack/share/spack/lmod
# What type of modules to use
enable:
- tcl
diff --git a/lib/spack/docs/config_yaml.rst b/lib/spack/docs/config_yaml.rst
index 3d21642749..6fb673e461 100644
--- a/lib/spack/docs/config_yaml.rst
+++ b/lib/spack/docs/config_yaml.rst
@@ -73,21 +73,6 @@ used to configure module names.
able to find the old installation directories.
--------------------
-``module_roots``
---------------------
-
-Controls where Spack installs generated module files. You can customize
-the location for each type of module. e.g.:
-
-.. code-block:: yaml
-
- module_roots:
- tcl: $spack/share/spack/modules
- lmod: $spack/share/spack/lmod
-
-See :ref:`modules` for details.
-
---------------------
``build_stage``
--------------------
diff --git a/lib/spack/docs/configuration.rst b/lib/spack/docs/configuration.rst
index a7b0a6c74b..21c8f4afaf 100644
--- a/lib/spack/docs/configuration.rst
+++ b/lib/spack/docs/configuration.rst
@@ -37,8 +37,6 @@ Here is an example ``config.yaml`` file:
config:
install_tree: $spack/opt/spack
- module_roots:
- lmod: $spack/share/spack/lmod
build_stage:
- $tempdir/$user/spack-stage
- ~/.spack/stage
@@ -253,8 +251,6 @@ your configurations look like this:
config:
install_tree: $spack/opt/spack
- module_roots:
- lmod: $spack/share/spack/lmod
build_stage:
- $tempdir/$user/spack-stage
- ~/.spack/stage
@@ -278,8 +274,6 @@ command:
$ spack config get config
config:
install_tree: /some/other/directory
- module_roots:
- lmod: $spack/share/spack/lmod
build_stage:
- $tempdir/$user/spack-stage
- ~/.spack/stage
@@ -345,13 +339,11 @@ higher-precedence scope is *prepended* to the defaults. ``spack config
get config`` shows the result:
.. code-block:: console
- :emphasize-lines: 7-10
+ :emphasize-lines: 5-8
$ spack config get config
config:
install_tree: /some/other/directory
- module_roots:
- lmod: $spack/share/spack/lmod
build_stage:
- /lustre-scratch/$user/spack
- ~/mystage
@@ -375,13 +367,11 @@ user config looked like this:
The merged configuration would look like this:
.. code-block:: console
- :emphasize-lines: 7-8
+ :emphasize-lines: 5-6
$ spack config get config
config:
install_tree: /some/other/directory
- module_roots:
- lmod: $spack/share/spack/lmod
build_stage:
- /lustre-scratch/$user/spack
- ~/mystage
@@ -502,9 +492,6 @@ account all scopes. For example, to see the fully merged
template_dirs:
- $spack/templates
directory_layout: {architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}
- module_roots:
- tcl: $spack/share/spack/modules
- lmod: $spack/share/spack/lmod
build_stage:
- $tempdir/$user/spack-stage
- ~/.spack/stage
@@ -552,9 +539,6 @@ down the problem:
/home/myuser/spack/etc/spack/defaults/config.yaml:23 template_dirs:
/home/myuser/spack/etc/spack/defaults/config.yaml:24 - $spack/templates
/home/myuser/spack/etc/spack/defaults/config.yaml:28 directory_layout: {architecture}/{compiler.name}-{compiler.version}/{name}-{version}-{hash}
- /home/myuser/spack/etc/spack/defaults/config.yaml:32 module_roots:
- /home/myuser/spack/etc/spack/defaults/config.yaml:33 tcl: $spack/share/spack/modules
- /home/myuser/spack/etc/spack/defaults/config.yaml:34 lmod: $spack/share/spack/lmod
/home/myuser/spack/etc/spack/defaults/config.yaml:49 build_stage:
/home/myuser/spack/etc/spack/defaults/config.yaml:50 - $tempdir/$user/spack-stage
/home/myuser/spack/etc/spack/defaults/config.yaml:51 - ~/.spack/stage
diff --git a/lib/spack/docs/module_file_support.rst b/lib/spack/docs/module_file_support.rst
index 5ce69dfc91..5aa57f95bf 100644
--- a/lib/spack/docs/module_file_support.rst
+++ b/lib/spack/docs/module_file_support.rst
@@ -181,10 +181,7 @@ to the environment variables listed below the folder name.
Spack modules can be configured for multiple module sets. The default
module set is named ``default``. All Spack commands which operate on
modules default to apply the ``default`` module set, but can be
-applied to any module set in the configuration. Settings applied at
-the root of the configuration (e.g. ``modules:enable`` rather than
-``modules:default:enable``) are applied to the default module set for
-backwards compatibility.
+applied to any module set in the configuration.
"""""""""""""""""""""""""
Changing the modules root
diff --git a/lib/spack/spack/modules/common.py b/lib/spack/spack/modules/common.py
index 884522b7c2..4b81b1a4b3 100644
--- a/lib/spack/spack/modules/common.py
+++ b/lib/spack/spack/modules/common.py
@@ -57,11 +57,7 @@ import spack.util.spack_yaml as syaml
#: config section for this file
def configuration(module_set_name):
config_path = 'modules:%s' % module_set_name
- config = spack.config.get(config_path, {})
- if not config and module_set_name == 'default':
- # return old format for backward compatibility
- return spack.config.get('modules', {})
- return config
+ return spack.config.get(config_path, {})
#: Valid tokens for naming scheme and env variable names
@@ -233,11 +229,6 @@ def root_path(name, module_set_name):
# Root folders where the various module files should be written
roots = spack.config.get('modules:%s:roots' % module_set_name, {})
- # For backwards compatibility, read the old module roots for default set
- if module_set_name == 'default':
- roots = spack.config.merge_yaml(
- spack.config.get('config:module_roots', {}), roots)
-
# Merge config values into the defaults so we prefer configured values
roots = spack.config.merge_yaml(defaults, roots)
@@ -698,12 +689,11 @@ class BaseContext(tengine.Context):
def environment_modifications(self):
"""List of environment modifications to be processed."""
# Modifications guessed by inspecting the spec prefix
- std_prefix_inspections = spack.config.get(
- 'modules:prefix_inspections', {})
- set_prefix_inspections = spack.config.get(
- 'modules:%s:prefix_inspections' % self.conf.name, {})
- prefix_inspections = spack.config.merge_yaml(
- std_prefix_inspections, set_prefix_inspections)
+ prefix_inspections = syaml.syaml_dict()
+ spack.config.merge_yaml(prefix_inspections, spack.config.get(
+ 'modules:prefix_inspections', {}))
+ spack.config.merge_yaml(prefix_inspections, spack.config.get(
+ 'modules:%s:prefix_inspections' % self.conf.name, {}))
use_view = spack.config.get(
'modules:%s:use_view' % self.conf.name, False)
@@ -939,7 +929,9 @@ def disable_modules():
"""Disable the generation of modulefiles within the context manager."""
data = {
'modules:': {
- 'enable': []
+ 'default': {
+ 'enable': []
+ }
}
}
disable_scope = spack.config.InternalConfigScope('disable_modules', data=data)
diff --git a/lib/spack/spack/schema/config.py b/lib/spack/spack/schema/config.py
index 06dadc802c..1000fcb5a8 100644
--- a/lib/spack/spack/schema/config.py
+++ b/lib/spack/spack/schema/config.py
@@ -56,22 +56,6 @@ properties = {
'type': 'array',
'items': {'type': 'string'}
},
- 'module_roots': {
- 'type': 'object',
- 'additionalProperties': False,
- 'properties': {
- 'tcl': {'type': 'string'},
- 'lmod': {'type': 'string'},
- 'dotkit': {'type': 'string'},
- },
- 'deprecatedProperties': {
- 'properties': ['dotkit'],
- 'message': 'specifying a "dotkit" module root has no '
- 'effect [support for "dotkit" has been '
- 'dropped in v0.13.0]',
- 'error': False
- },
- },
'source_cache': {'type': 'string'},
'misc_cache': {'type': 'string'},
'connect_timeout': {'type': 'integer', 'minimum': 0},
@@ -108,6 +92,12 @@ properties = {
'items': {'type': 'string'}
}
},
+ 'deprecatedProperties': {
+ 'properties': ['module_roots'],
+ 'message': 'config:module_roots has been replaced by '
+ 'modules:[module set]:roots and is ignored',
+ 'error': False
+ }
},
}
diff --git a/lib/spack/spack/schema/modules.py b/lib/spack/spack/schema/modules.py
index 981ccdbda6..a06a459253 100644
--- a/lib/spack/spack/schema/modules.py
+++ b/lib/spack/spack/schema/modules.py
@@ -8,6 +8,8 @@
.. literalinclude:: _spack_root/lib/spack/spack/schema/modules.py
:lines: 13-
"""
+import warnings
+
import spack.schema.environment
import spack.schema.projections
@@ -21,8 +23,8 @@ spec_regex = r'(?!hierarchy|core_specs|verbose|hash_length|whitelist|' \
r'defaults)(^\w[\w-]*)'
#: Matches a valid name for a module set
-# Banned names are valid entries at that level in the previous schema
-set_regex = r'(?!enable|lmod|tcl|dotkit|prefix_inspections)^\w[\w-]*'
+valid_module_set_name = r'^(?!arch_folder$|lmod$|roots$|enable$|prefix_inspections$|'\
+ r'tcl$|use_view$)\w[\w-]*$'
#: Matches an anonymous spec, i.e. a spec without a root name
anonymous_spec_regex = r'^[\^@%+~]'
@@ -117,24 +119,12 @@ module_type_configuration = {
}
-#: The "real" module properties -- the actual configuration parameters.
-#: They are separate from ``properties`` because they can appear both
-#: at the top level of a Spack ``modules:`` config (old, deprecated format),
-#: and within a named module set (new format with multiple module sets).
module_config_properties = {
'use_view': {'anyOf': [
{'type': 'string'},
{'type': 'boolean'}
]},
'arch_folder': {'type': 'boolean'},
- 'prefix_inspections': {
- 'type': 'object',
- 'additionalProperties': False,
- 'patternProperties': {
- # prefix-relative path to be inspected for existence
- r'^[\w-]*': array_of_strings
- }
- },
'roots': {
'type': 'object',
'properties': {
@@ -147,15 +137,8 @@ module_config_properties = {
'default': [],
'items': {
'type': 'string',
- 'enum': ['tcl', 'dotkit', 'lmod']
- },
- 'deprecatedProperties': {
- 'properties': ['dotkit'],
- 'message': 'cannot enable "dotkit" in modules.yaml '
- '[support for "dotkit" has been dropped '
- 'in v0.13.0]',
- 'error': False
- },
+ 'enum': ['tcl', 'lmod']
+ }
},
'lmod': {
'allOf': [
@@ -178,40 +161,54 @@ module_config_properties = {
{} # Specific tcl extensions
]
},
- 'dotkit': {
- 'allOf': [
- # Base configuration
- module_type_configuration,
- {} # Specific dotkit extensions
- ]
+ 'prefix_inspections': {
+ 'type': 'object',
+ 'additionalProperties': False,
+ 'patternProperties': {
+ # prefix-relative path to be inspected for existence
+ r'^[\w-]*': array_of_strings
+ }
},
}
+def deprecation_msg_default_module_set(instance, props):
+ return (
+ 'Top-level properties "{0}" in module config are ignored as of Spack v0.18. '
+ 'They should be set on the "default" module set. Run\n\n'
+ '\t$ spack config update modules\n\n'
+ 'to update the file to the new format'.format('", "'.join(instance))
+ )
+
+
# Properties for inclusion into other schemas (requires definitions)
properties = {
'modules': {
'type': 'object',
+ 'additionalProperties': False,
+ 'properties': {
+ 'prefix_inspections': {
+ 'type': 'object',
+ 'additionalProperties': False,
+ 'patternProperties': {
+ # prefix-relative path to be inspected for existence
+ r'^[\w-]*': array_of_strings
+ }
+ },
+ },
'patternProperties': {
- set_regex: {
+ valid_module_set_name: {
'type': 'object',
'default': {},
'additionalProperties': False,
- 'properties': module_config_properties,
- 'deprecatedProperties': {
- 'properties': ['dotkit'],
- 'message': 'the "dotkit" section in modules.yaml has no effect'
- ' [support for "dotkit" has been dropped in v0.13.0]',
- 'error': False
- }
+ 'properties': module_config_properties
},
+ # Deprecated top-level keys (ignored in 0.18 with a warning)
+ '^(arch_folder|lmod|roots|enable|tcl|use_view)$': {}
},
- # Available here for backwards compatibility
- 'properties': module_config_properties,
'deprecatedProperties': {
- 'properties': ['dotkit'],
- 'message': 'the "dotkit" section in modules.yaml has no effect'
- ' [support for "dotkit" has been dropped in v0.13.0]',
+ 'properties': ['arch_folder', 'lmod', 'roots', 'enable', 'tcl', 'use_view'],
+ 'message': deprecation_msg_default_module_set,
'error': False
}
}
@@ -225,3 +222,39 @@ schema = {
'additionalProperties': False,
'properties': properties,
}
+
+
+def update(data):
+ """Update the data in place to remove deprecated properties.
+
+ Args:
+ data (dict): dictionary to be updated
+
+ Returns:
+ True if data was changed, False otherwise
+ """
+ changed = False
+
+ deprecated_top_level_keys = ('arch_folder', 'lmod', 'roots', 'enable',
+ 'tcl', 'use_view')
+
+ # Don't update when we already have a default module set
+ if 'default' in data:
+ if any(key in data for key in deprecated_top_level_keys):
+ warnings.warn('Did not move top-level module properties into "default" '
+ 'module set, because the "default" module set is already '
+ 'defined')
+ return changed
+
+ default = {}
+
+ # Move deprecated top-level keys under "default" module set.
+ for key in deprecated_top_level_keys:
+ if key in data:
+ default[key] = data.pop(key)
+
+ if default:
+ changed = True
+ data['default'] = default
+
+ return changed
diff --git a/lib/spack/spack/test/bootstrap.py b/lib/spack/spack/test/bootstrap.py
index 8c56cd1cf5..f8eea0de52 100644
--- a/lib/spack/spack/test/bootstrap.py
+++ b/lib/spack/spack/test/bootstrap.py
@@ -73,15 +73,15 @@ def test_bootstrap_deactivates_environments(active_mock_environment):
@pytest.mark.regression('25805')
def test_bootstrap_disables_modulefile_generation(mutable_config):
# Be sure to enable both lmod and tcl in modules.yaml
- spack.config.set('modules:enable', ['tcl', 'lmod'])
+ spack.config.set('modules:default:enable', ['tcl', 'lmod'])
- assert 'tcl' in spack.config.get('modules:enable')
- assert 'lmod' in spack.config.get('modules:enable')
+ assert 'tcl' in spack.config.get('modules:default:enable')
+ assert 'lmod' in spack.config.get('modules:default:enable')
with spack.bootstrap.ensure_bootstrap_configuration():
- assert 'tcl' not in spack.config.get('modules:enable')
- assert 'lmod' not in spack.config.get('modules:enable')
- assert 'tcl' in spack.config.get('modules:enable')
- assert 'lmod' in spack.config.get('modules:enable')
+ assert 'tcl' not in spack.config.get('modules:default:enable')
+ assert 'lmod' not in spack.config.get('modules:default:enable')
+ assert 'tcl' in spack.config.get('modules:default:enable')
+ assert 'lmod' in spack.config.get('modules:default:enable')
@pytest.mark.regression('25992')
diff --git a/lib/spack/spack/test/config.py b/lib/spack/spack/test/config.py
index a765582a2b..4a27acf9ca 100644
--- a/lib/spack/spack/test/config.py
+++ b/lib/spack/spack/test/config.py
@@ -561,20 +561,6 @@ def test_read_config_override_all(mock_low_high_config, write_config_file):
}
-@pytest.mark.regression('23663')
-def test_read_with_default(mock_low_high_config):
- # this very synthetic example ensures that config.get(path, default)
- # returns default if any element of path doesn't exist, regardless
- # of the type of default.
- spack.config.set('modules', {'enable': []})
-
- default_conf = spack.config.get('modules:default', 'default')
- assert default_conf == 'default'
-
- default_enable = spack.config.get('modules:default:enable', [])
- assert default_enable == []
-
-
def test_read_config_override_key(mock_low_high_config, write_config_file):
write_config_file('config', config_low, 'low')
write_config_file('config', config_override_key, 'high')
@@ -1100,22 +1086,6 @@ compilers:
""")
-@pytest.mark.regression('13045')
-def test_dotkit_in_config_does_not_raise(
- mock_low_high_config, write_config_file, capsys
-):
- write_config_file('config',
- {'config': {'module_roots': {'dotkit': '/some/path'}}},
- 'high')
- spack.main.print_setup_info('sh')
- captured = capsys.readouterr()
-
- # Check that we set the variables we expect and that
- # we throw a a deprecation warning without raising
- assert '_sp_sys_type' in captured[0] # stdout
- assert 'Warning' in captured[1] # stderr
-
-
def test_internal_config_section_override(mock_low_high_config,
write_config_file):
write_config_file('config', config_merge_list, 'low')
diff --git a/lib/spack/spack/test/modules/lmod.py b/lib/spack/spack/test/modules/lmod.py
index f2aa048705..feaa9e7905 100644
--- a/lib/spack/spack/test/modules/lmod.py
+++ b/lib/spack/spack/test/modules/lmod.py
@@ -313,23 +313,6 @@ class TestLmod(object):
projection = writer.spec.format(writer.conf.projections['all'])
assert projection in writer.layout.use_name
- def test_config_backwards_compat(self, mutable_config):
- settings = {
- 'enable': ['lmod'],
- 'lmod': {
- 'core_compilers': ['%gcc@0.0.0']
- }
- }
-
- spack.config.set('modules:default', settings)
- new_format = spack.modules.lmod.configuration('default')
-
- spack.config.set('modules', settings)
- old_format = spack.modules.lmod.configuration('default')
-
- assert old_format == new_format
- assert old_format == settings['lmod']
-
def test_modules_relative_to_view(
self, tmpdir, modulefile_content, module_configuration, install_mockery,
mock_fetch
diff --git a/lib/spack/spack/test/modules/tcl.py b/lib/spack/spack/test/modules/tcl.py
index 121e47422b..d9e4ad17bd 100644
--- a/lib/spack/spack/test/modules/tcl.py
+++ b/lib/spack/spack/test/modules/tcl.py
@@ -391,25 +391,6 @@ class TestTcl(object):
content = modulefile_content('mpileaks ^mpich')
assert len([x for x in content if 'is-loaded' in x]) == 0
- def test_config_backwards_compat(self, mutable_config):
- settings = {
- 'enable': ['tcl'],
- 'tcl': {
- 'all': {
- 'conflict': ['{name}']
- }
- }
- }
-
- spack.config.set('modules:default', settings)
- new_format = spack.modules.tcl.configuration('default')
-
- spack.config.set('modules', settings)
- old_format = spack.modules.tcl.configuration('default')
-
- assert old_format == new_format
- assert old_format == settings['tcl']
-
def test_modules_no_arch(self, factory, module_configuration):
module_configuration('no_arch')
module, spec = factory(mpileaks_spec_string)