From cd04109e175fdc8223a5d922c7d3995496a23936 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Wed, 2 Feb 2022 19:05:24 +0100 Subject: Add a "sticky" property to variants (#28630) * Add sticky variants * Add unit tests for sticky variants * Add documentation for sticky variants * Revert "Revert 19736 because conflicts are avoided by clingo by default (#26721)" This reverts commit 33ef7d57c1a09d6ce454503e9ff5638747e4f4a2. * Add stickiness to "allow-unsupported-compiler" --- lib/spack/docs/packaging_guide.rst | 26 +++ lib/spack/spack/build_systems/cuda.py | 195 +++++++++++---------- lib/spack/spack/directives.py | 9 +- lib/spack/spack/solver/asp.py | 3 + lib/spack/spack/solver/concretize.lp | 9 + lib/spack/spack/test/concretize.py | 18 ++ lib/spack/spack/test/concretize_preferences.py | 10 ++ lib/spack/spack/variant.py | 7 +- .../packages/sticky-variant/package.py | 18 ++ var/spack/repos/builtin/packages/cuda/package.py | 2 + 10 files changed, 196 insertions(+), 101 deletions(-) create mode 100644 var/spack/repos/builtin.mock/packages/sticky-variant/package.py diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 66939ad3a8..a65a363d19 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -1441,6 +1441,32 @@ The ``when`` clause follows the same syntax and accepts the same values as the ``when`` argument of :py:func:`spack.directives.depends_on` +^^^^^^^^^^^^^^^ +Sticky Variants +^^^^^^^^^^^^^^^ + +The variant directive can be marked as ``sticky`` by setting to ``True`` the +corresponding argument: + +.. code-block:: python + + variant('bar', default=False, sticky=True) + +A ``sticky`` variant differs from a regular one in that it is always set +to either: + +#. An explicit value appearing in a spec literal or +#. Its default value + +The concretizer thus is not free to pick an alternate value to work +around conflicts, but will error out instead. +Setting this property on a variant is useful in cases where the +variant allows some dangerous or controversial options (e.g. using unsupported versions +of a compiler for a library) and the packager wants to ensure that +allowing these options is done on purpose by the user, rather than +automatically by the solver. + + ^^^^^^^^^^^^^^^^^^^ Overriding Variants ^^^^^^^^^^^^^^^^^^^ diff --git a/lib/spack/spack/build_systems/cuda.py b/lib/spack/spack/build_systems/cuda.py index 8200737d76..1c46086c63 100644 --- a/lib/spack/spack/build_systems/cuda.py +++ b/lib/spack/spack/build_systems/cuda.py @@ -5,6 +5,7 @@ import spack.variant from spack.directives import conflicts, depends_on, variant +from spack.multimethod import when from spack.package import PackageBase @@ -88,103 +89,103 @@ class CudaPackage(PackageBase): # Linux x86_64 compiler conflicts from here: # https://gist.github.com/ax3l/9489132 - - # GCC - # According to - # https://github.com/spack/spack/pull/25054#issuecomment-886531664 - # these conflicts are valid independently from the architecture - - # minimum supported versions - conflicts('%gcc@:4', when='+cuda ^cuda@11.0:') - conflicts('%gcc@:5', when='+cuda ^cuda@11.4:') - - # maximum supported version - # NOTE: - # in order to not constrain future cuda version to old gcc versions, - # it has been decided to use an upper bound for the latest version. - # This implies that the last one in the list has to be updated at - # each release of a new cuda minor version. - conflicts('%gcc@10:', when='+cuda ^cuda@:11.0') - conflicts('%gcc@12:', when='+cuda ^cuda@:11.6') - conflicts('%clang@13:', when='+cuda ^cuda@:11.5') - conflicts('%clang@14:', when='+cuda ^cuda@:11.6') - - # https://gist.github.com/ax3l/9489132#gistcomment-3860114 - conflicts('%gcc@10', when='+cuda ^cuda@:11.4.0') - conflicts('%gcc@5:', when='+cuda ^cuda@:7.5 target=x86_64:') - conflicts('%gcc@6:', when='+cuda ^cuda@:8 target=x86_64:') - conflicts('%gcc@7:', when='+cuda ^cuda@:9.1 target=x86_64:') - conflicts('%gcc@8:', when='+cuda ^cuda@:10.0.130 target=x86_64:') - conflicts('%gcc@9:', when='+cuda ^cuda@:10.2.89 target=x86_64:') - conflicts('%pgi@:14.8', when='+cuda ^cuda@:7.0.27 target=x86_64:') - conflicts('%pgi@:15.3,15.5:', when='+cuda ^cuda@7.5 target=x86_64:') - conflicts('%pgi@:16.2,16.0:16.3', when='+cuda ^cuda@8 target=x86_64:') - conflicts('%pgi@:15,18:', when='+cuda ^cuda@9.0:9.1 target=x86_64:') - conflicts('%pgi@:16,19:', when='+cuda ^cuda@9.2.88:10 target=x86_64:') - conflicts('%pgi@:17,20:', when='+cuda ^cuda@10.1.105:10.2.89 target=x86_64:') - conflicts('%pgi@:17,21:', when='+cuda ^cuda@11.0.2:11.1.0 target=x86_64:') - conflicts('%clang@:3.4', when='+cuda ^cuda@:7.5 target=x86_64:') - conflicts('%clang@:3.7,4:', when='+cuda ^cuda@8.0:9.0 target=x86_64:') - conflicts('%clang@:3.7,4.1:', when='+cuda ^cuda@9.1 target=x86_64:') - conflicts('%clang@:3.7,5.1:', when='+cuda ^cuda@9.2 target=x86_64:') - conflicts('%clang@:3.7,6.1:', when='+cuda ^cuda@10.0.130 target=x86_64:') - conflicts('%clang@:3.7,7.1:', when='+cuda ^cuda@10.1.105 target=x86_64:') - conflicts('%clang@:3.7,8.1:', - when='+cuda ^cuda@10.1.105:10.1.243 target=x86_64:') - conflicts('%clang@:3.2,9:', when='+cuda ^cuda@10.2.89 target=x86_64:') - conflicts('%clang@:5', when='+cuda ^cuda@11.0.2: target=x86_64:') - conflicts('%clang@10:', when='+cuda ^cuda@:11.0.3 target=x86_64:') - conflicts('%clang@11:', when='+cuda ^cuda@:11.1.0 target=x86_64:') - - # x86_64 vs. ppc64le differ according to NVidia docs - # Linux ppc64le compiler conflicts from Table from the docs below: - # https://docs.nvidia.com/cuda/cuda-installation-guide-linux/index.html - # https://docs.nvidia.com/cuda/archive/9.2/cuda-installation-guide-linux/index.html - # https://docs.nvidia.com/cuda/archive/9.1/cuda-installation-guide-linux/index.html - # https://docs.nvidia.com/cuda/archive/9.0/cuda-installation-guide-linux/index.html - # https://docs.nvidia.com/cuda/archive/8.0/cuda-installation-guide-linux/index.html - - # information prior to CUDA 9 difficult to find - conflicts('%gcc@6:', when='+cuda ^cuda@:9 target=ppc64le:') - conflicts('%gcc@8:', when='+cuda ^cuda@:10.0.130 target=ppc64le:') - conflicts('%gcc@9:', when='+cuda ^cuda@:10.1.243 target=ppc64le:') - # officially, CUDA 11.0.2 only supports the system GCC 8.3 on ppc64le - conflicts('%pgi', when='+cuda ^cuda@:8 target=ppc64le:') - conflicts('%pgi@:16', when='+cuda ^cuda@:9.1.185 target=ppc64le:') - conflicts('%pgi@:17', when='+cuda ^cuda@:10 target=ppc64le:') - conflicts('%clang@4:', when='+cuda ^cuda@:9.0.176 target=ppc64le:') - conflicts('%clang@5:', when='+cuda ^cuda@:9.1 target=ppc64le:') - conflicts('%clang@6:', when='+cuda ^cuda@:9.2 target=ppc64le:') - conflicts('%clang@7:', when='+cuda ^cuda@10.0.130 target=ppc64le:') - conflicts('%clang@7.1:', when='+cuda ^cuda@:10.1.105 target=ppc64le:') - conflicts('%clang@8.1:', when='+cuda ^cuda@:10.2.89 target=ppc64le:') - conflicts('%clang@:5', when='+cuda ^cuda@11.0.2: target=ppc64le:') - conflicts('%clang@10:', when='+cuda ^cuda@:11.0.2 target=ppc64le:') - conflicts('%clang@11:', when='+cuda ^cuda@:11.1.0 target=ppc64le:') - - # Intel is mostly relevant for x86_64 Linux, even though it also - # exists for Mac OS X. No information prior to CUDA 3.2 or Intel 11.1 - conflicts('%intel@:11.0', when='+cuda ^cuda@:3.1') - conflicts('%intel@:12.0', when='+cuda ^cuda@5.5:') - conflicts('%intel@:13.0', when='+cuda ^cuda@6.0:') - conflicts('%intel@:13.2', when='+cuda ^cuda@6.5:') - conflicts('%intel@:14.9', when='+cuda ^cuda@7:') - # Intel 15.x is compatible with CUDA 7 thru current CUDA - conflicts('%intel@16.0:', when='+cuda ^cuda@:8.0.43') - conflicts('%intel@17.0:', when='+cuda ^cuda@:8.0.60') - conflicts('%intel@18.0:', when='+cuda ^cuda@:9.9') - conflicts('%intel@19.0:', when='+cuda ^cuda@:10.0') - conflicts('%intel@19.1:', when='+cuda ^cuda@:10.1') - conflicts('%intel@19.2:', when='+cuda ^cuda@:11.1.0') - - # XL is mostly relevant for ppc64le Linux - conflicts('%xl@:12,14:', when='+cuda ^cuda@:9.1') - conflicts('%xl@:12,14:15,17:', when='+cuda ^cuda@9.2') - conflicts('%xl@:12,17:', when='+cuda ^cuda@:11.1.0') - - # Darwin. - # TODO: add missing conflicts for %apple-clang cuda@:10 - conflicts('platform=darwin', when='+cuda ^cuda@11.0.2: ') + with when('^cuda~allow-unsupported-compilers'): + # GCC + # According to + # https://github.com/spack/spack/pull/25054#issuecomment-886531664 + # these conflicts are valid independently from the architecture + + # minimum supported versions + conflicts('%gcc@:4', when='+cuda ^cuda@11.0:') + conflicts('%gcc@:5', when='+cuda ^cuda@11.4:') + + # maximum supported version + # NOTE: + # in order to not constrain future cuda version to old gcc versions, + # it has been decided to use an upper bound for the latest version. + # This implies that the last one in the list has to be updated at + # each release of a new cuda minor version. + conflicts('%gcc@10:', when='+cuda ^cuda@:11.0') + conflicts('%gcc@12:', when='+cuda ^cuda@:11.6') + conflicts('%clang@13:', when='+cuda ^cuda@:11.5') + conflicts('%clang@14:', when='+cuda ^cuda@:11.6') + + # https://gist.github.com/ax3l/9489132#gistcomment-3860114 + conflicts('%gcc@10', when='+cuda ^cuda@:11.4.0') + conflicts('%gcc@5:', when='+cuda ^cuda@:7.5 target=x86_64:') + conflicts('%gcc@6:', when='+cuda ^cuda@:8 target=x86_64:') + conflicts('%gcc@7:', when='+cuda ^cuda@:9.1 target=x86_64:') + conflicts('%gcc@8:', when='+cuda ^cuda@:10.0.130 target=x86_64:') + conflicts('%gcc@9:', when='+cuda ^cuda@:10.2.89 target=x86_64:') + conflicts('%pgi@:14.8', when='+cuda ^cuda@:7.0.27 target=x86_64:') + conflicts('%pgi@:15.3,15.5:', when='+cuda ^cuda@7.5 target=x86_64:') + conflicts('%pgi@:16.2,16.0:16.3', when='+cuda ^cuda@8 target=x86_64:') + conflicts('%pgi@:15,18:', when='+cuda ^cuda@9.0:9.1 target=x86_64:') + conflicts('%pgi@:16,19:', when='+cuda ^cuda@9.2.88:10 target=x86_64:') + conflicts('%pgi@:17,20:', when='+cuda ^cuda@10.1.105:10.2.89 target=x86_64:') + conflicts('%pgi@:17,21:', when='+cuda ^cuda@11.0.2:11.1.0 target=x86_64:') + conflicts('%clang@:3.4', when='+cuda ^cuda@:7.5 target=x86_64:') + conflicts('%clang@:3.7,4:', when='+cuda ^cuda@8.0:9.0 target=x86_64:') + conflicts('%clang@:3.7,4.1:', when='+cuda ^cuda@9.1 target=x86_64:') + conflicts('%clang@:3.7,5.1:', when='+cuda ^cuda@9.2 target=x86_64:') + conflicts('%clang@:3.7,6.1:', when='+cuda ^cuda@10.0.130 target=x86_64:') + conflicts('%clang@:3.7,7.1:', when='+cuda ^cuda@10.1.105 target=x86_64:') + conflicts('%clang@:3.7,8.1:', + when='+cuda ^cuda@10.1.105:10.1.243 target=x86_64:') + conflicts('%clang@:3.2,9:', when='+cuda ^cuda@10.2.89 target=x86_64:') + conflicts('%clang@:5', when='+cuda ^cuda@11.0.2: target=x86_64:') + conflicts('%clang@10:', when='+cuda ^cuda@:11.0.3 target=x86_64:') + conflicts('%clang@11:', when='+cuda ^cuda@:11.1.0 target=x86_64:') + + # x86_64 vs. ppc64le differ according to NVidia docs + # Linux ppc64le compiler conflicts from Table from the docs below: + # https://docs.nvidia.com/cuda/cuda-installation-guide-linux/index.html + # https://docs.nvidia.com/cuda/archive/9.2/cuda-installation-guide-linux/index.html + # https://docs.nvidia.com/cuda/archive/9.1/cuda-installation-guide-linux/index.html + # https://docs.nvidia.com/cuda/archive/9.0/cuda-installation-guide-linux/index.html + # https://docs.nvidia.com/cuda/archive/8.0/cuda-installation-guide-linux/index.html + + # information prior to CUDA 9 difficult to find + conflicts('%gcc@6:', when='+cuda ^cuda@:9 target=ppc64le:') + conflicts('%gcc@8:', when='+cuda ^cuda@:10.0.130 target=ppc64le:') + conflicts('%gcc@9:', when='+cuda ^cuda@:10.1.243 target=ppc64le:') + # officially, CUDA 11.0.2 only supports the system GCC 8.3 on ppc64le + conflicts('%pgi', when='+cuda ^cuda@:8 target=ppc64le:') + conflicts('%pgi@:16', when='+cuda ^cuda@:9.1.185 target=ppc64le:') + conflicts('%pgi@:17', when='+cuda ^cuda@:10 target=ppc64le:') + conflicts('%clang@4:', when='+cuda ^cuda@:9.0.176 target=ppc64le:') + conflicts('%clang@5:', when='+cuda ^cuda@:9.1 target=ppc64le:') + conflicts('%clang@6:', when='+cuda ^cuda@:9.2 target=ppc64le:') + conflicts('%clang@7:', when='+cuda ^cuda@10.0.130 target=ppc64le:') + conflicts('%clang@7.1:', when='+cuda ^cuda@:10.1.105 target=ppc64le:') + conflicts('%clang@8.1:', when='+cuda ^cuda@:10.2.89 target=ppc64le:') + conflicts('%clang@:5', when='+cuda ^cuda@11.0.2: target=ppc64le:') + conflicts('%clang@10:', when='+cuda ^cuda@:11.0.2 target=ppc64le:') + conflicts('%clang@11:', when='+cuda ^cuda@:11.1.0 target=ppc64le:') + + # Intel is mostly relevant for x86_64 Linux, even though it also + # exists for Mac OS X. No information prior to CUDA 3.2 or Intel 11.1 + conflicts('%intel@:11.0', when='+cuda ^cuda@:3.1') + conflicts('%intel@:12.0', when='+cuda ^cuda@5.5:') + conflicts('%intel@:13.0', when='+cuda ^cuda@6.0:') + conflicts('%intel@:13.2', when='+cuda ^cuda@6.5:') + conflicts('%intel@:14.9', when='+cuda ^cuda@7:') + # Intel 15.x is compatible with CUDA 7 thru current CUDA + conflicts('%intel@16.0:', when='+cuda ^cuda@:8.0.43') + conflicts('%intel@17.0:', when='+cuda ^cuda@:8.0.60') + conflicts('%intel@18.0:', when='+cuda ^cuda@:9.9') + conflicts('%intel@19.0:', when='+cuda ^cuda@:10.0') + conflicts('%intel@19.1:', when='+cuda ^cuda@:10.1') + conflicts('%intel@19.2:', when='+cuda ^cuda@:11.1.0') + + # XL is mostly relevant for ppc64le Linux + conflicts('%xl@:12,14:', when='+cuda ^cuda@:9.1') + conflicts('%xl@:12,14:15,17:', when='+cuda ^cuda@9.2') + conflicts('%xl@:12,17:', when='+cuda ^cuda@:11.1.0') + + # Darwin. + # TODO: add missing conflicts for %apple-clang cuda@:10 + conflicts('platform=darwin', when='+cuda ^cuda@11.0.2: ') # Make sure cuda_arch can not be used without +cuda for value in cuda_arch_values: diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 81f58f542d..7d92a203e5 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -562,7 +562,9 @@ def variant( values=None, multi=None, validator=None, - when=None): + when=None, + sticky=False +): """Define a variant for the package. Packager can specify a default value as well as a text description. @@ -583,7 +585,8 @@ def variant( doesn't meet the additional constraints when (spack.spec.Spec, bool): optional condition on which the variant applies - + sticky (bool): the variant should not be changed by the concretizer to + find a valid concrete spec. Raises: DirectiveError: if arguments passed to the directive are invalid """ @@ -657,7 +660,7 @@ def variant( when_specs += orig_when pkg.variants[name] = (spack.variant.Variant( - name, default, description, values, multi, validator + name, default, description, values, multi, validator, sticky ), when_specs) return _execute_variant diff --git a/lib/spack/spack/solver/asp.py b/lib/spack/spack/solver/asp.py index 4f727cdf48..ea603fd92f 100644 --- a/lib/spack/spack/solver/asp.py +++ b/lib/spack/spack/solver/asp.py @@ -852,6 +852,9 @@ class SpackSolverSetup(object): for value in sorted(values): self.gen.fact(fn.variant_possible_value(pkg.name, name, value)) + if variant.sticky: + self.gen.fact(fn.variant_sticky(pkg.name, name)) + self.gen.newline() # conflicts diff --git a/lib/spack/spack/solver/concretize.lp b/lib/spack/spack/solver/concretize.lp index 0b4c14796d..f4c15397e2 100644 --- a/lib/spack/spack/solver/concretize.lp +++ b/lib/spack/spack/solver/concretize.lp @@ -402,6 +402,14 @@ variant(Package, Variant) :- variant_condition(ID, Package, Variant), build(Package), error("Unsatisfied conditional variants cannot take on a variant value"). +% if a variant is sticky and not set its value is the default value +variant_value(Package, Variant, Value) :- + variant(Package, Variant), + not variant_set(Package, Variant), + variant_sticky(Package, Variant), + variant_default_value(Package, Variant, Value), + build(Package). + % one variant value for single-valued variants. 1 { variant_value(Package, Variant, Value) @@ -523,6 +531,7 @@ variant_single_value(Package, "dev_path") % spec or some package sets it, and without this, clingo will give % warnings like 'info: atom does not occur in any rule head'. #defined variant/2. +#defined variant_sticky/2. #defined variant_set/3. #defined variant_condition/3. #defined variant_single_value/2. diff --git a/lib/spack/spack/test/concretize.py b/lib/spack/spack/test/concretize.py index 9715568c06..fae852edf8 100644 --- a/lib/spack/spack/test/concretize.py +++ b/lib/spack/spack/test/concretize.py @@ -1357,3 +1357,21 @@ class TestConcretize(object): s = spack.spec.Spec(spec_str).concretized(reuse=True) assert s.package.installed is expect_installed assert s.satisfies(spec_str, strict=True) + + @pytest.mark.regression('26721,19736') + def test_sticky_variant_in_package(self): + if spack.config.get('config:concretizer') == 'original': + pytest.skip('Original concretizer cannot use sticky variants') + + # Here we test that a sticky variant cannot be changed from its default value + # by the ASP solver if not set explicitly. The package used in the test needs + # to have +allow-gcc set to be concretized with %gcc and clingo is not allowed + # to change the default ~allow-gcc + with pytest.raises(spack.error.SpackError): + spack.spec.Spec('sticky-variant %gcc').concretized() + + s = spack.spec.Spec('sticky-variant+allow-gcc %gcc').concretized() + assert s.satisfies('%gcc') and s.satisfies('+allow-gcc') + + s = spack.spec.Spec('sticky-variant %clang').concretized() + assert s.satisfies('%clang') and s.satisfies('~allow-gcc') diff --git a/lib/spack/spack/test/concretize_preferences.py b/lib/spack/spack/test/concretize_preferences.py index 885e601bd0..7bcfc75f7d 100644 --- a/lib/spack/spack/test/concretize_preferences.py +++ b/lib/spack/spack/test/concretize_preferences.py @@ -416,3 +416,13 @@ mpi: ): s = Spec('somevirtual').concretized() assert s.name == 'some-virtual-preferred' + + @pytest.mark.regression('26721,19736') + def test_sticky_variant_accounts_for_packages_yaml(self): + with spack.config.override( + 'packages:sticky-variant', { + 'variants': '+allow-gcc' + } + ): + s = Spec('sticky-variant %gcc').concretized() + assert s.satisfies('%gcc') and s.satisfies('+allow-gcc') diff --git a/lib/spack/spack/variant.py b/lib/spack/spack/variant.py index ccbc5db33e..13c8354ea9 100644 --- a/lib/spack/spack/variant.py +++ b/lib/spack/spack/variant.py @@ -37,7 +37,9 @@ class Variant(object): description, values=(True, False), multi=False, - validator=None): + validator=None, + sticky=False + ): """Initialize a package variant. Args: @@ -51,6 +53,8 @@ class Variant(object): multi (bool): whether multiple CSV are allowed validator (callable): optional callable used to enforce additional logic on the set of values being validated + sticky (bool): if true the variant is set to the default value at + concretization time """ self.name = name self.default = default @@ -83,6 +87,7 @@ class Variant(object): self.multi = multi self.group_validator = validator + self.sticky = sticky def validate_or_raise(self, vspec, pkg=None): """Validate a variant spec against this package variant. Raises an diff --git a/var/spack/repos/builtin.mock/packages/sticky-variant/package.py b/var/spack/repos/builtin.mock/packages/sticky-variant/package.py new file mode 100644 index 0000000000..a1fea2d531 --- /dev/null +++ b/var/spack/repos/builtin.mock/packages/sticky-variant/package.py @@ -0,0 +1,18 @@ +# Copyright 2013-2022 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 import * + + +class StickyVariant(AutotoolsPackage): + """Package with a sticky variant and a conflict""" + + homepage = "http://www.example.com" + url = "http://www.example.com/a-1.0.tar.gz" + + version('1.0', '0123456789abcdef0123456789abcdef') + + variant('allow-gcc', description='', default=False, sticky=True) + + conflicts('%gcc', when='~allow-gcc') diff --git a/var/spack/repos/builtin/packages/cuda/package.py b/var/spack/repos/builtin/packages/cuda/package.py index 378a9b0295..aa76935965 100644 --- a/var/spack/repos/builtin/packages/cuda/package.py +++ b/var/spack/repos/builtin/packages/cuda/package.py @@ -144,6 +144,8 @@ class Cuda(Package): conflicts('arch=darwin-mojave-x86_64') variant('dev', default=False, description='Enable development dependencies, i.e to use cuda-gdb') + variant('allow-unsupported-compilers', default=False, sticky=True, + description='Allow unsupported host compiler and CUDA version combinations') depends_on('libxml2', when='@10.1.243:') # cuda-gdb needed libncurses.so.5 before 11.4.0 -- cgit v1.2.3-60-g2f50