environment.matching_spec: linear time traversal (#35534)
... and use colors in disambiguate message for clarity. This commit avoids the loop: ``` for root in roots: for dep in deps(root): ... ``` instead it ensures each node is visited once and only once. Also adds a small optimization when searching for concrete specs, since we can assume uniqueness of dag hash, so it's fine to early exit.
This commit is contained in:
parent
1711e186fe
commit
96b205ce6c
1 changed files with 38 additions and 36 deletions
|
@ -1863,6 +1863,7 @@ def get_one_by_hash(self, dag_hash):
|
||||||
assert len(hash_matches) == 1
|
assert len(hash_matches) == 1
|
||||||
return hash_matches[0]
|
return hash_matches[0]
|
||||||
|
|
||||||
|
@spack.repo.autospec
|
||||||
def matching_spec(self, spec):
|
def matching_spec(self, spec):
|
||||||
"""
|
"""
|
||||||
Given a spec (likely not concretized), find a matching concretized
|
Given a spec (likely not concretized), find a matching concretized
|
||||||
|
@ -1880,59 +1881,60 @@ def matching_spec(self, spec):
|
||||||
and multiple dependency specs match, then this raises an error
|
and multiple dependency specs match, then this raises an error
|
||||||
and reports all matching specs.
|
and reports all matching specs.
|
||||||
"""
|
"""
|
||||||
# Root specs will be keyed by concrete spec, value abstract
|
env_root_to_user = {root.dag_hash(): user for user, root in self.concretized_specs()}
|
||||||
# Dependency-only specs will have value None
|
root_matches, dep_matches = [], []
|
||||||
matches = {}
|
|
||||||
|
|
||||||
if not isinstance(spec, spack.spec.Spec):
|
for env_spec in spack.traverse.traverse_nodes(
|
||||||
spec = spack.spec.Spec(spec)
|
specs=[root for _, root in self.concretized_specs()],
|
||||||
|
key=lambda s: s.dag_hash(),
|
||||||
for user_spec, concretized_user_spec in self.concretized_specs():
|
order="breadth",
|
||||||
# Deal with concrete specs differently
|
):
|
||||||
if spec.concrete:
|
if not env_spec.satisfies(spec):
|
||||||
if spec in concretized_user_spec:
|
|
||||||
matches[spec] = spec
|
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if concretized_user_spec.satisfies(spec):
|
# If the spec is concrete, then there is no possibility of multiple matches,
|
||||||
matches[concretized_user_spec] = user_spec
|
# and we immediately return the single match
|
||||||
for dep_spec in concretized_user_spec.traverse(root=False):
|
if spec.concrete:
|
||||||
if dep_spec.satisfies(spec):
|
return env_spec
|
||||||
# Don't overwrite the abstract spec if present
|
|
||||||
# If not present already, set to None
|
|
||||||
matches[dep_spec] = matches.get(dep_spec, None)
|
|
||||||
|
|
||||||
if not matches:
|
# Distinguish between environment roots and deps. Specs that are both
|
||||||
|
# are classified as environment roots.
|
||||||
|
user_spec = env_root_to_user.get(env_spec.dag_hash())
|
||||||
|
if user_spec:
|
||||||
|
root_matches.append((env_spec, user_spec))
|
||||||
|
else:
|
||||||
|
dep_matches.append(env_spec)
|
||||||
|
|
||||||
|
# No matching spec
|
||||||
|
if not root_matches and not dep_matches:
|
||||||
return None
|
return None
|
||||||
elif len(matches) == 1:
|
|
||||||
return list(matches.keys())[0]
|
|
||||||
|
|
||||||
root_matches = dict(
|
|
||||||
(concrete, abstract) for concrete, abstract in matches.items() if abstract
|
|
||||||
)
|
|
||||||
|
|
||||||
|
# Single root spec, any number of dep specs => return root spec.
|
||||||
if len(root_matches) == 1:
|
if len(root_matches) == 1:
|
||||||
return list(root_matches.items())[0][0]
|
return root_matches[0][0]
|
||||||
|
|
||||||
|
if not root_matches and len(dep_matches) == 1:
|
||||||
|
return dep_matches[0]
|
||||||
|
|
||||||
# More than one spec matched, and either multiple roots matched or
|
# More than one spec matched, and either multiple roots matched or
|
||||||
# none of the matches were roots
|
# none of the matches were roots
|
||||||
# If multiple root specs match, it is assumed that the abstract
|
# If multiple root specs match, it is assumed that the abstract
|
||||||
# spec will most-succinctly summarize the difference between them
|
# spec will most-succinctly summarize the difference between them
|
||||||
# (and the user can enter one of these to disambiguate)
|
# (and the user can enter one of these to disambiguate)
|
||||||
match_strings = []
|
|
||||||
fmt_str = "{hash:7} " + spack.spec.default_format
|
fmt_str = "{hash:7} " + spack.spec.default_format
|
||||||
for concrete, abstract in matches.items():
|
color = clr.get_color_when()
|
||||||
if abstract:
|
match_strings = [
|
||||||
s = "Root spec %s\n %s" % (abstract, concrete.format(fmt_str))
|
f"Root spec {abstract.format(color=color)}\n {concrete.format(fmt_str, color=color)}"
|
||||||
else:
|
for concrete, abstract in root_matches
|
||||||
s = "Dependency spec\n %s" % concrete.format(fmt_str)
|
]
|
||||||
match_strings.append(s)
|
match_strings.extend(
|
||||||
|
f"Dependency spec\n {s.format(fmt_str, color=color)}" for s in dep_matches
|
||||||
|
)
|
||||||
matches_str = "\n".join(match_strings)
|
matches_str = "\n".join(match_strings)
|
||||||
|
|
||||||
msg = "{0} matches multiple specs in the environment {1}: \n" "{2}".format(
|
raise SpackEnvironmentError(
|
||||||
str(spec), self.name, matches_str
|
f"{spec} matches multiple specs in the environment {self.name}: \n{matches_str}"
|
||||||
)
|
)
|
||||||
raise SpackEnvironmentError(msg)
|
|
||||||
|
|
||||||
def removed_specs(self):
|
def removed_specs(self):
|
||||||
"""Tuples of (user spec, concrete spec) for all specs that will be
|
"""Tuples of (user spec, concrete spec) for all specs that will be
|
||||||
|
|
Loading…
Reference in a new issue