environment: key by dag_hash instead of build_hash

This commit is contained in:
Scott Wittenburg 2022-01-24 21:42:45 -07:00 committed by Todd Gamblin
parent 32a2c22b2b
commit 512645ff2e
5 changed files with 68 additions and 45 deletions

View File

@ -202,7 +202,7 @@ def format_root_spec(spec, main_phase, strip_compiler):
return '{0}@{1} arch={2}'.format(
spec.name, spec.version, spec.architecture)
else:
return spec.build_hash()
return spec.dag_hash()
def spec_deps_key(s):

View File

@ -94,7 +94,7 @@
valid_environment_name_re = r'^\w[\w-]*$'
#: version of the lockfile format. Must increase monotonically.
lockfile_format_version = 3
lockfile_format_version = 4
# Magic names
# The name of the standalone spec list in the manifest yaml
@ -439,7 +439,7 @@ def _next_root(self, specs):
def content_hash(self, specs):
d = syaml.syaml_dict([
('descriptor', self.to_dict()),
('specs', [(spec.full_hash(), spec.prefix) for spec in sorted(specs)])
('specs', [(spec.dag_hash(), spec.prefix) for spec in sorted(specs)])
])
contents = sjson.dump(d)
return spack.util.hash.b32_hash(contents)
@ -1010,14 +1010,9 @@ def remove(self, query_spec, list_name=user_speclist_name, force=False):
if not matches:
# concrete specs match against concrete specs in the env
# by *dag hash*, not build hash.
dag_hashes_in_order = [
self.specs_by_hash[build_hash].dag_hash()
for build_hash in self.concretized_order
]
# by dag hash.
specs_hashes = zip(
self.concretized_user_specs, dag_hashes_in_order
self.concretized_user_specs, self.concretized_order
)
matches = [
@ -1331,7 +1326,7 @@ def concretize_and_add(self, user_spec, concrete_spec=None, tests=False):
spec = next(
s for s in self.user_specs if s.satisfies(user_spec)
)
concrete = self.specs_by_hash.get(spec.build_hash())
concrete = self.specs_by_hash.get(spec.dag_hash())
if not concrete:
concrete = spec.concretized(tests=tests)
self._add_concrete_spec(spec, concrete)
@ -1499,7 +1494,7 @@ def _add_concrete_spec(self, spec, concrete, new=True):
# update internal lists of specs
self.concretized_user_specs.append(spec)
h = concrete.build_hash()
h = concrete.dag_hash()
self.concretized_order.append(h)
self.specs_by_hash[h] = concrete
@ -1621,9 +1616,7 @@ def all_specs(self):
return sorted(all_specs)
def all_hashes(self):
"""Return hashes of all specs.
Note these hashes exclude build dependencies."""
"""Return hashes of all specs."""
return list(set(s.dag_hash() for s in self.all_specs()))
def roots(self):
@ -1695,11 +1688,10 @@ def matching_spec(self, spec):
for user_spec, concretized_user_spec in self.concretized_specs():
# Deal with concrete specs differently
if spec.concrete:
# Matching a concrete spec is more restrictive
# than just matching the dag hash
# TODO: do we still need the extra check comparing dag hashes?
is_match = (
spec in concretized_user_spec and
concretized_user_spec[spec.name].build_hash() == spec.build_hash()
concretized_user_spec[spec.name].dag_hash() == spec.dag_hash()
)
if is_match:
matches[spec] = spec
@ -1781,12 +1773,12 @@ def _to_lockfile_dict(self):
concrete_specs = {}
for spec in self.specs_by_hash.values():
for s in spec.traverse():
build_hash = s.build_hash()
if build_hash not in concrete_specs:
spec_dict = s.to_node_dict(hash=ht.build_hash)
dag_hash = s.dag_hash()
if dag_hash not in concrete_specs:
spec_dict = s.node_dict_with_hashes(hash=ht.dag_hash)
# Assumes no legacy formats, since this was just created.
spec_dict[ht.dag_hash.name] = s.dag_hash()
concrete_specs[build_hash] = spec_dict
concrete_specs[dag_hash] = spec_dict
hash_spec_list = zip(
self.concretized_order, self.concretized_user_specs)
@ -1828,33 +1820,53 @@ def _read_lockfile_dict(self, d):
root_hashes = set(self.concretized_order)
specs_by_hash = {}
for build_hash, node_dict in json_specs_by_hash.items():
for dag_hash, node_dict in json_specs_by_hash.items():
spec = Spec.from_node_dict(node_dict)
if d['_meta']['lockfile-version'] > 1:
# Build hash is stored as a key, but not as part of the node dict
# To ensure build hashes are not recomputed, we reattach here
setattr(spec, ht.build_hash.attr, build_hash)
specs_by_hash[build_hash] = spec
setattr(spec, ht.dag_hash.attr, dag_hash)
specs_by_hash[dag_hash] = spec
for build_hash, node_dict in json_specs_by_hash.items():
for dag_hash, node_dict in json_specs_by_hash.items():
for _, dep_hash, deptypes, _ in (
Spec.dependencies_from_node_dict(node_dict)):
specs_by_hash[build_hash]._add_dependency(
specs_by_hash[dag_hash]._add_dependency(
specs_by_hash[dep_hash], deptypes)
# If we are reading an older lockfile format (which uses dag hashes
# that exclude build deps), we use this to convert the old
# concretized_order to the full hashes (preserving the order)
# Current lockfile key: dag_hash() (dag_hash() == full_hash())
# Previous lockfile keys from most recent to least:
# 1. build_hash
# 2. dag_hash (computed *without* build deps)
# If we are reading an older lockfile format, the key may have been computed
# using a different hash type than the one spack uses currently (which
# includes build deps as well as the package hash). If this is the case
# the following code updates the keys in in 'concretized_order' to be computed
# using the hash type spack currently uses, while maintaining the order of the
# list.
old_hash_to_new = {}
self.specs_by_hash = {}
for _, spec in specs_by_hash.items():
dag_hash = spec.dag_hash()
# - to get old dag_hash() (w/out build deps) use runtime_hash() now
# - dag_hash() now includes build deps and package hash
# - i.e. dag_hash() == full_hash()
# - regardless of what hash type keyed the lockfile we're reading,
# the dag_hash we read from the file may appear appear in install
# trees and binary mirrors, and as such, must be considered the
# permanent id of the spec.
dag_hash = spec.dag_hash() # == full_hash()
build_hash = spec.build_hash()
if dag_hash in root_hashes:
old_hash_to_new[dag_hash] = build_hash
runtime_hash = spec.runtime_hash() # == old dag_hash()
if (dag_hash in root_hashes or build_hash in root_hashes):
self.specs_by_hash[build_hash] = spec
if runtime_hash in root_hashes:
old_hash_to_new[runtime_hash] = dag_hash
elif build_hash in root_hashes:
old_hash_to_new[build_hash] = dag_hash
if (runtime_hash in root_hashes or
build_hash in root_hashes or dag_hash in root_hashes):
self.specs_by_hash[dag_hash] = spec
if old_hash_to_new:
# Replace any older hashes in concretized_order with hashes

View File

@ -2103,6 +2103,8 @@ def node_dict_with_hashes(self, hash=ht.dag_hash):
node[hash.name] = self.build_hash()
elif hash.name == 'process_hash':
node[hash.name] = self.process_hash()
elif hash.name == 'runtime_hash':
node[hash.name] = self.runtime_hash()
return node
@ -2244,11 +2246,17 @@ def read_yaml_dep_specs(deps, hash_type=ht.dag_hash.name):
dep_hash, deptypes = elt
elif isinstance(elt, dict):
# new format: elements of dependency spec are keyed.
for key in (ht.full_hash.name,
for key in (ht.dag_hash.name,
ht.full_hash.name,
ht.build_hash.name,
ht.dag_hash.name,
ht.runtime_hash.name,
ht.process_hash.name):
if key in elt:
# FIXME: if the key is 'hash' it could mean the old
# dag hash without build deps, or the new dag hash which
# is equivalent to the full hash. If this was the old
# dag hash, we need to keep the hash value but set the
# key hash type to "runtime_hash".
dep_hash, deptypes = elt[key], elt['type']
hash_type = key
break
@ -3793,6 +3801,7 @@ def _dup(self, other, deps=True, cleardeps=True):
self._dunder_hash = other._dunder_hash
self._normal = True
self._full_hash = other._full_hash
self._runtime_hash = other._runtime_hash
self._package_hash = other._package_hash
else:
self._hash = None
@ -3802,6 +3811,7 @@ def _dup(self, other, deps=True, cleardeps=True):
# always set it False here to avoid the complexity of checking
self._normal = False
self._full_hash = None
self._runtime_hash = None
self._package_hash = None
return changed
@ -4741,6 +4751,7 @@ def from_self(name, transitive):
for dep in ret.traverse(root=True, order='post'):
opposite = other_nodes if dep.name in self_nodes else self_nodes
if any(name in dep for name in opposite.keys()):
# package hash cannot be affected by splice
dep.clear_cached_hashes(ignore=['package_hash'])

View File

@ -96,9 +96,9 @@ def test_get_concrete_specs(config, mutable_mock_env_path, mock_packages):
with e as active_env:
for s in active_env.all_specs():
hash_dict[s.name] = s.build_hash()
hash_dict[s.name] = s.dag_hash()
if s.name == 'dyninst':
dyninst_hash = s.build_hash()
dyninst_hash = s.dag_hash()
assert(dyninst_hash)
@ -107,7 +107,7 @@ def test_get_concrete_specs(config, mutable_mock_env_path, mock_packages):
assert 'root' in spec_map
concrete_root = spec_map['root']
assert(concrete_root.build_hash() == dyninst_hash)
assert(concrete_root.dag_hash() == dyninst_hash)
s = spec.Spec('dyninst')
print('nonconc spec name: {0}'.format(s.name))

View File

@ -975,14 +975,14 @@ def create_v1_lockfile_dict(roots, all_specs):
},
"roots": list(
{
"hash": s.dag_hash(),
"hash": s.runtime_hash(),
"spec": s.name
} for s in roots
),
# Version one lockfiles use the dag hash without build deps as keys,
# but they write out the full node dict (including build deps)
"concrete_specs": dict(
(s.dag_hash(), s.to_node_dict(hash=ht.build_hash))
(s.runtime_hash(), s.to_node_dict(hash=ht.build_hash))
for s in all_specs
)
}
@ -1016,8 +1016,8 @@ def test_read_old_lock_and_write_new(tmpdir):
# When the lockfile is rewritten, it should adopt the new hash scheme
# which accounts for all dependencies, including build dependencies
assert hashes == set([
x.build_hash(),
y.build_hash()])
x.dag_hash(),
y.dag_hash()])
@pytest.mark.usefixtures('config')
@ -1118,7 +1118,7 @@ def noop(*args):
# according to the DAG hash (since build deps are excluded from
# comparison by default). Although the dag hashes are equal, the specs
# are not considered equal because they compare build deps.
assert x_concretized['y'].dag_hash() == y_concretized.dag_hash()
assert x_concretized['y'].runtime_hash() == y_concretized.runtime_hash()
_env_create('test', with_view=False)
e = ev.read('test')