From abad16c1983685772f5c4ac36f1a9fecfe276793 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 21 Sep 2023 17:02:34 +0200
Subject: [PATCH] Restore virtuals normalization on edge construction (#40130)

Put back normalization of the "virtuals" input as a sorted tuple.

Without this we might get edges that differ just for the order of
virtuals, or that have lists, which are not hashable.

Add unit-tests to prevent regressions.
---
 lib/spack/spack/spec.py                |  2 +-
 lib/spack/spack/test/spec_semantics.py | 13 +++++++++++++
 lib/spack/spack/test/spec_yaml.py      |  5 +++++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/spec.py b/lib/spack/spack/spec.py
index c88998d54ed..13bf66f506e 100644
--- a/lib/spack/spack/spec.py
+++ b/lib/spack/spack/spec.py
@@ -740,7 +740,7 @@ def __init__(
         self.parent = parent
         self.spec = spec
         self.depflag = depflag
-        self.virtuals = virtuals
+        self.virtuals = tuple(sorted(set(virtuals)))
 
     def update_deptypes(self, depflag: dt.DepFlag) -> bool:
         """Update the current dependency types"""
diff --git a/lib/spack/spack/test/spec_semantics.py b/lib/spack/spack/test/spec_semantics.py
index 4f8ae8054a5..662ea5ef0e0 100644
--- a/lib/spack/spack/test/spec_semantics.py
+++ b/lib/spack/spack/test/spec_semantics.py
@@ -11,6 +11,7 @@
 from spack.spec import (
     ArchSpec,
     CompilerSpec,
+    DependencySpec,
     Spec,
     SpecFormatSigilError,
     SpecFormatStringError,
@@ -1326,3 +1327,15 @@ def assert_disjoint(a: Spec, b: Spec):
     # disjoint concretization space
     assert_disjoint(abstract_none, concrete)
     assert_disjoint(abstract_none, abstract_5)
+
+
+def test_edge_equality_does_not_depend_on_virtual_order():
+    """Tests that two edges that are constructed with just a different order of the virtuals in
+    the input parameters are equal to each other.
+    """
+    parent, child = Spec("parent"), Spec("child")
+    edge1 = DependencySpec(parent, child, depflag=0, virtuals=("mpi", "lapack"))
+    edge2 = DependencySpec(parent, child, depflag=0, virtuals=("lapack", "mpi"))
+    assert edge1 == edge2
+    assert tuple(sorted(edge1.virtuals)) == edge1.virtuals
+    assert tuple(sorted(edge2.virtuals)) == edge1.virtuals
diff --git a/lib/spack/spack/test/spec_yaml.py b/lib/spack/spack/test/spec_yaml.py
index 3599606f1cf..b19bb4308f9 100644
--- a/lib/spack/spack/test/spec_yaml.py
+++ b/lib/spack/spack/test/spec_yaml.py
@@ -500,3 +500,8 @@ def test_load_json_specfiles(specfile, expected_hash, reader_cls):
 
     openmpi_edges = s2.edges_to_dependencies(name="openmpi")
     assert len(openmpi_edges) == 1
+
+    # The virtuals attribute must be a tuple, when read from a
+    # JSON or YAML file, not a list
+    for edge in s2.traverse_edges():
+        assert isinstance(edge.virtuals, tuple), edge