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>

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@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))
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 b3a25d59d11..a4bf08b7d81 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))