From 8b1009a4a074d31b02defc6800ac0161c0bfdc39 Mon Sep 17 00:00:00 2001 From: Todd Gamblin Date: Mon, 25 Nov 2024 00:02:40 -0800 Subject: [PATCH] `resource`: clean up arguments and typing - [x] Clean up arguments on the `resource` directive. - [x] Add type annotations - [x] Add `resource` to type annotations on `PackageBase` - [x] Fix up `resource` docstrings Signed-off-by: Todd Gamblin --- lib/spack/spack/audit.py | 6 +-- lib/spack/spack/directives.py | 67 ++++++++++++++++----------------- lib/spack/spack/package_base.py | 2 + lib/spack/spack/resource.py | 5 ++- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/lib/spack/spack/audit.py b/lib/spack/spack/audit.py index 7e6b87c987c..77e85172d72 100644 --- a/lib/spack/spack/audit.py +++ b/lib/spack/spack/audit.py @@ -693,19 +693,19 @@ def invalid_sha256_digest(fetcher): return h, True return None, False - error_msg = "Package '{}' does not use sha256 checksum".format(pkg_name) + error_msg = f"Package '{pkg_name}' does not use sha256 checksum" details = [] for v, args in pkg.versions.items(): fetcher = spack.fetch_strategy.for_package_version(pkg, v) digest, is_bad = invalid_sha256_digest(fetcher) if is_bad: - details.append("{}@{} uses {}".format(pkg_name, v, digest)) + details.append(f"{pkg_name}@{v} uses {digest}") for _, resources in pkg.resources.items(): for resource in resources: digest, is_bad = invalid_sha256_digest(resource.fetcher) if is_bad: - details.append("Resource in '{}' uses {}".format(pkg_name, digest)) + details.append(f"Resource in '{pkg_name}' uses {digest}") if details: errors.append(error_cls(error_msg, details)) diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 87e68da002e..2f152f813d9 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -40,6 +40,7 @@ class OpenMpi(Package): import llnl.util.tty.color import spack.deptypes as dt +import spack.fetch_strategy import spack.package_base import spack.patch import spack.spec @@ -47,7 +48,6 @@ class OpenMpi(Package): import spack.variant from spack.dependency import Dependency from spack.directives_meta import DirectiveError, DirectiveMeta -from spack.fetch_strategy import from_kwargs from spack.resource import Resource from spack.version import ( GitVersion, @@ -740,58 +740,55 @@ def _execute_variant(pkg): @directive("resources") -def resource(**kwargs): - """Define an external resource to be fetched and staged when building the - package. Based on the keywords present in the dictionary the appropriate - FetchStrategy will be used for the resource. Resources are fetched and - staged in their own folder inside spack stage area, and then moved into - the stage area of the package that needs them. +def resource( + *, + name: Optional[str] = None, + destination: str = "", + placement: Optional[str] = None, + when: WhenType = None, + # additional kwargs are as for `version()` + **kwargs, +): + """Define an external resource to be fetched and staged when building the package. + Based on the keywords present in the dictionary the appropriate FetchStrategy will + be used for the resource. Resources are fetched and staged in their own folder + inside spack stage area, and then moved into the stage area of the package that + needs them. - List of recognized keywords: + Keyword Arguments: + name: name for the resource + when: condition defining when the resource is needed + destination: path, relative to the package stage area, to which resource should be moved + placement: optionally rename the expanded resource inside the destination directory - * 'when' : (optional) represents the condition upon which the resource is - needed - * 'destination' : (optional) path where to move the resource. This path - must be relative to the main package stage area. - * 'placement' : (optional) gives the possibility to fine tune how the - resource is moved into the main package stage area. """ def _execute_resource(pkg): - when = kwargs.get("when") when_spec = _make_when_spec(when) if not when_spec: return - destination = kwargs.get("destination", "") - placement = kwargs.get("placement", None) - # Check if the path is relative if os.path.isabs(destination): - message = ( - "The destination keyword of a resource directive " "can't be an absolute path.\n" - ) - message += "\tdestination : '{dest}\n'".format(dest=destination) - raise RuntimeError(message) + msg = "The destination keyword of a resource directive can't be an absolute path.\n" + msg += f"\tdestination : '{destination}\n'" + raise RuntimeError(msg) # Check if the path falls within the main package stage area test_path = "stage_folder_root" - normalized_destination = os.path.normpath( - os.path.join(test_path, destination) - ) # Normalized absolute path + + # Normalized absolute path + normalized_destination = os.path.normpath(os.path.join(test_path, destination)) if test_path not in normalized_destination: - message = ( - "The destination folder of a resource must fall " - "within the main package stage directory.\n" - ) - message += "\tdestination : '{dest}'\n".format(dest=destination) - raise RuntimeError(message) + msg = "Destination of a resource must be within the package stage directory.\n" + msg += f"\tdestination : '{destination}'\n" + raise RuntimeError(msg) resources = pkg.resources.setdefault(when_spec, []) - name = kwargs.get("name") - fetcher = from_kwargs(**kwargs) - resources.append(Resource(name, fetcher, destination, placement)) + resources.append( + Resource(name, spack.fetch_strategy.from_kwargs(**kwargs), destination, placement) + ) return _execute_resource diff --git a/lib/spack/spack/package_base.py b/lib/spack/spack/package_base.py index cfd28038054..85f23c4b8ef 100644 --- a/lib/spack/spack/package_base.py +++ b/lib/spack/spack/package_base.py @@ -54,6 +54,7 @@ import spack.variant from spack.error import InstallError, NoURLError, PackageError from spack.filesystem_view import YamlFilesystemView +from spack.resource import Resource from spack.solver.version_order import concretization_version_order from spack.stage import DevelopStage, ResourceStage, Stage, StageComposite, compute_stage_name from spack.util.package_hash import package_hash @@ -585,6 +586,7 @@ class PackageBase(WindowsRPath, PackageViewMixin, metaclass=PackageMeta): # Declare versions dictionary as placeholder for values. # This allows analysis tools to correctly interpret the class attributes. versions: dict + resources: Dict[spack.spec.Spec, List[Resource]] dependencies: Dict[spack.spec.Spec, Dict[str, spack.dependency.Dependency]] conflicts: Dict[spack.spec.Spec, List[Tuple[spack.spec.Spec, Optional[str]]]] requirements: Dict[ diff --git a/lib/spack/spack/resource.py b/lib/spack/spack/resource.py index ba855cbb638..5f222fe016a 100644 --- a/lib/spack/spack/resource.py +++ b/lib/spack/spack/resource.py @@ -12,7 +12,10 @@ class Resource: - """Represents an optional resource to be fetched by a package. + """Represents any resource to be fetched by a package. + + This includes the main tarball or source archive, as well as extra archives defined + by the resource() directive. Aggregates a name, a fetcher, a destination and a placement. """