From 7dd3592eabe2b1ab2739a3e54c5228465c4c6ae7 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 26 Aug 2021 18:28:47 +0200 Subject: [PATCH] Regression test for version selection with preferences (#25602) This commit adds a regression test for version selection with preferences in `packages.yaml`. Before PR 25585 we used negative weights in a minimization to select the optimal version. This may lead to situations where a dependency may make the version score of dependents "better" if it is preferred in packages.yaml. --- .../spack/test/concretize_preferences.py | 12 ++++++++++++ .../spack/test/data/config/packages.yaml | 2 ++ .../package.py | 16 ++++++++++++++++ .../packages/version-test-pkg/package.py | 19 +++++++++++++++++++ .../packages/version-test-root/package.py | 11 +++++++++++ 5 files changed, 60 insertions(+) create mode 100644 var/spack/repos/builtin.mock/packages/version-test-dependency-preferred/package.py create mode 100644 var/spack/repos/builtin.mock/packages/version-test-pkg/package.py create mode 100644 var/spack/repos/builtin.mock/packages/version-test-root/package.py diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index b5b631d6af..598e9d08fb 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -384,3 +384,15 @@ def test_variant_not_flipped_to_pull_externals(self): assert '~external' in s['vdefault-or-external'] assert 'externaltool' not in s + + @pytest.mark.regression('25585') + def test_dependencies_cant_make_version_parent_score_better(self): + """Test that a package can't select a worse version for a + dependent because doing so it can pull-in a dependency + that makes the overall version score even or better and maybe + has a better score in some lower priority criteria. + """ + s = Spec('version-test-root').concretized() + + assert s.satisfies('^version-test-pkg@2.4.6') + assert 'version-test-dependency-preferred' not in s diff --git a/lib/spack/spack/test/data/config/packages.yaml b/lib/spack/spack/test/data/config/packages.yaml index b2c3a33b42..84f470d208 100644 --- a/lib/spack/spack/test/data/config/packages.yaml +++ b/lib/spack/spack/test/data/config/packages.yaml @@ -47,3 +47,5 @@ packages: externals: - spec: external-non-default-variant@3.8.7~foo~bar prefix: /usr + version-test-dependency-preferred: + version: ['5.2.5'] \ No newline at end of file diff --git a/var/spack/repos/builtin.mock/packages/version-test-dependency-preferred/package.py b/var/spack/repos/builtin.mock/packages/version-test-dependency-preferred/package.py new file mode 100644 index 0000000000..672e4e64c4 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/version-test-dependency-preferred/package.py @@ -0,0 +1,16 @@ +# Copyright 2013-2021 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) +class VersionTestDependencyPreferred(AutotoolsPackage): + """Dependency of version-test-pkg, which has a multi-valued + variant with two default values (a very low priority optimization + criterion for clingo is to maximize their number) + """ + homepage = "http://www.spack.org" + url = "http://www.spack.org/downloads/xz-1.0.tar.gz" + + version('5.2.5', sha256='5117f930900b341493827d63aa910ff5e011e0b994197c3b71c08a20228a42df') + + variant('libs', default='shared,static', values=('shared', 'static'), + multi=True, description='Build shared libs, static libs or both') diff --git a/var/spack/repos/builtin.mock/packages/version-test-pkg/package.py b/var/spack/repos/builtin.mock/packages/version-test-pkg/package.py new file mode 100644 index 0000000000..5d2cd4b5cf --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/version-test-pkg/package.py @@ -0,0 +1,19 @@ +# Copyright 2013-2021 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) +class VersionTestPkg(AutotoolsPackage): + """Mock AutotoolsPackage to check proper version + selection by clingo. + """ + homepage = "https://www.gnu.org/software/make/" + url = "http://www.example.com/libtool-version-1.0.tar.gz" + + version('develop', git='https://git.savannah.gnu.org/git/libtool.git', + branch='master', submodules=True) + version('2.4.6', sha256='e40b8f018c1da64edd1cc9a6fce5fa63b2e707e404e20cad91fbae337c98a5b7') + + depends_on( + 'version-test-dependency-preferred', + when='@develop' + ) diff --git a/var/spack/repos/builtin.mock/packages/version-test-root/package.py b/var/spack/repos/builtin.mock/packages/version-test-root/package.py new file mode 100644 index 0000000000..e91a868740 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/version-test-root/package.py @@ -0,0 +1,11 @@ +# Copyright 2013-2021 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) +class VersionTestRoot(AutotoolsPackage): + """Uses version-test-pkg, as a build dependency""" + homepage = "http://www.spack.org" + url = "http://www.spack.org/downloads/aml-1.0.tar.gz" + + version('0.1.0', sha256='cc89a8768693f1f11539378b21cdca9f0ce3fc5cb564f9b3e4154a051dcea69b') + depends_on('version-test-pkg', type='build')