Spec.format: fix bug in dependency hash formatting (#37073)

Co-authored-by: becker33 <becker33@users.noreply.github.com>

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

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"),
+            ("{NAME}", "", "name", lambda spec: spec),
-            ("{VERSION}", "versions"),
+            ("{VERSION}", "", "versions", lambda spec: spec),
-            ("{compiler}", "compiler"),
+            ("{compiler}", "", "compiler", lambda spec: spec),
-            ("{compiler_flags}", "compiler_flags"),
+            ("{compiler_flags}", "", "compiler_flags", lambda spec: spec),
-            ("{variants}", "variants"),
+            ("{variants}", "", "variants", lambda spec: spec),
-            ("{architecture}", "architecture"),
+            ("{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 = [
+        hash_segments = [
-            ("{@VERSIONS}", "@" + str(spec.version)),
+            ("{hash:7}", "", lambda s: s.dag_hash(7)),
-            ("{%compiler}", "%" + str(spec.compiler)),
+            ("{/hash}", "/", lambda s: "/" + s.dag_hash()),
-            ("{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"),
         ]
 
         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:
+        def depify(depname, fmt_str, sigil):
-            expected = getattr(spec, prop, "")
+            sig = len(sigil)
-            actual = spec.format(named_str)
+            opening = fmt_str[: 1 + sig]
-            assert str(expected).strip() == actual
+            closing = fmt_str[1 + sig :]
+            return spec[depname], opening + f"^{depname}." + closing
 
-        for named_str, expected in sigil_package_segments:
+        def check_prop(check_spec, fmt_str, prop, getter):
-            actual = spec.format(named_str)
+            actual = spec.format(fmt_str)
-            assert expected == actual
+            expected = getter(check_spec)
+            assert actual == str(expected).strip()
 
-        compiler = spec.compiler
+        for named_str, sigil, prop, get_component in package_segments:
-        for named_str, prop in compiler_segments:
+            getter = lambda s: sigil + str(getattr(get_component(s), prop, ""))
-            expected = getattr(compiler, prop, "")
+            check_prop(spec, named_str, prop, getter)
-            actual = spec.format(named_str)
+            mpi, fmt_str = depify("mpi", named_str, sigil)
-            assert str(expected) == actual
+            check_prop(mpi, fmt_str, prop, getter)
 
-        for named_str, expected in sigil_compiler_segments:
+        for named_str, sigil, getter in hash_segments:
-            actual = spec.format(named_str)
+            assert spec.format(named_str) == getter(spec)
-            assert expected == actual
+            callpath, fmt_str = depify("callpath", named_str, sigil)
-
+            assert spec.format(fmt_str) == getter(callpath)
-        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, expected in other_segments:
             actual = spec.format(named_str)