Remove update functions used to ease the transition in v0.16 (#31216)

lib/spack/spack/schema/env.py

@@ -198,9 +198,6 @@ def update(data):
                " [files={0}]")
         warnings.warn(msg.format(', '.join(data['include'])))
 
-    if 'packages' in data:
-        updated |= spack.schema.packages.update(data['packages'])
-
     # Spack 0.19 drops support for `spack:concretization` in favor of
     # `spack:concretizer:unify`. Here we provide an upgrade path that changes the former
     # into the latter, or warns when there's an ambiguity. Note that Spack 0.17 is not

lib/spack/spack/schema/packages.py

@@ -9,54 +9,6 @@
 """
 
 
-def deprecate_paths_and_modules(instance, deprecated_properties):
-    """Function to produce warning/error messages if "paths" and "modules" are
-    found in "packages.yaml"
-
-    Args:
-        instance: instance of the configuration file
-        deprecated_properties: deprecated properties in instance
-
-    Returns:
-        Warning/Error message to be printed
-    """
-    import copy
-    import os.path
-
-    import llnl.util.tty
-
-    import spack.util.spack_yaml as syaml
-
-    # Copy the instance to remove default attributes that are not related
-    # to the part that needs to be reported
-    instance_copy = copy.copy(instance)
-
-    # Check if this configuration comes from an environment or not
-    absolute_path = instance_copy._end_mark.name
-    command_to_suggest = '$ spack config update packages'
-    if os.path.basename(absolute_path) == 'spack.yaml':
-        command_to_suggest = '$ spack env update <environment>'
-
-    # Retrieve the relevant part of the configuration as YAML
-    keys_to_be_removed = [
-        x for x in instance_copy if x not in deprecated_properties
-    ]
-    for key in keys_to_be_removed:
-        instance_copy.pop(key)
-    yaml_as_str = syaml.dump_config(instance_copy, blame=True)
-
-    if llnl.util.tty.is_debug():
-        msg = 'OUTDATED CONFIGURATION FILE [file={0}]\n{1}'
-        llnl.util.tty.debug(msg.format(absolute_path, yaml_as_str))
-
-    msg = ('detected deprecated properties in {0}\nActivate the debug '
-           'flag to have more information on the deprecated parts or '
-           'run:\n\n\t{2}\n\nto update the file to the new format\n')
-    return msg.format(
-        absolute_path, yaml_as_str, command_to_suggest
-    )
-
-
 #: Properties for inclusion in other schemas
 properties = {
     'packages': {
@@ -136,16 +88,7 @@ def deprecate_paths_and_modules(instance, deprecated_properties):
                             'required': ['spec']
                         }
                     },
-                    # Deprecated properties, will trigger an error with a
-                    # message telling how to update.
-                    'paths': {'type': 'object'},
-                    'modules': {'type': 'object'},
                 },
-                'deprecatedProperties': {
-                    'properties': ['modules', 'paths'],
-                    'message': deprecate_paths_and_modules,
-                    'error': False
-                }
             },
         },
     },
@@ -160,41 +103,3 @@ def deprecate_paths_and_modules(instance, deprecated_properties):
     '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
-    for cfg_object in data.values():
-        externals = []
-
-        # If we don't have these deprecated attributes, continue
-        if not any(x in cfg_object for x in ('paths', 'modules')):
-            continue
-
-        # If we arrive here we need to make some changes i.e.
-        # we need to remove and eventually convert some attributes
-        changed = True
-        paths = cfg_object.pop('paths', {})
-        for spec, prefix in paths.items():
-            externals.append({
-                'spec': str(spec),
-                'prefix': str(prefix)
-            })
-        modules = cfg_object.pop('modules', {})
-        for spec, module in modules.items():
-            externals.append({
-                'spec': str(spec),
-                'modules': [str(module)]
-            })
-        if externals:
-            cfg_object['externals'] = externals
-
-    return changed

lib/spack/spack/test/cmd/config.py

@@ -29,22 +29,6 @@ def _create_config(scope=None, data={}, section='packages'):
     return cfg_file
 
 
-@pytest.fixture()
-def packages_yaml_v015(mutable_config):
-    """Create a packages.yaml in the old format"""
-    old_data = {
-        'packages': {
-            'cmake': {
-                'paths': {'cmake@3.14.0': '/usr'}
-            },
-            'gcc': {
-                'modules': {'gcc@8.3.0': 'gcc-8'}
-            }
-        }
-    }
-    return functools.partial(_create_config, data=old_data, section='packages')
-
-
 @pytest.fixture()
 def config_yaml_v015(mutable_config):
     """Create a packages.yaml in the old format"""
@@ -493,19 +477,6 @@ def test_config_remove_from_env(mutable_empty_config, mutable_mock_env_path):
     assert output == expected
 
 
-def test_config_update_packages(packages_yaml_v015):
-    """Test Spack updating old packages.yaml format for externals
-    to new format. Ensure that data is preserved and converted
-    properly.
-    """
-    packages_yaml_v015()
-    config('update', '-y', 'packages')
-
-    # Check the entries have been transformed
-    data = spack.config.get('packages')
-    check_packages_updated(data)
-
-
 def test_config_update_config(config_yaml_v015):
     config_yaml_v015()
     config('update', '-y', 'config')
@@ -522,100 +493,26 @@ def test_config_update_not_needed(mutable_config):
     assert data_before == data_after
 
 
-def test_config_update_fail_on_permission_issue(
-        packages_yaml_v015, monkeypatch
-):
-    # The first time it will update and create the backup file
-    packages_yaml_v015()
-    # Mock a global scope where we cannot write
-    monkeypatch.setattr(
-        spack.cmd.config, '_can_update_config_file', lambda x, y: False
-    )
-    with pytest.raises(spack.main.SpackCommandError):
-        config('update', '-y', 'packages')
-
-
-def test_config_revert(packages_yaml_v015):
-    cfg_file = packages_yaml_v015()
-    bkp_file = cfg_file + '.bkp'
-
-    config('update', '-y', 'packages')
-
-    # Check that the backup file exists, compute its md5 sum
-    assert os.path.exists(bkp_file)
-    md5bkp = fs.md5sum(bkp_file)
-
-    config('revert', '-y', 'packages')
-
-    # Check that the backup file does not exist anymore and
-    # that the md5 sum of the configuration file is the same
-    # as that of the old backup file
-    assert not os.path.exists(bkp_file)
-    assert md5bkp == fs.md5sum(cfg_file)
-
-
-def test_config_revert_raise_if_cant_write(packages_yaml_v015, monkeypatch):
-    packages_yaml_v015()
-    config('update', '-y', 'packages')
-
-    # Mock a global scope where we cannot write
-    monkeypatch.setattr(
-        spack.cmd.config, '_can_revert_update', lambda x, y, z: False
-    )
-    # The command raises with an helpful error if a configuration
-    # file is to be deleted and we don't have sufficient permissions
-    with pytest.raises(spack.main.SpackCommandError):
-        config('revert', '-y', 'packages')
-
-
-def test_updating_config_implicitly_raises(packages_yaml_v015):
-    # Trying to write implicitly to a scope with a configuration file
-    # in the old format raises an exception
-    packages_yaml_v015()
-    with pytest.raises(RuntimeError):
-        config('add', 'packages:cmake:buildable:false')
-
-
-def test_updating_multiple_scopes_at_once(packages_yaml_v015):
-    # Create 2 config files in the old format
-    packages_yaml_v015(scope='user')
-    packages_yaml_v015(scope='site')
-
-    # Update both of them at once
-    config('update', '-y', 'packages')
-
-    for scope in ('user', 'site'):
-        data = spack.config.get('packages', scope=scope)
-        check_packages_updated(data)
-
-
 @pytest.mark.regression('18031')
 def test_config_update_can_handle_comments(mutable_config):
     # Create an outdated config file with comments
     scope = spack.config.default_modify_scope()
-    cfg_file = spack.config.config.get_config_filename(scope, 'packages')
+    cfg_file = spack.config.config.get_config_filename(scope, 'config')
     with open(cfg_file, mode='w') as f:
         f.write("""
