diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 28f39dbd416..ac05919f025 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -1356,14 +1356,8 @@ def _test_detection_by_executable(pkgs, debug_log, error_cls): def _compare_extra_attribute(_expected, _detected, *, _spec): result = [] - # Check items are of the same type - if not isinstance(_detected, type(_expected)): - _summary = f'{pkg_name}: error when trying to detect "{_expected}"' - _details = [f"{_detected} was detected instead"] - return [error_cls(summary=_summary, details=_details)] - # If they are string expected is a regex - if isinstance(_expected, str): + if isinstance(_expected, str) and isinstance(_detected, str): try: _regex = re.compile(_expected) except re.error: @@ -1379,7 +1373,7 @@ def _compare_extra_attribute(_expected, _detected, *, _spec): _details = [f"{_detected} does not match the regex"] return [error_cls(summary=_summary, details=_details)] - if isinstance(_expected, dict): + elif isinstance(_expected, dict) and isinstance(_detected, dict): _not_detected = set(_expected.keys()) - set(_detected.keys()) if _not_detected: _summary = f"{pkg_name}: cannot detect some attributes for spec {_spec}" @@ -1394,6 +1388,10 @@ def _compare_extra_attribute(_expected, _detected, *, _spec): result.extend( _compare_extra_attribute(_expected[_key], _detected[_key], _spec=_spec) ) + else: + _summary = f'{pkg_name}: error when trying to detect "{_expected}"' + _details = [f"{_detected} was detected instead"] + return [error_cls(summary=_summary, details=_details)] return result diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index 37076ce9933..9835ddaca69 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -2332,11 +2332,16 @@ def to_node_dict(self, hash=ht.dag_hash): ) if self.external: + if self.extra_attributes: + extra_attributes = syaml.sorted_dict(self.extra_attributes) + else: + extra_attributes = None + d["external"] = syaml.syaml_dict( [ ("path", self.external_path), ("module", self.external_modules), - ("extra_attributes", self.extra_attributes), + ("extra_attributes", extra_attributes), ] ) diff --git a/lib/spack/spack/test/spack_yaml.py b/lib/spack/spack/test/spack_yaml.py index d3bea4e66d0..0a27a219c36 100644 --- a/lib/spack/spack/test/spack_yaml.py +++ b/lib/spack/spack/test/spack_yaml.py @@ -137,3 +137,19 @@ def test_round_trip_configuration(initial_content, expected_final_content, tmp_p expected_final_content = initial_content assert final_content.getvalue() == expected_final_content + + +def test_sorted_dict(): + assert syaml.sorted_dict( + { + "z": 0, + "y": [{"x": 0, "w": [2, 1, 0]}, 0], + "v": ({"u": 0, "t": 0, "s": 0}, 0, {"r": 0, "q": 0}), + "p": 0, + } + ) == { + "p": 0, + "v": ({"s": 0, "t": 0, "u": 0}, 0, {"q": 0, "r": 0}), + "y": [{"w": [2, 1, 0], "x": 0}, 0], + "z": 0, + } diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index 59fe217b67d..8656a7ad011 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -21,6 +21,7 @@ import ruamel.yaml import spack.concretize +import spack.config import spack.hash_types as ht import spack.paths import spack.repo @@ -143,89 +144,83 @@ def descend_and_check(iterable, level=0): assert level >= 5 -def test_ordered_read_not_required_for_consistent_dag_hash(config, mock_packages): +@pytest.mark.parametrize("spec_str", ["mpileaks ^zmpi", "dttop", "dtuse"]) +def test_ordered_read_not_required_for_consistent_dag_hash( + spec_str, mutable_config: spack.config.Configuration, mock_packages +): """Make sure ordered serialization isn't required to preserve hashes. - For consistent hashes, we require that YAML and json documents - have their keys serialized in a deterministic order. However, we - don't want to require them to be serialized in order. This - ensures that is not required. - """ - specs = ["mpileaks ^zmpi", "dttop", "dtuse"] - for spec in specs: - spec = spack.concretize.concretize_one(spec) + For consistent hashes, we require that YAML and JSON serializations have their keys in a + deterministic order. However, we don't want to require them to be serialized in order. This + ensures that is not required.""" - # - # Dict & corresponding YAML & JSON from the original spec. - # - spec_dict = spec.to_dict() - spec_yaml = spec.to_yaml() - spec_json = spec.to_json() + # Make sure that `extra_attributes` of externals is order independent for hashing. + extra_attributes = { + "compilers": {"c": "/some/path/bin/cc", "cxx": "/some/path/bin/c++"}, + "foo": "bar", + "baz": "qux", + } + mutable_config.set( + "packages:dtuse", + { + "buildable": False, + "externals": [ + {"spec": "dtuse@=1.0", "prefix": "/usr", "extra_attributes": extra_attributes} + ], + }, + ) - # - # Make a spec with reversed OrderedDicts for every - # OrderedDict in the original. - # - reversed_spec_dict = reverse_all_dicts(spec.to_dict()) + spec = spack.concretize.concretize_one(spec_str) - # - # Dump to YAML and JSON - # - yaml_string = syaml.dump(spec_dict, default_flow_style=False) - reversed_yaml_string = syaml.dump(reversed_spec_dict, default_flow_style=False) - json_string = sjson.dump(spec_dict) - reversed_json_string = sjson.dump(reversed_spec_dict) + if spec_str == "dtuse": + assert spec.external and spec.extra_attributes == extra_attributes - # - # Do many consistency checks - # + spec_dict = spec.to_dict(hash=ht.dag_hash) + spec_yaml = spec.to_yaml() + spec_json = spec.to_json() - # spec yaml is ordered like the spec dict - assert yaml_string == spec_yaml - assert json_string == spec_json + # Make a spec with dict keys reversed recursively + spec_dict_rev = reverse_all_dicts(spec_dict) - # reversed string is different from the original, so it - # *would* generate a different hash - assert yaml_string != reversed_yaml_string - assert json_string != reversed_json_string + # Dump to YAML and JSON + yaml_string = syaml.dump(spec_dict, default_flow_style=False) + yaml_string_rev = syaml.dump(spec_dict_rev, default_flow_style=False) + json_string = sjson.dump(spec_dict) + json_string_rev = sjson.dump(spec_dict_rev) - # build specs from the "wrongly" ordered data - round_trip_yaml_spec = Spec.from_yaml(yaml_string) - round_trip_json_spec = Spec.from_json(json_string) - round_trip_reversed_yaml_spec = Spec.from_yaml(reversed_yaml_string) - round_trip_reversed_json_spec = Spec.from_yaml(reversed_json_string) + # spec yaml is ordered like the spec dict + assert yaml_string == spec_yaml + assert json_string == spec_json - # Strip spec if we stripped the yaml - spec = spec.copy(deps=ht.dag_hash.depflag) + # reversed string is different from the original, so it *would* generate a different hash + assert yaml_string != yaml_string_rev + assert json_string != json_string_rev - # specs are equal to the original - assert spec == round_trip_yaml_spec - assert spec == round_trip_json_spec + # build specs from the "wrongly" ordered data + from_yaml = Spec.from_yaml(yaml_string) + from_json = Spec.from_json(json_string) + from_yaml_rev = Spec.from_yaml(yaml_string_rev) + from_json_rev = Spec.from_json(json_string_rev) - assert spec == round_trip_reversed_yaml_spec - assert spec == round_trip_reversed_json_spec - assert round_trip_yaml_spec == round_trip_reversed_yaml_spec - assert round_trip_json_spec == round_trip_reversed_json_spec - # dag_hashes are equal - assert spec.dag_hash() == round_trip_yaml_spec.dag_hash() - assert spec.dag_hash() == round_trip_json_spec.dag_hash() - assert spec.dag_hash() == round_trip_reversed_yaml_spec.dag_hash() - assert spec.dag_hash() == round_trip_reversed_json_spec.dag_hash() + # Strip spec if we stripped the yaml + spec = spec.copy(deps=ht.dag_hash.depflag) - # dag_hash is equal after round-trip by dag_hash - spec = spack.concretize.concretize_one(spec) - round_trip_yaml_spec = spack.concretize.concretize_one(round_trip_yaml_spec) - round_trip_json_spec = spack.concretize.concretize_one(round_trip_json_spec) - round_trip_reversed_yaml_spec = spack.concretize.concretize_one( - round_trip_reversed_yaml_spec - ) - round_trip_reversed_json_spec = spack.concretize.concretize_one( - round_trip_reversed_json_spec - ) - assert spec.dag_hash() == round_trip_yaml_spec.dag_hash() - assert spec.dag_hash() == round_trip_json_spec.dag_hash() - assert spec.dag_hash() == round_trip_reversed_yaml_spec.dag_hash() - assert spec.dag_hash() == round_trip_reversed_json_spec.dag_hash() + # specs and their hashes are equal to the original + assert ( + spec.process_hash() + == from_yaml.process_hash() + == from_json.process_hash() + == from_yaml_rev.process_hash() + == from_json_rev.process_hash() + ) + assert ( + spec.dag_hash() + == from_yaml.dag_hash() + == from_json.dag_hash() + == from_yaml_rev.dag_hash() + == from_json_rev.dag_hash() + ) + assert spec == from_yaml == from_json == from_yaml_rev == from_json_rev @pytest.mark.parametrize("module", [spack.spec, spack.version]) @@ -296,13 +291,10 @@ def visit_Call(self, node): def reverse_all_dicts(data): """Descend into data and reverse all the dictionaries""" if isinstance(data, dict): - return syaml_dict( - reversed([(reverse_all_dicts(k), reverse_all_dicts(v)) for k, v in data.items()]) - ) + return type(data)((k, reverse_all_dicts(v)) for k, v in reversed(list(data.items()))) elif isinstance(data, (list, tuple)): return type(data)(reverse_all_dicts(elt) for elt in data) - else: - return data + return data def check_specs_equal(original_spec, spec_yaml_path): diff --git a/lib/spack/spack/util/spack_yaml.py b/lib/spack/spack/util/spack_yaml.py index 6d15875ac3f..6a6db7d4575 100644 --- a/lib/spack/spack/util/spack_yaml.py +++ b/lib/spack/spack/util/spack_yaml.py @@ -448,20 +448,13 @@ def _dump_annotated(handler, data, stream=None): return getvalue() -def sorted_dict(dict_like): - """Return an ordered dict with all the fields sorted recursively. - - Args: - dict_like (dict): dictionary to be sorted - - Returns: - dictionary sorted recursively - """ - result = syaml_dict(sorted(dict_like.items())) - for key, value in result.items(): - if isinstance(value, collections.abc.Mapping): - result[key] = sorted_dict(value) - return result +def sorted_dict(data): + """Descend into data and sort all dictionary keys.""" + if isinstance(data, dict): + return type(data)((k, sorted_dict(v)) for k, v in sorted(data.items())) + elif isinstance(data, (list, tuple)): + return type(data)(sorted_dict(v) for v in data) + return data def extract_comments(data):