patman: Support checking for review tags in patchwork
Before sending out a new version of a series for review, it is important
to add any review tags (e.g. Reviewed-by, Acked-by) collected by
patchwork. Otherwise people waste time reviewing the same patch
repeatedly, become frustrated and stop reviewing your patches.
To help with this, add a new 'status' subcommand that checks patchwork
for review tags, showing those which are not present in the local branch.
This allows users to see what new review tags have been received and then
add them.
Sample output:
$ patman status
1 Subject 1
Reviewed-by: Joe Bloggs <joe@napierwallies.co.nz>
2 Subject 2
Tested-by: Lord Edmund Blackaddër <weasel@blackadder.org>
Reviewed-by: Fred Bloggs <f.bloggs@napier.net>
+ Reviewed-by: Mary Bloggs <mary@napierwallies.co.nz>
1 new response available in patchwork
The '+' indicates a new tag. Colours are used to make it easier to read.
Signed-off-by: Simon Glass <sjg@chromium.org>
This commit is contained in:
Simon Glass
parent
be051c0c77
commit
dc6df972c9
|
|
@ -11,6 +11,8 @@ This tool is a Python script which:
|
|||
- Runs the patches through checkpatch.pl and its own checks
|
||||
- Optionally emails them out to selected people
|
||||
|
||||
It also shows review tags from Patchwork so you can update your local patches.
|
||||
|
||||
It is intended to automate patch creation and make it a less
|
||||
error-prone process. It is useful for U-Boot and Linux work so far,
|
||||
since it uses the checkpatch.pl script.
|
||||
|
|
@ -352,6 +354,38 @@ These people will get the cover letter even if they are not on the To/Cc
|
|||
list for any of the patches.
|
||||
|
||||
|
||||
Patchwork Integration
|
||||
=====================
|
||||
|
||||
Patman has a very basic integration with Patchwork. If you point patman to
|
||||
your series on patchwork it can show you what new reviews have appears since
|
||||
you sent your series.
|
||||
|
||||
To set this up, add a Series-link tag to one of the commits in your series
|
||||
(see above).
|
||||
|
||||
Then you can type
|
||||
|
||||
patman status
|
||||
|
||||
and patman will show you each patch and what review tags have been collected,
|
||||
for example:
|
||||
|
||||
...
|
||||
21 x86: mtrr: Update the command to use the new mtrr
|
||||
Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
|
||||
+ Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
|
||||
22 x86: mtrr: Restructure so command execution is in
|
||||
Reviewed-by: Wolfgang Wallner <wolfgang.wallner@br-automation.com>
|
||||
+ Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
|
||||
...
|
||||
|
||||
This shows that patch 21 and 22 were sent out with one review but have since
|
||||
attracted another review each. If the series needs changes, you can update
|
||||
these commits with the new review tag before sending the next version of the
|
||||
series.
|
||||
|
||||
|
||||
Example Work Flow
|
||||
=================
|
||||
|
||||
|
|
|
|||
|
|
@ -175,3 +175,49 @@ def send(args):
|
|||
its_a_go, args.ignore_bad_tags, args.add_maintainers,
|
||||
args.limit, args.dry_run, args.in_reply_to, args.thread,
|
||||
args.smtp_server)
|
||||
|
||||
def patchwork_status(branch, count, start, end):
|
||||
"""Check the status of patches in patchwork
|
||||
|
||||
This finds the series in patchwork using the Series-link tag, checks for new
|
||||
comments / review tags and displays them
|
||||
|
||||
Args:
|
||||
branch (str): Branch to create patches from (None = current)
|
||||
count (int): Number of patches to produce, or -1 to produce patches for
|
||||
the current branch back to the upstream commit
|
||||
start (int): Start partch to use (0=first / top of branch)
|
||||
end (int): End patch to use (0=last one in series, 1=one before that,
|
||||
etc.)
|
||||
|
||||
Raises:
|
||||
ValueError: if the branch has no Series-link value
|
||||
"""
|
||||
if count == -1:
|
||||
# Work out how many patches to send if we can
|
||||
count = (gitutil.CountCommitsToBranch(branch) - start)
|
||||
|
||||
series = patchstream.get_metadata(branch, start, count - end)
|
||||
warnings = 0
|
||||
for cmt in series.commits:
|
||||
if cmt.warn:
|
||||
print('%d warnings for %s:' % (len(cmt.warn), cmt.hash))
|
||||
for warn in cmt.warn:
|
||||
print('\t', warn)
|
||||
warnings += 1
|
||||
print
|
||||
if warnings:
|
||||
raise ValueError('Please fix warnings before running status')
|
||||
links = series.get('links')
|
||||
if not links:
|
||||
raise ValueError("Branch has no Series-links value")
|
||||
|
||||
# Find the link without a version number (we don't support versions yet)
|
||||
found = [link for link in links.split() if not ':' in link]
|
||||
if not found:
|
||||
raise ValueError('Series-links has no current version (without :)')
|
||||
|
||||
# Import this here to avoid failing on other commands if the dependencies
|
||||
# are not present
|
||||
from patman import status
|
||||
status.check_patchwork_status(series, found[0])
|
||||
|
|
|
|||
|
|
@ -13,10 +13,13 @@ import sys
|
|||
import tempfile
|
||||
import unittest
|
||||
|
||||
|
||||
from patman.commit import Commit
|
||||
from patman import control
|
||||
from patman import gitutil
|
||||
from patman import patchstream
|
||||
from patman.patchstream import PatchStream
|
||||
from patman.series import Series
|
||||
from patman import settings
|
||||
from patman import terminal
|
||||
from patman import tools
|
||||
|
|
@ -25,6 +28,7 @@ from patman.test_util import capture_sys_output
|
|||
try:
|
||||
import pygit2
|
||||
HAVE_PYGIT2 = True
|
||||
from patman import status
|
||||
except ModuleNotFoundError:
|
||||
HAVE_PYGIT2 = False
|
||||
|
||||
|
|
@ -36,6 +40,8 @@ class TestFunctional(unittest.TestCase):
|
|||
fred = 'Fred Bloggs <f.bloggs@napier.net>'
|
||||
joe = 'Joe Bloggs <joe@napierwallies.co.nz>'
|
||||
mary = 'Mary Bloggs <mary@napierwallies.co.nz>'
|
||||
commits = None
|
||||
patches = None
|
||||
|
||||
def setUp(self):
|
||||
self.tmpdir = tempfile.mkdtemp(prefix='patman.')
|
||||
|
|
@ -44,6 +50,7 @@ class TestFunctional(unittest.TestCase):
|
|||
|
||||
def tearDown(self):
|
||||
shutil.rmtree(self.tmpdir)
|
||||
terminal.SetPrintTestMode(False)
|
||||
|
||||
@staticmethod
|
||||
def _get_path(fname):
|
||||
|
|
@ -607,3 +614,311 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
|
|||
str(exc.exception))
|
||||
finally:
|
||||
os.chdir(orig_dir)
|
||||
|
||||
@staticmethod
|
||||
def _fake_patchwork(subpath):
|
||||
"""Fake Patchwork server for the function below
|
||||
|
||||
This handles accessing a series, providing a list consisting of a
|
||||
single patch
|
||||
"""
|
||||
re_series = re.match(r'series/(\d*)/$', subpath)
|
||||
if re_series:
|
||||
series_num = re_series.group(1)
|
||||
if series_num == '1234':
|
||||
return {'patches': [
|
||||
{'id': '1', 'name': 'Some patch'}]}
|
||||
raise ValueError('Fake Patchwork does not understand: %s' % subpath)
|
||||
|
||||
@unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
|
||||
def testStatusMismatch(self):
|
||||
"""Test Patchwork patches not matching the series"""
|
||||
series = Series()
|
||||
|
||||
with capture_sys_output() as (_, err):
|
||||
status.collect_patches(series, 1234, self._fake_patchwork)
|
||||
self.assertIn('Warning: Patchwork reports 1 patches, series has 0',
|
||||
err.getvalue())
|
||||
|
||||
@unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
|
||||
def testStatusReadPatch(self):
|
||||
"""Test handling a single patch in Patchwork"""
|
||||
series = Series()
|
||||
series.commits = [Commit('abcd')]
|
||||
|
||||
patches = status.collect_patches(series, 1234, self._fake_patchwork)
|
||||
self.assertEqual(1, len(patches))
|
||||
patch = patches[0]
|
||||
self.assertEqual('1', patch.id)
|
||||
self.assertEqual('Some patch', patch.raw_subject)
|
||||
|
||||
@unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
|
||||
def testParseSubject(self):
|
||||
"""Test parsing of the patch subject"""
|
||||
patch = status.Patch('1')
|
||||
|
||||
# Simple patch not in a series
|
||||
patch.parse_subject('Testing')
|
||||
self.assertEqual('Testing', patch.raw_subject)
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(1, patch.seq)
|
||||
self.assertEqual(1, patch.count)
|
||||
self.assertEqual(None, patch.prefix)
|
||||
self.assertEqual(None, patch.version)
|
||||
|
||||
# First patch in a series
|
||||
patch.parse_subject('[1/2] Testing')
|
||||
self.assertEqual('[1/2] Testing', patch.raw_subject)
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(1, patch.seq)
|
||||
self.assertEqual(2, patch.count)
|
||||
self.assertEqual(None, patch.prefix)
|
||||
self.assertEqual(None, patch.version)
|
||||
|
||||
# Second patch in a series
|
||||
patch.parse_subject('[2/2] Testing')
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(2, patch.seq)
|
||||
self.assertEqual(2, patch.count)
|
||||
self.assertEqual(None, patch.prefix)
|
||||
self.assertEqual(None, patch.version)
|
||||
|
||||
# RFC patch
|
||||
patch.parse_subject('[RFC,3/7] Testing')
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(3, patch.seq)
|
||||
self.assertEqual(7, patch.count)
|
||||
self.assertEqual('RFC', patch.prefix)
|
||||
self.assertEqual(None, patch.version)
|
||||
|
||||
# Version patch
|
||||
patch.parse_subject('[v2,3/7] Testing')
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(3, patch.seq)
|
||||
self.assertEqual(7, patch.count)
|
||||
self.assertEqual(None, patch.prefix)
|
||||
self.assertEqual('v2', patch.version)
|
||||
|
||||
# All fields
|
||||
patch.parse_subject('[RESEND,v2,3/7] Testing')
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(3, patch.seq)
|
||||
self.assertEqual(7, patch.count)
|
||||
self.assertEqual('RESEND', patch.prefix)
|
||||
self.assertEqual('v2', patch.version)
|
||||
|
||||
# RFC only
|
||||
patch.parse_subject('[RESEND] Testing')
|
||||
self.assertEqual('Testing', patch.subject)
|
||||
self.assertEqual(1, patch.seq)
|
||||
self.assertEqual(1, patch.count)
|
||||
self.assertEqual('RESEND', patch.prefix)
|
||||
self.assertEqual(None, patch.version)
|
||||
|
||||
@unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
|
||||
def testCompareSeries(self):
|
||||
"""Test operation of compare_with_series()"""
|
||||
commit1 = Commit('abcd')
|
||||
commit1.subject = 'Subject 1'
|
||||
commit2 = Commit('ef12')
|
||||
commit2.subject = 'Subject 2'
|
||||
commit3 = Commit('3456')
|
||||
commit3.subject = 'Subject 2'
|
||||
|
||||
patch1 = status.Patch('1')
|
||||
patch1.subject = 'Subject 1'
|
||||
patch2 = status.Patch('2')
|
||||
patch2.subject = 'Subject 2'
|
||||
patch3 = status.Patch('3')
|
||||
patch3.subject = 'Subject 2'
|
||||
|
||||
series = Series()
|
||||
series.commits = [commit1]
|
||||
patches = [patch1]
|
||||
patch_for_commit, commit_for_patch, warnings = (
|
||||
status.compare_with_series(series, patches))
|
||||
self.assertEqual(1, len(patch_for_commit))
|
||||
self.assertEqual(patch1, patch_for_commit[0])
|
||||
self.assertEqual(1, len(commit_for_patch))
|
||||
self.assertEqual(commit1, commit_for_patch[0])
|
||||
|
||||
series.commits = [commit1]
|
||||
patches = [patch1, patch2]
|
||||
patch_for_commit, commit_for_patch, warnings = (
|
||||
status.compare_with_series(series, patches))
|
||||
self.assertEqual(1, len(patch_for_commit))
|
||||
self.assertEqual(patch1, patch_for_commit[0])
|
||||
self.assertEqual(1, len(commit_for_patch))
|
||||
self.assertEqual(commit1, commit_for_patch[0])
|
||||
self.assertEqual(["Cannot find commit for patch 2 ('Subject 2')"],
|
||||
warnings)
|
||||
|
||||
series.commits = [commit1, commit2]
|
||||
patches = [patch1]
|
||||
patch_for_commit, commit_for_patch, warnings = (
|
||||
status.compare_with_series(series, patches))
|
||||
self.assertEqual(1, len(patch_for_commit))
|
||||
self.assertEqual(patch1, patch_for_commit[0])
|
||||
self.assertEqual(1, len(commit_for_patch))
|
||||
self.assertEqual(commit1, commit_for_patch[0])
|
||||
self.assertEqual(["Cannot find patch for commit 2 ('Subject 2')"],
|
||||
warnings)
|
||||
|
||||
series.commits = [commit1, commit2, commit3]
|
||||
patches = [patch1, patch2]
|
||||
patch_for_commit, commit_for_patch, warnings = (
|
||||
status.compare_with_series(series, patches))
|
||||
self.assertEqual(2, len(patch_for_commit))
|
||||
self.assertEqual(patch1, patch_for_commit[0])
|
||||
self.assertEqual(patch2, patch_for_commit[1])
|
||||
self.assertEqual(1, len(commit_for_patch))
|
||||
self.assertEqual(commit1, commit_for_patch[0])
|
||||
self.assertEqual(["Cannot find patch for commit 3 ('Subject 2')",
|
||||
"Multiple commits match patch 2 ('Subject 2'):\n"
|
||||
' Subject 2\n Subject 2'],
|
||||
warnings)
|
||||
|
||||
series.commits = [commit1, commit2]
|
||||
patches = [patch1, patch2, patch3]
|
||||
patch_for_commit, commit_for_patch, warnings = (
|
||||
status.compare_with_series(series, patches))
|
||||
self.assertEqual(1, len(patch_for_commit))
|
||||
self.assertEqual(patch1, patch_for_commit[0])
|
||||
self.assertEqual(2, len(commit_for_patch))
|
||||
self.assertEqual(commit1, commit_for_patch[0])
|
||||
self.assertEqual(["Multiple patches match commit 2 ('Subject 2'):\n"
|
||||
' Subject 2\n Subject 2',
|
||||
"Cannot find commit for patch 3 ('Subject 2')"],
|
||||
warnings)
|
||||
|
||||
def _fake_patchwork2(self, subpath):
|
||||
"""Fake Patchwork server for the function below
|
||||
|
||||
This handles accessing series, patches and comments, providing the data
|
||||
in self.patches to the caller
|
||||
"""
|
||||
re_series = re.match(r'series/(\d*)/$', subpath)
|
||||
re_patch = re.match(r'patches/(\d*)/$', subpath)
|
||||
re_comments = re.match(r'patches/(\d*)/comments/$', subpath)
|
||||
if re_series:
|
||||
series_num = re_series.group(1)
|
||||
if series_num == '1234':
|
||||
return {'patches': self.patches}
|
||||
elif re_patch:
|
||||
patch_num = int(re_patch.group(1))
|
||||
patch = self.patches[patch_num - 1]
|
||||
return patch
|
||||
elif re_comments:
|
||||
patch_num = int(re_comments.group(1))
|
||||
patch = self.patches[patch_num - 1]
|
||||
return patch.comments
|
||||
raise ValueError('Fake Patchwork does not understand: %s' % subpath)
|
||||
|
||||
@unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2')
|
||||
def testFindNewResponses(self):
|
||||
"""Test operation of find_new_responses()"""
|
||||
commit1 = Commit('abcd')
|
||||
commit1.subject = 'Subject 1'
|
||||
commit2 = Commit('ef12')
|
||||
commit2.subject = 'Subject 2'
|
||||
|
||||
patch1 = status.Patch('1')
|
||||
patch1.parse_subject('[1/2] Subject 1')
|
||||
patch1.name = patch1.raw_subject
|
||||
patch1.content = 'This is my patch content'
|
||||
comment1a = {'content': 'Reviewed-by: %s\n' % self.joe}
|
||||
|
||||
patch1.comments = [comment1a]
|
||||
|
||||
patch2 = status.Patch('2')
|
||||
patch2.parse_subject('[2/2] Subject 2')
|
||||
patch2.name = patch2.raw_subject
|
||||
patch2.content = 'Some other patch content'
|
||||
comment2a = {
|
||||
'content': 'Reviewed-by: %s\nTested-by: %s\n' %
|
||||
(self.mary, self.leb)}
|
||||
comment2b = {'content': 'Reviewed-by: %s' % self.fred}
|
||||
patch2.comments = [comment2a, comment2b]
|
||||
|
||||
# This test works by setting up commits and patch for use by the fake
|
||||
# Rest API function _fake_patchwork2(). It calls various functions in
|
||||
# the status module after setting up tags in the commits, checking that
|
||||
# things behaves as expected
|
||||
self.commits = [commit1, commit2]
|
||||
self.patches = [patch1, patch2]
|
||||
count = 2
|
||||
new_rtag_list = [None] * count
|
||||
|
||||
# Check that the tags are picked up on the first patch
|
||||
status.find_new_responses(new_rtag_list, 0, commit1, patch1,
|
||||
self._fake_patchwork2)
|
||||
self.assertEqual(new_rtag_list[0], {'Reviewed-by': {self.joe}})
|
||||
|
||||
# Now the second patch
|
||||
status.find_new_responses(new_rtag_list, 1, commit2, patch2,
|
||||
self._fake_patchwork2)
|
||||
self.assertEqual(new_rtag_list[1], {
|
||||
'Reviewed-by': {self.mary, self.fred},
|
||||
'Tested-by': {self.leb}})
|
||||
|
||||
# Now add some tags to the commit, which means they should not appear as
|
||||
# 'new' tags when scanning comments
|
||||
new_rtag_list = [None] * count
|
||||
commit1.rtags = {'Reviewed-by': {self.joe}}
|
||||
status.find_new_responses(new_rtag_list, 0, commit1, patch1,
|
||||
self._fake_patchwork2)
|
||||
self.assertEqual(new_rtag_list[0], {})
|
||||
|
||||
# For the second commit, add Ed and Fred, so only Mary should be left
|
||||
commit2.rtags = {
|
||||
'Tested-by': {self.leb},
|
||||
'Reviewed-by': {self.fred}}
|
||||
status.find_new_responses(new_rtag_list, 1, commit2, patch2,
|
||||
self._fake_patchwork2)
|
||||
self.assertEqual(new_rtag_list[1], {'Reviewed-by': {self.mary}})
|
||||
|
||||
# Check that the output patches expectations:
|
||||
# 1 Subject 1
|
||||
# Reviewed-by: Joe Bloggs <joe@napierwallies.co.nz>
|
||||
# 2 Subject 2
|
||||
# Tested-by: Lord Edmund Blackaddër <weasel@blackadder.org>
|
||||
# Reviewed-by: Fred Bloggs <f.bloggs@napier.net>
|
||||
# + Reviewed-by: Mary Bloggs <mary@napierwallies.co.nz>
|
||||
# 1 new response available in patchwork
|
||||
|
||||
series = Series()
|
||||
series.commits = [commit1, commit2]
|
||||
terminal.SetPrintTestMode()
|
||||
status.check_patchwork_status(series, '1234', self._fake_patchwork2)
|
||||
lines = iter(terminal.GetPrintTestLines())
|
||||
col = terminal.Color()
|
||||
self.assertEqual(terminal.PrintLine(' 1 Subject 1', col.BLUE),
|
||||
next(lines))
|
||||
self.assertEqual(
|
||||
terminal.PrintLine(' Reviewed-by: ', col.GREEN, newline=False,
|
||||
bright=False),
|
||||
next(lines))
|
||||
self.assertEqual(terminal.PrintLine(self.joe, col.WHITE, bright=False),
|
||||
next(lines))
|
||||
|
||||
self.assertEqual(terminal.PrintLine(' 2 Subject 2', col.BLUE),
|
||||
next(lines))
|
||||
self.assertEqual(
|
||||
terminal.PrintLine(' Tested-by: ', col.GREEN, newline=False,
|
||||
bright=False),
|
||||
next(lines))
|
||||
self.assertEqual(terminal.PrintLine(self.leb, col.WHITE, bright=False),
|
||||
next(lines))
|
||||
self.assertEqual(
|
||||
terminal.PrintLine(' Reviewed-by: ', col.GREEN, newline=False,
|
||||
bright=False),
|
||||
next(lines))
|
||||
self.assertEqual(terminal.PrintLine(self.fred, col.WHITE, bright=False),
|
||||
next(lines))
|
||||
self.assertEqual(
|
||||
terminal.PrintLine(' + Reviewed-by: ', col.GREEN, newline=False),
|
||||
next(lines))
|
||||
self.assertEqual(terminal.PrintLine(self.mary, col.WHITE),
|
||||
next(lines))
|
||||
self.assertEqual(terminal.PrintLine(
|
||||
'1 new response available in patchwork', None), next(lines))
|
||||
|
|
|
|||
|
|
@ -90,6 +90,10 @@ 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')
|
||||
AddCommonArgs(status)
|
||||
|
||||
# Parse options twice: first to get the project and second to handle
|
||||
# defaults properly (which depends on project).
|
||||
argv = sys.argv[1:]
|
||||
|
|
@ -157,3 +161,17 @@ elif args.cmd == 'send':
|
|||
|
||||
else:
|
||||
control.send(args)
|
||||
|
||||
# Check status of patches in patchwork
|
||||
elif args.cmd == 'status':
|
||||
ret_code = 0
|
||||
try:
|
||||
control.patchwork_status(args.branch, args.count, args.start, args.end)
|
||||
except Exception as e:
|
||||
terminal.Print('patman: %s: %s' % (type(e).__name__, e),
|
||||
colour=terminal.Color.RED)
|
||||
if args.debug:
|
||||
print()
|
||||
traceback.print_exc()
|
||||
ret_code = 1
|
||||
sys.exit(ret_code)
|
||||
|
|
|
|||
|
|
@ -0,0 +1,356 @@
|
|||
# SPDX-License-Identifier: GPL-2.0+
|
||||
#
|
||||
# Copyright 2020 Google LLC
|
||||
#
|
||||
"""Talks to the patchwork service to figure out what patches have been reviewed
|
||||
and commented on.
|
||||
"""
|
||||
|
||||
import collections
|
||||
import concurrent.futures
|
||||
from itertools import repeat
|
||||
import re
|
||||
import requests
|
||||
|
||||
from patman.patchstream import PatchStream
|
||||
from patman import terminal
|
||||
from patman import tout
|
||||
|
||||
# Patches which are part of a multi-patch series are shown with a prefix like
|
||||
# [prefix, version, sequence], for example '[RFC, v2, 3/5]'. All but the last
|
||||
# part is optional. This decodes the string into groups. For single patches
|
||||
# the [] part is not present:
|
||||
# Groups: (ignore, ignore, ignore, prefix, version, sequence, subject)
|
||||
RE_PATCH = re.compile(r'(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$')
|
||||
|
||||
# This decodes the sequence string into a patch number and patch count
|
||||
RE_SEQ = re.compile(r'(\d+)/(\d+)')
|
||||
|
||||
def to_int(vals):
|
||||
"""Convert a list of strings into integers, using 0 if not an integer
|
||||
|
||||
Args:
|
||||
vals (list): List of strings
|
||||
|
||||
Returns:
|
||||
list: List of integers, one for each input string
|
||||
"""
|
||||
out = [int(val) if val.isdigit() else 0 for val in vals]
|
||||
return out
|
||||
|
||||
|
||||
class Patch(dict):
|
||||
"""Models a patch in patchwork
|
||||
|
||||
This class records information obtained from patchwork
|
||||
|
||||
Some of this information comes from the 'Patch' column:
|
||||
|
||||
[RFC,v2,1/3] dm: Driver and uclass changes for tiny-dm
|
||||
|
||||
This shows the prefix, version, seq, count and subject.
|
||||
|
||||
The other properties come from other columns in the display.
|
||||
|
||||
Properties:
|
||||
pid (str): ID of the patch (typically an integer)
|
||||
seq (int): Sequence number within series (1=first) parsed from sequence
|
||||
string
|
||||
count (int): Number of patches in series, parsed from sequence string
|
||||
raw_subject (str): Entire subject line, e.g.
|
||||
"[1/2,v2] efi_loader: Sort header file ordering"
|
||||
prefix (str): Prefix string or None (e.g. 'RFC')
|
||||
version (str): Version string or None (e.g. 'v2')
|
||||
raw_subject (str): Raw patch subject
|
||||
subject (str): Patch subject with [..] part removed (same as commit
|
||||
subject)
|
||||
"""
|
||||
def __init__(self, pid):
|
||||
super().__init__()
|
||||
self.id = pid # Use 'id' to match what the Rest API provides
|
||||
self.seq = None
|
||||
self.count = None
|
||||
self.prefix = None
|
||||
self.version = None
|
||||
self.raw_subject = None
|
||||
self.subject = None
|
||||
|
||||
# These make us more like a dictionary
|
||||
def __setattr__(self, name, value):
|
||||
self[name] = value
|
||||
|
||||
def __getattr__(self, name):
|
||||
return self[name]
|
||||
|
||||
def __hash__(self):
|
||||
return hash(frozenset(self.items()))
|
||||
|
||||
def __str__(self):
|
||||
return self.raw_subject
|
||||
|
||||
def parse_subject(self, raw_subject):
|
||||
"""Parse the subject of a patch into its component parts
|
||||
|
||||
See RE_PATCH for details. The parsed info is placed into seq, count,
|
||||
prefix, version, subject
|
||||
|
||||
Args:
|
||||
raw_subject (str): Subject string to parse
|
||||
|
||||
Raises:
|
||||
ValueError: the subject cannot be parsed
|
||||
"""
|
||||
self.raw_subject = raw_subject.strip()
|
||||
mat = RE_PATCH.search(raw_subject.strip())
|
||||
if not mat:
|
||||
raise ValueError("Cannot parse subject '%s'" % raw_subject)
|
||||
self.prefix, self.version, seq_info, self.subject = mat.groups()[3:]
|
||||
mat_seq = RE_SEQ.match(seq_info) if seq_info else False
|
||||
if mat_seq is None:
|
||||
self.version = seq_info
|
||||
seq_info = None
|
||||
if self.version and not self.version.startswith('v'):
|
||||
self.prefix = self.version
|
||||
self.version = None
|
||||
if seq_info:
|
||||
if mat_seq:
|
||||
self.seq = int(mat_seq.group(1))
|
||||
self.count = int(mat_seq.group(2))
|
||||
else:
|
||||
self.seq = 1
|
||||
self.count = 1
|
||||
|
||||
def compare_with_series(series, patches):
|
||||
"""Compare a list of patches with a series it came from
|
||||
|
||||
This prints any problems as warnings
|
||||
|
||||
Args:
|
||||
series (Series): Series to compare against
|
||||
patches (:type: list of Patch): list of Patch objects to compare with
|
||||
|
||||
Returns:
|
||||
tuple
|
||||
dict:
|
||||
key: Commit number (0...n-1)
|
||||
value: Patch object for that commit
|
||||
dict:
|
||||
key: Patch number (0...n-1)
|
||||
value: Commit object for that patch
|
||||
"""
|
||||
# Check the names match
|
||||
warnings = []
|
||||
patch_for_commit = {}
|
||||
all_patches = set(patches)
|
||||
for seq, cmt in enumerate(series.commits):
|
||||
pmatch = [p for p in all_patches if p.subject == cmt.subject]
|
||||
if len(pmatch) == 1:
|
||||
patch_for_commit[seq] = pmatch[0]
|
||||
all_patches.remove(pmatch[0])
|
||||
elif len(pmatch) > 1:
|
||||
warnings.append("Multiple patches match commit %d ('%s'):\n %s" %
|
||||
(seq + 1, cmt.subject,
|
||||
'\n '.join([p.subject for p in pmatch])))
|
||||
else:
|
||||
warnings.append("Cannot find patch for commit %d ('%s')" %
|
||||
(seq + 1, cmt.subject))
|
||||
|
||||
|
||||
# Check the names match
|
||||
commit_for_patch = {}
|
||||
all_commits = set(series.commits)
|
||||
for seq, patch in enumerate(patches):
|
||||
cmatch = [c for c in all_commits if c.subject == patch.subject]
|
||||
if len(cmatch) == 1:
|
||||
commit_for_patch[seq] = cmatch[0]
|
||||
all_commits.remove(cmatch[0])
|
||||
elif len(cmatch) > 1:
|
||||
warnings.append("Multiple commits match patch %d ('%s'):\n %s" %
|
||||
(seq + 1, patch.subject,
|
||||
'\n '.join([c.subject for c in cmatch])))
|
||||
else:
|
||||
warnings.append("Cannot find commit for patch %d ('%s')" %
|
||||
(seq + 1, patch.subject))
|
||||
|
||||
return patch_for_commit, commit_for_patch, warnings
|
||||
|
||||
def call_rest_api(subpath):
|
||||
"""Call the patchwork API and return the result as JSON
|
||||
|
||||
Args:
|
||||
subpath (str): URL subpath to use
|
||||
|
||||
Returns:
|
||||
dict: Json result
|
||||
|
||||
Raises:
|
||||
ValueError: the URL could not be read
|
||||
"""
|
||||
url = 'https://patchwork.ozlabs.org/api/1.2/%s' % subpath
|
||||
response = requests.get(url)
|
||||
if response.status_code != 200:
|
||||
raise ValueError("Could not read URL '%s'" % url)
|
||||
return response.json()
|
||||
|
||||
def collect_patches(series, series_id, rest_api=call_rest_api):
|
||||
"""Collect patch information about a series from patchwork
|
||||
|
||||
Uses the Patchwork REST API to collect information provided by patchwork
|
||||
about the status of each patch.
|
||||
|
||||
Args:
|
||||
series (Series): Series object corresponding to the local branch
|
||||
containing the series
|
||||
series_id (str): Patch series ID number
|
||||
rest_api (function): API function to call to access Patchwork, for
|
||||
testing
|
||||
|
||||
Returns:
|
||||
list: List of patches sorted by sequence number, each a Patch object
|
||||
|
||||
Raises:
|
||||
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)
|
||||
|
||||
# Get all the rows, which are patches
|
||||
patch_dict = data['patches']
|
||||
count = len(patch_dict)
|
||||
num_commits = len(series.commits)
|
||||
if count != num_commits:
|
||||
tout.Warning('Warning: Patchwork reports %d patches, series has %d' %
|
||||
(count, num_commits))
|
||||
|
||||
patches = []
|
||||
|
||||
# Work through each row (patch) one at a time, collecting the information
|
||||
warn_count = 0
|
||||
for pw_patch in patch_dict:
|
||||
patch = Patch(pw_patch['id'])
|
||||
patch.parse_subject(pw_patch['name'])
|
||||
patches.append(patch)
|
||||
if warn_count > 1:
|
||||
tout.Warning(' (total of %d warnings)' % warn_count)
|
||||
|
||||
# Sort patches by patch number
|
||||
patches = sorted(patches, key=lambda x: x.seq)
|
||||
return patches
|
||||
|
||||
def find_new_responses(new_rtag_list, seq, cmt, patch, rest_api=call_rest_api):
|
||||
"""Find new rtags collected by patchwork that we don't know about
|
||||
|
||||
This is designed to be run in parallel, once for each commit/patch
|
||||
|
||||
Args:
|
||||
new_rtag_list (list): New rtags are written to new_rtag_list[seq]
|
||||
list, each a dict:
|
||||
key: Response tag (e.g. 'Reviewed-by')
|
||||
value: Set of people who gave that response, each a name/email
|
||||
string
|
||||
seq (int): Position in new_rtag_list to update
|
||||
cmt (Commit): Commit object for this commit
|
||||
patch (Patch): Corresponding Patch object for this patch
|
||||
rest_api (function): API function to call to access Patchwork, for
|
||||
testing
|
||||
"""
|
||||
if not patch:
|
||||
return
|
||||
|
||||
# Get the content for the patch email itself as well as all comments
|
||||
data = rest_api('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)
|
||||
|
||||
for comment in data:
|
||||
pstrm = PatchStream.process_text(comment['content'], True)
|
||||
for response, people in pstrm.commit.rtags.items():
|
||||
rtags[response].update(people)
|
||||
|
||||
# Find the tags that are not in the commit
|
||||
new_rtags = collections.defaultdict(set)
|
||||
base_rtags = cmt.rtags
|
||||
for tag, people in rtags.items():
|
||||
for who in people:
|
||||
is_new = (tag not in base_rtags or
|
||||
who not in base_rtags[tag])
|
||||
if is_new:
|
||||
new_rtags[tag].add(who)
|
||||
new_rtag_list[seq] = new_rtags
|
||||
|
||||
def show_responses(rtags, indent, is_new):
|
||||
"""Show rtags collected
|
||||
|
||||
Args:
|
||||
rtags (dict): review tags to show
|
||||
key: Response tag (e.g. 'Reviewed-by')
|
||||
value: Set of people who gave that response, each a name/email string
|
||||
indent (str): Indentation string to write before each line
|
||||
is_new (bool): True if this output should be highlighted
|
||||
|
||||
Returns:
|
||||
int: Number of review tags displayed
|
||||
"""
|
||||
col = terminal.Color()
|
||||
count = 0
|
||||
for tag, people in rtags.items():
|
||||
for who in people:
|
||||
terminal.Print(indent + '%s %s: ' % ('+' if is_new else ' ', tag),
|
||||
newline=False, colour=col.GREEN, bright=is_new)
|
||||
terminal.Print(who, colour=col.WHITE, bright=is_new)
|
||||
count += 1
|
||||
return count
|
||||
|
||||
def check_patchwork_status(series, series_id, rest_api=call_rest_api):
|
||||
"""Check the status of a series on Patchwork
|
||||
|
||||
This finds review tags and comments for a series in Patchwork, displaying
|
||||
them to show what is new compared to the local series.
|
||||
|
||||
Args:
|
||||
series (Series): Series object for the existing branch
|
||||
series_id (str): Patch series ID number
|
||||
rest_api (function): API function to call to access Patchwork, for
|
||||
testing
|
||||
"""
|
||||
patches = collect_patches(series, series_id, rest_api)
|
||||
col = terminal.Color()
|
||||
count = len(series.commits)
|
||||
new_rtag_list = [None] * count
|
||||
|
||||
patch_for_commit, _, warnings = compare_with_series(series, patches)
|
||||
for warn in warnings:
|
||||
tout.Warning(warn)
|
||||
|
||||
patch_list = [patch_for_commit.get(c) for c in range(len(series.commits))]
|
||||
|
||||
with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor:
|
||||
futures = executor.map(
|
||||
find_new_responses, repeat(new_rtag_list), range(count),
|
||||
series.commits, patch_list, repeat(rest_api))
|
||||
for fresponse in futures:
|
||||
if fresponse:
|
||||
raise fresponse.exception()
|
||||
|
||||
num_to_add = 0
|
||||
for seq, cmt in enumerate(series.commits):
|
||||
patch = patch_for_commit.get(seq)
|
||||
if not patch:
|
||||
continue
|
||||
terminal.Print('%3d %s' % (patch.seq, patch.subject[:50]),
|
||||
colour=col.BLUE)
|
||||
cmt = series.commits[seq]
|
||||
base_rtags = cmt.rtags
|
||||
new_rtags = new_rtag_list[seq]
|
||||
|
||||
indent = ' ' * 2
|
||||
show_responses(base_rtags, indent, False)
|
||||
num_to_add += show_responses(new_rtags, indent, True)
|
||||
|
||||
terminal.Print("%d new response%s available in patchwork" %
|
||||
(num_to_add, 's' if num_to_add != 1 else ''))
|
||||
|
|
@ -34,14 +34,22 @@ class PrintLine:
|
|||
newline: True to output a newline after the text
|
||||
colour: Text colour to use
|
||||
"""
|
||||
def __init__(self, text, newline, colour):
|
||||
def __init__(self, text, colour, newline=True, bright=True):
|
||||
self.text = text
|
||||
self.newline = newline
|
||||
self.colour = colour
|
||||
self.bright = bright
|
||||
|
||||
def __eq__(self, other):
|
||||
return (self.text == other.text and
|
||||
self.newline == other.newline and
|
||||
self.colour == other.colour and
|
||||
self.bright == other.bright)
|
||||
|
||||
def __str__(self):
|
||||
return 'newline=%s, colour=%s, text=%s' % (self.newline, self.colour,
|
||||
self.text)
|
||||
return ("newline=%s, colour=%s, bright=%d, text='%s'" %
|
||||
(self.newline, self.colour, self.bright, self.text))
|
||||
|
||||
|
||||
def CalcAsciiLen(text):
|
||||
"""Calculate the length of a string, ignoring any ANSI sequences
|
||||
|
|
@ -136,7 +144,7 @@ def Print(text='', newline=True, colour=None, limit_to_line=False, bright=True):
|
|||
global last_print_len
|
||||
|
||||
if print_test_mode:
|
||||
print_test_list.append(PrintLine(text, newline, colour))
|
||||
print_test_list.append(PrintLine(text, colour, newline, bright))
|
||||
else:
|
||||
if colour:
|
||||
col = Color()
|
||||
|
|
@ -159,11 +167,12 @@ def PrintClear():
|
|||
print('\r%s\r' % (' '* last_print_len), end='', flush=True)
|
||||
last_print_len = None
|
||||
|
||||
def SetPrintTestMode():
|
||||
def SetPrintTestMode(enable=True):
|
||||
"""Go into test mode, where all printing is recorded"""
|
||||
global print_test_mode
|
||||
|
||||
print_test_mode = True
|
||||
print_test_mode = enable
|
||||
GetPrintTestLines()
|
||||
|
||||
def GetPrintTestLines():
|
||||
"""Get a list of all lines output through Print()
|
||||
|
|
|
|||
Loading…
Reference in New Issue