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 sjg@chromium.org ---

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 732bd401067..8c10eab2645 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 zhikang.zhang@nxp.com ''' % (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.

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 sjg@chromium.org ---

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 f4a6ca145d4..a3c50cd9c52 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 5933fcfd14c..74a144dc2d5 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 e0e03a40a7a..5f319eaa9b8 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 a369d655c5e..f3fbc661b2f 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()

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 sjg@chromium.org ---

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 49b73590cf0..639c9943ace 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 5f319eaa9b8..342fd446a12 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)

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 sjg@chromium.org ---

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 639c9943ace..6b806632f8c 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 a3c50cd9c52..2330682df4a 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 4457719f2ef..1d92bdb9103 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.