From 579fadacd0a93e6676caed8bda5d3e6e5a20ecc9 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Tue, 21 May 2024 08:41:09 +0200 Subject: [PATCH] ASP-based solver: fix version optimization for roots (#44272) This fixes a bug occurring when two root specs need to select old versions, and these versions have the same penalty in the optimization. This sometimes caused an older version to be preferred to a more recent one. The issue was the omission of `PackageNode` in the optimization tuple. --- lib/spack/spack/solver/concretize.lp | 7 ++++--- lib/spack/spack/test/concretize.py | 4 ++-- lib/spack/spack/test/env.py | 30 ++++++++++++++++++++++++++++ 3 files changed, 36 insertions(+), 5 deletions(-) diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 60165338c6..4d8a202ce3 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -1432,11 +1432,11 @@ opt_criterion(73, "deprecated versions used"). % 1. Version weight % 2. Number of variants with a non default value, if not set % for the root package. -opt_criterion(70, "version weight"). +opt_criterion(70, "version badness (roots)"). #minimize{ 0@270: #true }. #minimize{ 0@70: #true }. #minimize { - Weight@70+Priority + Weight@70+Priority,PackageNode : attr("root", PackageNode), version_weight(PackageNode, Weight), build_priority(PackageNode, Priority) @@ -1526,13 +1526,14 @@ opt_criterion(30, "non-preferred OS's"). }. % Choose more recent versions for nodes -opt_criterion(25, "version badness"). +opt_criterion(25, "version badness (non roots)"). #minimize{ 0@225: #true }. #minimize{ 0@25: #true }. #minimize{ Weight@25+Priority,node(X, Package) : version_weight(node(X, Package), Weight), build_priority(node(X, Package), Priority), + not attr("root", node(X, Package)), not runtime(Package) }. diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index bfa774e3f4..797c865a4f 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1914,11 +1914,11 @@ def test_version_weight_and_provenance(self): libc_offset = 1 if spack.solver.asp.using_libc_compatibility() else 0 criteria = [ (num_specs - 1 - libc_offset, None, "number of packages to build (vs. reuse)"), - (2, 0, "version badness"), + (2, 0, "version badness (non roots)"), ] for criterion in criteria: - assert criterion in result.criteria, result_spec + assert criterion in result.criteria, criterion assert result_spec.satisfies("^b@1.0") @pytest.mark.only_clingo("Use case not supported by the original concretizer") diff --git a/lib/spack/spack/test/env.py b/lib/spack/spack/test/env.py index 54ac9a3732..d8bcc0797e 100644 --- a/lib/spack/spack/test/env.py +++ b/lib/spack/spack/test/env.py @@ -813,3 +813,33 @@ def test_deconcretize_then_concretize_does_not_error(mutable_mock_env_path, mock assert len(e.concrete_roots()) == 3 all_root_hashes = set(x.dag_hash() for x in e.concrete_roots()) assert len(all_root_hashes) == 2 + + +@pytest.mark.regression("44216") +@pytest.mark.only_clingo() +def test_root_version_weights_for_old_versions(mutable_mock_env_path, mock_packages): + """Tests that, when we select two old versions of root specs that have the same version + optimization penalty, both are considered. + """ + mutable_mock_env_path.mkdir() + spack_yaml = mutable_mock_env_path / ev.manifest_name + spack_yaml.write_text( + """spack: + specs: + # allow any version, but the most recent + - bowtie@:1.3 + # allows only the third most recent, so penalty is 2 + - gcc@1 + concretizer: + unify: true + """ + ) + e = ev.Environment(mutable_mock_env_path) + with e: + e.concretize() + + bowtie = [x for x in e.concrete_roots() if x.name == "bowtie"][0] + gcc = [x for x in e.concrete_roots() if x.name == "gcc"][0] + + assert bowtie.satisfies("@=1.3.0") + assert gcc.satisfies("@=1.0")