Remove unnecessary copy.deepcopy calls (#45135)

This commit is contained in:
Massimiliano Culpo 2024-07-10 09:33:48 +02:00 committed by GitHub
parent b237ee3689
commit 9001e9328a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 36 additions and 53 deletions

View File

@ -156,7 +156,7 @@ def print_flattened_configuration(*, blame: bool) -> None:
"""
env = ev.active_environment()
if env is not None:
pristine = env.manifest.pristine_yaml_content
pristine = env.manifest.yaml_content
flattened = pristine.copy()
flattened[spack.schema.env.TOP_LEVEL_KEY] = pristine[spack.schema.env.TOP_LEVEL_KEY].copy()
else:

View File

@ -174,9 +174,7 @@ def _write_section(self, section: str) -> None:
if data is None:
return
# We copy data here to avoid adding defaults at write time
validate_data = copy.deepcopy(data)
validate(validate_data, SECTION_SCHEMAS[section])
validate(data, SECTION_SCHEMAS[section])
try:
filesystem.mkdirp(self.path)
@ -1080,11 +1078,8 @@ def validate(
"""
import jsonschema
# Validate a copy to avoid adding defaults
# This allows us to round-trip data without adding to it.
test_data = syaml.deepcopy(data)
try:
spack.schema.Validator(schema).validate(test_data)
spack.schema.Validator(schema).validate(data)
except jsonschema.ValidationError as e:
if hasattr(e.instance, "lc"):
line_number = e.instance.lc.line + 1
@ -1093,7 +1088,7 @@ def validate(
raise ConfigFormatError(e, data, filename, line_number) from e
# return the validated data so that we can access the raw data
# mostly relevant for environments
return test_data
return data
def read_config_file(

View File

@ -5,7 +5,6 @@
import collections
import collections.abc
import contextlib
import copy
import os
import pathlib
import re
@ -528,8 +527,8 @@ def _read_yaml(str_or_file):
)
filename = getattr(str_or_file, "name", None)
default_data = spack.config.validate(data, spack.schema.env.schema, filename)
return data, default_data
spack.config.validate(data, spack.schema.env.schema, filename)
return data
def _write_yaml(data, str_or_file):
@ -957,13 +956,20 @@ def write_transaction(self):
"""Get a write lock context manager for use in a `with` block."""
return lk.WriteTransaction(self.txlock, acquire=self._re_read)
def _process_definition(self, item):
def _process_definition(self, entry):
"""Process a single spec definition item."""
entry = copy.deepcopy(item)
when = _eval_conditional(entry.pop("when", "True"))
when_string = entry.get("when")
if when_string is not None:
when = _eval_conditional(when_string)
assert len([x for x in entry if x != "when"]) == 1
else:
when = True
assert len(entry) == 1
if when:
name, spec_list = next(iter(entry.items()))
for name, spec_list in entry.items():
if name == "when":
continue
user_specs = SpecList(name, spec_list, self.spec_lists.copy())
if name in self.spec_lists:
self.spec_lists[name].extend(user_specs)
@ -2767,12 +2773,8 @@ def __init__(self, manifest_dir: Union[pathlib.Path, str], name: Optional[str] =
raise SpackEnvironmentError(msg)
with self.manifest_file.open() as f:
raw, with_defaults_added = _read_yaml(f)
self.yaml_content = _read_yaml(f)
#: Pristine YAML content, without defaults being added
self.pristine_yaml_content = raw
#: YAML content with defaults added by Spack, if they're missing
self.yaml_content = with_defaults_added
self.changed = False
def _all_matches(self, user_spec: str) -> List[str]:
@ -2786,7 +2788,7 @@ def _all_matches(self, user_spec: str) -> List[str]:
ValueError: if no equivalent match is found
"""
result = []
for yaml_spec_str in self.pristine_configuration["specs"]:
for yaml_spec_str in self.configuration["specs"]:
if Spec(yaml_spec_str) == Spec(user_spec):
result.append(yaml_spec_str)
@ -2801,7 +2803,6 @@ def add_user_spec(self, user_spec: str) -> None:
Args:
user_spec: user spec to be appended
"""
self.pristine_configuration.setdefault("specs", []).append(user_spec)
self.configuration.setdefault("specs", []).append(user_spec)
self.changed = True
@ -2816,7 +2817,6 @@ def remove_user_spec(self, user_spec: str) -> None:
"""
try:
for key in self._all_matches(user_spec):
self.pristine_configuration["specs"].remove(key)
self.configuration["specs"].remove(key)
except ValueError as e:
msg = f"cannot remove {user_spec} from {self}, no such spec exists"
@ -2834,7 +2834,6 @@ def override_user_spec(self, user_spec: str, idx: int) -> None:
SpackEnvironmentError: when the user spec cannot be overridden
"""
try:
self.pristine_configuration["specs"][idx] = user_spec
self.configuration["specs"][idx] = user_spec
except ValueError as e:
msg = f"cannot override {user_spec} from {self}"
@ -2847,10 +2846,10 @@ def set_include_concrete(self, include_concrete: List[str]) -> None:
Args:
include_concrete: list of already existing concrete environments to include
"""
self.pristine_configuration[included_concrete_name] = []
self.configuration[included_concrete_name] = []
for env_path in include_concrete:
self.pristine_configuration[included_concrete_name].append(env_path)
self.configuration[included_concrete_name].append(env_path)
self.changed = True
@ -2864,14 +2863,13 @@ def add_definition(self, user_spec: str, list_name: str) -> None:
Raises:
SpackEnvironmentError: is no valid definition exists already
"""
defs = self.pristine_configuration.get("definitions", [])
defs = self.configuration.get("definitions", [])
msg = f"cannot add {user_spec} to the '{list_name}' definition, no valid list exists"
for idx, item in self._iterate_on_definitions(defs, list_name=list_name, err_msg=msg):
item[list_name].append(user_spec)
break
self.configuration["definitions"][idx][list_name].append(user_spec)
self.changed = True
def remove_definition(self, user_spec: str, list_name: str) -> None:
@ -2885,7 +2883,7 @@ def remove_definition(self, user_spec: str, list_name: str) -> None:
SpackEnvironmentError: if the user spec cannot be removed from the list,
or the list does not exist
"""
defs = self.pristine_configuration.get("definitions", [])
defs = self.configuration.get("definitions", [])
msg = (
f"cannot remove {user_spec} from the '{list_name}' definition, "
f"no valid list exists"
@ -2898,7 +2896,6 @@ def remove_definition(self, user_spec: str, list_name: str) -> None:
except ValueError:
pass
self.configuration["definitions"][idx][list_name].remove(user_spec)
self.changed = True
def override_definition(self, user_spec: str, *, override: str, list_name: str) -> None:
@ -2913,7 +2910,7 @@ def override_definition(self, user_spec: str, *, override: str, list_name: str)
Raises:
SpackEnvironmentError: if the user spec cannot be overridden
"""
defs = self.pristine_configuration.get("definitions", [])
defs = self.configuration.get("definitions", [])
msg = f"cannot override {user_spec} with {override} in the '{list_name}' definition"
for idx, item in self._iterate_on_definitions(defs, list_name=list_name, err_msg=msg):
@ -2924,7 +2921,6 @@ def override_definition(self, user_spec: str, *, override: str, list_name: str)
except ValueError:
pass
self.configuration["definitions"][idx][list_name][sub_index] = override
self.changed = True
def _iterate_on_definitions(self, definitions, *, list_name, err_msg):
@ -2956,7 +2952,6 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]])
True the default view is used for the environment, if False there's no view.
"""
if isinstance(view, dict):
self.pristine_configuration["view"][default_view_name].update(view)
self.configuration["view"][default_view_name].update(view)
self.changed = True
return
@ -2964,15 +2959,13 @@ def set_default_view(self, view: Union[bool, str, pathlib.Path, Dict[str, str]])
if not isinstance(view, bool):
view = str(view)
self.pristine_configuration["view"] = view
self.configuration["view"] = view
self.changed = True
def remove_default_view(self) -> None:
"""Removes the default view from the manifest file"""
view_data = self.pristine_configuration.get("view")
view_data = self.configuration.get("view")
if isinstance(view_data, collections.abc.Mapping):
self.pristine_configuration["view"].pop(default_view_name)
self.configuration["view"].pop(default_view_name)
self.changed = True
return
@ -2985,17 +2978,12 @@ def flush(self) -> None:
return
with fs.write_tmp_and_move(os.path.realpath(self.manifest_file)) as f:
_write_yaml(self.pristine_yaml_content, f)
_write_yaml(self.yaml_content, f)
self.changed = False
@property
def pristine_configuration(self):
"""Return the dictionaries in the pristine YAML, without the top level attribute"""
return self.pristine_yaml_content[TOP_LEVEL_KEY]
@property
def configuration(self):
"""Return the dictionaries in the YAML, without the top level attribute"""
"""Return the dictionaries in the pristine YAML, without the top level attribute"""
return self.yaml_content[TOP_LEVEL_KEY]
def __len__(self):

View File

@ -640,4 +640,4 @@ def update_config(data):
config("update", "-y", "config")
with ev.Environment(str(tmpdir)) as e:
assert not e.manifest.pristine_yaml_content["spack"]["config"]["ccache"]
assert not e.manifest.yaml_content["spack"]["config"]["ccache"]

View File

@ -253,7 +253,7 @@ def test_update_default_view(init_view, update_value, tmp_path, mock_packages, c
if isinstance(init_view, str) and update_value is True:
expected_value = init_view
assert env.manifest.pristine_yaml_content["spack"]["view"] == expected_value
assert env.manifest.yaml_content["spack"]["view"] == expected_value
@pytest.mark.parametrize(
@ -384,7 +384,7 @@ def test_can_add_specs_to_environment_without_specs_attribute(tmp_path, mock_pac
env.add("a")
assert len(env.user_specs) == 1
assert env.manifest.pristine_yaml_content["spack"]["specs"] == ["a"]
assert env.manifest.yaml_content["spack"]["specs"] == ["a"]
@pytest.mark.parametrize(