From 21cadf96e02da56a13bc9657e5740a770763e8f9 Mon Sep 17 00:00:00 2001 From: Greg Becker Date: Fri, 28 Apr 2023 14:33:05 -0700 Subject: [PATCH] Spec.format: fix bug in dependency hash formatting (#37073) Co-authored-by: becker33 --- lib/spack/spack/spec.py | 25 ++++---- lib/spack/spack/test/spec_semantics.py | 89 +++++++++++++------------- 2 files changed, 56 insertions(+), 58 deletions(-) diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py index ced35d5c7d0..78530f7a7c8 100644 --- a/lib/spack/spack/spec.py +++ b/lib/spack/spack/spec.py @@ -4115,6 +4115,17 @@ def write(s, c=None): clr.cwrite(f, stream=out, color=color) def write_attribute(spec, attribute, color): + attribute = attribute.lower() + + sig = "" + if attribute.startswith(("@", "%", "/")): + # color sigils that are inside braces + sig = attribute[0] + attribute = attribute[1:] + elif attribute.startswith("arch="): + sig = " arch=" # include space as separator + attribute = attribute[5:] + current = spec if attribute.startswith("^"): attribute = attribute[1:] @@ -4123,16 +4134,6 @@ def write_attribute(spec, attribute, color): if attribute == "": raise SpecFormatStringError("Format string attributes must be non-empty") - attribute = attribute.lower() - - sig = "" - if attribute[0] in "@%/": - # color sigils that are inside braces - sig = attribute[0] - attribute = attribute[1:] - elif attribute.startswith("arch="): - sig = " arch=" # include space as separator - attribute = attribute[5:] parts = attribute.split(".") assert parts @@ -4162,9 +4163,9 @@ def write_attribute(spec, attribute, color): col = "#" if ":" in attribute: _, length = attribute.split(":") - write(sig + morph(spec, spec.dag_hash(int(length))), col) + write(sig + morph(spec, current.dag_hash(int(length))), col) else: - write(sig + morph(spec, spec.dag_hash()), col) + write(sig + morph(spec, current.dag_hash()), col) return # Iterate over components using getattr to get next element diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py index 63e2253b688..01a8de5e46a 100644 --- a/lib/spack/spack/test/spec_semantics.py +++ b/lib/spack/spack/test/spec_semantics.py @@ -621,65 +621,62 @@ def test_spec_formatting(self, default_mock_concretization): # Testing named strings ie {string} and whether we get # the correct component # Mixed case intentional to test both + # Fields are as follow + # fmt_str: the format string to test + # sigil: the portion that is a sigil (may be empty string) + # prop: the property to get + # component: subcomponent of spec from which to get property package_segments = [ - ("{NAME}", "name"), - ("{VERSION}", "versions"), - ("{compiler}", "compiler"), - ("{compiler_flags}", "compiler_flags"), - ("{variants}", "variants"), - ("{architecture}", "architecture"), + ("{NAME}", "", "name", lambda spec: spec), + ("{VERSION}", "", "versions", lambda spec: spec), + ("{compiler}", "", "compiler", lambda spec: spec), + ("{compiler_flags}", "", "compiler_flags", lambda spec: spec), + ("{variants}", "", "variants", lambda spec: spec), + ("{architecture}", "", "architecture", lambda spec: spec), + ("{@VERSIONS}", "@", "version", lambda spec: spec), + ("{%compiler}", "%", "compiler", lambda spec: spec), + ("{arch=architecture}", "arch=", "architecture", lambda spec: spec), + ("{compiler.name}", "", "name", lambda spec: spec.compiler), + ("{compiler.version}", "", "versions", lambda spec: spec.compiler), + ("{%compiler.name}", "%", "name", lambda spec: spec.compiler), + ("{@compiler.version}", "@", "version", lambda spec: spec.compiler), + ("{architecture.platform}", "", "platform", lambda spec: spec.architecture), + ("{architecture.os}", "", "os", lambda spec: spec.architecture), + ("{architecture.target}", "", "target", lambda spec: spec.architecture), + ("{prefix}", "", "prefix", lambda spec: spec), ] - sigil_package_segments = [ - ("{@VERSIONS}", "@" + str(spec.version)), - ("{%compiler}", "%" + str(spec.compiler)), - ("{arch=architecture}", "arch=" + str(spec.architecture)), - ] - - compiler_segments = [("{compiler.name}", "name"), ("{compiler.version}", "versions")] - - sigil_compiler_segments = [ - ("{%compiler.name}", "%" + spec.compiler.name), - ("{@compiler.version}", "@" + str(spec.compiler.version)), - ] - - architecture_segments = [ - ("{architecture.platform}", "platform"), - ("{architecture.os}", "os"), - ("{architecture.target}", "target"), + hash_segments = [ + ("{hash:7}", "", lambda s: s.dag_hash(7)), + ("{/hash}", "/", lambda s: "/" + s.dag_hash()), ] other_segments = [ ("{spack_root}", spack.paths.spack_root), ("{spack_install}", spack.store.layout.root), - ("{hash:7}", spec.dag_hash(7)), - ("{/hash}", "/" + spec.dag_hash()), ] - for named_str, prop in package_segments: - expected = getattr(spec, prop, "") - actual = spec.format(named_str) - assert str(expected).strip() == actual + def depify(depname, fmt_str, sigil): + sig = len(sigil) + opening = fmt_str[: 1 + sig] + closing = fmt_str[1 + sig :] + return spec[depname], opening + f"^{depname}." + closing - for named_str, expected in sigil_package_segments: - actual = spec.format(named_str) - assert expected == actual + def check_prop(check_spec, fmt_str, prop, getter): + actual = spec.format(fmt_str) + expected = getter(check_spec) + assert actual == str(expected).strip() - compiler = spec.compiler - for named_str, prop in compiler_segments: - expected = getattr(compiler, prop, "") - actual = spec.format(named_str) - assert str(expected) == actual + for named_str, sigil, prop, get_component in package_segments: + getter = lambda s: sigil + str(getattr(get_component(s), prop, "")) + check_prop(spec, named_str, prop, getter) + mpi, fmt_str = depify("mpi", named_str, sigil) + check_prop(mpi, fmt_str, prop, getter) - for named_str, expected in sigil_compiler_segments: - actual = spec.format(named_str) - assert expected == actual - - arch = spec.architecture - for named_str, prop in architecture_segments: - expected = getattr(arch, prop, "") - actual = spec.format(named_str) - assert str(expected) == actual + for named_str, sigil, getter in hash_segments: + assert spec.format(named_str) == getter(spec) + callpath, fmt_str = depify("callpath", named_str, sigil) + assert spec.format(fmt_str) == getter(callpath) for named_str, expected in other_segments: actual = spec.format(named_str)