diff --git a/lib/spack/spack/cmd/config.py b/lib/spack/spack/cmd/config.py index 06f6477dfd6..0efbbd8f5ab 100644 --- a/lib/spack/spack/cmd/config.py +++ b/lib/spack/spack/cmd/config.py @@ -244,30 +244,35 @@ def config_remove(args): spack.config.set(path, existing, scope) -def _can_update_config_file(scope_dir, cfg_file): - dir_ok = fs.can_write_to_dir(scope_dir) - cfg_ok = fs.can_access(cfg_file) - return dir_ok and cfg_ok +def _can_update_config_file(scope, cfg_file): + if isinstance(scope, spack.config.SingleFileScope): + return fs.can_access(cfg_file) + return fs.can_write_to_dir(scope.path) and fs.can_access(cfg_file) def config_update(args): # Read the configuration files spack.config.config.get_config(args.section, scope=args.scope) - updates = spack.config.config.format_updates[args.section] + updates = list( + filter( + lambda s: not isinstance( + s, (spack.config.InternalConfigScope, spack.config.ImmutableConfigScope) + ), + spack.config.config.format_updates[args.section], + ) + ) cannot_overwrite, skip_system_scope = [], False for scope in updates: cfg_file = spack.config.config.get_config_filename(scope.name, args.section) - scope_dir = scope.path - can_be_updated = _can_update_config_file(scope_dir, cfg_file) + can_be_updated = _can_update_config_file(scope, cfg_file) if not can_be_updated: if scope.name == "system": skip_system_scope = True - msg = ( + tty.warn( 'Not enough permissions to write to "system" scope. ' - "Skipping update at that location [cfg={0}]" + "Skipping update at that location [cfg={0}]".format(cfg_file) ) - tty.warn(msg.format(cfg_file)) continue cannot_overwrite.append((scope, cfg_file)) @@ -315,18 +320,14 @@ def config_update(args): # Get a function to update the format update_fn = spack.config.ensure_latest_format_fn(args.section) for scope in updates: - cfg_file = spack.config.config.get_config_filename(scope.name, args.section) - with open(cfg_file) as f: - data = syaml.load_config(f) or {} - data = data.pop(args.section, {}) + data = scope.get_section(args.section).pop(args.section) update_fn(data) # Make a backup copy and rewrite the file bkp_file = cfg_file + ".bkp" shutil.copy(cfg_file, bkp_file) spack.config.config.update_config(args.section, data, scope=scope.name, force=True) - msg = 'File "{0}" updated [backup={1}]' - tty.msg(msg.format(cfg_file, bkp_file)) + tty.msg('File "{}" update [backup={}]'.format(cfg_file, bkp_file)) def _can_revert_update(scope_dir, cfg_file, bkp_file): diff --git a/lib/spack/spack/config.py b/lib/spack/spack/config.py index e9a3b1d9568..a43b39e0481 100644 --- a/lib/spack/spack/config.py +++ b/lib/spack/spack/config.py @@ -36,10 +36,11 @@ import re import sys from contextlib import contextmanager -from typing import List # novm +from typing import List # novm # noqa: F401 import ruamel.yaml as yaml import six +from ruamel.yaml.comments import Comment from ruamel.yaml.error import MarkedYAMLError from six import iteritems @@ -532,16 +533,14 @@ def update_config(self, section, update_data, scope=None, force=False): scope = self._validate_scope(scope) # get ConfigScope object # manually preserve comments - need_comment_copy = section in scope.sections and scope.sections[section] is not None + need_comment_copy = section in scope.sections and scope.sections[section] if need_comment_copy: - comments = getattr( - scope.sections[section][section], yaml.comments.Comment.attrib, None - ) + comments = getattr(scope.sections[section][section], Comment.attrib, None) # read only the requested section's data. scope.sections[section] = syaml.syaml_dict({section: update_data}) if need_comment_copy and comments: - setattr(scope.sections[section][section], yaml.comments.Comment.attrib, comments) + setattr(scope.sections[section][section], Comment.attrib, comments) scope._write_section(section) diff --git a/lib/spack/spack/test/cmd/config.py b/lib/spack/spack/test/cmd/config.py index 56943f5d8b6..dd26a92ef04 100644 --- a/lib/spack/spack/test/cmd/config.py +++ b/lib/spack/spack/test/cmd/config.py @@ -13,6 +13,7 @@ import spack.database import spack.environment as ev import spack.main +import spack.schema.config import spack.spec import spack.store import spack.util.spack_yaml as syaml @@ -652,3 +653,26 @@ def test_config_prefer_upstream( # Make sure a message about the conflicting hdf5's was given. assert "- hdf5" in output + + +def test_environment_config_update(tmpdir, mutable_config, monkeypatch): + with open(str(tmpdir.join("spack.yaml")), "w") as f: + f.write( + """\ +spack: + config: + ccache: true +""" + ) + + def update_config(data): + data["ccache"] = False + return True + + monkeypatch.setattr(spack.schema.config, "update", update_config) + + with ev.Environment(str(tmpdir)): + config("update", "-y", "config") + + with ev.Environment(str(tmpdir)) as e: + assert not e.raw_yaml["spack"]["config"]["ccache"]