Skip to content

Commit

Permalink
Fix filtering external specs (#44093)
Browse files Browse the repository at this point in the history
When an include filter on externals is present, implicitly
include libcs.

Also, do not penalize deprecated versions if they come
from externals.
  • Loading branch information
alalazo committed May 9, 2024
1 parent d88fa5c commit c7cf5ea
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 0 deletions.
4 changes: 4 additions & 0 deletions lib/spack/spack/solver/asp.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
1 change: 1 addition & 0 deletions lib/spack/spack/solver/concretize.lp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}.

Expand Down
43 changes: 43 additions & 0 deletions lib/spack/spack/test/concretize.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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():
Expand Down
16 changes: 16 additions & 0 deletions var/spack/repos/builtin.mock/packages/deprecated-client/package.py
Original file line number Diff line number Diff line change
@@ -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")

0 comments on commit c7cf5ea

Please sign in to comment.