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

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:")