From 31640652c7b5e1a430ab7dfb894347aff96cddd7 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 7 Dec 2023 10:21:01 +0100 Subject: audit: forbid nested dependencies in depends_on declarations (#41428) Forbid nested dependencies in depends_on declarations, by running an audit in CI. Fix the packages not passing the new audit: - amd-aocl - exago - palace - shapemapper - xsdk-examples ginkgo: add a commit sha to v1.5.0.glu_experimental --- lib/spack/spack/audit.py | 28 ++++++++++++++-- .../repos/builtin/packages/amd-aocl/package.py | 37 +++++++++++----------- var/spack/repos/builtin/packages/exago/package.py | 17 +++++----- var/spack/repos/builtin/packages/ginkgo/package.py | 6 +++- var/spack/repos/builtin/packages/palace/package.py | 3 +- .../repos/builtin/packages/shapemapper/package.py | 3 +- .../builtin/packages/xsdk-examples/package.py | 22 ++++++++----- 7 files changed, 77 insertions(+), 39 deletions(-) diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 970e4a3b36..4fe1b1dddf 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -726,13 +726,37 @@ def _unknown_variants_in_directives(pkgs, error_cls): @package_directives -def _unknown_variants_in_dependencies(pkgs, error_cls): - """Report unknown dependencies and wrong variants for dependencies""" +def _issues_in_depends_on_directive(pkgs, error_cls): + """Reports issues with 'depends_on' directives. + + Issues might be unknown dependencies, unknown variants or variant values, or declaration + of nested dependencies. + """ errors = [] for pkg_name in pkgs: pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) filename = spack.repo.PATH.filename_for_package_name(pkg_name) for dependency_name, dependency_data in pkg_cls.dependencies.items(): + # Check if there are nested dependencies declared. We don't want directives like: + # + # depends_on('foo+bar ^fee+baz') + # + # but we'd like to have two dependencies listed instead. + for when, dependency_edge in dependency_data.items(): + dependency_spec = dependency_edge.spec + nested_dependencies = dependency_spec.dependencies() + if nested_dependencies: + summary = ( + f"{pkg_name}: invalid nested dependency " + f"declaration '{str(dependency_spec)}'" + ) + details = [ + f"split depends_on('{str(dependency_spec)}', when='{str(when)}') " + f"into {len(nested_dependencies) + 1} directives", + f"in {filename}", + ] + errors.append(error_cls(summary=summary, details=details)) + # No need to analyze virtual packages if spack.repo.PATH.is_virtual(dependency_name): continue diff --git a/var/spack/repos/builtin/packages/amd-aocl/package.py b/var/spack/repos/builtin/packages/amd-aocl/package.py index a1fb1cb061..c8c1bfee12 100644 --- a/var/spack/repos/builtin/packages/amd-aocl/package.py +++ b/var/spack/repos/builtin/packages/amd-aocl/package.py @@ -32,21 +32,22 @@ class AmdAocl(BundlePackage): version("2.2") variant("openmp", default=False, description="Enable OpenMP support.") - for vers in ["2.2", "3.0", "3.1", "3.2", "4.0", "4.1"]: - depends_on("amdblis@{0} threads=openmp".format(vers), when="@{0} +openmp".format(vers)) - depends_on("amdblis@{0} threads=none".format(vers), when="@{0} ~openmp".format(vers)) - depends_on("amdfftw@{0} +openmp".format(vers), when="@{0} +openmp".format(vers)) - depends_on("amdfftw@{0} ~openmp".format(vers), when="@{0} ~openmp".format(vers)) - depends_on("amdlibflame@{0}".format(vers), when="@{0}".format(vers)) - depends_on("amdlibm@{0}".format(vers), when="@{0}".format(vers)) - depends_on( - "amdscalapack@{0} ^amdblis@{0} ^amdlibflame@{0} threads=none".format(vers), - when="@{0} ~openmp".format(vers), - ) - depends_on( - "amdscalapack@{0} ^amdblis@{0} ^amdlibflame@{0} threads=openmp".format(vers), - when="@{0} +openmp".format(vers), - ) - depends_on( - "aocl-sparse@{0} ^amdblis@{0} ^amdlibflame@{0}".format(vers), when="@{0}".format(vers) - ) + + with when("+openmp"): + depends_on("amdblis threads=openmp") + depends_on("amdfftw +openmp") + depends_on("amdlibflame threads=openmp") + + with when("~openmp"): + depends_on("amdblis threads=none") + depends_on("amdfftw ~openmp") + depends_on("amdlibflame threads=none") + + for vers in ("2.2", "3.0", "3.1", "3.2", "4.0", "4.1"): + with when(f"@{vers}"): + depends_on(f"amdblis@{vers}") + depends_on(f"amdfftw@{vers}") + depends_on(f"amdlibflame@{vers}") + depends_on(f"amdlibm@{vers}") + depends_on(f"amdscalapack@{vers}") + depends_on(f"aocl-sparse@{vers}") diff --git a/var/spack/repos/builtin/packages/exago/package.py b/var/spack/repos/builtin/packages/exago/package.py index 44ede70fa6..415a27e077 100644 --- a/var/spack/repos/builtin/packages/exago/package.py +++ b/var/spack/repos/builtin/packages/exago/package.py @@ -124,14 +124,15 @@ class Exago(CMakePackage, CudaPackage, ROCmPackage): when="+{0} build_type=RelWithDebInfo".format(pkg[1]), ) - depends_on( - "{0} build_type=Release".format("hiop+ginkgo ^ginkgo"), - when="+{0} build_type=Release".format("hiop ^hiop+ginkgo"), - ) - depends_on( - "{0} build_type=Debug".format("hiop+ginkgo ^ginkgo"), - when="+{0} build_type=RelWithDebInfo".format("hiop ^hiop+ginkgo"), - ) + with when("+hiop"): + depends_on("hiop") + with when("build_type=Release"): + depends_on("hiop build_type=Release") + depends_on("ginkgo build_type=Release", when="^hiop+ginkgo") + with when("build_type=Debug"): + depends_on("hiop build_type=RelWithDebInfo") + depends_on("ginkgo build_type=Debug", when="^hiop+ginkgo") + # depends_on("hpctoolkit", when="with_profiling=hpctoolkit") # depends_on("tau", when="with_profiling=tau") # ^ need to depend when both hpctoolkit and tau diff --git a/var/spack/repos/builtin/packages/ginkgo/package.py b/var/spack/repos/builtin/packages/ginkgo/package.py index 37d974f9b7..4df0671449 100644 --- a/var/spack/repos/builtin/packages/ginkgo/package.py +++ b/var/spack/repos/builtin/packages/ginkgo/package.py @@ -27,7 +27,11 @@ class Ginkgo(CMakePackage, CudaPackage, ROCmPackage): version("1.7.0", commit="49242ff89af1e695d7794f6d50ed9933024b66fe") # v1.7.0 version("1.6.0", commit="1f1ed46e724334626f016f105213c047e16bc1ae") # v1.6.0 version("1.5.0", commit="234594c92b58e2384dfb43c2d08e7f43e2b58e7a") # v1.5.0 - version("1.5.0.glu_experimental", branch="glu_experimental") + version( + "1.5.0.glu_experimental", + branch="glu_experimental", + commit="e234eab1bd7afe85dd594638e291a2caf464bfb1", + ) version("1.4.0", commit="f811917c1def4d0fcd8db3fe5c948ce13409e28e") # v1.4.0 version("1.3.0", commit="4678668c66f634169def81620a85c9a20b7cec78") # v1.3.0 version("1.2.0", commit="b4be2be961fd5db45c3d02b5e004d73550722e31") # v1.2.0 diff --git a/var/spack/repos/builtin/packages/palace/package.py b/var/spack/repos/builtin/packages/palace/package.py index 9da8c20967..6fdbbd7bd7 100644 --- a/var/spack/repos/builtin/packages/palace/package.py +++ b/var/spack/repos/builtin/packages/palace/package.py @@ -81,7 +81,8 @@ class Palace(CMakePackage): depends_on("mumps~openmp", when="~openmp") with when("+slepc"): - depends_on("slepc ^petsc+mpi+double+complex") + depends_on("slepc") + depends_on("petsc+mpi+double+complex") depends_on("petsc+shared", when="+shared") depends_on("petsc~shared", when="~shared") depends_on("petsc+int64", when="+int64") diff --git a/var/spack/repos/builtin/packages/shapemapper/package.py b/var/spack/repos/builtin/packages/shapemapper/package.py index 1037f44b6b..03decdac72 100644 --- a/var/spack/repos/builtin/packages/shapemapper/package.py +++ b/var/spack/repos/builtin/packages/shapemapper/package.py @@ -24,7 +24,8 @@ class Shapemapper(CMakePackage): url="https://github.com/Weeks-UNC/shapemapper2/releases/download/2.1.5/shapemapper-2.1.5-source-only.tar.gz", ) - depends_on("bowtie2@2.3.0: ^perl+threads", type="run") + depends_on("bowtie2@2.3.0:", type="run") + depends_on("perl+threads", type="run") # hard version dep due to jni depends_on("bbmap@37.78", type="run") depends_on("boost+filesystem+program_options+iostreams+system") diff --git a/var/spack/repos/builtin/packages/xsdk-examples/package.py b/var/spack/repos/builtin/packages/xsdk-examples/package.py index 1f39bcada7..cf7ce092a8 100644 --- a/var/spack/repos/builtin/packages/xsdk-examples/package.py +++ b/var/spack/repos/builtin/packages/xsdk-examples/package.py @@ -39,14 +39,20 @@ class XsdkExamples(CMakePackage, CudaPackage, ROCmPackage): # Use ^dealii~hdf5 because of HDF5 linking issue in deal.II 9.4.0. # Disable 'arborx' to remove the 'kokkos' dependency which conflicts with # the internal Kokkos used by 'trilinos': - depends_on("xsdk@0.8.0 ~arborx ^mfem+pumi ^dealii~hdf5", when="@0.4.0") - depends_on("xsdk@0.8.0 ^mfem+strumpack", when="@0.4.0 ^xsdk+strumpack") - depends_on("xsdk@0.8.0 ^mfem+ginkgo", when="@0.4.0 ^xsdk+ginkgo") - depends_on("xsdk@0.8.0 ^mfem+hiop", when="@0.4.0 ^xsdk+hiop") - depends_on("xsdk@0.8.0 ^sundials+magma", when="@0.4.0 +cuda") - depends_on("xsdk@0.7.0", when="@0.3.0") - depends_on("xsdk@0.7.0 ^mfem+strumpack", when="@0.3.0 ^xsdk+strumpack") - depends_on("xsdk@0.7.0 ^sundials+magma", when="@0.3.0 +cuda") + with when("@0.4.0"): + depends_on("xsdk@0.8.0 ~arborx") + depends_on("mfem+pumi") + depends_on("dealii~hdf5") + depends_on("mfem+strumpack", when="^xsdk+strumpack") + depends_on("mfem+ginkgo", when="^xsdk+ginkgo") + depends_on("mfem+hiop", when="^xsdk+hiop") + depends_on("sundials+magma", when="+cuda") + + with when("@0.3.0"): + depends_on("xsdk@0.7.0") + depends_on("mfem+strumpack", when="^xsdk+strumpack") + depends_on("sundials+magma", when="+cuda") + depends_on("mpi") depends_on("cmake@3.21:", type="build", when="@0.3.0:") -- cgit v1.2.3-60-g2f50