From 8c042fb7f9f475367804b26a892fd522ad8fcfcc Mon Sep 17 00:00:00 2001 From: Maxim Cournoyer Date: Tue, 20 Dec 2022 00:28:46 -0500 Subject: [PATCH] patman: add '--get-maintainer-script' argument 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 Signed-off-by: Maxim Cournoyer --- 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 37489304f7..749e6348b6 100755 --- a/tools/patman/__main__.py +++ b/tools/patman/__main__.py @@ -21,6 +21,7 @@ if __name__ == "__main__": # Our modules 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 @@ -64,6 +65,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/' + +@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@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@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@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 # 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 +.. Maxim Cournoyer .. 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 others: Mike Frysinger , Fred Bloggs -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)) -- 2.39.5