From de5a396ecb28bbe284fcca82dd4c773806a703a8 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 19 Feb 2021 00:51:00 -0800 Subject: bugfix: add build deps to 'full hash' (#21735) The "full hash" was only including the link/run deps, but it should include build deps as well. --- lib/spack/spack/directory_layout.py | 5 ++++- lib/spack/spack/hash_types.py | 4 ++-- lib/spack/spack/package.py | 8 ++++++- lib/spack/spack/test/spec_yaml.py | 43 ++++++++++++++++++++++--------------- 4 files changed, 39 insertions(+), 21 deletions(-) (limited to 'lib') diff --git a/lib/spack/spack/directory_layout.py b/lib/spack/spack/directory_layout.py index 145421130a..cbaa9b47fd 100644 --- a/lib/spack/spack/directory_layout.py +++ b/lib/spack/spack/directory_layout.py @@ -15,6 +15,7 @@ import ruamel.yaml as yaml from llnl.util.filesystem import mkdirp import spack.config +import spack.hash_types as ht import spack.spec from spack.error import SpackError @@ -242,7 +243,9 @@ class YamlDirectoryLayout(DirectoryLayout): """Write a spec out to a file.""" _check_concrete(spec) with open(path, 'w') as f: - spec.to_yaml(f) + # The hash the the projection is the DAG hash but we write out the + # full provenance by full hash so it's availabe if we want it later + spec.to_yaml(f, hash=ht.full_hash) def read_spec(self, path): """Read the contents of a file and parse them as a spec""" diff --git a/lib/spack/spack/hash_types.py b/lib/spack/spack/hash_types.py index 38429f5e41..0ad321dec6 100644 --- a/lib/spack/spack/hash_types.py +++ b/lib/spack/spack/hash_types.py @@ -35,5 +35,5 @@ build_hash = SpecHashDescriptor( #: Full hash used in build pipelines to determine when to rebuild packages. -full_hash = SpecHashDescriptor(deptype=('link', 'run'), package_hash=True, - attr='_full_hash') +full_hash = SpecHashDescriptor( + deptype=('build', 'link', 'run'), package_hash=True, attr='_full_hash') diff --git a/lib/spack/spack/package.py b/lib/spack/spack/package.py index 0c09410c6d..9dfd8bf01f 100644 --- a/lib/spack/spack/package.py +++ b/lib/spack/spack/package.py @@ -1526,10 +1526,16 @@ class PackageBase(six.with_metaclass(PackageMeta, PackageViewMixin, object)): hash_content.extend(':'.join((p.sha256, str(p.level))).encode('utf-8') for p in self.spec.patches) hash_content.append(package_hash(self.spec, content)) - return base64.b32encode( + b32_hash = base64.b32encode( hashlib.sha256(bytes().join( sorted(hash_content))).digest()).lower() + # convert from bytes if running python 3 + if sys.version_info[0] >= 3: + b32_hash = b32_hash.decode('utf-8') + + return b32_hash + def _has_make_target(self, target): """Checks to see if 'target' is a valid target in a Makefile. diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py index 2de0ca09e8..74ebf0a541 100644 --- a/lib/spack/spack/test/spec_yaml.py +++ b/lib/spack/spack/test/spec_yaml.py @@ -130,8 +130,13 @@ def test_to_record_dict(mock_packages, config): assert record[key] == value +@pytest.mark.parametrize("hash_type", [ + ht.dag_hash, + ht.build_hash, + ht.full_hash +]) def test_ordered_read_not_required_for_consistent_dag_hash( - config, mock_packages + hash_type, config, mock_packages ): """Make sure ordered serialization isn't required to preserve hashes. @@ -148,15 +153,15 @@ def test_ordered_read_not_required_for_consistent_dag_hash( # # Dict & corresponding YAML & JSON from the original spec. # - spec_dict = spec.to_dict() - spec_yaml = spec.to_yaml() - spec_json = spec.to_json() + spec_dict = spec.to_dict(hash=hash_type) + spec_yaml = spec.to_yaml(hash=hash_type) + spec_json = spec.to_json(hash=hash_type) # # Make a spec with reversed OrderedDicts for every # OrderedDict in the original. # - reversed_spec_dict = reverse_all_dicts(spec.to_dict()) + reversed_spec_dict = reverse_all_dicts(spec.to_dict(hash=hash_type)) # # Dump to YAML and JSON @@ -190,11 +195,13 @@ def test_ordered_read_not_required_for_consistent_dag_hash( reversed_json_string ) - # TODO: remove this when build deps are in provenance. - spec = spec.copy(deps=('link', 'run')) + # Strip spec if we stripped the yaml + spec = spec.copy(deps=hash_type.deptype) + # specs are equal to the original assert spec == round_trip_yaml_spec assert spec == round_trip_json_spec + assert spec == round_trip_reversed_yaml_spec assert spec == round_trip_reversed_json_spec assert round_trip_yaml_spec == round_trip_reversed_yaml_spec @@ -204,16 +211,18 @@ def test_ordered_read_not_required_for_consistent_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() - # full_hashes are equal - spec.concretize() - round_trip_yaml_spec.concretize() - round_trip_json_spec.concretize() - round_trip_reversed_yaml_spec.concretize() - round_trip_reversed_json_spec.concretize() - assert spec.full_hash() == round_trip_yaml_spec.full_hash() - assert spec.full_hash() == round_trip_json_spec.full_hash() - assert spec.full_hash() == round_trip_reversed_yaml_spec.full_hash() - assert spec.full_hash() == round_trip_reversed_json_spec.full_hash() + + # full_hashes are equal if we round-tripped by build_hash or full_hash + if hash_type in (ht.build_hash, ht.full_hash): + spec.concretize() + round_trip_yaml_spec.concretize() + round_trip_json_spec.concretize() + round_trip_reversed_yaml_spec.concretize() + round_trip_reversed_json_spec.concretize() + assert spec.full_hash() == round_trip_yaml_spec.full_hash() + assert spec.full_hash() == round_trip_json_spec.full_hash() + assert spec.full_hash() == round_trip_reversed_yaml_spec.full_hash() + assert spec.full_hash() == round_trip_reversed_json_spec.full_hash() @pytest.mark.parametrize("module", [ -- cgit v1.2.3-60-g2f50