[PATCH v2] patman: add '--get-maintainer-script' argument

Maxim Cournoyer maxim.cournoyer at gmail.com
Tue Dec 20 06:28:46 CET 2022


This makes it possible to configure a project to use some other
location or script than the default scripts/get_maintainer.pl one used
in the U-Boot and Linux projects. It can be configured via a .patman
configuration file and accepts arguments, as documented.

Reviewed-by: Simon Glass <sjg at chromium.org>
Signed-off-by: Maxim Cournoyer <maxim.cournoyer at savoirfairelinux.com>
---

Changes in v2:
- Add functional test
- Fix 'os.path.exist' typo in 'find_get_maintainer'

 tools/patman/__main__.py       |  7 +++++
 tools/patman/control.py        | 12 ++++---
 tools/patman/func_test.py      | 49 +++++++++++++++++++++++++++--
 tools/patman/get_maintainer.py | 57 +++++++++++++++++++++-------------
 tools/patman/gitutil.py        |  3 +-
 tools/patman/patman.rst        | 30 +++++++++++++-----
 tools/patman/series.py         |  9 ++++--
 7 files changed, 127 insertions(+), 40 deletions(-)

diff --git a/tools/patman/__main__.py b/tools/patman/__main__.py
index 15fd7603d7..05968037ed 100755
--- a/tools/patman/__main__.py
+++ b/tools/patman/__main__.py
@@ -22,6 +22,7 @@ if __name__ == "__main__":
 from patman import command
 from patman import control
 from patman import func_test
+from patman import gitutil
 from patman import project
 from patman import settings
 from patman import terminal
@@ -65,6 +66,12 @@ send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None,
 send.add_argument('-m', '--no-maintainers', action='store_false',
        dest='add_maintainers', default=True,
        help="Don't cc the file maintainers automatically")
+send.add_argument(
+    '--get-maintainer-script', dest='get_maintainer_script', type=str,
+    action='store',
+    default=os.path.join(gitutil.get_top_level(), 'scripts',
+                         'get_maintainer.pl') + ' --norolestats',
+    help='File name of the get_maintainer.pl (or compatible) script.')
 send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
        default=False, help="Do a dry run (create but don't email patches)")
 send.add_argument('-r', '--in-reply-to', type=str, action='store',
diff --git a/tools/patman/control.py b/tools/patman/control.py
index bf426cf7bc..38e98dab84 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -94,8 +94,8 @@ def check_patches(series, patch_files, run_checkpatch, verbose, use_tree):
 
 
 def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
-                  ignore_bad_tags, add_maintainers, limit, dry_run, in_reply_to,
-                  thread, smtp_server):
+                  ignore_bad_tags, add_maintainers, get_maintainer_script, limit,
+                  dry_run, in_reply_to, thread, smtp_server):
     """Email patches to the recipients
 
     This emails out the patches and cover letter using 'git send-email'. Each
@@ -123,6 +123,8 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
         ignore_bad_tags (bool): True to just print a warning for unknown tags,
             False to halt with an error
         add_maintainers (bool): Run the get_maintainer.pl script for each patch
+        get_maintainer_script (str): The script used to retrieve which
+            maintainers to cc
         limit (int): Limit on the number of people that can be cc'd on a single
             patch or the cover letter (None if no limit)
         dry_run (bool): Don't actually email the patches, just print out what
@@ -134,7 +136,7 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go,
         smtp_server (str): SMTP server to use to send patches (None for default)
     """
     cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags,
-                                add_maintainers, limit)
+                                add_maintainers, limit, get_maintainer_script)
 
     # Email the patches out (giving the user time to check / cancel)
     cmd = ''
@@ -174,8 +176,8 @@ def send(args):
     email_patches(
         col, series, cover_fname, patch_files, args.process_tags,
         its_a_go, args.ignore_bad_tags, args.add_maintainers,
-        args.limit, args.dry_run, args.in_reply_to, args.thread,
-        args.smtp_server)
+        args.get_maintainer_script, args.limit, args.dry_run,
+        args.in_reply_to, args.thread, args.smtp_server)
 
 def patchwork_status(branch, count, start, end, dest_branch, force,
                      show_comments, url):
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 7fa4a00786..c25a47bdeb 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -6,6 +6,7 @@
 
 """Functional tests for checking that patman behaves correctly"""
 
