diff options
| -rw-r--r-- | etc/spack/defaults/modules.yaml | 9 | ||||
| -rw-r--r-- | lib/spack/docs/config_yaml.rst | 15 | ||||
| -rw-r--r-- | lib/spack/docs/configuration.rst | 20 | ||||
| -rw-r--r-- | lib/spack/docs/module_file_support.rst | 5 | ||||
| -rw-r--r-- | lib/spack/spack/modules/common.py | 26 | ||||
| -rw-r--r-- | lib/spack/spack/schema/config.py | 22 | ||||
| -rw-r--r-- | lib/spack/spack/schema/modules.py | 117 | ||||
| -rw-r--r-- | lib/spack/spack/test/bootstrap.py | 14 | ||||
| -rw-r--r-- | lib/spack/spack/test/config.py | 30 | ||||
| -rw-r--r-- | lib/spack/spack/test/modules/lmod.py | 17 | ||||
| -rw-r--r-- | lib/spack/spack/test/modules/tcl.py | 19 |
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) |