From 476f2a63e2ece92143a7614b97fa6d12579ae662 Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Wed, 12 Feb 2025 22:02:30 +0100
Subject: [PATCH] Add support for externals with compiler specified

Compiler annotation is taken into account when
selecting externals as dependencies.
---
 lib/spack/spack/solver/asp.py               |  4 +-
 lib/spack/spack/solver/concretize.lp        | 10 ++++
 lib/spack/spack/test/concretization/core.py | 66 +++++++++++++++++++++
 3 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 6bddd2d6b79..c28c1c3087d 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -2286,8 +2286,10 @@ def external_requirement(input_spec, requirements):
                 def external_imposition(input_spec, requirements):
                     result = []
                     for asp_fn in requirements:
-                        if asp_fn.args[0] in ("depends_on", "build_requirement"):
+                        if asp_fn.args[0] == "depends_on":
                             continue
+                        elif asp_fn.args[0] == "build_requirement":
+                            asp_fn.args = "external_build_requirement", *asp_fn.args[1:]
                         if asp_fn.args[1] != input_spec.name:
                             continue
                         result.append(asp_fn)
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index b9439724c72..0bd9f09fb15 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -831,6 +831,16 @@ attr("external_spec_selected", node(ID, Package), LocalIndex) :-
     attr("node", node(ID, Package)),
     not attr("hash", node(ID, Package), _).
 
+% Account for compiler annotation on externals
+:- not attr("root", ExternalNode),
+   attr("external_build_requirement", ExternalNode, build_requirement("node", Compiler)),
+   not node_compiler(_, node(_, Compiler)).
+
+1 { attr("node_version_satisfies", node(X, Compiler), Constraint) : node_compiler(_, node(X, Compiler)) }
+  :- not attr("root", ExternalNode),
+     attr("external_build_requirement", ExternalNode, build_requirement("node", Compiler)),
+     attr("external_build_requirement", ExternalNode, build_requirement("node_version_satisfies", Compiler, Constraint)).
+
 % it cannot happen that a spec is external, but none of the external specs
 % conditions hold.
 error(100, "Attempted to use external for '{0}' which does not satisfy any configured external spec", Package)
diff --git a/lib/spack/spack/test/concretization/core.py b/lib/spack/spack/test/concretization/core.py
index e88ba082093..df3ef609036 100644
--- a/lib/spack/spack/test/concretization/core.py
+++ b/lib/spack/spack/test/concretization/core.py
@@ -3247,3 +3247,69 @@ def test_compiler_can_be_built_with_other_compilers(config, mock_packages):
 
     c_compiler = s.dependencies(virtuals=("c",))
     assert len(c_compiler) == 1 and c_compiler[0].satisfies("gcc@10")
+
+
+@pytest.mark.parametrize(
+    "spec_str,expected",
+    [
+        # Only one compiler is in the DAG, so pick the external associated with it
+        ("dyninst %clang", "clang"),
+        ("dyninst %gcc", "gcc"),
+        # Both compilers are in the DAG, so pick the best external according to other criteria
+        ("dyninst %clang ^libdwarf%gcc", "clang"),
+        ("dyninst %gcc ^libdwarf%clang", "clang"),
+    ],
+)
+def test_compiler_match_for_externals_is_taken_into_account(
+    spec_str, expected, mutable_config, mock_packages, tmp_path
+):
+    """Tests that compiler annotation for externals are somehow taken into account for a match"""
+    packages_yaml = syaml.load_config(
+        f"""
+packages:
+  libelf:
+    externals:
+    - spec: "libelf@0.8.12 %gcc"
+      prefix: {tmp_path / 'gcc'}
+    - spec: "libelf@0.8.13 %clang"
+      prefix: {tmp_path / 'clang'}
+"""
+    )
+    mutable_config.set("packages", packages_yaml["packages"])
+    s = spack.concretize.concretize_one(spec_str)
+    libelf = s["libelf"]
+    assert libelf.external and libelf.external_path == str(tmp_path / expected)
+
+
+@pytest.mark.parametrize(
+    "spec_str,expected",
+    [
+        # Only one compiler is in the DAG, so pick the external associated with it
+        ("dyninst %gcc@10", "libelf-gcc10"),
+        ("dyninst %gcc@9", "libelf-gcc9"),
+        # Both compilers are in the DAG, so pick the best external according to other criteria
+        ("dyninst %gcc@10 ^libdwarf%gcc@9", "libelf-gcc9"),
+    ],
+)
+def test_compiler_match_for_externals_with_versions(
+    spec_str, expected, mutable_config, mock_packages, tmp_path
+):
+    """Tests that version constraints are taken into account for compiler annotations
+    on externals
+    """
+    packages_yaml = syaml.load_config(
+        f"""
+packages:
+  libelf:
+    buildable: false
+    externals:
+    - spec: "libelf@0.8.12 %gcc@10"
+      prefix: {tmp_path / 'libelf-gcc10'}
+    - spec: "libelf@0.8.13 %gcc@9"
+      prefix: {tmp_path / 'libelf-gcc9'}
+"""
+    )
+    mutable_config.set("packages", packages_yaml["packages"])
+    s = spack.concretize.concretize_one(spec_str)
+    libelf = s["libelf"]
+    assert libelf.external and libelf.external_path == str(tmp_path / expected)