+import contextlib
 import os
 import pathlib
 import re
@@ -29,8 +30,19 @@ from patman.test_util import capture_sys_output
 import pygit2
 from patman import status
 
+PATMAN_DIR = pathlib.Path(__file__).parent
+TEST_DATA_DIR = PATMAN_DIR / 'test/'
 
-TEST_DATA_DIR = pathlib.Path(__file__).parent / 'test/'
+
+ at contextlib.contextmanager
+def directory_excursion(directory):
+    """Change directory to `directory` for a limited to the context block."""
+    current = os.getcwd()
+    try:
+        os.chdir(directory)
+        yield
+    finally:
+        os.chdir(current)
 
 
 class TestFunctional(unittest.TestCase):
@@ -204,6 +216,8 @@ class TestFunctional(unittest.TestCase):
         text = self._get_text('test01.txt')
         series = patchstream.get_metadata_for_test(text)
         cover_fname, args = self._create_patches_for_test(series)
+        get_maintainer_script = str(pathlib.Path(__file__).parent.parent.parent
+                                    / 'get_maintainer.pl') + ' --norolestats'
         with capture_sys_output() as out:
             patchstream.fix_patches(series, args)
             if cover_fname and series.get('cover'):
@@ -211,7 +225,7 @@ class TestFunctional(unittest.TestCase):
             series.DoChecks()
             cc_file = series.MakeCcFile(process_tags, cover_fname,
                                         not ignore_bad_tags, add_maintainers,
-                                        None)
+                                        None, get_maintainer_script)
             cmd = gitutil.email_patches(
                 series, cover_fname, args, dry_run, not ignore_bad_tags,
                 cc_file, in_reply_to=in_reply_to, thread=None)
@@ -506,6 +520,37 @@ complicated as possible''')
         finally:
             os.chdir(orig_dir)
 
+    def test_custom_get_maintainer_script(self):
+        """Validate that a custom get_maintainer script gets used."""
+        self.make_git_tree()
+        with directory_excursion(self.gitdir):
+            # Setup git.
+            os.environ['GIT_CONFIG_GLOBAL'] = '/dev/null'
+            os.environ['GIT_CONFIG_SYSTEM'] = '/dev/null'
+            tools.run('git', 'config', 'user.name', 'Dummy')
+            tools.run('git', 'config', 'user.email', 'dumdum at dummy.com')
+            tools.run('git', 'branch', 'upstream')
+            tools.run('git', 'branch', '--set-upstream-to=upstream')
+            tools.run('git', 'add', '.')
+            tools.run('git', 'commit', '-m', 'new commit')
+
+            # Setup patman configuration.
+            with open('.patman', 'w', buffering=1) as f:
+                f.write('[settings]\n'
+                        'get_maintainer_script: dummy-script.sh\n'
+                        'check_patch: False\n')
+            with open('dummy-script.sh', 'w', buffering=1) as f:
+                f.write('#!/usr/bin/env python\n'
+                        'print("hello at there.com")\n')
+            os.chmod('dummy-script.sh', 0x555)
+
+            # Finally, do the test
+            with capture_sys_output():
+                output = tools.run(PATMAN_DIR / 'patman', '--dry-run')
+                # Assert the email address is part of the dry-run
+                # output.
+                self.assertIn('hello at there.com', output)
+
     def test_tags(self):
         """Test collection of tags in a patchstream"""
         text = '''This is a patch
diff --git a/tools/patman/get_maintainer.py b/tools/patman/get_maintainer.py
index e1d15ff6ab..f7011be1e4 100644
--- a/tools/patman/get_maintainer.py
+++ b/tools/patman/get_maintainer.py
@@ -1,48 +1,61 @@
 # SPDX-License-Identifier: GPL-2.0+
 # Copyright (c) 2012 The Chromium OS Authors.
