From 1e1cb68b849af2fc26b2a52b74a26fd42e6137d8 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Thu, 16 Nov 2023 14:19:05 +0100 Subject: Add audit check to spot `when=` arguments using wrong named specs (#41107) * Add audit check to spot when= arguments using named specs * Fix package issues caught by the new audit --- lib/spack/spack/audit.py | 49 +++++++++++++++++++++- var/spack/repos/builtin/packages/cpr/package.py | 2 +- .../repos/builtin/packages/interproscan/package.py | 6 +-- var/spack/repos/builtin/packages/lbann/package.py | 2 +- .../repos/builtin/packages/py-abipy/package.py | 2 +- .../repos/builtin/packages/py-kombu/package.py | 4 +- .../builtin/packages/py-nvidia-dali/package.py | 24 +++++------ .../repos/builtin/packages/py-pdbfixer/package.py | 2 +- .../packages/py-tensorflow-datasets/package.py | 4 +- 9 files changed, 71 insertions(+), 24 deletions(-) diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 8b13ffc7cf..66c7008580 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -776,7 +776,7 @@ def _version_constraints_are_satisfiable_by_some_version_in_repo(pkgs, error_cls ) except Exception: summary = ( - "{0}: dependency on {1} cannot be satisfied " "by known versions of {1.name}" + "{0}: dependency on {1} cannot be satisfied by known versions of {1.name}" ).format(pkg_name, s) details = ["happening in " + filename] if dependency_pkg_cls is not None: @@ -818,6 +818,53 @@ def _analyze_variants_in_directive(pkg, constraint, directive, error_cls): return errors +@package_directives +def _named_specs_in_when_arguments(pkgs, error_cls): + """Reports named specs in the 'when=' attribute of a directive. + + Note that 'conflicts' is the only directive allowing that. + """ + errors = [] + for pkg_name in pkgs: + pkg_cls = spack.repo.PATH.get_pkg_class(pkg_name) + + def _extracts_errors(triggers, summary): + _errors = [] + for trigger in list(triggers): + when_spec = spack.spec.Spec(trigger) + if when_spec.name is not None and when_spec.name != pkg_name: + details = [f"using '{trigger}', should be '^{trigger}'"] + _errors.append(error_cls(summary=summary, details=details)) + return _errors + + for dname, triggers in pkg_cls.dependencies.items(): + summary = f"{pkg_name}: wrong 'when=' condition for the '{dname}' dependency" + errors.extend(_extracts_errors(triggers, summary)) + + for vname, (variant, triggers) in pkg_cls.variants.items(): + summary = f"{pkg_name}: wrong 'when=' condition for the '{vname}' variant" + errors.extend(_extracts_errors(triggers, summary)) + + for provided, triggers in pkg_cls.provided.items(): + summary = f"{pkg_name}: wrong 'when=' condition for the '{provided}' virtual" + errors.extend(_extracts_errors(triggers, summary)) + + for _, triggers in pkg_cls.requirements.items(): + triggers = [when_spec for when_spec, _, _ in triggers] + summary = f"{pkg_name}: wrong 'when=' condition in 'requires' directive" + errors.extend(_extracts_errors(triggers, summary)) + + triggers = list(pkg_cls.patches) + summary = f"{pkg_name}: wrong 'when=' condition in 'patch' directives" + errors.extend(_extracts_errors(triggers, summary)) + + triggers = list(pkg_cls.resources) + summary = f"{pkg_name}: wrong 'when=' condition in 'resource' directives" + errors.extend(_extracts_errors(triggers, summary)) + + return llnl.util.lang.dedupe(errors) + + #: Sanity checks on package directives external_detection = AuditClass( group="externals", diff --git a/var/spack/repos/builtin/packages/cpr/package.py b/var/spack/repos/builtin/packages/cpr/package.py index 71e32d9960..0d18a6a919 100644 --- a/var/spack/repos/builtin/packages/cpr/package.py +++ b/var/spack/repos/builtin/packages/cpr/package.py @@ -18,7 +18,7 @@ class Cpr(CMakePackage): version("1.9.2", sha256="3bfbffb22c51f322780d10d3ca8f79424190d7ac4b5ad6ad896de08dbd06bf31") depends_on("curl") - depends_on("git", when="build") + depends_on("git", type="build") def cmake_args(self): _force = "_FORCE" if self.spec.satisfies("@:1.9") else "" diff --git a/var/spack/repos/builtin/packages/interproscan/package.py b/var/spack/repos/builtin/packages/interproscan/package.py index 82380135a7..4143dc6ff2 100644 --- a/var/spack/repos/builtin/packages/interproscan/package.py +++ b/var/spack/repos/builtin/packages/interproscan/package.py @@ -45,21 +45,21 @@ class Interproscan(Package): ) resource( - when="5.56-89.0 +databases", + when="@5.56-89.0 +databases", name="databases", url="https://ftp.ebi.ac.uk/pub/databases/interpro/iprscan/5/5.56-89.0/alt/interproscan-data-5.56-89.0.tar.gz", sha256="49cd0c69711f9469f3b68857f4581b23ff12765ca2b12893d18e5a9a5cd8032d", ) resource( - when="5.38-76.0 +databases", + when="@5.38-76.0 +databases", name="databases", url="https://ftp.ebi.ac.uk/pub/databases/interpro/iprscan/5/5.38-76.0/alt/interproscan-data-5.38-76.0.tar.gz", sha256="e05e15d701037504f92ecf849c20317e70df28e78ff1945826b3c1e16d9b9cce", ) resource( - when="5.36-75.0 +databases", + when="@5.36-75.0 +databases", name="databases", url="https://ftp.ebi.ac.uk/pub/databases/interpro/iprscan/5/5.36-75.0/alt/interproscan-data-5.36-75.0.tar.gz", sha256="e9b1e6f2d1c20d06661a31a08c973bc8ddf039a4cf1e45ec4443200375e5d6a4", diff --git a/var/spack/repos/builtin/packages/lbann/package.py b/var/spack/repos/builtin/packages/lbann/package.py index ebe68f39db..14f257a341 100644 --- a/var/spack/repos/builtin/packages/lbann/package.py +++ b/var/spack/repos/builtin/packages/lbann/package.py @@ -209,7 +209,7 @@ class Lbann(CachedCMakePackage, CudaPackage, ROCmPackage): depends_on("py-protobuf+cpp@3.10.0:4.21.12", type=("build", "run"), when="+pfe") depends_on("protobuf+shared@3.10.0:3.21.12") - depends_on("zlib-api", when="protobuf@3.11.0:") + depends_on("zlib-api", when="^protobuf@3.11.0:") # using cereal@1.3.1 and above requires changing the # find_package call to lowercase, so stick with :1.3.0 diff --git a/var/spack/repos/builtin/packages/py-abipy/package.py b/var/spack/repos/builtin/packages/py-abipy/package.py index 3e868f5607..dfaed29c7d 100644 --- a/var/spack/repos/builtin/packages/py-abipy/package.py +++ b/var/spack/repos/builtin/packages/py-abipy/package.py @@ -17,7 +17,7 @@ class PyAbipy(PythonPackage): version("0.2.0", sha256="c72b796ba0f9ea4299eac3085bede092d2652e9e5e8074d3badd19ef7b600792") variant("gui", default=False, description="Build the GUI") - variant("ipython", default=False, when="0.2.0", description="Build IPython support") + variant("ipython", default=False, when="@0.2.0", description="Build IPython support") depends_on("py-setuptools", type="build") # in newer pip versions --install-option does not exist diff --git a/var/spack/repos/builtin/packages/py-kombu/package.py b/var/spack/repos/builtin/packages/py-kombu/package.py index 6f13c380ff..257b0acd7f 100644 --- a/var/spack/repos/builtin/packages/py-kombu/package.py +++ b/var/spack/repos/builtin/packages/py-kombu/package.py @@ -32,7 +32,7 @@ class PyKombu(PythonPackage): depends_on("py-amqp@5.0.0:5", when="@5.0.0:5.0.2", type=("build", "run")) depends_on("py-amqp@5.0.9:5.0", when="@5.2.3", type=("build", "run")) depends_on("py-vine", when="@5.1.0:", type=("build", "run")) - depends_on("py-importlib-metadata@0.18:", type=("build", "run"), when="python@:3.7") - depends_on("py-cached-property", type=("build", "run"), when="python@:3.7") + depends_on("py-importlib-metadata@0.18:", type=("build", "run"), when="^python@:3.7") + depends_on("py-cached-property", type=("build", "run"), when="^python@:3.7") depends_on("py-redis@3.4.1:3,4.0.2:", when="+redis", type=("build", "run")) diff --git a/var/spack/repos/builtin/packages/py-nvidia-dali/package.py b/var/spack/repos/builtin/packages/py-nvidia-dali/package.py index 2b1af9e19a..93804505fb 100644 --- a/var/spack/repos/builtin/packages/py-nvidia-dali/package.py +++ b/var/spack/repos/builtin/packages/py-nvidia-dali/package.py @@ -170,20 +170,20 @@ class PyNvidiaDali(PythonPackage): ) cuda120_versions = ( - "1.27.0-cuda120", - "1.26.0-cuda120", - "1.25.0-cuda120", - "1.24.0-cuda120", - "1.23.0-cuda120", - "1.22.0-cuda120", + "@1.27.0-cuda120", + "@1.26.0-cuda120", + "@1.25.0-cuda120", + "@1.24.0-cuda120", + "@1.23.0-cuda120", + "@1.22.0-cuda120", ) cuda110_versions = ( - "1.27.0-cuda110", - "1.26.0-cuda110", - "1.25.0-cuda110", - "1.24.0-cuda110", - "1.23.0-cuda110", - "1.22.0-cuda110", + "@1.27.0-cuda110", + "@1.26.0-cuda110", + "@1.25.0-cuda110", + "@1.24.0-cuda110", + "@1.23.0-cuda110", + "@1.22.0-cuda110", ) for v in cuda120_versions: diff --git a/var/spack/repos/builtin/packages/py-pdbfixer/package.py b/var/spack/repos/builtin/packages/py-pdbfixer/package.py index 2da9f24d1a..2dbd4aa3ee 100644 --- a/var/spack/repos/builtin/packages/py-pdbfixer/package.py +++ b/var/spack/repos/builtin/packages/py-pdbfixer/package.py @@ -18,6 +18,6 @@ class PyPdbfixer(PythonPackage): version("1.7", sha256="a0bef3c52a7bbe69a6aea5333f51f3e7d158339be5829aed19b0344bd66d4eea") depends_on("py-setuptools", type="build") - depends_on("openmm@7.1:7.5", type=("build", "run"), when="1.7") + depends_on("openmm@7.1:7.5", type=("build", "run"), when="@1.7") depends_on("openmm@7.6:", type=("build", "run"), when="@1.8:") depends_on("py-numpy", type=("build", "run")) diff --git a/var/spack/repos/builtin/packages/py-tensorflow-datasets/package.py b/var/spack/repos/builtin/packages/py-tensorflow-datasets/package.py index 1ad767902d..8189fa0c49 100644 --- a/var/spack/repos/builtin/packages/py-tensorflow-datasets/package.py +++ b/var/spack/repos/builtin/packages/py-tensorflow-datasets/package.py @@ -29,5 +29,5 @@ class PyTensorflowDatasets(PythonPackage): depends_on("py-tensorflow-metadata", type=("build", "run")) depends_on("py-termcolor", type=("build", "run")) depends_on("py-tqdm", type=("build", "run")) - depends_on("py-typing-extensions", type=("build", "run"), when="python@:3.7") - depends_on("py-importlib-resources", type=("build", "run"), when="python@:3.8") + depends_on("py-typing-extensions", type=("build", "run"), when="^python@:3.7") + depends_on("py-importlib-resources", type=("build", "run"), when="^python@:3.8") -- cgit v1.2.3-60-g2f50