From cc2ae9f270befa554ba8b09c68e89bb8248ea650 Mon Sep 17 00:00:00 2001 From: Massimiliano Culpo Date: Fri, 27 Jan 2023 07:51:24 +0100 Subject: Add a `maintainers` directive (#35083) fixes #34879 This commit adds a new maintainer directive, which by default extend the list of maintainers for a given package. The directive is backward compatible with the current practice of having a "maintainers" list declared at the class level. --- lib/spack/docs/features.rst | 2 +- lib/spack/docs/packaging_guide.rst | 37 ++++++++++++++++++++------- lib/spack/spack/cmd/create.py | 2 +- lib/spack/spack/directives.py | 17 +++++++++++++ lib/spack/spack/test/cmd/maintainers.py | 45 +++++++++++++++++++++++---------- lib/spack/spack/test/directives.py | 16 ++++++++++++ 6 files changed, 95 insertions(+), 24 deletions(-) (limited to 'lib') diff --git a/lib/spack/docs/features.rst b/lib/spack/docs/features.rst index bfc484f518..c39075fe10 100644 --- a/lib/spack/docs/features.rst +++ b/lib/spack/docs/features.rst @@ -116,7 +116,7 @@ creates a simple python file: # FIXME: Add a list of GitHub accounts to # notify when the package is updated. - # maintainers = ["github_user1", "github_user2"] + # maintainers("github_user1", "github_user2") version("0.8.13", sha256="591a9b4ec81c1f2042a97aa60564e0cb79d041c52faa7416acb38bc95bd2c76d") diff --git a/lib/spack/docs/packaging_guide.rst b/lib/spack/docs/packaging_guide.rst index 93a049e69a..4ebb62da07 100644 --- a/lib/spack/docs/packaging_guide.rst +++ b/lib/spack/docs/packaging_guide.rst @@ -268,7 +268,7 @@ generates a boilerplate template for your package, and opens up the new # FIXME: Add a list of GitHub accounts to # notify when the package is updated. - # maintainers = ["github_user1", "github_user2"] + # maintainers("github_user1", "github_user2") version("6.2.1", sha256="eae9326beb4158c386e39a356818031bd28f3124cf915f8c5b1dc4c7a36b4d7c") @@ -319,14 +319,8 @@ The rest of the tasks you need to do are as follows: #. Add a comma-separated list of maintainers. - The ``maintainers`` field is a list of GitHub accounts of people - who want to be notified any time the package is modified. When a - pull request is submitted that updates the package, these people - will be requested to review the PR. This is useful for developers - who maintain a Spack package for their own software, as well as - users who rely on a piece of software and want to ensure that the - package doesn't break. It also gives users a list of people to - contact for help when someone reports a build error with the package. + Add a list of Github accounts of people who want to be notified + any time the package is modified. See :ref:`package_maintainers`. #. Add ``depends_on()`` calls for the package's dependencies. @@ -497,6 +491,31 @@ some examples: In general, you won't have to remember this naming convention because :ref:`cmd-spack-create` and :ref:`cmd-spack-edit` handle the details for you. +.. _package_maintainers: + +----------- +Maintainers +----------- + +Each package in Spack may have one or more maintainers, i.e. one or more +GitHub accounts of people who want to be notified any time the package is +modified. + +When a pull request is submitted that updates the package, these people will +be requested to review the PR. This is useful for developers who maintain a +Spack package for their own software, as well as users who rely on a piece of +software and want to ensure that the package doesn't break. It also gives users +a list of people to contact for help when someone reports a build error with +the package. + +To add maintainers to a package, simply declare them with the ``maintainers`` directive: + +.. code-block:: python + + maintainers("user1", "user2") + +The list of maintainers is additive, and includes all the accounts eventually declared in base classes. + ----------------- Trusted Downloads ----------------- diff --git a/lib/spack/spack/cmd/create.py b/lib/spack/spack/cmd/create.py index b62daa22d3..bf796e47dc 100644 --- a/lib/spack/spack/cmd/create.py +++ b/lib/spack/spack/cmd/create.py @@ -70,7 +70,7 @@ class {class_name}({base_class_name}): # FIXME: Add a list of GitHub accounts to # notify when the package is updated. - # maintainers = ["github_user1", "github_user2"] + # maintainers("github_user1", "github_user2") {versions} diff --git a/lib/spack/spack/directives.py b/lib/spack/spack/directives.py index 8c247b7aff..c4d5865d9e 100644 --- a/lib/spack/spack/directives.py +++ b/lib/spack/spack/directives.py @@ -54,6 +54,7 @@ __all__ = [ "conflicts", "depends_on", "extends", + "maintainers", "provides", "patch", "variant", @@ -767,6 +768,22 @@ def build_system(*values, **kwargs): ) +@directive(dicts=()) +def maintainers(*names: str): + """Add a new maintainer directive, to specify maintainers in a declarative way. + + Args: + names: GitHub username for the maintainer + """ + + def _execute_maintainer(pkg): + maintainers_from_base = getattr(pkg, "maintainers", []) + # Here it is essential to copy, otherwise we might add to an empty list in the parent + pkg.maintainers = list(sorted(set(maintainers_from_base + list(names)))) + + return _execute_maintainer + + class DirectiveError(spack.error.SpackError): """This is raised when something is wrong with a package directive.""" diff --git a/lib/spack/spack/test/cmd/maintainers.py b/lib/spack/spack/test/cmd/maintainers.py index 11560a6ce5..fba42bfc8e 100644 --- a/lib/spack/spack/test/cmd/maintainers.py +++ b/lib/spack/spack/test/cmd/maintainers.py @@ -14,6 +14,8 @@ import spack.repo maintainers = spack.main.SpackCommand("maintainers") +MAINTAINED_PACKAGES = ["maintainers-1", "maintainers-2", "maintainers-3", "py-extension1"] + def split(output): """Split command line output into an array.""" @@ -23,14 +25,12 @@ def split(output): def test_maintained(mock_packages): out = split(maintainers("--maintained")) - assert out == ["maintainers-1", "maintainers-2"] + assert out == MAINTAINED_PACKAGES def test_unmaintained(mock_packages): out = split(maintainers("--unmaintained")) - assert out == sorted( - set(spack.repo.all_package_names()) - set(["maintainers-1", "maintainers-2"]) - ) + assert out == sorted(set(spack.repo.all_package_names()) - set(MAINTAINED_PACKAGES)) def test_all(mock_packages, capfd): @@ -43,6 +43,14 @@ def test_all(mock_packages, capfd): "maintainers-2:", "user2,", "user3", + "maintainers-3:", + "user0,", + "user1,", + "user2,", + "user3", + "py-extension1:", + "user1,", + "user2", ] with capfd.disabled(): @@ -58,23 +66,34 @@ def test_all_by_user(mock_packages, capfd): with capfd.disabled(): out = split(maintainers("--all", "--by-user")) assert out == [ + "user0:", + "maintainers-3", "user1:", - "maintainers-1", + "maintainers-1,", + "maintainers-3,", + "py-extension1", "user2:", "maintainers-1,", - "maintainers-2", + "maintainers-2,", + "maintainers-3,", + "py-extension1", "user3:", - "maintainers-2", + "maintainers-2,", + "maintainers-3", ] with capfd.disabled(): out = split(maintainers("--all", "--by-user", "user1", "user2")) assert out == [ "user1:", - "maintainers-1", + "maintainers-1,", + "maintainers-3,", + "py-extension1", "user2:", "maintainers-1,", - "maintainers-2", + "maintainers-2,", + "maintainers-3,", + "py-extension1", ] @@ -116,16 +135,16 @@ def test_maintainers_list_fails(mock_packages, capfd): def test_maintainers_list_by_user(mock_packages, capfd): with capfd.disabled(): out = split(maintainers("--by-user", "user1")) - assert out == ["maintainers-1"] + assert out == ["maintainers-1", "maintainers-3", "py-extension1"] with capfd.disabled(): out = split(maintainers("--by-user", "user1", "user2")) - assert out == ["maintainers-1", "maintainers-2"] + assert out == ["maintainers-1", "maintainers-2", "maintainers-3", "py-extension1"] with capfd.disabled(): out = split(maintainers("--by-user", "user2")) - assert out == ["maintainers-1", "maintainers-2"] + assert out == ["maintainers-1", "maintainers-2", "maintainers-3", "py-extension1"] with capfd.disabled(): out = split(maintainers("--by-user", "user3")) - assert out == ["maintainers-2"] + assert out == ["maintainers-2", "maintainers-3"] diff --git a/lib/spack/spack/test/directives.py b/lib/spack/spack/test/directives.py index 841e820083..18242feb67 100644 --- a/lib/spack/spack/test/directives.py +++ b/lib/spack/spack/test/directives.py @@ -68,3 +68,19 @@ def test_error_on_anonymous_dependency(config, mock_packages): pkg = spack.repo.path.get_pkg_class("a") with pytest.raises(spack.directives.DependencyError): spack.directives._depends_on(pkg, "@4.5") + + +@pytest.mark.regression("34879") +@pytest.mark.parametrize( + "package_name,expected_maintainers", + [ + ("maintainers-1", ["user1", "user2"]), + # Reset from PythonPackage + ("py-extension1", ["user1", "user2"]), + # Extends maintainers-1 + ("maintainers-3", ["user0", "user1", "user2", "user3"]), + ], +) +def test_maintainer_directive(config, mock_packages, package_name, expected_maintainers): + pkg_cls = spack.repo.path.get_pkg_class(package_name) + assert pkg_cls.maintainers == expected_maintainers -- cgit v1.2.3-60-g2f50