diff mbox series

[v2,28/30] patman: Support updating a branch with review tags

Message ID 20201026010442.1606893-29-sjg@chromium.org
State Superseded
Delegated to: Simon Glass
Headers show
Series patman: Collect review tags and comments from Patchwork | expand

Commit Message

Simon Glass Oct. 26, 2020, 1:04 a.m. UTC
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>
---

(no changes since v1)

 tools/patman/README       |  17 ++++++-
 tools/patman/control.py   |   9 ++--
 tools/patman/func_test.py | 100 +++++++++++++++++++++++++++++++++++++-
 tools/patman/main.py      |   7 ++-
 tools/patman/status.py    |  89 +++++++++++++++++++++++++++++++--
 5 files changed, 210 insertions(+), 12 deletions(-)
diff mbox series

Patch

diff --git a/tools/patman/README b/tools/patman/README
index db941b9d0b4..d99bfb3127b 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
@@ -386,6 +388,17 @@  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. 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 09542401983..5ccf46570bd 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 0d38cb21c01..873cb31202b 100644
--- a/tools/patman/func_test.py
+++ b/tools/patman/func_test.py
@@ -911,7 +911,8 @@  Series-changes: 2
         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),
@@ -943,4 +944,99 @@  Series-changes: 2
         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)
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/status.py b/tools/patman/status.py
index f41b2d4c776..52f401e7a40 100644
--- a/tools/patman/status.py
+++ b/tools/patman/status.py
@@ -3,13 +3,16 @@ 
 # 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.patchstream import PatchStream
@@ -306,7 +309,72 @@  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 = cherry.message.rstrip() + '\n' + '\n'.join(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 +383,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 +424,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))