These changes correspond to commits 9ff7500ace..3154de3dd6 already merged to the u-boot-dm custodian repo (at https://source.denx.de/u-boot/custodians/u-boot-dm/-/commits/next), scheduled to be pulled after the next release. diff --git a/tools/patman/__init__.py b/tools/patman/__init__.py index c9d3e35052..1b98ec7fee 100644 --- a/tools/patman/__init__.py +++ b/tools/patman/__init__.py @@ -1,6 +1,6 @@ # SPDX-License-Identifier: GPL-2.0+ __all__ = ['checkpatch', 'command', 'commit', 'control', 'cros_subprocess', - 'func_test', 'get_maintainer', 'gitutil', 'main', 'patchstream', + 'func_test', 'get_maintainer', 'gitutil', '__main__', 'patchstream', 'project', 'series', 'setup', 'settings', 'terminal', 'test_checkpatch', 'test_util', 'tools', 'tout'] diff --git a/tools/patman/main.py b/tools/patman/__main__.py similarity index 89% rename from tools/patman/main.py rename to tools/patman/__main__.py index 8067a288ab..749e6348b6 100755 --- a/tools/patman/main.py +++ b/tools/patman/__main__.py @@ -7,6 +7,7 @@ """See README for more information""" from argparse import ArgumentParser +import importlib.resources import os import re import sys @@ -19,6 +20,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 @@ -53,7 +55,8 @@ parser.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') subparsers = parser.add_subparsers(dest='cmd') -send = subparsers.add_parser('send') +send = subparsers.add_parser( + 'send', help='Format, check and email patches (default command)') send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') @@ -62,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', @@ -94,9 +103,11 @@ send.add_argument('--smtp-server', type=str, send.add_argument('patchfiles', nargs='*') -test_parser = subparsers.add_parser('test', help='Run tests') -test_parser.add_argument('testname', type=str, default=None, nargs='?', - help="Specify the test to run") +# Only add the 'test' action if the test data files are available. +if os.path.exists(func_test.TEST_DATA_DIR): + test_parser = subparsers.add_parser('test', help='Run tests') + test_parser.add_argument('testname', type=str, default=None, nargs='?', + help="Specify the test to run") status = subparsers.add_parser('status', help='Check status of patches in patchwork') @@ -113,7 +124,7 @@ status.add_argument('-f', '--force', action='store_true', argv = sys.argv[1:] args, rest = parser.parse_known_args(argv) if hasattr(args, 'project'): - settings.Setup(gitutil, parser, args.project, '') + settings.Setup(parser, args.project) args, rest = parser.parse_known_args(argv) # If we have a command, it is safe to parse all arguments @@ -160,11 +171,8 @@ elif args.cmd == 'send': fd.close() elif args.full_help: - tools.print_full_help( - os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), - 'README.rst') - ) - + with importlib.resources.path('patman', 'README.rst') as readme: + tools.print_full_help(str(readme)) else: # If we are not processing tags, no need to warning about bad ones if not args.process_tags: diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index d1b902dd96..012c0d895c 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -211,7 +211,7 @@ def check_patch(fname, verbose=False, show_types=False, use_tree=False): stdout: Full output of checkpatch """ chk = find_check_patch() - args = [chk] + args = [chk, '--u-boot', '--strict'] if not use_tree: args.append('--no-tree') if show_types: 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 7b92bc67be..c25a47bdeb 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -6,7 +6,9 @@ """Functional tests for checking that patman behaves correctly""" +import contextlib import os +import pathlib import re import shutil import sys @@ -28,6 +30,21 @@ 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/' + + +@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): """Functional tests for checking that patman behaves correctly""" leb = (b'Lord Edmund Blackadd\xc3\xabr '. @@ -57,8 +74,7 @@ class TestFunctional(unittest.TestCase): Returns: str: Full path to file in the test directory """ - return os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), - 'test', fname) + return TEST_DATA_DIR / fname @classmethod def _get_text(cls, fname): @@ -200,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'): @@ -207,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) @@ -502,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 ceaf2ce150..5e742102c2 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -2,21 +2,19 @@ # Copyright (c) 2011 The Chromium OS Authors. # -import re import os -import subprocess import sys from patman import command from patman import settings from patman import terminal -from patman import tools # True to use --no-decorate - we check this in setup() use_no_decorate = True + def log_cmd(commit_range, git_dir=None, oneline=False, reverse=False, - count=None): + count=None): """Create a command to perform a 'git log' Args: @@ -49,6 +47,7 @@ def log_cmd(commit_range, git_dir=None, oneline=False, reverse=False, cmd.append('--') return cmd + def count_commits_to_branch(branch): """Returns number of commits between HEAD and the tracking branch. @@ -68,13 +67,14 @@ def count_commits_to_branch(branch): rev_range = '@{upstream}..' pipe = [log_cmd(rev_range, oneline=True)] result = command.run_pipe(pipe, capture=True, capture_stderr=True, - oneline=True, raise_on_error=False) + oneline=True, raise_on_error=False) if result.return_code: raise ValueError('Failed to determine upstream: %s' % result.stderr.strip()) patch_count = len(result.stdout.splitlines()) return patch_count + def name_revision(commit_hash): """Gets the revision name for a commit @@ -91,6 +91,7 @@ def name_revision(commit_hash): name = stdout.split(' ')[1].strip() return name + def guess_upstream(git_dir, branch): """Tries to guess the upstream for a branch @@ -109,7 +110,7 @@ def guess_upstream(git_dir, branch): """ pipe = [log_cmd(branch, git_dir=git_dir, oneline=True, count=100)] result = command.run_pipe(pipe, capture=True, capture_stderr=True, - raise_on_error=False) + raise_on_error=False) if result.return_code: return None, "Branch '%s' not found" % branch for line in result.stdout.splitlines()[1:]: @@ -121,6 +122,7 @@ def guess_upstream(git_dir, branch): return name, "Guessing upstream as '%s'" % name return None, "Cannot find a suitable upstream for branch '%s'" % branch + def get_upstream(git_dir, branch): """Returns the name of the upstream for a branch @@ -135,10 +137,10 @@ def get_upstream(git_dir, branch): """ try: remote = command.output_one_line('git', '--git-dir', git_dir, 'config', - 'branch.%s.remote' % branch) + 'branch.%s.remote' % branch) merge = command.output_one_line('git', '--git-dir', git_dir, 'config', - 'branch.%s.merge' % branch) - except: + 'branch.%s.merge' % branch) + except Exception: upstream, msg = guess_upstream(git_dir, branch) return upstream, msg @@ -149,7 +151,8 @@ def get_upstream(git_dir, branch): return '%s/%s' % (remote, leaf), None else: raise ValueError("Cannot determine upstream branch for branch " - "'%s' remote='%s', merge='%s'" % (branch, remote, merge)) + "'%s' remote='%s', merge='%s'" + % (branch, remote, merge)) def get_range_in_branch(git_dir, branch, include_upstream=False): @@ -168,6 +171,7 @@ def get_range_in_branch(git_dir, branch, include_upstream=False): rstr = '%s%s..%s' % (upstream, '~' if include_upstream else '', branch) return rstr, msg + def count_commits_in_range(git_dir, range_expr): """Returns the number of commits in the given range. @@ -180,12 +184,13 @@ def count_commits_in_range(git_dir, range_expr): """ pipe = [log_cmd(range_expr, git_dir=git_dir, oneline=True)] result = command.run_pipe(pipe, capture=True, capture_stderr=True, - raise_on_error=False) + raise_on_error=False) if result.return_code: return None, "Range '%s' not found or is invalid" % range_expr patch_count = len(result.stdout.splitlines()) return patch_count, None + def count_commits_in_branch(git_dir, branch, include_upstream=False): """Returns the number of commits in the given branch. @@ -201,6 +206,7 @@ def count_commits_in_branch(git_dir, branch, include_upstream=False): return None, msg return count_commits_in_range(git_dir, range_expr) + def count_commits(commit_range): """Returns the number of commits in the given range. @@ -215,6 +221,7 @@ def count_commits(commit_range): patch_count = int(stdout) return patch_count + def checkout(commit_hash, git_dir=None, work_tree=None, force=False): """Checkout the selected commit for this build @@ -231,10 +238,11 @@ def checkout(commit_hash, git_dir=None, work_tree=None, force=False): pipe.append('-f') pipe.append(commit_hash) result = command.run_pipe([pipe], capture=True, raise_on_error=False, - capture_stderr=True) + capture_stderr=True) if result.return_code != 0: raise OSError('git checkout (%s): %s' % (pipe, result.stderr)) + def clone(git_dir, output_dir): """Checkout the selected commit for this build @@ -243,10 +251,11 @@ def clone(git_dir, output_dir): """ pipe = ['git', 'clone', git_dir, '.'] result = command.run_pipe([pipe], capture=True, cwd=output_dir, - capture_stderr=True) + capture_stderr=True) if result.return_code != 0: raise OSError('git clone: %s' % result.stderr) + def fetch(git_dir=None, work_tree=None): """Fetch from the origin repo @@ -263,6 +272,7 @@ def fetch(git_dir=None, work_tree=None): if result.return_code != 0: raise OSError('git fetch: %s' % result.stderr) + def check_worktree_is_available(git_dir): """Check if git-worktree functionality is available @@ -274,9 +284,10 @@ def check_worktree_is_available(git_dir): """ pipe = ['git', '--git-dir', git_dir, 'worktree', 'list'] result = command.run_pipe([pipe], capture=True, capture_stderr=True, - raise_on_error=False) + raise_on_error=False) return result.return_code == 0 + def add_worktree(git_dir, output_dir, commit_hash=None): """Create and checkout a new git worktree for this build @@ -290,10 +301,11 @@ def add_worktree(git_dir, output_dir, commit_hash=None): if commit_hash: pipe.append(commit_hash) result = command.run_pipe([pipe], capture=True, cwd=output_dir, - capture_stderr=True) + capture_stderr=True) if result.return_code != 0: raise OSError('git worktree add: %s' % result.stderr) + def prune_worktrees(git_dir): """Remove administrative files for deleted worktrees @@ -305,7 +317,8 @@ def prune_worktrees(git_dir): if result.return_code != 0: raise OSError('git worktree prune: %s' % result.stderr) -def create_patches(branch, start, count, ignore_binary, series, signoff = True): + +def create_patches(branch, start, count, ignore_binary, series, signoff=True): """Create a series of patches from the top of the current branch. The patch files are written to the current directory using @@ -321,9 +334,7 @@ def create_patches(branch, start, count, ignore_binary, series, signoff = True): Filename of cover letter (None if none) List of filenames of patch files """ - if series.get('version'): - version = '%s ' % series['version'] - cmd = ['git', 'format-patch', '-M' ] + cmd = ['git', 'format-patch', '-M'] if signoff: cmd.append('--signoff') if ignore_binary: @@ -341,9 +352,10 @@ def create_patches(branch, start, count, ignore_binary, series, signoff = True): # We have an extra file if there is a cover letter if series.get('cover'): - return files[0], files[1:] + return files[0], files[1:] else: - return None, files + return None, files + def build_email_list(in_list, tag=None, alias=None, warn_on_error=True): """Build a list of email addresses based on an input list. @@ -385,40 +397,43 @@ def build_email_list(in_list, tag=None, alias=None, warn_on_error=True): raw += lookup_email(item, alias, warn_on_error=warn_on_error) result = [] for item in raw: - if not item in result: + if item not in result: result.append(item) if tag: return ['%s %s%s%s' % (tag, quote, email, quote) for email in result] return result + def check_suppress_cc_config(): """Check if sendemail.suppresscc is configured correctly. Returns: True if the option is configured correctly, False otherwise. """ - suppresscc = command.output_one_line('git', 'config', 'sendemail.suppresscc', - raise_on_error=False) + suppresscc = command.output_one_line( + 'git', 'config', 'sendemail.suppresscc', raise_on_error=False) # Other settings should be fine. if suppresscc == 'all' or suppresscc == 'cccmd': col = terminal.Color() print((col.build(col.RED, "error") + - ": git config sendemail.suppresscc set to %s\n" % (suppresscc)) + - " patman needs --cc-cmd to be run to set the cc list.\n" + - " Please run:\n" + - " git config --unset sendemail.suppresscc\n" + - " Or read the man page:\n" + - " git send-email --help\n" + - " and set an option that runs --cc-cmd\n") + ": git config sendemail.suppresscc set to %s\n" + % (suppresscc)) + + " patman needs --cc-cmd to be run to set the cc list.\n" + + " Please run:\n" + + " git config --unset sendemail.suppresscc\n" + + " Or read the man page:\n" + + " git send-email --help\n" + + " and set an option that runs --cc-cmd\n") return False return True + 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): + self_only=False, alias=None, in_reply_to=None, thread=False, + smtp_server=None, get_maintainer_script=None): """Email a patch series. Args: @@ -435,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 @@ -487,9 +503,10 @@ send --cc-cmd cc-fname" cover p1 p2' "git config sendemail.to u-boot@lists.denx.de") return cc = build_email_list(list(set(series.get('cc')) - set(series.get('to'))), - '--cc', alias, warn_on_error) + '--cc', alias, warn_on_error) if self_only: - to = build_email_list([os.getenv('USER')], '--to', alias, warn_on_error) + to = build_email_list([os.getenv('USER')], '--to', + alias, warn_on_error) cc = [] cmd = ['git', 'send-email', '--annotate'] if smtp_server: @@ -565,7 +582,7 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0): if not alias: alias = settings.alias lookup_name = lookup_name.strip() - if '@' in lookup_name: # Perhaps a real email address + if '@' in lookup_name: # Perhaps a real email address return [lookup_name] lookup_name = lookup_name.lower() @@ -581,7 +598,7 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0): return out_list if lookup_name: - if not lookup_name in alias: + if lookup_name not in alias: msg = "Alias '%s' not found" % lookup_name if warn_on_error: print(col.build(col.RED, msg)) @@ -589,11 +606,12 @@ def lookup_email(lookup_name, alias=None, warn_on_error=True, level=0): for item in alias[lookup_name]: todo = lookup_email(item, alias, warn_on_error, level + 1) for new_item in todo: - if not new_item in out_list: + if new_item not in out_list: out_list.append(new_item) return out_list + def get_top_level(): """Return name of top-level directory for this git repo. @@ -608,6 +626,7 @@ def get_top_level(): """ return command.output_one_line('git', 'rev-parse', '--show-toplevel') + def get_alias_file(): """Gets the name of the git alias file. @@ -615,7 +634,7 @@ def get_alias_file(): Filename of git alias file, or None if none """ fname = command.output_one_line('git', 'config', 'sendemail.aliasesfile', - raise_on_error=False) + raise_on_error=False) if not fname: return None @@ -625,6 +644,7 @@ def get_alias_file(): return os.path.join(get_top_level(), fname) + def get_default_user_name(): """Gets the user.name from .gitconfig file. @@ -634,6 +654,7 @@ def get_default_user_name(): uname = command.output_one_line('git', 'config', '--global', 'user.name') return uname + def get_default_user_email(): """Gets the user.email from the global .gitconfig file. @@ -643,17 +664,19 @@ def get_default_user_email(): uemail = command.output_one_line('git', 'config', '--global', 'user.email') return uemail + def get_default_subject_prefix(): """Gets the format.subjectprefix from local .git/config file. Returns: Subject prefix found in local .git/config file, or None if none """ - sub_prefix = command.output_one_line('git', 'config', 'format.subjectprefix', - raise_on_error=False) + sub_prefix = command.output_one_line( + 'git', 'config', 'format.subjectprefix', raise_on_error=False) return sub_prefix + def setup(): """Set up git utils, by reading the alias files.""" # Check for a git alias file also @@ -666,6 +689,7 @@ def setup(): use_no_decorate = (command.run_pipe([cmd], raise_on_error=False) .return_code == 0) + def get_head(): """Get the hash of the current HEAD @@ -674,6 +698,7 @@ def get_head(): """ return command.output_one_line('git', 'show', '-s', '--pretty=format:%H') + if __name__ == "__main__": import doctest diff --git a/tools/patman/patman b/tools/patman/patman index 11a5d8e18a..5a427d1942 120000 --- a/tools/patman/patman +++ b/tools/patman/patman @@ -1 +1 @@ -main.py \ No newline at end of file +__main__.py \ No newline at end of file diff --git a/tools/patman/patman.rst b/tools/patman/patman.rst index 8c5c9cc2cc..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,13 +69,28 @@ 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. -To add your own, create a file ~/.patman like this:: +To add your own, create a file `~/.patman` like this:: # patman alias file @@ -85,6 +101,12 @@ To add your own, create a file ~/.patman like this:: wolfgang: Wolfgang Denk others: Mike Frysinger , Fred Bloggs +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. The checkpatch.pl in the U-Boot tools/ subdirectory will be located and @@ -680,6 +702,16 @@ them: $ tools/patman/patman test +Note that since the test suite depends on data files only available in +the git checkout, the `test` command is hidden unless `patman` is +invoked from the U-Boot git repository. + +Alternatively, you can run the test suite via Pytest: + +.. code-block:: bash + + $ cd tools/patman && pytest + Error handling doesn't always produce friendly error messages - e.g. putting an incorrect tag in a commit may provide a confusing message. diff --git a/tools/patman/pytest.ini b/tools/patman/pytest.ini new file mode 100644 index 0000000000..df3eb518d0 --- /dev/null +++ b/tools/patman/pytest.ini @@ -0,0 +1,2 @@ +[pytest] +addopts = --doctest-modules 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)) diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 903d6fcb0b..636983e32d 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -1,18 +1,18 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2011 The Chromium OS Authors. +# Copyright (c) 2022 Maxim Cournoyer # try: import configparser as ConfigParser -except: +except Exception: import ConfigParser import argparse import os import re -from patman import command -from patman import tools +from patman import gitutil """Default settings per-project. @@ -32,7 +32,8 @@ _default_settings = { }, } -class _ProjectConfigParser(ConfigParser.SafeConfigParser): + +class _ProjectConfigParser(ConfigParser.ConfigParser): """ConfigParser that handles projects. There are two main goals of this class: @@ -83,14 +84,14 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser): def __init__(self, project_name): """Construct _ProjectConfigParser. - In addition to standard SafeConfigParser initialization, this also loads - project defaults. + In addition to standard ConfigParser initialization, this also + loads project defaults. Args: project_name: The name of the project. """ self._project_name = project_name - ConfigParser.SafeConfigParser.__init__(self) + ConfigParser.ConfigParser.__init__(self) # Update the project settings in the config based on # the _default_settings global. @@ -102,31 +103,31 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser): self.set(project_settings, setting_name, setting_value) def get(self, section, option, *args, **kwargs): - """Extend SafeConfigParser to try project_section before section. + """Extend ConfigParser to try project_section before section. Args: - See SafeConfigParser. + See ConfigParser. Returns: - See SafeConfigParser. + See ConfigParser. """ try: - val = ConfigParser.SafeConfigParser.get( + val = ConfigParser.ConfigParser.get( self, "%s_%s" % (self._project_name, section), option, *args, **kwargs ) except (ConfigParser.NoSectionError, ConfigParser.NoOptionError): - val = ConfigParser.SafeConfigParser.get( + val = ConfigParser.ConfigParser.get( self, section, option, *args, **kwargs ) return val def items(self, section, *args, **kwargs): - """Extend SafeConfigParser to add project_section to section. + """Extend ConfigParser to add project_section to section. Args: - See SafeConfigParser. + See ConfigParser. Returns: - See SafeConfigParser. + See ConfigParser. """ project_items = [] has_project_section = False @@ -134,7 +135,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser): # Get items from the project section try: - project_items = ConfigParser.SafeConfigParser.items( + project_items = ConfigParser.ConfigParser.items( self, "%s_%s" % (self._project_name, section), *args, **kwargs ) has_project_section = True @@ -143,7 +144,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser): # Get top-level items try: - top_items = ConfigParser.SafeConfigParser.items( + top_items = ConfigParser.ConfigParser.items( self, section, *args, **kwargs ) except ConfigParser.NoSectionError: @@ -155,6 +156,7 @@ class _ProjectConfigParser(ConfigParser.SafeConfigParser): item_dict.update(project_items) return {(item, val) for item, val in item_dict.items()} + def ReadGitAliases(fname): """Read a git alias file. This is in the form used by git: @@ -170,7 +172,7 @@ def ReadGitAliases(fname): print("Warning: Cannot find alias file '%s'" % fname) return - re_line = re.compile('alias\s+(\S+)\s+(.*)') + re_line = re.compile(r'alias\s+(\S+)\s+(.*)') for line in fd.readlines(): line = line.strip() if not line or line[0] == '#': @@ -190,7 +192,8 @@ def ReadGitAliases(fname): fd.close() -def CreatePatmanConfigFile(gitutil, config_fname): + +def CreatePatmanConfigFile(config_fname): """Creates a config file under $(HOME)/.patman if it can't find one. Args: @@ -200,12 +203,12 @@ def CreatePatmanConfigFile(gitutil, config_fname): None """ name = gitutil.get_default_user_name() - if name == None: + if name is None: name = input("Enter name: ") email = gitutil.get_default_user_email() - if email == None: + if email is None: email = input("Enter email: ") try: @@ -220,7 +223,8 @@ me: %s <%s> [bounces] nxp = Zhikang Zhang ''' % (name, email), file=f) - f.close(); + f.close() + def _UpdateDefaults(main_parser, config): """Update the given OptionParser defaults based on config. @@ -242,8 +246,8 @@ def _UpdateDefaults(main_parser, config): # Find all the parsers and subparsers parsers = [main_parser] parsers += [subparser for action in main_parser._actions - if isinstance(action, argparse._SubParsersAction) - for _, subparser in action.choices.items()] + if isinstance(action, argparse._SubParsersAction) + for _, subparser in action.choices.items()] # Collect the defaults from each parser defaults = {} @@ -270,8 +274,9 @@ def _UpdateDefaults(main_parser, config): # Set all the defaults and manually propagate them to subparsers main_parser.set_defaults(**defaults) for parser, pdefs in zip(parsers, parser_defaults): - parser.set_defaults(**{ k: v for k, v in defaults.items() - if k in pdefs }) + parser.set_defaults(**{k: v for k, v in defaults.items() + if k in pdefs}) + def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists. @@ -298,6 +303,7 @@ def _ReadAliasFile(fname): if bad_line: print(bad_line) + def _ReadBouncesFile(fname): """Read in the bounces file if it exists @@ -311,6 +317,7 @@ def _ReadBouncesFile(fname): continue bounces.add(line.strip()) + def GetItems(config, section): """Get the items from a section of the config. @@ -323,31 +330,50 @@ def GetItems(config, section): """ try: return config.items(section) - except ConfigParser.NoSectionError as e: + except ConfigParser.NoSectionError: return [] - except: - raise -def Setup(gitutil, parser, project_name, config_fname=''): + +def Setup(parser, project_name, config_fname=None): """Set up the settings module by reading config files. + Unless `config_fname` is specified, a `.patman` config file local + to the git repository is consulted, followed by the global + `$HOME/.patman`. If none exists, the later is created. Values + defined in the local config file take precedence over those + defined in the global one. + Args: - parser: The parser to update + parser: The parser to update. project_name: Name of project that we're working on; we'll look for sections named "project_section" as well. - config_fname: Config filename to read ('' for default) + config_fname: Config filename to read. An error is raised if it + does not exist. """ # First read the git alias file if available _ReadAliasFile('doc/git-mailrc') config = _ProjectConfigParser(project_name) - if config_fname == '': + + if config_fname and not os.path.exists(config_fname): + raise Exception(f'provided {config_fname} does not exist') + + if not config_fname: config_fname = '%s/.patman' % os.getenv('HOME') + has_config = os.path.exists(config_fname) + + git_local_config_fname = os.path.join(gitutil.get_top_level(), '.patman') + has_git_local_config = os.path.exists(git_local_config_fname) - if not os.path.exists(config_fname): - print("No config file found ~/.patman\nCreating one...\n") - CreatePatmanConfigFile(gitutil, config_fname) + # Read the git local config last, so that its values override + # those of the global config, if any. + if has_config: + config.read(config_fname) + if has_git_local_config: + config.read(git_local_config_fname) - config.read(config_fname) + if not (has_config or has_git_local_config): + print("No config file found.\nCreating ~/.patman...\n") + CreatePatmanConfigFile(config_fname) for name, value in GetItems(config, 'alias'): alias[name] = value.split(',') @@ -358,6 +384,7 @@ def Setup(gitutil, parser, project_name, config_fname=''): _UpdateDefaults(parser, config) + # These are the aliases we understand, indexed by alias. Each member is a list. alias = {} bounces = set() diff --git a/tools/patman/setup.py b/tools/patman/setup.py index 5643bf1503..2ff791da0f 100644 --- a/tools/patman/setup.py +++ b/tools/patman/setup.py @@ -7,6 +7,6 @@ setup(name='patman', scripts=['patman'], packages=['patman'], package_dir={'patman': ''}, - package_data={'patman': ['README']}, + package_data={'patman': ['README.rst']}, classifiers=['Environment :: Console', 'Topic :: Software Development']) diff --git a/tools/patman/test_settings.py b/tools/patman/test_settings.py new file mode 100644 index 0000000000..c768a2fc64 --- /dev/null +++ b/tools/patman/test_settings.py @@ -0,0 +1,67 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (c) 2022 Maxim Cournoyer +# + +import argparse +import contextlib +import os +import sys +import tempfile + +from patman import settings +from patman import tools + + +@contextlib.contextmanager +def empty_git_repository(): + with tempfile.TemporaryDirectory() as tmpdir: + os.chdir(tmpdir) + tools.run('git', 'init', raise_on_error=True) + yield tmpdir + + +@contextlib.contextmanager +def cleared_command_line_args(): + old_value = sys.argv[:] + sys.argv = [sys.argv[0]] + try: + yield + finally: + sys.argv = old_value + + +def test_git_local_config(): + # Clearing the command line arguments is required, otherwise + # arguments passed to the test running such as in 'pytest -k + # filter' would be processed by _UpdateDefaults and fail. + with cleared_command_line_args(): + with empty_git_repository(): + with tempfile.NamedTemporaryFile() as global_config: + global_config.write(b'[settings]\n' + b'project=u-boot\n') + global_config.flush() + parser = argparse.ArgumentParser() + parser.add_argument('-p', '--project', default='unknown') + subparsers = parser.add_subparsers(dest='cmd') + send = subparsers.add_parser('send') + send.add_argument('--no-check', action='store_false', + dest='check_patch', default=True) + + # Test "global" config is used. + settings.Setup(parser, 'unknown', global_config.name) + args, _ = parser.parse_known_args([]) + assert args.project == 'u-boot' + send_args, _ = send.parse_known_args([]) + assert send_args.check_patch + + # Test local config can shadow it. + with open('.patman', 'w', buffering=1) as f: + f.write('[settings]\n' + 'project: guix-patches\n' + 'check_patch: False\n') + settings.Setup(parser, 'unknown', global_config.name) + args, _ = parser.parse_known_args([]) + assert args.project == 'guix-patches' + send_args, _ = send.parse_known_args([]) + assert not send_args.check_patch