-packages:
+config:
   # system cmake in /usr
-  cmake:
+  install_tree: './foo'
-    paths:
+  # Another comment after the outdated section
-      cmake@3.14.0:  /usr
+  install_hash_length: 7
-    # Another comment after the outdated section
-    buildable: False
 """)
 
     # Try to update it, it should not raise errors
-    config('update', '-y', 'packages')
+    config('update', '-y', 'config')
 
     # Check data
-    data = spack.config.get('packages', scope=scope)
+    data = spack.config.get('config', scope=scope)
-    assert 'paths' not in data['cmake']
+    assert 'root' in data['install_tree']
-    assert 'externals' in data['cmake']
-    externals = data['cmake']['externals']
-    assert len(externals) == 1
-    assert externals[0]['spec'] == 'cmake@3.14.0'
-    assert externals[0]['prefix'] == '/usr'
 
     # Check the comment is there
     with open(cfg_file) as f:
@@ -627,39 +524,21 @@ def test_config_update_can_handle_comments(mutable_config):
 
 @pytest.mark.regression('18050')
 def test_config_update_works_for_empty_paths(mutable_config):
-    # Create an outdated config file with empty "paths" and "modules"
     scope = spack.config.default_modify_scope()
-    cfg_file = spack.config.config.get_config_filename(scope, 'packages')
+    cfg_file = spack.config.config.get_config_filename(scope, 'config')
     with open(cfg_file, mode='w') as f:
         f.write("""
