From 7105cc8c0131bd219fc688787e3ed36822a1cadb Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 12 Dec 2024 14:19:05 +0100 Subject: [PATCH] Make use of `^` in 'depends_on' an error (#48062) The use of `^` in `depends_on` directives has never been allowed, since the dawn of Spack. Up to now, we used to have an audit to catch this kind of issue, mainly because in that way we could easily collect all issues and report them to packagers at once. Due to implementation details, this audit doesn't work if a dependency without a `^` is followed by the same dependency with a `^`. This PR makes this pattern an error, which will be reported eagerly, and removes the corresponding audit. It also fixes a package using the wrong idiom. Co-authored-by: Harmen Stoppels --- lib/spack/spack/audit.py | 14 -------------- lib/spack/spack/directives.py | 7 +++++++ .../repos/builtin/packages/amd-aocl/package.py | 11 ++++++++--- 3 files changed, 15 insertions(+), 17 deletions(-) diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 77e85172d72..500c8282e5a 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -1009,20 +1009,6 @@ def _issues_in_depends_on_directive(pkgs, error_cls): for when, deps_by_name in pkg_cls.dependencies.items(): for dep_name, dep in deps_by_name.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. - nested_dependencies = dep.spec.dependencies() - if nested_dependencies: - summary = f"{pkg_name}: nested dependency declaration '{dep.spec}'" - ndir = len(nested_dependencies) + 1 - details = [ - f"split depends_on('{dep.spec}', when='{when}') into {ndir} directives", - f"in {filename}", - ] - errors.append(error_cls(summary=summary, details=details)) def check_virtual_with_variants(spec, msg): if not spec.virtual or not spec.variants: diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 08f6ec9ba95..e20515bd509 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -297,6 +297,13 @@ def _depends_on( deps_by_name = pkg.dependencies.setdefault(when_spec, {}) dependency = deps_by_name.get(spec.name) + if spec.dependencies(): + raise DirectiveError( + f"the '^' sigil cannot be used in 'depends_on' directives. Please reformulate " + f"the directive below as multiple directives:\n\n" + f'\tdepends_on("{spec}", when="{when_spec}")\n' + ) + if not dependency: dependency = Dependency(pkg, spec, depflag=depflag) deps_by_name[spec.name] = dependency diff --git a/var/spack/repos/builtin/packages/amd-aocl/package.py b/var/spack/repos/builtin/packages/amd-aocl/package.py index 85f7148c818..93616521a16 100644 --- a/var/spack/repos/builtin/packages/amd-aocl/package.py +++ b/var/spack/repos/builtin/packages/amd-aocl/package.py @@ -35,6 +35,14 @@ class AmdAocl(BundlePackage): variant("openmp", default=False, description="Enable OpenMP support.") + depends_on("scalapack") + depends_on("lapack") + depends_on("blas") + + requires("^[virtuals=scalapack] amdscalapack") + requires("^[virtuals=lapack] amdlibflame") + requires("^[virtuals=blas] amdblis") + with when("+openmp"): depends_on("amdblis threads=openmp") depends_on("amdfftw +openmp") @@ -56,11 +64,8 @@ class AmdAocl(BundlePackage): depends_on(f"amdblis@={vers}") depends_on(f"amdfftw@={vers}") depends_on(f"amdlibflame@={vers}") - depends_on("amdlibflame ^[virtuals=blas] amdblis") depends_on(f"amdlibm@={vers}") depends_on(f"amdscalapack@={vers}") - depends_on("amdscalapack ^[virtuals=blas] amdblis") - depends_on("amdscalapack ^[virtuals=lapack] amdlibflame") depends_on(f"aocl-sparse@={vers}") if Version(vers) >= Version("4.2"): depends_on(f"aocl-compression@={vers}")