+# Copyright (c) 2022 Maxim Cournoyer <maxim.cournoyer at savoirfairelinux.com>
 #
 
 import os
+import shlex
+import shutil
 
 from patman import command
+from patman import gitutil
 
-def find_get_maintainer(try_list):
-    """Look for the get_maintainer.pl script.
 
-    Args:
-        try_list: List of directories to try for the get_maintainer.pl script
+def find_get_maintainer(script_file_name):
+    """Try to find where `script_file_name` is.
 
-    Returns:
-        If the script is found we'll return a path to it; else None.
+    It searches in PATH and falls back to a path relative to the top
+    of the current git repository.
     """
-    # Look in the list
-    for path in try_list:
-        fname = os.path.join(path, 'get_maintainer.pl')
-        if os.path.isfile(fname):
-            return fname
+    get_maintainer = shutil.which(script_file_name)
+    if get_maintainer:
+        return get_maintainer
+
+    git_relative_script = os.path.join(gitutil.get_top_level(),
+                                       script_file_name)
+    if os.path.exists(git_relative_script):
+        return git_relative_script
 
-    return None
 
-def get_maintainer(dir_list, fname, verbose=False):
-    """Run get_maintainer.pl on a file if we find it.
+def get_maintainer(script_file_name, fname, verbose=False):
+    """Run `script_file_name` on a file.
 
-    We look for get_maintainer.pl in the 'scripts' directory at the top of
-    git.  If we find it we'll run it.  If we don't find get_maintainer.pl
-    then we fail silently.
+    `script_file_name` should be a get_maintainer.pl-like script that
+    takes a patch file name as an input and return the email addresses
+    of the associated maintainers to standard output, one per line.
+
+    If `script_file_name` does not exist we fail silently.
 
     Args:
-        dir_list: List of directories to try for the get_maintainer.pl script
-        fname: Path to the patch file to run get_maintainer.pl on.
+        script_file_name: The file name of the get_maintainer.pl script
+            (or compatible).
+        fname: File name of the patch to process with get_maintainer.pl.
 
     Returns:
         A list of email addresses to CC to.
     """
-    get_maintainer = find_get_maintainer(dir_list)
+    # Expand `script_file_name` into a file name and its arguments, if
+    # any.
+    cmd_args = shlex.split(script_file_name)
+    file_name = cmd_args[0]
+    arguments = cmd_args[1:]
+
+    get_maintainer = find_get_maintainer(file_name)
     if not get_maintainer:
         if verbose:
             print("WARNING: Couldn't find get_maintainer.pl")
         return []
 
-    stdout = command.output(get_maintainer, '--norolestats', fname)
+    stdout = command.output(get_maintainer, *arguments, fname)
     lines = stdout.splitlines()
