From 4600767d294d892ce52cb0af3b24e324a3df4818 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:10 -0700 Subject: patman: Refactor how the default subcommand works At present patman tries to assume a default subcommand of 'send', to maintain backwards compatibility. However it does not cope with arguments added to the default command, so for example 'patman -t' does not work. Update the logic to handle this. Also update the CC command to use 'send' explicitly, since otherwise patman gets confused with the patch-filename argument. Signed-off-by: Simon Glass --- tools/patman/func_test.py | 2 +- tools/patman/gitutil.py | 10 ++++---- tools/patman/main.py | 59 +++++++++++++++++++++++++---------------------- 3 files changed, 38 insertions(+), 33 deletions(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index e2adf32..5933fcf 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -248,7 +248,7 @@ class TestFunctional(unittest.TestCase): self.assertEqual(' Cc: %s' % rick, next(lines)) expected = ('Git command: git send-email --annotate ' '--in-reply-to="%s" --to "u-boot@lists.denx.de" ' - '--cc "%s" --cc-cmd "%s --cc-cmd %s" %s %s' + '--cc "%s" --cc-cmd "%s send --cc-cmd %s" %s %s' % (in_reply_to, stefan, sys.argv[0], cc_file, cover_fname, ' '.join(args))) self.assertEqual(expected, tools.ToUnicode(next(lines))) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 3a2366bc..31fb3b2 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -455,21 +455,21 @@ def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname, >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ False, alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ -"m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" cover p1 p2' +"m.poppins@cloud.net" --cc-cmd "./patman send --cc-cmd cc-fname" cover p1 p2' >>> EmailPatches(series, None, ['p1'], True, True, 'cc-fname', False, \ alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ -"m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" p1' +"m.poppins@cloud.net" --cc-cmd "./patman send --cc-cmd cc-fname" p1' >>> series['cc'] = ['all'] >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ True, alias) 'git send-email --annotate --to "this-is-me@me.com" --cc-cmd "./patman \ ---cc-cmd cc-fname" cover p1 p2' +send --cc-cmd cc-fname" cover p1 p2' >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ False, alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ "f.bloggs@napier.co.nz" --cc "j.bloggs@napier.co.nz" --cc \ -"m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" cover p1 p2' +"m.poppins@cloud.net" --cc-cmd "./patman send --cc-cmd cc-fname" cover p1 p2' # Restore argv[0] since we clobbered it. >>> sys.argv[0] = _old_argv0 @@ -500,7 +500,7 @@ def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname, cmd += to cmd += cc - cmd += ['--cc-cmd', '"%s --cc-cmd %s"' % (sys.argv[0], cc_fname)] + cmd += ['--cc-cmd', '"%s send --cc-cmd %s"' % (sys.argv[0], cc_fname)] if cover_fname: cmd.append(cover_fname) cmd += args diff --git a/tools/patman/main.py b/tools/patman/main.py index c7f4255..e0e03a4 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -28,26 +28,30 @@ from patman import terminal from patman import test_util from patman import test_checkpatch -def AddCommonArgs(parser): - parser.add_argument('-b', '--branch', type=str, - help="Branch to process (by default, the current branch)") - parser.add_argument('-c', '--count', dest='count', type=int, - default=-1, help='Automatically create patches from top n commits') - parser.add_argument('-e', '--end', type=int, default=0, - help='Commits to skip at end of patch list') - parser.add_argument('-D', '--debug', action='store_true', - help='Enabling debugging (provides a full traceback on error)') - parser.add_argument('-s', '--start', dest='start', type=int, - default=0, help='Commit to start creating patches from (0 = HEAD)') - epilog = '''Create patches from commits in a branch, check them and email them as specified by tags you place in the commits. Use -n to do a dry run first.''' parser = ArgumentParser(epilog=epilog) +parser.add_argument('-b', '--branch', type=str, + help="Branch to process (by default, the current branch)") +parser.add_argument('-c', '--count', dest='count', type=int, + default=-1, help='Automatically create patches from top n commits') +parser.add_argument('-e', '--end', type=int, default=0, + help='Commits to skip at end of patch list') +parser.add_argument('-D', '--debug', action='store_true', + help='Enabling debugging (provides a full traceback on error)') +parser.add_argument('-p', '--project', default=project.DetectProject(), + help="Project name; affects default option values and " + "aliases [default: %(default)s]") +parser.add_argument('-s', '--start', dest='start', type=int, + default=0, help='Commit to start creating patches from (0 = HEAD)') +parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', + default=False, help='Verbose output of errors and warnings') +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.add_argument('-H', '--full-help', action='store_true', dest='full_help', - default=False, help='Display the README file') send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') @@ -58,15 +62,10 @@ send.add_argument('-m', '--no-maintainers', action='store_false', help="Don't cc the file maintainers automatically") 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('-p', '--project', default=project.DetectProject(), - help="Project name; affects default option values and " - "aliases [default: %(default)s]") send.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") send.add_argument('-t', '--ignore-bad-tags', action='store_true', default=False, help='Ignore bad tags / aliases') -send.add_argument('-v', '--verbose', action='store_true', dest='verbose', - default=False, help='Verbose output of errors and warnings') send.add_argument('-T', '--thread', action='store_true', dest='thread', default=False, help='Create patches as a single thread') send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', @@ -81,14 +80,12 @@ send.add_argument('--no-tags', action='store_false', dest='process_tags', default=True, help="Don't process subject tags as aliases") send.add_argument('--smtp-server', type=str, help="Specify the SMTP server to 'git send-email'") -AddCommonArgs(send) 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") -AddCommonArgs(test_parser) status = subparsers.add_parser('status', help='Check status of patches in patchwork') @@ -98,16 +95,24 @@ status.add_argument('-d', '--dest-branch', type=str, help='Name of branch to create with collected responses') status.add_argument('-f', '--force', action='store_true', help='Force overwriting an existing branch') -AddCommonArgs(status) # Parse options twice: first to get the project and second to handle -# defaults properly (which depends on project). +# defaults properly (which depends on project) +# Use parse_known_args() in case 'cmd' is omitted argv = sys.argv[1:] -if len(argv) < 1 or argv[0].startswith('-'): - argv = ['send'] + argv -args = parser.parse_args(argv) +args, rest = parser.parse_known_args(argv) if hasattr(args, 'project'): - settings.Setup(gitutil, send, args.project, '') + settings.Setup(gitutil, parser, args.project, '') + args, rest = parser.parse_known_args(argv) + +# If we have a command, it is safe to parse all arguments +if args.cmd: + args = parser.parse_args(argv) +else: + # No command, so insert it after the known arguments and before the ones + # that presumably relate to the 'send' subcommand + nargs = len(rest) + argv = argv[:-nargs] + ['send'] + rest args = parser.parse_args(argv) if __name__ != "__main__": -- cgit v1.1 From 53336e6ca8ca82124857cb56b5e2c89c08774e51 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:11 -0700 Subject: patman: Correct Change-Ids error message args The arguments of this error are incorrectly formatted. Fix it. Signed-off-by: Simon Glass --- tools/patman/patchstream.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 772e4b5..cdcd50a 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -452,8 +452,8 @@ class PatchStream: if self.is_log: if self.commit.change_id: raise ValueError( - "%s: Two Change-Ids: '%s' vs. '%s'" % self.commit.hash, - self.commit.change_id, value) + "%s: Two Change-Ids: '%s' vs. '%s'" % + (self.commit.hash, self.commit.change_id, value)) self.commit.change_id = value self.skip_blank = True -- cgit v1.1 From 3145b63513552fc77f6584a89baf0966617eeadf Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:13 -0700 Subject: patman: Update defaults in subparsers At present values from the settings file are only applied to the main parser. With the new parser structure this means that some settings are ignored. Update the implementation to set defaults across the main parser and all subparsers. Also fix up the comments, since ArgumentParser is being used now. Signed-off-by: Simon Glass --- tools/patman/settings.py | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 732bd40..8c10eab 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -7,6 +7,7 @@ try: except: import ConfigParser +import argparse import os import re @@ -216,10 +217,10 @@ nxp = Zhikang Zhang ''' % (name, email), file=f) f.close(); -def _UpdateDefaults(parser, config): +def _UpdateDefaults(main_parser, config): """Update the given OptionParser defaults based on config. - We'll walk through all of the settings from the parser + We'll walk through all of the settings from all parsers. For each setting we'll look for a default in the option parser. If it's found we'll update the option parser default. @@ -228,13 +229,24 @@ def _UpdateDefaults(parser, config): say. Args: - parser: An instance of an OptionParser whose defaults will be + parser: An instance of an ArgumentParser whose defaults will be updated. config: An instance of _ProjectConfigParser that we will query for settings. """ - defaults = parser.parse_known_args()[0] - defaults = vars(defaults) + # 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()] + + # Collect the defaults from each parser + defaults = {} + for parser in parsers: + pdefs = parser.parse_known_args()[0] + defaults.update(vars(pdefs)) + + # Go through the settings and collect defaults for name, val in config.items('settings'): if name in defaults: default_val = defaults[name] @@ -242,10 +254,14 @@ def _UpdateDefaults(parser, config): val = config.getboolean('settings', name) elif isinstance(default_val, int): val = config.getint('settings', name) + elif isinstance(default_val, str): + val = config.get('settings', name) defaults[name] = val else: print("WARNING: Unknown setting %s" % name) - parser.set_defaults(**defaults) + + # Set all the defaults (this propagates through all subparsers) + main_parser.set_defaults(**defaults) def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists. -- cgit v1.1 From 7cbf02e94df820b77b53ae113828b294688bf623 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:14 -0700 Subject: patman: Allow specifying the patchwork URL Add a new argument to allow the URL of the patchwork server to be speciified. For now this is hard-coded in the main file, but future patches will move it to the settings file. Signed-off-by: Simon Glass --- tools/patman/control.py | 5 +++-- tools/patman/func_test.py | 38 ++++++++++++++++++++++++++------------ tools/patman/main.py | 3 ++- tools/patman/status.py | 29 +++++++++++++++++------------ 4 files changed, 48 insertions(+), 27 deletions(-) diff --git a/tools/patman/control.py b/tools/patman/control.py index f4a6ca1..a3c50cd 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -177,7 +177,7 @@ def send(args): args.smtp_server) def patchwork_status(branch, count, start, end, dest_branch, force, - show_comments): + show_comments, url): """Check the status of patches in patchwork This finds the series in patchwork using the Series-link tag, checks for new @@ -196,6 +196,7 @@ def patchwork_status(branch, count, start, end, dest_branch, force, force (bool): With dest_branch, force overwriting an existing branch show_comments (bool): True to display snippets from the comments provided by reviewers + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' Raises: ValueError: if the branch has no Series-link value @@ -228,4 +229,4 @@ def patchwork_status(branch, count, start, end, dest_branch, force, # are not present from patman import status status.check_patchwork_status(series, found[0], branch, dest_branch, force, - show_comments) + show_comments, url) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 5933fcf..74a144d 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -625,11 +625,15 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c os.chdir(orig_dir) @staticmethod - def _fake_patchwork(subpath): + def _fake_patchwork(url, subpath): """Fake Patchwork server for the function below This handles accessing a series, providing a list consisting of a single patch + + Args: + url (str): URL of patchwork server + subpath (str): URL subpath to use """ re_series = re.match(r'series/(\d*)/$', subpath) if re_series: @@ -645,7 +649,7 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c series = Series() with capture_sys_output() as (_, err): - status.collect_patches(series, 1234, self._fake_patchwork) + status.collect_patches(series, 1234, None, self._fake_patchwork) self.assertIn('Warning: Patchwork reports 1 patches, series has 0', err.getvalue()) @@ -655,7 +659,8 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c series = Series() series.commits = [Commit('abcd')] - patches = status.collect_patches(series, 1234, self._fake_patchwork) + patches = status.collect_patches(series, 1234, None, + self._fake_patchwork) self.assertEqual(1, len(patches)) patch = patches[0] self.assertEqual('1', patch.id) @@ -800,11 +805,15 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c "Cannot find commit for patch 3 ('Subject 2')"], warnings) - def _fake_patchwork2(self, subpath): + def _fake_patchwork2(self, url, subpath): """Fake Patchwork server for the function below This handles accessing series, patches and comments, providing the data in self.patches to the caller + + Args: + url (str): URL of patchwork server + subpath (str): URL subpath to use """ re_series = re.match(r'series/(\d*)/$', subpath) re_patch = re.match(r'patches/(\d*)/$', subpath) @@ -861,12 +870,12 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c # Check that the tags are picked up on the first patch status.find_new_responses(new_rtag_list, review_list, 0, commit1, - patch1, self._fake_patchwork2) + patch1, None, self._fake_patchwork2) self.assertEqual(new_rtag_list[0], {'Reviewed-by': {self.joe}}) # Now the second patch status.find_new_responses(new_rtag_list, review_list, 1, commit2, - patch2, self._fake_patchwork2) + patch2, None, self._fake_patchwork2) self.assertEqual(new_rtag_list[1], { 'Reviewed-by': {self.mary, self.fred}, 'Tested-by': {self.leb}}) @@ -876,7 +885,7 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c new_rtag_list = [None] * count commit1.rtags = {'Reviewed-by': {self.joe}} status.find_new_responses(new_rtag_list, review_list, 0, commit1, - patch1, self._fake_patchwork2) + patch1, None, self._fake_patchwork2) self.assertEqual(new_rtag_list[0], {}) # For the second commit, add Ed and Fred, so only Mary should be left @@ -884,7 +893,7 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c 'Tested-by': {self.leb}, 'Reviewed-by': {self.fred}} status.find_new_responses(new_rtag_list, review_list, 1, commit2, - patch2, self._fake_patchwork2) + patch2, None, self._fake_patchwork2) self.assertEqual(new_rtag_list[1], {'Reviewed-by': {self.mary}}) # Check that the output patches expectations: @@ -900,7 +909,7 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c series.commits = [commit1, commit2] terminal.SetPrintTestMode() status.check_patchwork_status(series, '1234', None, None, False, False, - self._fake_patchwork2) + None, self._fake_patchwork2) lines = iter(terminal.GetPrintTestLines()) col = terminal.Color() self.assertEqual(terminal.PrintLine(' 1 Subject 1', col.BLUE), @@ -935,11 +944,15 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c '1 new response available in patchwork (use -d to write them to a new branch)', None), next(lines)) - def _fake_patchwork3(self, subpath): + def _fake_patchwork3(self, url, subpath): """Fake Patchwork server for the function below This handles accessing series, patches and comments, providing the data in self.patches to the caller + + Args: + url (str): URL of patchwork server + subpath (str): URL subpath to use """ re_series = re.match(r'series/(\d*)/$', subpath) re_patch = re.match(r'patches/(\d*)/$', subpath) @@ -1011,7 +1024,8 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c terminal.SetPrintTestMode() status.check_patchwork_status(series, '1234', branch, dest_branch, - False, False, self._fake_patchwork3, repo) + False, False, None, self._fake_patchwork3, + repo) lines = terminal.GetPrintTestLines() self.assertEqual(12, len(lines)) self.assertEqual( @@ -1214,7 +1228,7 @@ Reviewed-by: %s series.commits = [commit1, commit2] terminal.SetPrintTestMode() status.check_patchwork_status(series, '1234', None, None, False, True, - self._fake_patchwork2) + None, self._fake_patchwork2) lines = iter(terminal.GetPrintTestLines()) col = terminal.Color() self.assertEqual(terminal.PrintLine(' 1 Subject 1', col.BLUE), diff --git a/tools/patman/main.py b/tools/patman/main.py index e0e03a4..5f319ea 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -179,7 +179,8 @@ elif args.cmd == 'status': try: control.patchwork_status(args.branch, args.count, args.start, args.end, args.dest_branch, args.force, - args.show_comments) + args.show_comments, + 'https://patchwork.ozlabs.org') except Exception as e: terminal.Print('patman: %s: %s' % (type(e).__name__, e), colour=terminal.Color.RED) diff --git a/tools/patman/status.py b/tools/patman/status.py index a369d65..f3fbc66 100644 --- a/tools/patman/status.py +++ b/tools/patman/status.py @@ -198,10 +198,11 @@ def compare_with_series(series, patches): return patch_for_commit, commit_for_patch, warnings -def call_rest_api(subpath): +def call_rest_api(url, subpath): """Call the patchwork API and return the result as JSON Args: + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' subpath (str): URL subpath to use Returns: @@ -210,13 +211,13 @@ def call_rest_api(subpath): Raises: ValueError: the URL could not be read """ - url = 'https://patchwork.ozlabs.org/api/1.2/%s' % subpath - response = requests.get(url) + full_url = '%s/api/1.2/%s' % (url, subpath) + response = requests.get(full_url) if response.status_code != 200: - raise ValueError("Could not read URL '%s'" % url) + raise ValueError("Could not read URL '%s'" % full_url) return response.json() -def collect_patches(series, series_id, rest_api=call_rest_api): +def collect_patches(series, series_id, url, rest_api=call_rest_api): """Collect patch information about a series from patchwork Uses the Patchwork REST API to collect information provided by patchwork @@ -226,6 +227,7 @@ def collect_patches(series, series_id, rest_api=call_rest_api): series (Series): Series object corresponding to the local branch containing the series series_id (str): Patch series ID number + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' rest_api (function): API function to call to access Patchwork, for testing @@ -236,7 +238,7 @@ def collect_patches(series, series_id, rest_api=call_rest_api): ValueError: if the URL could not be read or the web page does not follow the expected structure """ - data = rest_api('series/%s/' % series_id) + data = rest_api(url, 'series/%s/' % series_id) # Get all the rows, which are patches patch_dict = data['patches'] @@ -261,7 +263,7 @@ def collect_patches(series, series_id, rest_api=call_rest_api): patches = sorted(patches, key=lambda x: x.seq) return patches -def find_new_responses(new_rtag_list, review_list, seq, cmt, patch, +def find_new_responses(new_rtag_list, review_list, seq, cmt, patch, url, rest_api=call_rest_api): """Find new rtags collected by patchwork that we don't know about @@ -279,6 +281,7 @@ def find_new_responses(new_rtag_list, review_list, seq, cmt, patch, seq (int): Position in new_rtag_list to update cmt (Commit): Commit object for this commit patch (Patch): Corresponding Patch object for this patch + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' rest_api (function): API function to call to access Patchwork, for testing """ @@ -286,14 +289,14 @@ def find_new_responses(new_rtag_list, review_list, seq, cmt, patch, return # Get the content for the patch email itself as well as all comments - data = rest_api('patches/%s/' % patch.id) + data = rest_api(url, 'patches/%s/' % patch.id) pstrm = PatchStream.process_text(data['content'], True) rtags = collections.defaultdict(set) for response, people in pstrm.commit.rtags.items(): rtags[response].update(people) - data = rest_api('patches/%s/comments/' % patch.id) + data = rest_api(url, 'patches/%s/comments/' % patch.id) reviews = [] for comment in data: @@ -407,7 +410,7 @@ def create_branch(series, new_rtag_list, branch, dest_branch, overwrite, return num_added def check_patchwork_status(series, series_id, branch, dest_branch, force, - show_comments, rest_api=call_rest_api, + show_comments, url, rest_api=call_rest_api, test_repo=None): """Check the status of a series on Patchwork @@ -421,11 +424,12 @@ def check_patchwork_status(series, series_id, branch, dest_branch, force, dest_branch (str): Name of new branch to create, or None force (bool): True to force overwriting dest_branch if it exists show_comments (bool): True to show the comments on each patch + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' rest_api (function): API function to call to access Patchwork, for testing test_repo (pygit2.Repository): Repo to use (use None unless testing) """ - patches = collect_patches(series, series_id, rest_api) + patches = collect_patches(series, series_id, url, rest_api) col = terminal.Color() count = len(series.commits) new_rtag_list = [None] * count @@ -440,7 +444,8 @@ def check_patchwork_status(series, series_id, branch, dest_branch, force, with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor: futures = executor.map( find_new_responses, repeat(new_rtag_list), repeat(review_list), - range(count), series.commits, patch_list, repeat(rest_api)) + range(count), series.commits, patch_list, repeat(url), + repeat(rest_api)) for fresponse in futures: if fresponse: raise fresponse.exception() -- cgit v1.1 From a55be354c02d3dc322d9e8b927618d378f7b81ce Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:15 -0700 Subject: patman: Add a setting for the Patchwork URL Add an argument to allow specifying the the patchwork URL. This also adds this feature to the settings file, either globally, or on a per-project basis. Signed-off-by: Simon Glass --- tools/patman/README | 1 + tools/patman/main.py | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/tools/patman/README b/tools/patman/README index 49b7359..639c994 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -113,6 +113,7 @@ ignore_errors: True process_tags: False verbose: True smtp_server: /path/to/sendmail +patchwork_server: https://patchwork.ozlabs.org <<< diff --git a/tools/patman/main.py b/tools/patman/main.py index 5f319ea..342fd44 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -43,6 +43,9 @@ parser.add_argument('-D', '--debug', action='store_true', parser.add_argument('-p', '--project', default=project.DetectProject(), help="Project name; affects default option values and " "aliases [default: %(default)s]") +parser.add_argument('-P', '--patchwork-url', + default='https://patchwork.ozlabs.org', + help='URL of patchwork server [default: %(default)s]') parser.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', @@ -179,8 +182,7 @@ elif args.cmd == 'status': try: control.patchwork_status(args.branch, args.count, args.start, args.end, args.dest_branch, args.force, - args.show_comments, - 'https://patchwork.ozlabs.org') + args.show_comments, args.patchwork_url) except Exception as e: terminal.Print('patman: %s: %s' % (type(e).__name__, e), colour=terminal.Color.RED) -- cgit v1.1 From fcbec650e6216fdba0ffe6fc017a34ceed0c86cb Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 3 Nov 2020 13:54:16 -0700 Subject: patman: Add a Series-patchwork-url option Add a commit tag to allow the Patchwork URL to be specified in a commit. This can be handy for when you submit code to multiple projects but don't want to use the -p option. Signed-off-by: Simon Glass --- tools/patman/README | 6 ++++++ tools/patman/control.py | 7 ++++++- tools/patman/series.py | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/tools/patman/README b/tools/patman/README index 639c994..6b80663 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -208,6 +208,12 @@ Series-links: [id | version:id]... branch against patchwork to see what new reviews your series has collected ('patman status'). +Series-patchwork-url: url + This allows specifying the Patchwork URL for a branch. This overrides + both the setting files and the command-line argument. The URL should + include the protocol and web site, with no trailing slash, for example + 'https://patchwork.ozlabs.org/project' + Cover-letter: This is the patch set title blah blah diff --git a/tools/patman/control.py b/tools/patman/control.py index a3c50cd..2330682 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -196,7 +196,8 @@ def patchwork_status(branch, count, start, end, dest_branch, force, force (bool): With dest_branch, force overwriting an existing branch show_comments (bool): True to display snippets from the comments provided by reviewers - url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org' + url (str): URL of patchwork server, e.g. 'https://patchwork.ozlabs.org'. + This is ignored if the series provides a Series-patchwork-url tag. Raises: ValueError: if the branch has no Series-link value @@ -225,6 +226,10 @@ def patchwork_status(branch, count, start, end, dest_branch, force, if not found: raise ValueError('Series-links has no current version (without :)') + # Allow the series to override the URL + if 'patchwork_url' in series: + url = series.patchwork_url + # Import this here to avoid failing on other commands if the dependencies # are not present from patman import status diff --git a/tools/patman/series.py b/tools/patman/series.py index 4457719..1d92bdb 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -16,7 +16,7 @@ from patman import tools # Series-xxx tags that we understand valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name', - 'cover_cc', 'process_log', 'links'] + 'cover_cc', 'process_log', 'links', 'patchwork_url'] class Series(dict): """Holds information about a patch series, including all tags. -- cgit v1.1 From d237e9c7c00d131d808761d0fe7f44768598c074 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 9 Nov 2020 07:14:43 -0700 Subject: cros_ec: Correct collection of EC hash The EC now requires that the offset field be set correctly when checking on hash status. Update the code to handle this. Use the same message struct in both functions to reduce stack space. Signed-off-by: Simon Glass --- drivers/misc/cros_ec.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/drivers/misc/cros_ec.c b/drivers/misc/cros_ec.c index c367490..1b22f18 100644 --- a/drivers/misc/cros_ec.c +++ b/drivers/misc/cros_ec.c @@ -495,18 +495,18 @@ int cros_ec_read_current_image(struct udevice *dev, } static int cros_ec_wait_on_hash_done(struct udevice *dev, + struct ec_params_vboot_hash *p, struct ec_response_vboot_hash *hash) { - struct ec_params_vboot_hash p; ulong start; start = get_timer(0); while (hash->status == EC_VBOOT_HASH_STATUS_BUSY) { mdelay(50); /* Insert some reasonable delay */ - p.cmd = EC_VBOOT_HASH_GET; - if (ec_command(dev, EC_CMD_VBOOT_HASH, 0, &p, sizeof(p), - hash, sizeof(*hash)) < 0) + p->cmd = EC_VBOOT_HASH_GET; + if (ec_command(dev, EC_CMD_VBOOT_HASH, 0, p, sizeof(*p), hash, + sizeof(*hash)) < 0) return -1; if (get_timer(start) > CROS_EC_CMD_HASH_TIMEOUT_MS) { @@ -530,7 +530,7 @@ int cros_ec_read_hash(struct udevice *dev, uint hash_offset, return -1; /* If the EC is busy calculating the hash, fidget until it's done. */ - rv = cros_ec_wait_on_hash_done(dev, hash); + rv = cros_ec_wait_on_hash_done(dev, &p, hash); if (rv) return rv; @@ -553,9 +553,13 @@ int cros_ec_read_hash(struct udevice *dev, uint hash_offset, hash, sizeof(*hash)) < 0) return -1; - rv = cros_ec_wait_on_hash_done(dev, hash); + rv = cros_ec_wait_on_hash_done(dev, &p, hash); if (rv) return rv; + if (hash->status != EC_VBOOT_HASH_STATUS_DONE) { + log_err("Hash did not complete, status=%d\n", hash->status); + return -EIO; + } debug("%s: hash done\n", __func__); -- cgit v1.1 From 4cb862fe28332f187f636b37b48fb0fbaf3432ec Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Mon, 9 Nov 2020 07:14:44 -0700 Subject: cros_ec: Increase command timeout for flash erase Erasing the flash can take over a second on some devices and the EC is not responsive during this time. Update the timeout to 5 seconds to cope with this. Signed-off-by: Simon Glass --- drivers/misc/cros_ec_lpc.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/misc/cros_ec_lpc.c b/drivers/misc/cros_ec_lpc.c index 63702f9..e0002b9 100644 --- a/drivers/misc/cros_ec_lpc.c +++ b/drivers/misc/cros_ec_lpc.c @@ -25,13 +25,16 @@ #define debug_trace(fmt, b...) #endif +/* Timeout waiting for a flash erase command to complete */ +static const int CROS_EC_CMD_TIMEOUT_MS = 5000; + static int wait_for_sync(struct cros_ec_dev *dev) { unsigned long start; start = get_timer(0); while (inb(EC_LPC_ADDR_HOST_CMD) & EC_LPC_STATUS_BUSY_MASK) { - if (get_timer(start) > 1000) { + if (get_timer(start) > CROS_EC_CMD_TIMEOUT_MS) { debug("%s: Timeout waiting for CROS_EC sync\n", __func__); return -1; -- cgit v1.1 From a3e458524c15710e4ac9ce1556a5f0898084d09a Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Mon, 9 Nov 2020 21:34:25 +0100 Subject: cros_ec: Handling EC_CMD_GET_NEXT_EVENT With commit 690079767803 ("cros_ec: Support keyboard scanning with EC_CMD_GET_NEXT_EVENT") check_for_keys() tries to read keyboard strokes using EC_CMD_GET_NEXT_EVENT. But the sandbox driver does not understand this command. We need to reply with -EC_RES_INVALID_COMMAND to force check_for_keys() to fall back to use EC_CMD_MKBP_STATE. Currently the driver prints ** Unknown EC command 0x67 in this case. With the patch the message is suppressed. In a future patch we should upgrade the sandbox driver to provide EC_CMD_GET_NEXT_EVENT support. Fixes: 690079767803 ("cros_ec: Support keyboard scanning with EC_CMD_GET_NEXT_EVENT") Signed-off-by: Heinrich Schuchardt --- drivers/misc/cros_ec_sandbox.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/misc/cros_ec_sandbox.c b/drivers/misc/cros_ec_sandbox.c index a191f06..ff7f782 100644 --- a/drivers/misc/cros_ec_sandbox.c +++ b/drivers/misc/cros_ec_sandbox.c @@ -460,6 +460,16 @@ static int process_cmd(struct ec_state *ec, case EC_CMD_ENTERING_MODE: len = 0; break; + case EC_CMD_GET_NEXT_EVENT: + /* + * TODO: + * This driver emulates an old keyboard device supporting + * EC_CMD_MKBP_STATE. Current Chrome OS keyboards use + * EC_CMD_GET_NEXT_EVENT. Cf. + * "mkbp: Add support for buttons and switches" + * https://chromium.googlesource.com/chromiumos/platform/ec/+/87a071941b89e3f7fd3eb329b682e60b3fbd6c73 + */ + return -EC_RES_INVALID_COMMAND; default: printf(" ** Unknown EC command %#02x\n", req_hdr->command); return -1; -- cgit v1.1