[PATCH v3 27/29] patman: Support updating a branch with review tags
Simon Glass
sjg at chromium.org
Fri Oct 30 04:46:36 CET 2020
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 at chromium.org>
---
Changes in v3:
- Allow tags to be inserted in the middle of the commit message
tools/patman/README | 19 +++++-
tools/patman/control.py | 9 ++-
tools/patman/func_test.py | 127 +++++++++++++++++++++++++++++++++++-
tools/patman/main.py | 7 +-
tools/patman/patchstream.py | 52 ++++++++++++++-
tools/patman/status.py | 91 ++++++++++++++++++++++++--
6 files changed, 289 insertions(+), 16 deletions(-)
diff --git a/tools/patman/README b/tools/patman/README
index 46b8e251ca5..15da6dc33cb 100644
--- a/tools/patman/README
+++ b/tools/patman/README
@@ -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
=================
diff --git a/tools/patman/control.py b/tools/patman/control.py
index 7a5469add1b..6ac258d41d7 100644
--- a/tools/patman/control.py
+++ b/tools/patman/control.py
@@ -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)
diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py
index 722844e15d3..2e1529525eb 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -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 at napierwallies.co.nz>
+ # 2 spi: SPI fixes
+ # + Reviewed-by: Fred Bloggs <f.bloggs at napier.net>
+ # + Reviewed-by: Mary Bloggs <mary at napierwallies.co.nz>
+ # + Tested-by: Lord Edmund Blackaddër <weasel at 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))
diff --git a/tools/patman/main.py b/tools/patman/main.py
index 7f4ae1125a1..8f139a6e3ba 100755
--- a/tools/patman/main.py
+++ b/tools/patman/main.py
@@ -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)
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py
index cf6a6c6fc3e..7b0805e9e14 100644
--- a/tools/patman/patchstream.py
+++ b/tools/patman/patchstream.py
@@ -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)
diff --git a/tools/patman/status.py b/tools/patman/status.py
index f41b2d4c776..f3a654160ec 100644
--- a/tools/patman/status.py
+++ b/tools/patman/status.py
@@ -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))
--
2.29.1.341.ge80a0c044ae-goog
More information about the U-Boot
mailing list