patman: Support updating a branch with review tags

It is tedious to add review tags into the local branch and errors can
sometimes be made. Add an option to create a new branch with the review
tags obtained from patchwork.

Signed-off-by: Simon Glass <sjg@chromium.org>
This commit is contained in:
Simon Glass 2020-10-29 21:46:36 -06:00
parent dc6df972c9
commit 8f9ba3ab56
6 changed files with 289 additions and 16 deletions

View File

@ -11,11 +11,13 @@ 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 also has some Patchwork features:
- shows review tags from Patchwork so you can update your local patches
- pulls these down into a new branch on request
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.
since they use the checkpatch.pl script.
It is configured almost entirely by tags it finds in your commits.
This means that you can work on a number of different branches at
@ -385,6 +387,19 @@ 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.
To automatically pull into these tags into a new branch, use the -d option:
patman status -d mtrr4
This will create a new 'mtrr4' branch which is the same as your current branch
but has the new review tags in it. The tags are added in alphabetic order and
are placed immediately after any existing ack/review/test/fixes tags, or at the
end. You can check that this worked with:
patman -b mtrr4 status
which should show that there are no new responses compared to this new branch.
Example Work Flow
=================

View File

@ -176,11 +176,11 @@ def send(args):
args.limit, args.dry_run, args.in_reply_to, args.thread,
args.smtp_server)
def patchwork_status(branch, count, start, end):
def patchwork_status(branch, count, start, end, dest_branch, force):
"""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
review tags, displays then and creates a new branch with the review tags.
Args:
branch (str): Branch to create patches from (None = current)
@ -189,6 +189,9 @@ def patchwork_status(branch, count, start, end):
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.)
dest_branch (str): Name of new branch to create with the updated tags
(None to not create a branch)
force (bool): With dest_branch, force overwriting an existing branch
Raises:
ValueError: if the branch has no Series-link value
@ -220,4 +223,4 @@ def patchwork_status(branch, count, start, end):
# 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])
status.check_patchwork_status(series, found[0], branch, dest_branch, force)

View File

@ -403,7 +403,16 @@ with some I2C-related things in it''')
self.make_commit_with_file('spi: SPI fixes', '''
SPI needs some fixes
and here they are
''', 'spi.c', '''Some fixes for SPI in this
Signed-off-by: %s
Series-to: u-boot
Commit-notes:
title of the series
This is the cover letter for the series
with various details
END
''' % self.leb, 'spi.c', '''Some fixes for SPI in this
file to make SPI work
better than before''')
first_target = repo.revparse_single('HEAD')
@ -889,7 +898,8 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
series = Series()
series.commits = [commit1, commit2]
terminal.SetPrintTestMode()
status.check_patchwork_status(series, '1234', self._fake_patchwork2)
status.check_patchwork_status(series, '1234', None, None, False,
self._fake_patchwork2)
lines = iter(terminal.GetPrintTestLines())
col = terminal.Color()
self.assertEqual(terminal.PrintLine(' 1 Subject 1', col.BLUE),
@ -921,4 +931,115 @@ diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
self.assertEqual(terminal.PrintLine(self.mary, col.WHITE),
next(lines))
self.assertEqual(terminal.PrintLine(
'1 new response available in patchwork', None), next(lines))
'1 new response available in patchwork (use -d to write them to a new branch)',
None), next(lines))
def _fake_patchwork3(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 testCreateBranch(self):
"""Test operation of create_branch()"""
repo = self.make_git_tree()
branch = 'first'
dest_branch = 'first2'
count = 2
gitdir = os.path.join(self.gitdir, '.git')
# Set up the test git tree. We use branch 'first' which has two commits
# in it
series = patchstream.get_metadata_for_list(branch, gitdir, count)
self.assertEqual(2, len(series.commits))
patch1 = status.Patch('1')
patch1.parse_subject('[1/2] %s' % series.commits[0].subject)
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] %s' % series.commits[1].subject)
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 patches for use by the fake Rest API
# function _fake_patchwork3(). The fake patch comments above should
# result in new review tags that are collected and added to the commits
# created in the destination branch.
self.patches = [patch1, patch2]
count = 2
# Expected output:
# 1 i2c: I2C things
# + Reviewed-by: Joe Bloggs <joe@napierwallies.co.nz>
# 2 spi: SPI fixes
# + Reviewed-by: Fred Bloggs <f.bloggs@napier.net>
# + Reviewed-by: Mary Bloggs <mary@napierwallies.co.nz>
# + Tested-by: Lord Edmund Blackaddër <weasel@blackadder.org>
# 4 new responses available in patchwork
# 4 responses added from patchwork into new branch 'first2'
# <unittest.result.TestResult run=8 errors=0 failures=0>
terminal.SetPrintTestMode()
status.check_patchwork_status(series, '1234', branch, dest_branch,
False, self._fake_patchwork3, repo)
lines = terminal.GetPrintTestLines()
self.assertEqual(12, len(lines))
self.assertEqual(
"4 responses added from patchwork into new branch 'first2'",
lines[11].text)
# Check that the destination branch has the new tags
new_series = patchstream.get_metadata_for_list(dest_branch, gitdir,
count)
self.assertEqual(
{'Reviewed-by': {self.joe}},
new_series.commits[0].rtags)
self.assertEqual(
{'Tested-by': {self.leb},
'Reviewed-by': {self.fred, self.mary}},
new_series.commits[1].rtags)
# Now check the actual test of the first commit message. We expect to
# see the new tags immediately below the old ones.
stdout = patchstream.get_list(dest_branch, count=count, git_dir=gitdir)
lines = iter([line.strip() for line in stdout.splitlines()
if '-by:' in line])
# First patch should have the review tag
self.assertEqual('Reviewed-by: %s' % self.joe, next(lines))
# Second patch should have the sign-off then the tested-by and two
# reviewed-by tags
self.assertEqual('Signed-off-by: %s' % self.leb, next(lines))
self.assertEqual('Reviewed-by: %s' % self.fred, next(lines))
self.assertEqual('Reviewed-by: %s' % self.mary, next(lines))
self.assertEqual('Tested-by: %s' % self.leb, next(lines))

View File

@ -92,6 +92,10 @@ AddCommonArgs(test_parser)
status = subparsers.add_parser('status',
help='Check status of patches in patchwork')
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
@ -166,7 +170,8 @@ elif args.cmd == 'send':
elif args.cmd == 'status':
ret_code = 0
try:
control.patchwork_status(args.branch, args.count, args.start, args.end)
control.patchwork_status(args.branch, args.count, args.start, args.end,
args.dest_branch, args.force)
except Exception as e:
terminal.Print('patman: %s: %s' % (type(e).__name__, e),
colour=terminal.Color.RED)

View File

@ -551,6 +551,54 @@ class PatchStream:
self.blank_count = 0
self.finalise()
def insert_tags(msg, tags_to_emit):
"""Add extra tags to a commit message
The tags are added after an existing block of tags if found, otherwise at
the end.
Args:
msg (str): Commit message
tags_to_emit (list): List of tags to emit, each a str
Returns:
(str) new message
"""
out = []
done = False
emit_tags = False
for line in msg.splitlines():
if not done:
signoff_match = RE_SIGNOFF.match(line)
tag_match = RE_TAG.match(line)
if tag_match or signoff_match:
emit_tags = True
if emit_tags and not tag_match and not signoff_match:
out += tags_to_emit
emit_tags = False
done = True
out.append(line)
if not done:
out.append('')
out += tags_to_emit
return '\n'.join(out)
def get_list(commit_range, git_dir=None, count=None):
"""Get a log of a list of comments
This returns the output of 'git log' for the selected commits
Args:
commit_range (str): Range of commits to count (e.g. 'HEAD..base')
git_dir (str): Path to git repositiory (None to use default)
count (int): Number of commits to list, or None for no limit
Returns
str: String containing the contents of the git log
"""
params = gitutil.LogCmd(commit_range, reverse=True, count=count,
git_dir=git_dir)
return command.RunPipe([params], capture=True).stdout
def get_metadata_for_list(commit_range, git_dir=None, count=None,
series=None, allow_overwrite=False):
@ -573,9 +621,7 @@ def get_metadata_for_list(commit_range, git_dir=None, count=None,
if not series:
series = Series()
series.allow_overwrite = allow_overwrite
params = gitutil.LogCmd(commit_range, reverse=True, count=count,
git_dir=git_dir)
stdout = command.RunPipe([params], capture=True).stdout
stdout = get_list(commit_range, git_dir, count)
pst = PatchStream(series, is_log=True)
for line in stdout.splitlines():
pst.process_line(line)

View File

@ -3,15 +3,19 @@
# Copyright 2020 Google LLC
#
"""Talks to the patchwork service to figure out what patches have been reviewed
and commented on.
and commented on. Allows creation of a new branch based on the old but with the
review tags collected from patchwork.
"""
import collections
import concurrent.futures
from itertools import repeat
import re
import pygit2
import requests
from patman import patchstream
from patman.patchstream import PatchStream
from patman import terminal
from patman import tout
@ -306,7 +310,73 @@ def show_responses(rtags, indent, is_new):
count += 1
return count
def check_patchwork_status(series, series_id, rest_api=call_rest_api):
def create_branch(series, new_rtag_list, branch, dest_branch, overwrite,
repo=None):
"""Create a new branch with review tags added
Args:
series (Series): Series object for the existing branch
new_rtag_list (list): List of review tags to add, one for each commit,
each a dict:
key: Response tag (e.g. 'Reviewed-by')
value: Set of people who gave that response, each a name/email
string
branch (str): Existing branch to update
dest_branch (str): Name of new branch to create
overwrite (bool): True to force overwriting dest_branch if it exists
repo (pygit2.Repository): Repo to use (use None unless testing)
Returns:
int: Total number of review tags added across all commits
Raises:
ValueError: if the destination branch name is the same as the original
branch, or it already exists and @overwrite is False
"""
if branch == dest_branch:
raise ValueError(
'Destination branch must not be the same as the original branch')
if not repo:
repo = pygit2.Repository('.')
count = len(series.commits)
new_br = repo.branches.get(dest_branch)
if new_br:
if not overwrite:
raise ValueError("Branch '%s' already exists (-f to overwrite)" %
dest_branch)
new_br.delete()
if not branch:
branch = 'HEAD'
target = repo.revparse_single('%s~%d' % (branch, count))
repo.branches.local.create(dest_branch, target)
num_added = 0
for seq in range(count):
parent = repo.branches.get(dest_branch)
cherry = repo.revparse_single('%s~%d' % (branch, count - seq - 1))
repo.merge_base(cherry.oid, parent.target)
base_tree = cherry.parents[0].tree
index = repo.merge_trees(base_tree, parent, cherry)
tree_id = index.write_tree(repo)
lines = []
if new_rtag_list[seq]:
for tag, people in new_rtag_list[seq].items():
for who in people:
lines.append('%s: %s' % (tag, who))
num_added += 1
message = patchstream.insert_tags(cherry.message.rstrip(),
sorted(lines))
repo.create_commit(
parent.name, cherry.author, cherry.committer, message, tree_id,
[parent.target])
return num_added
def check_patchwork_status(series, series_id, branch, dest_branch, force,
rest_api=call_rest_api, test_repo=None):
"""Check the status of a series on Patchwork
This finds review tags and comments for a series in Patchwork, displaying
@ -315,8 +385,12 @@ def check_patchwork_status(series, series_id, rest_api=call_rest_api):
Args:
series (Series): Series object for the existing branch
series_id (str): Patch series ID number
branch (str): Existing branch to update, or None
dest_branch (str): Name of new branch to create, or None
force (bool): True to force overwriting dest_branch if it exists
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)
col = terminal.Color()
@ -352,5 +426,14 @@ def check_patchwork_status(series, series_id, rest_api=call_rest_api):
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 ''))
terminal.Print("%d new response%s available in patchwork%s" %
(num_to_add, 's' if num_to_add != 1 else '',
'' if dest_branch
else ' (use -d to write them to a new branch)'))
if dest_branch:
num_added = create_branch(series, new_rtag_list, branch,
dest_branch, force, test_repo)
terminal.Print(
"%d response%s added from patchwork into new branch '%s'" %
(num_added, 's' if num_added != 1 else '', dest_branch))