-packages:
+config:
-  cmake:
+    install_tree: ''
-    paths: {}
-    modules: {}
-    buildable: False
 """)
 
     # Try to update it, it should not raise errors
-    output = config('update', '-y', 'packages')
+    output = config('update', '-y', 'config')
 
     # This ensures that we updated the configuration
     assert '[backup=' in output
 
 
-def check_packages_updated(data):
-    """Check that the data from the packages_yaml_v015
-    has been updated.
-    """
-    assert 'externals' in data['cmake']
-    externals = data['cmake']['externals']
-    assert {'spec': 'cmake@3.14.0', 'prefix': '/usr'} in externals
-    assert 'paths' not in data['cmake']
-    assert 'externals' in data['gcc']
-    externals = data['gcc']['externals']
-    assert {'spec': 'gcc@8.3.0', 'modules': ['gcc-8']} in externals
-    assert 'modules' not in data['gcc']
-
-
 def check_config_updated(data):
     assert isinstance(data['install_tree'], dict)
     assert data['install_tree']['root'] == '/fake/path'

lib/spack/spack/test/cmd/env.py

@@ -2288,76 +2288,6 @@ def test_env_write_only_non_default_nested(tmpdir):
     assert manifest == contents
 
 
-@pytest.fixture
-def packages_yaml_v015(tmpdir):
-    """Return the path to an existing manifest in the v0.15.x format
-    and the path to a non yet existing backup file.
-    """
-    raw_yaml = """
-spack:
-  specs:
-  - mpich
-  packages:
-    cmake:
-      paths:
-        cmake@3.17.3: /usr
-"""
-    manifest = tmpdir.ensure('spack.yaml')
-    backup_file = tmpdir.join('spack.yaml.bkp')
-    manifest.write(raw_yaml)
-    return manifest, backup_file
-
-
-def test_update_anonymous_env(packages_yaml_v015):
-    manifest, backup_file = packages_yaml_v015
-    env('update', '-y', str(manifest.dirname))
-
-    # The environment is now at the latest format
-    assert ev.is_latest_format(str(manifest))
-    # A backup file has been created and it's not at the latest format
-    assert os.path.exists(str(backup_file))
-    assert not ev.is_latest_format(str(backup_file))
-
-
-def test_double_update(packages_yaml_v015):
-    manifest, backup_file = packages_yaml_v015
-
-    # Update the environment
-    env('update', '-y', str(manifest.dirname))
-    # Try to read the environment (it should not error)
-    ev.create('test', str(manifest))
-    # Updating again does nothing since the manifest is up-to-date
-    env('update', '-y', str(manifest.dirname))
-
-    # The environment is at the latest format
-    assert ev.is_latest_format(str(manifest))
-    # A backup file has been created and it's not at the latest format
-    assert os.path.exists(str(backup_file))
-    assert not ev.is_latest_format(str(backup_file))
-
-
-def test_update_and_revert(packages_yaml_v015):
-    manifest, backup_file = packages_yaml_v015
-
-    # Update the environment
-    env('update', '-y', str(manifest.dirname))
-    assert os.path.exists(str(backup_file))
-    assert not ev.is_latest_format(str(backup_file))
-    assert ev.is_latest_format(str(manifest))
-
-    # Revert to previous state
-    env('revert', '-y', str(manifest.dirname))
-    assert not os.path.exists(str(backup_file))
-    assert not ev.is_latest_format(str(manifest))
-
-
-def test_old_format_cant_be_updated_implicitly(packages_yaml_v015):
-    manifest, backup_file = packages_yaml_v015
-    env('activate', str(manifest.dirname))
-    with pytest.raises(spack.main.SpackCommandError):
-        add('hdf5')
-
-
 @pytest.mark.parametrize('concretization,unify', [
     ('together', 'true'),
     ('separately', 'false')