-    return [ x.replace('"', '') for x in lines ]
+    return [x.replace('"', '') for x in lines]
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py
index 74c6e94494..5e742102c2 100644
--- a/tools/patman/gitutil.py
+++ b/tools/patman/gitutil.py
@@ -433,7 +433,7 @@ def check_suppress_cc_config():
 
 def email_patches(series, cover_fname, args, dry_run, warn_on_error, cc_fname,
                   self_only=False, alias=None, in_reply_to=None, thread=False,
-                  smtp_server=None):
+                  smtp_server=None, get_maintainer_script=None):
     """Email a patch series.
 
     Args:
@@ -450,6 +450,7 @@ def email_patches(series, cover_fname, args, dry_run, warn_on_error, cc_fname,
         thread: True to add --thread to git send-email (make
             all patches reply to cover-letter or first patch in series)
         smtp_server: SMTP server to use to send patches
+        get_maintainer_script: File name of script to get maintainers emails
 
     Returns:
         Git command that was/would be run
diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst
index d7994c888b..6113962fb4 100644
--- a/tools/patman/patman.rst
+++ b/tools/patman/patman.rst
@@ -1,6 +1,7 @@
 .. SPDX-License-Identifier: GPL-2.0+
 .. Copyright (c) 2011 The Chromium OS Authors
 .. Simon Glass <sjg at chromium.org>
+.. Maxim Cournoyer <maxim.cournoyer at savoirfairelinux.com>
 .. v1, v2, 19-Oct-11
 .. revised v3 24-Nov-11
 .. revised v4 Independence Day 2020, with Patchwork integration
@@ -68,8 +69,23 @@ this once::
 
     git config sendemail.aliasesfile doc/git-mailrc
 
-For both Linux and U-Boot the 'scripts/get_maintainer.pl' handles figuring
-out where to send patches pretty well.
+For both Linux and U-Boot the 'scripts/get_maintainer.pl' handles
+figuring out where to send patches pretty well. For other projects,
+you may want to specify a different script to be run, for example via
+a project-specific `.patman` file::
+
+    # .patman configuration file at the root of some project
+
+    [settings]
+    get_maintainer_script: etc/teams.scm get-maintainer
+
+The `get_maintainer_script` option corresponds to the
+`--get-maintainer-script` argument of the `send` command.  It is
+looked relatively to the root of the current git repository, as well
+as on PATH.  It can also be provided arguments, as shown above.  The
+contract is that the script should accept a patch file name and return
+a list of email addresses, one per line, like `get_maintainer.pl`
+does.
 
 During the first run patman creates a config file for you by taking the default
 user name and email address from the global .gitconfig file.
@@ -85,11 +101,11 @@ To add your own, create a file `~/.patman` like this::
     wolfgang: Wolfgang Denk <wd at denx.de>
     others: Mike Frysinger <vapier at gentoo.org>, Fred Bloggs <f.bloggs at napier.net>
 
-Patman will also look for a `.patman` configuration file at the root
-of the current project git repository, which makes it possible to
-override the `project` settings variable or anything else in a
-project-specific way. The values of this "local" configuration file
-take precedence over those of the "global" one.
+As hinted above, Patman will also look for a `.patman` configuration
+file at the root of the current project git repository, which makes it
+possible to override the `project` settings variable or anything else
+in a project-specific way. The values of this "local" configuration
+file take precedence over those of the "global" one.
 
 Aliases are recursive.
 
diff --git a/tools/patman/series.py b/tools/patman/series.py
index 3075378ac1..2eeeef71dc 100644
--- a/tools/patman/series.py
+++ b/tools/patman/series.py
@@ -235,7 +235,7 @@ class Series(dict):
             print(col.build(col.RED, str))
 
     def MakeCcFile(self, process_tags, cover_fname, warn_on_error,
-                   add_maintainers, limit):
+                   add_maintainers, limit, get_maintainer_script):
         """Make a cc file for us to use for per-commit Cc automation
 
         Also stores in self._generated_cc to make ShowActions() faster.
@@ -249,6 +249,8 @@ class Series(dict):
                 True/False to call the get_maintainers to CC maintainers
                 List of maintainers to include (for testing)
             limit: Limit the length of the Cc list (None if no limit)
+            get_maintainer_script: The file name of the get_maintainer.pl
+                script (or compatible).
         Return:
             Filename of temp file created
         """
@@ -267,8 +269,9 @@ class Series(dict):
             if type(add_maintainers) == type(cc):
                 cc += add_maintainers
             elif add_maintainers:
-                dir_list = [os.path.join(gitutil.get_top_level(), 'scripts')]
-                cc += get_maintainer.get_maintainer(dir_list, commit.patch)
+
+                cc += get_maintainer.get_maintainer(get_maintainer_script,
+                                                    commit.patch)
             for x in set(cc) & set(settings.bounces):
                 print(col.build(col.YELLOW, 'Skipping "%s"' % x))
             cc = list(set(cc) - set(settings.bounces))

base-commit: 9f051a5bb3e3c63d66d1ae5e8fdeba5ee1dd536d
-- 
2.38.1



More information about the U-Boot mailing list