From d51af675ef345a191aa995b82609e5f6776184a3 Mon Sep 17 00:00:00 2001 From: Harmen Stoppels Date: Wed, 19 Apr 2023 23:17:47 +0200 Subject: make version(...) kwargs explicit (#36998) - [x] Replace `version(ver, checksum=None, **kwargs)` signature with `version(ver, checksum=None, *, sha256=..., ...)` explicitly listing all arguments. - [x] Fix various issues in packages: - `tags` instead of `tag` - `default` instead of `preferred` - `sha26` instead of `sha256` - etc Also, use `sha256=...` consistently. Note: setting `sha256` currently doesn't validate the checksum length, so you could do `sha256="a"*32` and it would get checked as `md5`... but that's something for another PR. --- lib/spack/docs/packaging_guide.rst | 30 ++++++------ lib/spack/spack/cmd/create.py | 2 +- lib/spack/spack/directives.py | 94 ++++++++++++++++++++++++++++++++------ lib/spack/spack/test/cmd/pkg.py | 2 +- 4 files changed, 97 insertions(+), 31 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index d7d63f634e..969da6de20 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -604,9 +604,9 @@ add a line like this in the package class: url = "http://example.com/foo-1.0.tar.gz" - version("8.2.1", "4136d7b4c04df68b686570afa26988ac") - version("8.2.0", "1c9f62f0778697a09d36121ead88e08e") - version("8.1.2", "d47dd09ed7ae6e7fd6f9a816d7f5fdf6") + version("8.2.1", md5="4136d7b4c04df68b686570afa26988ac") + version("8.2.0", md5="1c9f62f0778697a09d36121ead88e08e") + version("8.1.2", md5="d47dd09ed7ae6e7fd6f9a816d7f5fdf6") Versions should be listed in descending order, from newest to oldest. @@ -695,7 +695,7 @@ of its versions, you can add an explicit URL for a particular version: .. code-block:: python - version("8.2.1", "4136d7b4c04df68b686570afa26988ac", + version("8.2.1", md5="4136d7b4c04df68b686570afa26988ac", url="http://example.com/foo-8.2.1-special-version.tar.gz") @@ -757,7 +757,7 @@ executables and other custom archive types), you can add ``expand=False`` to a .. code-block:: python - version("8.2.1", "4136d7b4c04df68b686570afa26988ac", + version("8.2.1", md5="4136d7b4c04df68b686570afa26988ac", url="http://example.com/foo-8.2.1-special-version.sh", expand=False) When ``expand`` is set to ``False``, Spack sets the current working @@ -955,10 +955,10 @@ file: .. code-block:: console ==> Checksummed new versions of libelf: - version("0.8.13", "4136d7b4c04df68b686570afa26988ac") - version("0.8.12", "e21f8273d9f5f6d43a59878dc274fec7") - version("0.8.11", "e931910b6d100f6caa32239849947fbf") - version("0.8.10", "9db4d36c283d9790d8fa7df1f4d7b4d9") + version("0.8.13", md5="4136d7b4c04df68b686570afa26988ac") + version("0.8.12", md5="e21f8273d9f5f6d43a59878dc274fec7") + version("0.8.11", md5="e931910b6d100f6caa32239849947fbf") + version("0.8.10", md5="9db4d36c283d9790d8fa7df1f4d7b4d9") By default, Spack will search for new tarball downloads by scraping the parent directory of the tarball you gave it. So, if your tarball @@ -1112,8 +1112,8 @@ class-level tarball URL and VCS. For example: version("develop", branch="develop") version("master", branch="master") - version("12.12.1", "ecd4606fa332212433c98bf950a69cc7") - version("12.10.1", "667333dbd7c0f031d47d7c5511fd0810") + version("12.12.1", md5="ecd4606fa332212433c98bf950a69cc7") + version("12.10.1", md5="667333dbd7c0f031d47d7c5511fd0810") version("12.8.1", "9f37f683ee2b427b5540db8a20ed6b15") If a package contains both a ``url`` and ``git`` class-level attribute, @@ -1276,7 +1276,7 @@ checksum. .. code-block:: python - version("1.9.5.1.1", "d035e4bc704d136db79b43ab371b27d2", + version("1.9.5.1.1", md5="d035e4bc704d136db79b43ab371b27d2", url="https://www.github.com/jswhit/pyproj/tarball/0be612cc9f972e38b50a90c946a9b353e2ab140f") .. _hg-fetch: @@ -2212,7 +2212,7 @@ looks like this: homepage = "http://www.openssl.org" url = "http://www.openssl.org/source/openssl-1.0.1h.tar.gz" - version("1.0.1h", "8d6d684a9430d5cc98a62a5d8fbda8cf") + version("1.0.1h", md5="8d6d684a9430d5cc98a62a5d8fbda8cf") depends_on("zlib") parallel = False @@ -2309,7 +2309,7 @@ Spack makes this relatively easy. Let's take a look at the url = "http://www.prevanders.net/libdwarf-20130729.tar.gz" list_url = homepage - version("20130729", "4cc5e48693f7b93b7aa0261e63c0e21d") + version("20130729", md5="4cc5e48693f7b93b7aa0261e63c0e21d") ... depends_on("libelf") @@ -2808,7 +2808,7 @@ supplying a ``depends_on`` call in the package definition. For example: homepage = "https://github.com/hpc/mpileaks" url = "https://github.com/hpc/mpileaks/releases/download/v1.0/mpileaks-1.0.tar.gz" - version("1.0", "8838c574b39202a57d7c2d68692718aa") + version("1.0", md5="8838c574b39202a57d7c2d68692718aa") depends_on("mpi") depends_on("adept-utils") diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py index 8ee259c52d..3582877ee1 100644 --- a/lib/spack/spack/cmd/create.py +++ b/lib/spack/spack/cmd/create.py @@ -807,7 +807,7 @@ def get_versions(args, name): # Default version with hash hashed_versions = """\ # FIXME: Add proper versions and checksums here. - # version("1.2.3", "0123456789abcdef0123456789abcdef")""" + # version("1.2.3", md5="0123456789abcdef0123456789abcdef")""" # Default version without hash unhashed_versions = """\ diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index c4d5865d9e..341b8b2c1f 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -32,7 +32,7 @@ import collections.abc import functools import os.path import re -from typing import List, Set +from typing import List, Optional, Set import llnl.util.lang import llnl.util.tty.color @@ -317,34 +317,100 @@ directive = DirectiveMeta.directive @directive("versions") -def version(ver, checksum=None, **kwargs): +def version( + ver: str, + # this positional argument is deprecated, use sha256=... instead + checksum: Optional[str] = None, + *, + # generic version options + preferred: Optional[bool] = None, + deprecated: Optional[bool] = None, + no_cache: Optional[bool] = None, + # url fetch options + url: Optional[str] = None, + extension: Optional[str] = None, + expand: Optional[bool] = None, + fetch_options: Optional[dict] = None, + # url archive verification options + md5: Optional[str] = None, + sha1: Optional[str] = None, + sha224: Optional[str] = None, + sha256: Optional[str] = None, + sha384: Optional[str] = None, + sha512: Optional[str] = None, + # git fetch options + git: Optional[str] = None, + commit: Optional[str] = None, + tag: Optional[str] = None, + branch: Optional[str] = None, + get_full_repo: Optional[bool] = None, + submodules: Optional[bool] = None, + submodules_delete: Optional[bool] = None, + # other version control + svn: Optional[str] = None, + hg: Optional[str] = None, + cvs: Optional[str] = None, + revision: Optional[str] = None, + date: Optional[str] = None, +): """Adds a version and, if appropriate, metadata for fetching its code. The ``version`` directives are aggregated into a ``versions`` dictionary attribute with ``Version`` keys and metadata values, where the metadata is stored as a dictionary of ``kwargs``. - The ``dict`` of arguments is turned into a valid fetch strategy for + The (keyword) arguments are turned into a valid fetch strategy for code packages later. See ``spack.fetch_strategy.for_package_version()``. - - Keyword Arguments: - deprecated (bool): whether or not this version is deprecated """ def _execute_version(pkg): - if checksum is not None: - if hasattr(pkg, "has_code") and not pkg.has_code: - raise VersionChecksumError( - "{0}: Checksums not allowed in no-code packages" - "(see '{1}' version).".format(pkg.name, ver) - ) + if ( + any((sha256, sha384, sha512, md5, sha1, sha224, checksum)) + and hasattr(pkg, "has_code") + and not pkg.has_code + ): + raise VersionChecksumError( + "{0}: Checksums not allowed in no-code packages " + "(see '{1}' version).".format(pkg.name, ver) + ) - kwargs["checksum"] = checksum + kwargs = { + key: value + for key, value in ( + ("sha256", sha256), + ("sha384", sha384), + ("sha512", sha512), + ("preferred", preferred), + ("deprecated", deprecated), + ("expand", expand), + ("url", url), + ("extension", extension), + ("no_cache", no_cache), + ("fetch_options", fetch_options), + ("git", git), + ("svn", svn), + ("hg", hg), + ("cvs", cvs), + ("get_full_repo", get_full_repo), + ("branch", branch), + ("submodules", submodules), + ("submodules_delete", submodules_delete), + ("commit", commit), + ("tag", tag), + ("revision", revision), + ("date", date), + ("md5", md5), + ("sha1", sha1), + ("sha224", sha224), + ("checksum", checksum), + ) + if value is not None + } # Store kwargs for the package to later with a fetch_strategy. version = Version(ver) if isinstance(version, GitVersion): - if not hasattr(pkg, "git") and "git" not in kwargs: + if git is None and not hasattr(pkg, "git"): msg = "Spack version directives cannot include git hashes fetched from" msg += " URLs. Error in package '%s'\n" % pkg.name msg += " version('%s', " % version.string diff --git a/lib/spack/spack/test/cmd/pkg.py b/lib/spack/spack/test/cmd/pkg.py index 4ae8b516c0..7c5aae590e 100644 --- a/lib/spack/spack/test/cmd/pkg.py +++ b/lib/spack/spack/test/cmd/pkg.py @@ -25,7 +25,7 @@ class {name}(Package): homepage = "http://www.example.com" url = "http://www.example.com/test-1.0.tar.gz" - version('1.0', '0123456789abcdef0123456789abcdef') + version("1.0", md5="0123456789abcdef0123456789abcdef") def install(self, spec, prefix): pass -- cgit v1.2.3-60-g2f50