Reconstruct virtual attributes on edges

alalazo · alalazo · commit 0b2db308fe19 · 2023-01-18T16:42:20.000+01:00
diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
@@ -4805,6 +4805,18 @@ def merge_abstract_anonymous_specs(*abstract_specs: Spec):
     return merged_spec
 
 
+def _reconstruct_virtuals_on_edges(spec):
+    """Reconstruct virtuals on edges. Used to read from old DB
+    and reindex.
+    """
+    possible_virtuals = [x for x in spec.package.dependencies if Spec(x).virtual]
+    for vspec in possible_virtuals:
+        if vspec in spec:
+            name = spec[vspec].name
+            for edge in spec.edges_to_dependencies(name=name):
+                edge.update_virtuals([vspec])
+
+
 class SpecfileReaderBase:
     @classmethod
     def from_node_dict(cls, node):
@@ -4869,6 +4881,64 @@ def from_node_dict(cls, node):
 
         return spec
 
+    @classmethod
+    def _load(cls, data):
+        """Construct a spec from JSON/YAML using the format version 2.
+
+        This format is used in Spack v0.17, was introduced in
+        https://github.com/spack/spack/pull/22845
+
+        Args:
+            data: a nested dict/list data structure read from YAML or JSON.
+        """
+        # Current specfile format
+        nodes = data["spec"]["nodes"]
+        hash_type = None
+        any_deps = False
+
+        # Pass 0: Determine hash type
+        for node in nodes:
+            for _, _, _, dhash_type, _ in cls.dependencies_from_node_dict(node):
+                any_deps = True
+                if dhash_type:
+                    hash_type = dhash_type
+                    break
+
+        if not any_deps:  # If we never see a dependency...
+            hash_type = ht.dag_hash.name
+        elif not hash_type:  # Seen a dependency, still don't know hash_type
+            raise spack.error.SpecError(
+                "Spec dictionary contains malformed dependencies. Old format?"
+            )
+
+        hash_dict = {}
+        root_spec_hash = None
+
+        # Pass 1: Create a single lookup dictionary by hash
+        for i, node in enumerate(nodes):
+            node_hash = node[hash_type]
+            node_spec = cls.from_node_dict(node)
+            hash_dict[node_hash] = node
+            hash_dict[node_hash]["node_spec"] = node_spec
+            if i == 0:
+                root_spec_hash = node_hash
+
+        if not root_spec_hash:
+            raise spack.error.SpecError("Spec dictionary contains no nodes.")
+
+        # Pass 2: Finish construction of all DAG edges (including build specs)
+        for node_hash, node in hash_dict.items():
+            node_spec = node["node_spec"]
+            for _, dhash, dtypes, _, virtuals in cls.dependencies_from_node_dict(node):
+                node_spec._add_dependency(
+                    hash_dict[dhash]["node_spec"], deptypes=dtypes, virtuals=virtuals
+                )
+            if "build_spec" in node.keys():
+                _, bhash, _ = cls.build_spec_from_node_dict(node, hash_type=hash_type)
+                node_spec._build_spec = hash_dict[bhash]["node_spec"]
+
+        return hash_dict[root_spec_hash]["node_spec"]
+
 
 class SpecfileV1(SpecfileReaderBase):
     @classmethod
@@ -4899,6 +4969,7 @@ def load(cls, data):
             for dname, _, dtypes, _, virtuals in cls.dependencies_from_node_dict(data):
                 deps[name]._add_dependency(deps[dname], deptypes=dtypes, virtuals=virtuals)
 
+        _reconstruct_virtuals_on_edges(result)
         return result
 
     @classmethod
@@ -4939,61 +5010,9 @@ def read_specfile_dep_specs(cls, deps, hash_type=ht.dag_hash.name):
 class SpecfileV2(SpecfileReaderBase):
     @classmethod
     def load(cls, data):
-        """Construct a spec from JSON/YAML using the format version 2.
-
-        This format is used in Spack v0.17, was introduced in
-        https://github.com/spack/spack/pull/22845
-
-        Args:
-            data: a nested dict/list data structure read from YAML or JSON.
-        """
-        # Current specfile format
-        nodes = data["spec"]["nodes"]
-        hash_type = None
-        any_deps = False
-
-        # Pass 0: Determine hash type
-        for node in nodes:
-            for _, _, _, dhash_type, _ in cls.dependencies_from_node_dict(node):
-                any_deps = True
-                if dhash_type:
-                    hash_type = dhash_type
-                    break
-
-        if not any_deps:  # If we never see a dependency...
-            hash_type = ht.dag_hash.name
-        elif not hash_type:  # Seen a dependency, still don't know hash_type
-            raise spack.error.SpecError(
-                "Spec dictionary contains malformed dependencies. Old format?"
-            )
-
-        hash_dict = {}
-        root_spec_hash = None
-
-        # Pass 1: Create a single lookup dictionary by hash
-        for i, node in enumerate(nodes):
-            node_hash = node[hash_type]
-            node_spec = cls.from_node_dict(node)
-            hash_dict[node_hash] = node
-            hash_dict[node_hash]["node_spec"] = node_spec
-            if i == 0:
-                root_spec_hash = node_hash
-
-        if not root_spec_hash:
-            raise spack.error.SpecError("Spec dictionary contains no nodes.")
-
-        # Pass 2: Finish construction of all DAG edges (including build specs)
-        for node_hash, node in hash_dict.items():
-            node_spec = node["node_spec"]
-            for _, dhash, dtypes, _, virtuals in cls.dependencies_from_node_dict(node):
-                node_spec._add_dependency(
-                    hash_dict[dhash]["node_spec"], deptypes=dtypes, virtuals=virtuals
-                )
-            if "build_spec" in node.keys():
-                _, bhash, _ = cls.build_spec_from_node_dict(node, hash_type=hash_type)
-                node_spec._build_spec = hash_dict[bhash]["node_spec"]
-
-        return hash_dict[root_spec_hash]["node_spec"]
+        result = cls._load(data)
+        _reconstruct_virtuals_on_edges(result)
+        return result
 
     @classmethod
     def name_and_data(cls, node):
@@ -5055,6 +5074,10 @@ def extract_info_from_dep(cls, elt, hash):
         virtuals = elt["parameters"]["virtuals"]
         return dep_hash, deptypes, hash_type, virtuals
 
+    @classmethod
+    def load(cls, data):
+        return cls._load(data)
+
 
 class LazySpecCache(collections.defaultdict):
     """Cache for Specs that uses a spec_like as key, and computes lazily
diff --git a/lib/spack/spack/test/cmd/test.py b/lib/spack/spack/test/cmd/test.py
@@ -258,7 +258,7 @@ def test_has_test_method_fails(capsys):
     assert "is not a class" in captured
 
 
-def test_read_old_results(mock_test_stage):
+def test_read_old_results(mock_packages, mock_test_stage):
     """Take test data generated before the switch to full hash everywhere
     and make sure we can still read it in"""
     # Test data was generated with:
diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py
@@ -515,3 +515,7 @@ def test_load_json_specfiles(specfile, expected_hash, reader_cls):
     assert s1.dag_hash() == s2.dag_hash()
     assert s1 == s2
     assert Spec.from_json(s2.to_json()).dag_hash() == s2.dag_hash()
+
+    openmpi_edges = s2.edges_to_dependencies(name="openmpi")
+    assert len(openmpi_edges) == 1
+    assert openmpi_edges[0].virtuals == ("mpi",)
