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.
2023-04-19 23:17:47 +02:00
parent c7508dc216
commit d51af675ef
308 changed files with 661 additions and 580 deletions
--- 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")
--- 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 = """\
--- a/lib/spack/spack/directives.py
+++ b/lib/spack/spack/directives.py
@@ -32,7 +32,7 @@ class OpenMpi(Package):
 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 @@ def remove_directives(arg):


@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
--- 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