ASP-based solver: improve reusing nodes with gcc-runtime (#42408)

* ASP-based solver: improve reusing nodes with gcc-runtime

This PR skips emitting dependency constraints on "gcc-runtime",
for concrete specs that are considered for reuse.

Instead, an appropriate version of gcc-runtime is recomputed
considering also the concrete nodes from reused specs.

This ensures that root nodes in a DAG have always a runtime
that is at a version greater or equal than their dependencies.

* Add unit-test for view with multiple runtimes
* Select latest version of runtimes in views
* Construct result keeping track of latest
* Keep ordering stable, just in case

3 files changed, 90 insertions, 6 deletions

diff --git a/lib/spack/spack/environment/environment.py b/lib/spack/spack/environment/environment.py
index 2ffa760ccd..727b46d048 100644
--- a/lib/spack/spack/environment/environment.py
+++ b/lib/spack/spack/environment/environment.py
@@ -626,14 +626,13 @@ class ViewDescriptor:
             new: If a string, create a FilesystemView rooted at that path. Default None. This
                 should only be used to regenerate the view, and cannot be used to access specs.
         """
-        root = new if new else self._current_root
-        if not root:
+        path = new if new else self._current_root
+        if not path:
             # This can only be hit if we write a future bug
             raise SpackEnvironmentViewError(
-                "Attempting to get nonexistent view from environment. "
-                f"View root is at {self.root}"
+                f"Attempting to get nonexistent view from environment. View root is at {self.root}"
             )
-        return self._view(root)
+        return self._view(path)
 
     def _view(self, root: str) -> SimpleFilesystemView:
         """Returns a view object for a given root dir."""
@@ -678,7 +677,9 @@ class ViewDescriptor:
 
         # Filter selected, installed specs
         with spack.store.STORE.db.read_transaction():
-            return [s for s in specs if s in self and s.installed]
+            result = [s for s in specs if s in self and s.installed]
+
+        return self._exclude_duplicate_runtimes(result)
 
     def regenerate(self, concrete_roots: List[Spec]) -> None:
         specs = self.specs_for_view(concrete_roots)
@@ -765,6 +766,16 @@ class ViewDescriptor:
                 msg += str(e)
                 tty.warn(msg)
 
+    def _exclude_duplicate_runtimes(self, nodes):
+        all_runtimes = spack.repo.PATH.packages_with_tags("runtime")
+        runtimes_by_name = {}
+        for s in nodes:
+            if s.name not in all_runtimes:
+                continue
+            current_runtime = runtimes_by_name.get(s.name, s)
+            runtimes_by_name[s.name] = max(current_runtime, s, key=lambda x: x.version)
+        return [x for x in nodes if x.name not in all_runtimes or runtimes_by_name[x.name] == x]
+
 
 def _create_environment(path):
     return Environment(path)
diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 8137daea63..0ae359c006 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1786,6 +1786,11 @@ class SpackSolverSetup:
                 dep = dspec.spec
 
                 if spec.concrete:
+                    # GCC runtime is solved again by clingo, even on concrete specs, to give
+                    # the possibility to reuse specs built against a different runtime.
+                    if dep.name == "gcc-runtime":
+                        continue
+
                     # We know dependencies are real for concrete specs. For abstract
                     # specs they just mean the dep is somehow in the DAG.
                     for dtype in dt.ALL_FLAGS:
diff --git a/lib/spack/spack/test/concretize_compiler_runtimes.py b/lib/spack/spack/test/concretize_compiler_runtimes.py
index f331e9a7f8..12c9b2454d 100644
--- a/lib/spack/spack/test/concretize_compiler_runtimes.py
+++ b/lib/spack/spack/test/concretize_compiler_runtimes.py
@@ -11,6 +11,7 @@ import spack.paths
 import spack.repo
 import spack.solver.asp
 import spack.spec
+from spack.environment.environment import ViewDescriptor
 from spack.version import Version
 
 pytestmark = [
@@ -19,6 +20,17 @@ pytestmark = [
 ]
 
 
+def _concretize_with_reuse(*, root_str, reused_str):
+    reused_spec = spack.spec.Spec(reused_str).concretized()
+    setup = spack.solver.asp.SpackSolverSetup(tests=False)
+    driver = spack.solver.asp.PyclingoDriver()
+    result, _, _ = driver.solve(
+        setup, [spack.spec.Spec(f"{root_str} ^{reused_str}")], reuse=[reused_spec]
+    )
+    root = result.specs[0]
+    return root, reused_spec
+
+
 @pytest.fixture
 def runtime_repo(config):
     repo = os.path.join(spack.paths.repos_path, "compiler_runtime.test")
@@ -60,3 +72,59 @@ def test_external_nodes_do_not_have_runtimes(runtime_repo, mutable_config, tmp_p
     assert a.dependencies("gcc-runtime")
     assert a.dependencies("b")
     assert not b.dependencies("gcc-runtime")
+
+
+@pytest.mark.parametrize(
+    "root_str,reused_str,expected,nruntime",
+    [
+        # The reused runtime is older than we need, thus we'll add a more recent one for a
+        ("a%gcc@10.2.1", "b%gcc@4.5.0", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@4.5.0"}, 2),
+        # The root is compiled with an older compiler, thus we'll reuse the runtime from b
+        ("a%gcc@4.5.0", "b%gcc@10.2.1", {"a": "gcc-runtime@10.2.1", "b": "gcc-runtime@10.2.1"}, 1),
+    ],
+)
+def test_reusing_specs_with_gcc_runtime(root_str, reused_str, expected, nruntime, runtime_repo):
+    """Tests that we can reuse specs with a "gcc-runtime" leaf node. In particular, checks
+    that the semantic for gcc-runtimes versions accounts for reused packages too.
+    """
+    root, reused_spec = _concretize_with_reuse(root_str=root_str, reused_str=reused_str)
+
+    assert f"{expected['b']}" in reused_spec
+    runtime_a = root.dependencies("gcc-runtime")[0]
+    assert runtime_a.satisfies(expected["a"])
+    runtime_b = root["b"].dependencies("gcc-runtime")[0]
+    assert runtime_b.satisfies(expected["b"])
+
+    runtimes = [x for x in root.traverse() if x.name == "gcc-runtime"]
+    assert len(runtimes) == nruntime
+
+
+@pytest.mark.parametrize(
+    "root_str,reused_str,expected,not_expected",
+    [
+        # Ensure that, whether we have multiple runtimes in the DAG or not,
+        # we always link only the latest version
+        ("a%gcc@10.2.1", "b%gcc@4.5.0", ["gcc-runtime@10.2.1"], ["gcc-runtime@4.5.0"]),
+        ("a%gcc@4.5.0", "b%gcc@10.2.1", ["gcc-runtime@10.2.1"], ["gcc-runtime@4.5.0"]),
+    ],
+)
+def test_views_can_handle_duplicate_runtime_nodes(
+    root_str, reused_str, expected, not_expected, runtime_repo, tmp_path, monkeypatch
+):
+    """Tests that an environment is able to select the latest version of a runtime node to be
+    linked in a view, in case more than one compatible version is in the DAG.
+    """
+    root, reused_spec = _concretize_with_reuse(root_str=root_str, reused_str=reused_str)
+
+    # Mock the installation status to allow selecting nodes for the view
+    monkeypatch.setattr(spack.spec.Spec, "installed", True)
+    nodes = list(root.traverse())
+
+    view = ViewDescriptor(str(tmp_path), str(tmp_path))
+    candidate_specs = view.specs_for_view(nodes)
+
+    for x in expected:
+        assert any(node.satisfies(x) for node in candidate_specs)
+
+    for x in not_expected:
+        assert all(not node.satisfies(x) for node in candidate_specs)