From 9f99e8ad6484165dd957857ec26c68654ba0b8f8 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Sat, 27 Mar 2021 22:22:11 +0100 Subject: [PATCH] Externals are preferred even when they have non-default variant values fixes #22596 Variants which are specified in an external spec are not scored negatively if they encode a non-default value. --- lib/spack/spack/solver/concretize.lp | 19 ++++++++++++++++++- lib/spack/spack/test/concretize.py | 10 ++++++++++ .../spack/test/data/config/packages.yaml | 12 +++++++++++- .../external-non-default-variant/package.py | 13 +++++++++++++ .../package.py | 12 ++++++++++++ 5 files changed, 64 insertions(+), 2 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/external-non-default-variant/package.py create mode 100644 var/spack/repos/builtin.mock/packages/trigger-external-non-default-variant/package.py diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index b875c58527..9e66760695 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -360,23 +360,40 @@ variant_value(Package, Variant, Value) variant(Package, Variant), variant_set(Package, Variant, Value). -% prefer default values. +% The rules below allow us to prefer default values for variants +% whenever possible. If a variant is set in a spec, or if it is +% specified in an external, we score it as if it was a default value. variant_not_default(Package, Variant, Value, 1) :- variant_value(Package, Variant, Value), not variant_default_value(Package, Variant, Value), not variant_set(Package, Variant, Value), + not external_with_variant_set(Package, Variant, Value), node(Package). +% We are using the default value for a variant variant_not_default(Package, Variant, Value, 0) :- variant_value(Package, Variant, Value), variant_default_value(Package, Variant, Value), node(Package). +% The variant is set in the spec variant_not_default(Package, Variant, Value, 0) :- variant_value(Package, Variant, Value), variant_set(Package, Variant, Value), node(Package). +% The variant is set in an external spec +external_with_variant_set(Package, Variant, Value) + :- variant_value(Package, Variant, Value), + condition_requirement(ID, "variant_value", Package, Variant, Value), + possible_external(ID, Package, _), + external(Package), + node(Package). + +variant_not_default(Package, Variant, Value, 0) + :- variant_value(Package, Variant, Value), + external_with_variant_set(Package, Variant, Value), + node(Package). % The default value for a variant in a package is what is written % in the package.py file, unless some preference is set in packages.yaml diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index c118304e64..f5294ae596 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1142,3 +1142,13 @@ def test_mv_variants_disjoint_sets_from_packages_yaml(self): s = Spec('mvapich2').concretized() assert set(s.variants['file_systems'].value) == set(['ufs', 'nfs']) + + @pytest.mark.regression('22596') + def test_external_with_non_default_variant_as_dependency(self): + # This package depends on another that is registered as an external + # with 'buildable: true' and a variant with a non-default value set + s = Spec('trigger-external-non-default-variant').concretized() + + assert '~foo' in s['external-non-default-variant'] + assert '~bar' in s['external-non-default-variant'] + assert s['external-non-default-variant'].external diff --git a/lib/spack/spack/test/data/config/packages.yaml b/lib/spack/spack/test/data/config/packages.yaml index 83f8cf1bb3..496c3623cc 100644 --- a/lib/spack/spack/test/data/config/packages.yaml +++ b/lib/spack/spack/test/data/config/packages.yaml @@ -35,4 +35,14 @@ packages: - spec: external-buildable-with-variant@1.1.special +baz prefix: /usr - spec: external-buildable-with-variant@0.9 +baz - prefix: /usr \ No newline at end of file + prefix: /usr + 'old-external': + buildable: True + externals: + - spec: old-external@1.0.0 + prefix: /usr + 'external-non-default-variant': + buildable: True + externals: + - spec: external-non-default-variant@3.8.7~foo~bar + prefix: /usr diff --git a/var/spack/repos/builtin.mock/packages/external-non-default-variant/package.py b/var/spack/repos/builtin.mock/packages/external-non-default-variant/package.py new file mode 100644 index 0000000000..f5a4e0f07d --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/external-non-default-variant/package.py @@ -0,0 +1,13 @@ +# 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 ExternalNonDefaultVariant(Package): + """An external that is registered with a non-default value""" + homepage = "http://www.python.org" + url = "http://www.python.org/ftp/python/3.8.7/Python-3.8.7.tgz" + + version('3.8.7', 'be78e48cdfc1a7ad90efff146dce6cfe') + + variant('foo', default=True, description='just a variant') + variant('bar', default=True, description='just a variant') diff --git a/var/spack/repos/builtin.mock/packages/trigger-external-non-default-variant/package.py b/var/spack/repos/builtin.mock/packages/trigger-external-non-default-variant/package.py new file mode 100644 index 0000000000..8b2b2be70d --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/trigger-external-non-default-variant/package.py @@ -0,0 +1,12 @@ +# 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 TriggerExternalNonDefaultVariant(Package): + """This ackage depends on an external with a non-default variant""" + homepage = "http://www.example.com" + url = "http://www.someurl.tar.gz" + + version('1.0', 'foobarbaz') + + depends_on('external-non-default-variant')