From f55224f161bd81a69108d9113d1a53d59c6723fe Mon Sep 17 00:00:00 2001
From: Massimiliano Culpo <massimiliano.culpo@gmail.com>
Date: Thu, 9 May 2024 18:50:15 +0200
Subject: [PATCH] Fix filtering external specs (#44093)

When an include filter on externals is present, implicitly
include libcs.

Also, do not penalize deprecated versions if they come
from externals.
---
 lib/spack/spack/solver/asp.py                 |  4 ++
 lib/spack/spack/solver/concretize.lp          |  1 +
 lib/spack/spack/test/concretize.py            | 43 +++++++++++++++++++
 .../packages/deprecated-client/package.py     | 16 +++++++
 4 files changed, 64 insertions(+)
 create mode 100644 var/spack/repos/builtin.mock/packages/deprecated-client/package.py

diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py
index 0d84c53970..46e892a835 100644
--- a/lib/spack/spack/solver/asp.py
+++ b/lib/spack/spack/solver/asp.py
@@ -1649,11 +1649,15 @@ def external_packages(self):
         if isinstance(reuse_yaml, typing.Mapping):
             default_include = reuse_yaml.get("include", [])
             default_exclude = reuse_yaml.get("exclude", [])
+            libc_externals = list(all_libcs())
             for source in reuse_yaml.get("from", []):
                 if source["type"] != "external":
                     continue
 
                 include = source.get("include", default_include)
+                if include:
+                    # Since libcs are implicit externals, we need to implicitly include them
+                    include = include + libc_externals
                 exclude = source.get("exclude", default_exclude)
                 spec_filters.append(
                     SpecFilter(
diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp
index 85cc697a2d..60165338c6 100644
--- a/lib/spack/spack/solver/concretize.lp
+++ b/lib/spack/spack/solver/concretize.lp
@@ -1424,6 +1424,7 @@ opt_criterion(73, "deprecated versions used").
 #minimize{
     1@73+Priority,PackageNode
     : attr("deprecated", PackageNode, _),
+      not external(PackageNode),
       build_priority(PackageNode, Priority)
 }.
 
diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py
index f0b92c98f5..bfa774e3f4 100644
--- a/lib/spack/spack/test/concretize.py
+++ b/lib/spack/spack/test/concretize.py
@@ -2464,6 +2464,7 @@ def test_spec_with_build_dep_from_json(self, tmp_path):
         assert s["dttop"].dag_hash() == build_dep.dag_hash()
 
     @pytest.mark.regression("44040")
+    @pytest.mark.only_clingo("Use case not supported by the original concretizer")
     def test_exclude_specs_from_reuse(self, monkeypatch):
         """Tests that we can exclude a spec from reuse when concretizing, and that the spec
         is not added back to the solve as a dependency of another reusable spec.
@@ -2503,6 +2504,48 @@ def test_exclude_specs_from_reuse(self, monkeypatch):
         for dep in result["dyninst"].traverse(root=False):
             assert dep.dag_hash() == reused[dep.name].dag_hash()
 
+    @pytest.mark.regression("44091")
+    @pytest.mark.parametrize(
+        "included_externals",
+        [
+            ["deprecated-versions"],
+            # Try the empty list, to ensure that in that case everything will be included
+            # since filtering should happen only when the list is non-empty
+            [],
+        ],
+    )
+    @pytest.mark.only_clingo("Use case not supported by the original concretizer")
+    def test_include_specs_from_externals_and_libcs(
+        self, included_externals, mutable_config, tmp_path
+    ):
+        """Tests that when we include specs from externals, we always include libcs."""
+        mutable_config.set(
+            "packages",
+            {
+                "deprecated-versions": {
+                    "externals": [{"spec": "deprecated-versions@1.1.0", "prefix": str(tmp_path)}]
+                }
+            },
+        )
+        request_str = "deprecated-client"
+
+        # When using the external the version is selected even if deprecated
+        with spack.config.override(
+            "concretizer:reuse", {"from": [{"type": "external", "include": included_externals}]}
+        ):
+            result = Spec(request_str).concretized()
+
+        assert result["deprecated-versions"].satisfies("@1.1.0")
+
+        # When excluding it, we pick the non-deprecated version
+        with spack.config.override(
+            "concretizer:reuse",
+            {"from": [{"type": "external", "exclude": ["deprecated-versions"]}]},
+        ):
+            result = Spec(request_str).concretized()
+
+        assert result["deprecated-versions"].satisfies("@1.0.0")
+
 
 @pytest.fixture()
 def duplicates_test_repository():
diff --git a/var/spack/repos/builtin.mock/packages/deprecated-client/package.py b/var/spack/repos/builtin.mock/packages/deprecated-client/package.py
new file mode 100644
index 0000000000..9806871d81
--- /dev/null
+++ b/var/spack/repos/builtin.mock/packages/deprecated-client/package.py
@@ -0,0 +1,16 @@
+# Copyright 2013-2024 Lawrence Livermore National Security, LLC and other
+# Spack Project Developers. See the top-level COPYRIGHT file for details.
+#
+# SPDX-License-Identifier: (Apache-2.0 OR MIT)
+from spack.package import *
+
+
+class DeprecatedClient(Package):
+    """A package depending on another which has deprecated versions."""
+
+    homepage = "http://www.example.com"
+    url = "http://www.example.com/c-1.0.tar.gz"
+
+    version("1.1.0", sha256="abcdef1234567890abcdef1234567890abcdef1234567890abcdef1234567890")
+
+    depends_on("deprecated-versions")