From patchwork Mon Jul 6 03:41:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240784 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:48 -0600 Subject: [RFC PATCH 01/16] patman: Use test_util to show test results In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-2-sjg@chromium.org> This handles skipped tests correctly, so use it instead of the existing code. Signed-off-by: Simon Glass --- tools/patman/main.py | 8 ++------ tools/patman/test_util.py | 6 +++--- 2 files changed, 5 insertions(+), 9 deletions(-) diff --git a/tools/patman/main.py b/tools/patman/main.py index 28a9a26087..03668d1bb8 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -25,6 +25,7 @@ from patman import patchstream from patman import project from patman import settings from patman import terminal +from patman import test_util from patman import test_checkpatch @@ -101,12 +102,7 @@ elif options.test: suite = doctest.DocTestSuite(module) suite.run(result) - # TODO: Surely we can just 'print' result? - print(result) - for test, err in result.errors: - print(err) - for test, err in result.failures: - print(err) + sys.exit(test_util.ReportResult('patman', None, result)) # Called from git with a patch filename as argument # Printout a list of additional CC recipients for this patch diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index aac58fb72f..0827488f33 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -123,12 +123,12 @@ def ReportResult(toolname:str, test_name: str, result: unittest.TestResult): for test, err in result.failures: print(err, result.failures) if result.skipped: - print('%d binman test%s SKIPPED:' % - (len(result.skipped), 's' if len(result.skipped) > 1 else '')) + print('%d %s test%s SKIPPED:' % (len(result.skipped), toolname, + 's' if len(result.skipped) > 1 else '')) for skip_info in result.skipped: print('%s: %s' % (skip_info[0], skip_info[1])) if result.errors or result.failures: - print('binman tests FAILED') + print('%s tests FAILED' % toolname) return 1 return 0 From patchwork Mon Jul 6 03:41:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240783 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:49 -0600 Subject: [RFC PATCH 02/16] patman: Move main code out to a control module In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-3-sjg@chromium.org> To make testing easier, move the code out from main into a separate 'control' module and split it into four parts: setup, preparing patches, checking patches and emailing patches. Add comments and fix a few code-style issues while we are here. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 6 ++ tools/patman/control.py | 170 +++++++++++++++++++++++++++++++++++++ tools/patman/gitutil.py | 4 +- tools/patman/main.py | 57 +------------ tools/patman/series.py | 2 +- 5 files changed, 182 insertions(+), 57 deletions(-) create mode 100644 tools/patman/control.py diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 07c3e2739a..263bac3fc9 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -41,6 +41,12 @@ def FindCheckPatch(): def CheckPatch(fname, verbose=False, show_types=False): """Run checkpatch.pl on a file. + Args: + fname: Filename to check + verbose: True to print out every line of the checkpatch output as it is + parsed + show_types: Tell checkpatch to show the type (number) of each message + Returns: namedtuple containing: ok: False=failure, True=ok diff --git a/tools/patman/control.py b/tools/patman/control.py new file mode 100644 index 0000000000..a896c924b5 --- /dev/null +++ b/tools/patman/control.py @@ -0,0 +1,170 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2020 Google LLC +# +"""Handles the main control logic of patman + +This module provides various functions called by the main program to implement +the features of patman. +""" + +import os +import sys + +from patman import checkpatch +from patman import gitutil +from patman import patchstream +from patman import terminal + +def setup(): + """Do required setup before doing anything""" + gitutil.Setup() + +def prepare_patches(col, count, start, ignore_binary): + """Figure out what patches to generate, then generate them + + The patch files are written to the current directory, e.g. 0001_xxx.patch + 0002_yyy.patch + + Args: + col (terminal.Color): Colour output object + 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) + ignore_binary (bool): Don't generate patches for binary files + + Returns: + Tuple: + Series object for this series (set of patches) + Filename of the cover letter as a string (None if none) + patch_files: List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + """ + if count == -1: + # Work out how many patches to send if we can + count = (gitutil.CountCommitsToBranch() - start) + + if not count: + sys.exit(col.Color(col.RED, + 'No commits found to process - please use -c flag')) + + # Read the metadata from the commits + to_do = count + series = patchstream.GetMetaData(start, to_do) + cover_fname, patch_files = gitutil.CreatePatches( + start, to_do, ignore_binary, series) + + # Fix up the patch files to our liking, and insert the cover letter + patchstream.FixPatches(series, patch_files) + if cover_fname and series.get('cover'): + patchstream.InsertCoverLetter(cover_fname, series, to_do) + return series, cover_fname, patch_files + +def check_patches(series, patch_files, run_checkpatch, verbose): + """Run some checks on a set of patches + + This santiy-checks the patman tags like Series-version and runs the patches + through checkpatch + + Args: + series (Series): Series object for this series (set of patches) + patch_files (list): List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + run_checkpatch (bool): True to run checkpatch.pl + verbose (bool): True to print out every line of the checkpatch output as + it is parsed + + Returns: + bool: True if the patches had no errors, False if they did + """ + # Do a few checks on the series + series.DoChecks() + + # Check the patches, and run them through 'git am' just to be sure + if run_checkpatch: + ok = checkpatch.CheckPatches(verbose, patch_files) + else: + ok = True + return ok + + +def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, + ignore_bad_tags, add_maintainers, limit, dry_run, in_reply_to, + thread, smtp_server): + """Email patches to the recipients + + This emails out the patches and cover letter using 'git send-email'. Each + patch is copied to recipients identified by the patch tag and output from + the get_maintainer.pl script. The cover letter is copied to all recipients + of any patch. + + To make this work a CC file is created holding the recipients for each patch + and the cover letter. See the main program 'cc_cmd' for this logic. + + Args: + col (terminal.Color): Colour output object + series (Series): Series object for this series (set of patches) + cover_fname (str): Filename of the cover letter as a string (None if + none) + patch_files (list): List of patch filenames, each a string, e.g. + ['0001_xxx.patch', '0002_yyy.patch'] + process_tags (bool): True to process subject tags in each patch, e.g. + for 'dm: spi: Add SPI support' this would be 'dm' and 'spi'. The + tags are looked up in the configured sendemail.aliasesfile and also + in ~/.patman (see README) + its_a_go (bool): True if we are going to actually send the patches, + False if the patches have errors and will not be sent unless + @ignore_errors + ignore_bad_tags (bool): True to just print a warning for unknown tags, + False to halt with an error + add_maintainers (bool): Run the get_maintainer.pl script for each patch + limit (int): Limit on the number of people that can be cc'd on a single + patch or the cover letter (None if no limit) + dry_run (bool): Don't actually email the patches, just print out what + would be sent + in_reply_to (str): If not None we'll pass this to git as --in-reply-to. + Should be a message ID that this is in reply to. + thread (bool): True to add --thread to git send-email (make all patches + reply to cover-letter or first patch in series) + smtp_server (str): SMTP server to use to send patches (None for default) + """ + cc_file = series.MakeCcFile(process_tags, cover_fname, not ignore_bad_tags, + add_maintainers, limit) + + # Email the patches out (giving the user time to check / cancel) + cmd = '' + if its_a_go: + cmd = gitutil.EmailPatches( + series, cover_fname, patch_files, dry_run, not ignore_bad_tags, + cc_file, in_reply_to=in_reply_to, thread=thread, + smtp_server=smtp_server) + else: + print(col.Color(col.RED, "Not sending emails due to errors/warnings")) + + # For a dry run, just show our actions as a sanity check + if dry_run: + series.ShowActions(patch_files, cmd, process_tags) + if not its_a_go: + print(col.Color(col.RED, "Email would not be sent")) + + os.remove(cc_file) + +def send(options): + """Create, check and send patches by email + + Args: + options (optparse.Values): Arguments to patman + """ + setup() + col = terminal.Color() + series, cover_fname, patch_files = prepare_patches( + col, options.count, options.start, options.ignore_binary) + ok = check_patches(series, patch_files, options.check_patch, + options.verbose) + its_a_go = ok or options.ignore_errors + if its_a_go: + email_patches( + col, series, cover_fname, patch_files, options.process_tags, + its_a_go, options.ignore_bad_tags, options.add_maintainers, + options.limit, options.dry_run, options.in_reply_to, options.thread, + options.smtp_server) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 5189840eab..29444bf8e9 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -261,8 +261,10 @@ def CreatePatches(start, count, ignore_binary, series): Args: start: Commit to start from: 0=HEAD, 1=next one, etc. count: number of commits to include + ignore_binary: Don't generate patches for binary files + series: Series object for this series (set of patches) Return: - Filename of cover letter + Filename of cover letter (None if none) List of filenames of patch files """ if series.get('version'): diff --git a/tools/patman/main.py b/tools/patman/main.py index 03668d1bb8..2432d31871 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -18,10 +18,9 @@ if __name__ == "__main__": sys.path.append(os.path.join(our_path, '..')) # Our modules -from patman import checkpatch from patman import command +from patman import control from patman import gitutil -from patman import patchstream from patman import project from patman import settings from patman import terminal @@ -128,56 +127,4 @@ elif options.full_help: # Process commits, produce patches files, check them, email them else: - gitutil.Setup() - - if options.count == -1: - # Work out how many patches to send if we can - options.count = gitutil.CountCommitsToBranch() - options.start - - col = terminal.Color() - if not options.count: - str = 'No commits found to process - please use -c flag' - sys.exit(col.Color(col.RED, str)) - - # Read the metadata from the commits - if options.count: - series = patchstream.GetMetaData(options.start, options.count) - cover_fname, args = gitutil.CreatePatches(options.start, options.count, - options.ignore_binary, series) - - # Fix up the patch files to our liking, and insert the cover letter - patchstream.FixPatches(series, args) - if cover_fname and series.get('cover'): - patchstream.InsertCoverLetter(cover_fname, series, options.count) - - # Do a few checks on the series - series.DoChecks() - - # Check the patches, and run them through 'git am' just to be sure - if options.check_patch: - ok = checkpatch.CheckPatches(options.verbose, args) - else: - ok = True - - cc_file = series.MakeCcFile(options.process_tags, cover_fname, - not options.ignore_bad_tags, - options.add_maintainers, options.limit) - - # Email the patches out (giving the user time to check / cancel) - cmd = '' - its_a_go = ok or options.ignore_errors - if its_a_go: - cmd = gitutil.EmailPatches(series, cover_fname, args, - options.dry_run, not options.ignore_bad_tags, cc_file, - in_reply_to=options.in_reply_to, thread=options.thread, - smtp_server=options.smtp_server) - else: - print(col.Color(col.RED, "Not sending emails due to errors/warnings")) - - # For a dry run, just show our actions as a sanity check - if options.dry_run: - series.ShowActions(args, cmd, options.process_tags) - if not its_a_go: - print(col.Color(col.RED, "Email would not be sent")) - - os.remove(cc_file) + control.send(options) diff --git a/tools/patman/series.py b/tools/patman/series.py index b7eef37d03..9f885c8987 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -244,7 +244,7 @@ class Series(dict): add_maintainers: Either: True/False to call the get_maintainers to CC maintainers List of maintainers to include (for testing) - limit: Limit the length of the Cc list + limit: Limit the length of the Cc list (None if no limit) Return: Filename of temp file created """ From patchwork Mon Jul 6 03:41:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240785 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:50 -0600 Subject: [RFC PATCH 03/16] patman: Add a test that uses gitpython In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-4-sjg@chromium.org> It is convenient to use gitpython to create a real git repo for testing patman's operation. Add a test for this. So far it just checks that patman produces the right number of patches for a branch. Signed-off-by: Simon Glass --- tools/patman/func_test.py | 151 +++++++++++++++++++++++++++++++++++++- tools/patman/tools.py | 4 +- 2 files changed, 152 insertions(+), 3 deletions(-) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index dc30078cce..211952154a 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -14,15 +14,23 @@ import unittest from io import StringIO +from patman import control from patman import gitutil from patman import patchstream from patman import settings +from patman import terminal from patman import tools +from patman.test_util import capture_sys_output + +try: + import pygit2 + HAVE_PYGIT2= True +except ModuleNotFoundError: + HAVE_PYGIT2 = False @contextlib.contextmanager def capture(): - import sys oldout,olderr = sys.stdout, sys.stderr try: out=[StringIO(), StringIO()] @@ -37,6 +45,8 @@ def capture(): class TestFunctional(unittest.TestCase): def setUp(self): self.tmpdir = tempfile.mkdtemp(prefix='patman.') + self.gitdir = os.path.join(self.tmpdir, 'git') + self.repo = None def tearDown(self): shutil.rmtree(self.tmpdir) @@ -286,3 +296,142 @@ Changes in v2: if expected: expected = expected.splitlines() self.assertEqual(expected, lines[start:(start+len(expected))]) + + def make_commit_with_file(self, subject, body, fname, text): + """Create a file and add it to the git repo with a new commit + + Args: + subject (str): Subject for the commit + body (str): Body text of the commit + fname (str): Filename of file to create + text (str): Text to put into the file + """ + path = os.path.join(self.gitdir, fname) + tools.WriteFile(path, text, binary=False) + index = self.repo.index + index.add(fname) + author = pygit2.Signature('Test user', 'test at email.com') + committer = author + tree = index.write_tree() + message = subject + '\n' + body + self.repo.create_commit('HEAD', author, committer, message, tree, + [self.repo.head.target]) + + def make_git_tree(self): + """Make a simple git tree suitable for testing + + It has three branches: + 'base' has two commits: PCI, main + 'first' has base as upstream and two more commits: I2C, SPI + 'second' has base as upstream and three more: video, serial, bootm + + Returns: + pygit2 repository + """ + repo = pygit2.init_repository(self.gitdir) + self.repo = repo + new_tree = repo.TreeBuilder().write() + + author = pygit2.Signature('Test user', 'test at email.com') + committer = author + commit = repo.create_commit('HEAD', author, committer, + 'Created master', new_tree, []) + + self.make_commit_with_file('Initial commit', ''' +Add a README + +''', 'README', '''This is the README file +describing this project +in very little detail''') + + self.make_commit_with_file('pci: PCI implementation', ''' +Here is a basic PCI implementation + +''', 'pci.c', '''This is a file +it has some contents +and some more things''') + self.make_commit_with_file('main: Main program', ''' +Hello here is the second commit. +''', 'main.c', '''This is the main file +there is very little here +but we can always add more later +if we want to + +Series-to: u-boot +Series-cc: Barry Crump +''') + base_target = repo.revparse_single('HEAD') + self.make_commit_with_file('i2c: I2C things', ''' +This has some stuff to do with I2C +''', 'i2c.c', '''And this is the file contents +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 +file to make SPI work +better than before''') + first_target = repo.revparse_single('HEAD') + + target = repo.revparse_single('HEAD~2') + repo.reset(target.oid, pygit2.GIT_CHECKOUT_FORCE) + self.make_commit_with_file('video: Some video improvements', ''' +Fix up the video so that +it looks more purple. Purple is +a very nice colour. +''', 'video.c', '''More purple here +Purple and purple +Even more purple +Could not be any more purple''') + self.make_commit_with_file('serial: Add a serial driver', ''' +Here is the serial driver +for my chip. + +Cover-letter: +Series for my board +This series implements support +for my glorious board. +END +''', 'serial.c', '''The code for the +serial driver is here''') + self.make_commit_with_file('bootm: Make it boot', ''' +This makes my board boot +with a fix to the bootm +command +''', 'bootm.c', '''Fix up the bootm +command to make the code as +complicated as possible''') + second_target = repo.revparse_single('HEAD') + + repo.branches.local.create('first', first_target) + repo.config.set_multivar('branch.first.remote', '', '.') + repo.config.set_multivar('branch.first.merge', '', 'refs/heads/base') + + repo.branches.local.create('second', second_target) + repo.config.set_multivar('branch.second.remote', '', '.') + repo.config.set_multivar('branch.second.merge', '', 'refs/heads/base') + + repo.branches.local.create('base', base_target) + return repo + + @unittest.skipIf(not HAVE_PYGIT2, 'Missing python3-pygit2') + def testBranch(self): + """Test creating patches from a branch""" + repo = self.make_git_tree() + target = repo.lookup_reference('refs/heads/first') + self.repo.checkout(target, strategy=pygit2.GIT_CHECKOUT_FORCE) + control.setup() + try: + orig_dir = os.getcwd() + os.chdir(self.gitdir) + + # Check that it can detect the current branch + self.assertEqual(2, gitutil.CountCommitsToBranch()) + col = terminal.Color() + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, count=-1, start=0, ignore_binary=False) + self.assertIsNone(cover_fname) + self.assertEqual(2, len(patch_files)) + finally: + os.chdir(orig_dir) diff --git a/tools/patman/tools.py b/tools/patman/tools.py index b50370dfe8..f402b9aab8 100644 --- a/tools/patman/tools.py +++ b/tools/patman/tools.py @@ -270,7 +270,7 @@ def ReadFile(fname, binary=True): #(fname, len(data), len(data))) return data -def WriteFile(fname, data): +def WriteFile(fname, data, binary=True): """Write data into a file. Args: @@ -279,7 +279,7 @@ def WriteFile(fname, data): """ #self._out.Info("Write file '%s' size %d (%#0x)" % #(fname, len(data), len(data))) - with open(Filename(fname), 'wb') as fd: + with open(Filename(fname), binary and 'wb' or 'w') as fd: fd.write(data) def GetBytes(byte, size): From patchwork Mon Jul 6 03:41:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240786 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:51 -0600 Subject: [RFC PATCH 04/16] patman: Allow creating patches for another branch In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-5-sjg@chromium.org> Add a -b option to allow patches to be created from a branch other than the current one. Signed-off-by: Simon Glass --- tools/patman/control.py | 13 ++++++++----- tools/patman/func_test.py | 13 +++++++++++-- tools/patman/gitutil.py | 19 ++++++++++++++----- tools/patman/main.py | 2 ++ tools/patman/patchstream.py | 8 +++++--- 5 files changed, 40 insertions(+), 15 deletions(-) diff --git a/tools/patman/control.py b/tools/patman/control.py index a896c924b5..b48eac41fd 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -20,7 +20,7 @@ def setup(): """Do required setup before doing anything""" gitutil.Setup() -def prepare_patches(col, count, start, ignore_binary): +def prepare_patches(col, branch, count, start, ignore_binary): """Figure out what patches to generate, then generate them The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -28,6 +28,7 @@ def prepare_patches(col, count, start, ignore_binary): Args: col (terminal.Color): Colour output object + 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) @@ -42,7 +43,7 @@ def prepare_patches(col, count, start, ignore_binary): """ if count == -1: # Work out how many patches to send if we can - count = (gitutil.CountCommitsToBranch() - start) + count = (gitutil.CountCommitsToBranch(branch) - start) if not count: sys.exit(col.Color(col.RED, @@ -50,9 +51,9 @@ def prepare_patches(col, count, start, ignore_binary): # Read the metadata from the commits to_do = count - series = patchstream.GetMetaData(start, to_do) + series = patchstream.GetMetaData(branch, start, to_do) cover_fname, patch_files = gitutil.CreatePatches( - start, to_do, ignore_binary, series) + branch, start, to_do, ignore_binary, series) # Fix up the patch files to our liking, and insert the cover letter patchstream.FixPatches(series, patch_files) @@ -158,9 +159,11 @@ def send(options): setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.count, options.start, options.ignore_binary) + col, options.branch, options.count, options.start, + options.ignore_binary) ok = check_patches(series, patch_files, options.check_patch, options.verbose) + its_a_go = ok or options.ignore_errors if its_a_go: email_patches( diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 211952154a..588be73ef4 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -426,12 +426,21 @@ complicated as possible''') os.chdir(self.gitdir) # Check that it can detect the current branch - self.assertEqual(2, gitutil.CountCommitsToBranch()) + self.assertEqual(2, gitutil.CountCommitsToBranch(None)) col = terminal.Color() with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, count=-1, start=0, ignore_binary=False) + col, branch=None, count=-1, start=0, ignore_binary=False) self.assertIsNone(cover_fname) self.assertEqual(2, len(patch_files)) + + # Check that it can detect a different branch + self.assertEqual(3, gitutil.CountCommitsToBranch('second')) + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, branch='second', count=-1, start=0, + ignore_binary=False) + self.assertIsNotNone(cover_fname) + self.assertEqual(3, len(patch_files)) finally: os.chdir(orig_dir) diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index 29444bf8e9..b683481a57 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -49,17 +49,24 @@ def LogCmd(commit_range, git_dir=None, oneline=False, reverse=False, cmd.append('--') return cmd -def CountCommitsToBranch(): +def CountCommitsToBranch(branch): """Returns number of commits between HEAD and the tracking branch. This looks back to the tracking branch and works out the number of commits since then. + Args: + branch: Branch to count from (None for current branch) + Return: Number of patches that exist on top of the branch """ - pipe = [LogCmd('@{upstream}..', oneline=True), - ['wc', '-l']] + if branch: + us, msg = GetUpstream('.git', branch) + rev_range = '%s..%s' % (us, branch) + else: + rev_range = '@{upstream}..' + pipe = [LogCmd(rev_range, oneline=True), ['wc', '-l']] stdout = command.RunPipe(pipe, capture=True, oneline=True).stdout patch_count = int(stdout) return patch_count @@ -252,13 +259,14 @@ def Fetch(git_dir=None, work_tree=None): if result.return_code != 0: raise OSError('git fetch: %s' % result.stderr) -def CreatePatches(start, count, ignore_binary, series): +def CreatePatches(branch, start, count, ignore_binary, series): """Create a series of patches from the top of the current branch. The patch files are written to the current directory using git format-patch. Args: + branch: Branch to create patches from (None for current branch) start: Commit to start from: 0=HEAD, 1=next one, etc. count: number of commits to include ignore_binary: Don't generate patches for binary files @@ -277,7 +285,8 @@ def CreatePatches(start, count, ignore_binary, series): prefix = series.GetPatchPrefix() if prefix: cmd += ['--subject-prefix=%s' % prefix] - cmd += ['HEAD~%d..HEAD~%d' % (start + count, start)] + brname = branch or 'HEAD' + cmd += ['%s~%d..%s~%d' % (brname, start + count, brname, start)] stdout = command.RunList(cmd) files = stdout.splitlines() diff --git a/tools/patman/main.py b/tools/patman/main.py index 2432d31871..066754196e 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -31,6 +31,8 @@ from patman import test_checkpatch parser = OptionParser() parser.add_option('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') +parser.add_option('-b', '--branch', type='str', + help="Branch to process (by default, the current branch)") parser.add_option('-c', '--count', dest='count', type='int', default=-1, help='Automatically create patches from top n commits') parser.add_option('-i', '--ignore-errors', action='store_true', diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 4fe465e9ab..2ea8ebcc3f 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -512,17 +512,19 @@ def GetMetaDataForList(commit_range, git_dir=None, count=None, ps.Finalize() return series -def GetMetaData(start, count): +def GetMetaData(branch, start, count): """Reads out patch series metadata from the commits This does a 'git log' on the relevant commits and pulls out the tags we are interested in. Args: - start: Commit to start from: 0=HEAD, 1=next one, etc. + branch: Branch to use (None for current branch) + start: Commit to start from: 0=branch HEAD, 1=next one, etc. count: Number of commits to list """ - return GetMetaDataForList('HEAD~%d' % start, None, count) + return GetMetaDataForList('%s~%d' % (branch if branch else 'HEAD', start), + None, count) def GetMetaDataForTest(text): """Process metadata from a file containing a git log. Used for tests From patchwork Mon Jul 6 03:41:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240787 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:52 -0600 Subject: [RFC PATCH 05/16] patman: Allow skipping patches at the end In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-6-sjg@chromium.org> The -s option allows skipping patches at the top of the branch. Sometimes there are commits at the bottom that need to be skipped. At present it is necessary to count the number of commits and then use -c to tell patman how many to process. Add a -e option to easily skip a number of commits at the bottom of the branch. Signed-off-by: Simon Glass --- tools/patman/control.py | 8 +++++--- tools/patman/func_test.py | 13 +++++++++++-- tools/patman/main.py | 2 ++ 3 files changed, 18 insertions(+), 5 deletions(-) diff --git a/tools/patman/control.py b/tools/patman/control.py index b48eac41fd..b481ff6b27 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -20,7 +20,7 @@ def setup(): """Do required setup before doing anything""" gitutil.Setup() -def prepare_patches(col, branch, count, start, ignore_binary): +def prepare_patches(col, branch, count, start, end, ignore_binary): """Figure out what patches to generate, then generate them The patch files are written to the current directory, e.g. 0001_xxx.patch @@ -32,6 +32,8 @@ def prepare_patches(col, branch, count, start, ignore_binary): 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.) ignore_binary (bool): Don't generate patches for binary files Returns: @@ -50,7 +52,7 @@ def prepare_patches(col, branch, count, start, ignore_binary): 'No commits found to process - please use -c flag')) # Read the metadata from the commits - to_do = count + to_do = count - end series = patchstream.GetMetaData(branch, start, to_do) cover_fname, patch_files = gitutil.CreatePatches( branch, start, to_do, ignore_binary, series) @@ -159,7 +161,7 @@ def send(options): setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.branch, options.count, options.start, + col, options.branch, options.count, options.start, options.end, options.ignore_binary) ok = check_patches(series, patch_files, options.check_patch, options.verbose) diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 588be73ef4..810af9c604 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -430,7 +430,8 @@ complicated as possible''') col = terminal.Color() with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, branch=None, count=-1, start=0, ignore_binary=False) + col, branch=None, count=-1, start=0, end=0, + ignore_binary=False) self.assertIsNone(cover_fname) self.assertEqual(2, len(patch_files)) @@ -438,9 +439,17 @@ complicated as possible''') self.assertEqual(3, gitutil.CountCommitsToBranch('second')) with capture_sys_output() as _: _, cover_fname, patch_files = control.prepare_patches( - col, branch='second', count=-1, start=0, + col, branch='second', count=-1, start=0, end=0, ignore_binary=False) self.assertIsNotNone(cover_fname) self.assertEqual(3, len(patch_files)) + + # Check that it can skip patches at the end + with capture_sys_output() as _: + _, cover_fname, patch_files = control.prepare_patches( + col, branch='second', count=-1, start=0, end=1, + ignore_binary=False) + self.assertIsNotNone(cover_fname) + self.assertEqual(2, len(patch_files)) finally: os.chdir(orig_dir) diff --git a/tools/patman/main.py b/tools/patman/main.py index 066754196e..4d7a3044ea 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -35,6 +35,8 @@ parser.add_option('-b', '--branch', type='str', help="Branch to process (by default, the current branch)") parser.add_option('-c', '--count', dest='count', type='int', default=-1, help='Automatically create patches from top n commits') +parser.add_option('-e', '--end', type='int', default=0, + help='Commits to skip at end of patch list') parser.add_option('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') From patchwork Mon Jul 6 03:41:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240788 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:53 -0600 Subject: [RFC PATCH 06/16] patman: Convert to ArgumentParser In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-7-sjg@chromium.org> Convert from OptionParser to ArgumentParser to match binman. With this we can easily add sub-commands. Signed-off-by: Simon Glass --- tools/patman/control.py | 22 ++++----- tools/patman/main.py | 97 ++++++++++++++++++++-------------------- tools/patman/settings.py | 10 +++-- 3 files changed, 65 insertions(+), 64 deletions(-) diff --git a/tools/patman/control.py b/tools/patman/control.py index b481ff6b27..e67867b845 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -152,24 +152,24 @@ def email_patches(col, series, cover_fname, patch_files, process_tags, its_a_go, os.remove(cc_file) -def send(options): +def send(args): """Create, check and send patches by email Args: - options (optparse.Values): Arguments to patman + args (argparse.Namespace): Arguments to patman """ setup() col = terminal.Color() series, cover_fname, patch_files = prepare_patches( - col, options.branch, options.count, options.start, options.end, - options.ignore_binary) - ok = check_patches(series, patch_files, options.check_patch, - options.verbose) + col, args.branch, args.count, args.start, args.end, + args.ignore_binary) + ok = check_patches(series, patch_files, args.check_patch, + args.verbose) - its_a_go = ok or options.ignore_errors + its_a_go = ok or args.ignore_errors if its_a_go: email_patches( - col, series, cover_fname, patch_files, options.process_tags, - its_a_go, options.ignore_bad_tags, options.add_maintainers, - options.limit, options.dry_run, options.in_reply_to, options.thread, - options.smtp_server) + col, series, cover_fname, patch_files, args.process_tags, + its_a_go, args.ignore_bad_tags, args.add_maintainers, + args.limit, args.dry_run, args.in_reply_to, args.thread, + args.smtp_server) diff --git a/tools/patman/main.py b/tools/patman/main.py index 4d7a3044ea..34cad9a562 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -6,7 +6,7 @@ """See README for more information""" -from optparse import OptionParser +from argparse import ArgumentParser import os import re import sys @@ -27,71 +27,70 @@ from patman import terminal from patman import test_util from patman import test_checkpatch +epilog = '''Create patches from commits in a branch, check them and email them +as specified by tags you place in the commits. Use -n to do a dry run first.''' -parser = OptionParser() -parser.add_option('-H', '--full-help', action='store_true', dest='full_help', +parser = ArgumentParser(epilog=epilog) +parser.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -parser.add_option('-b', '--branch', type='str', +parser.add_argument('-b', '--branch', type=str, help="Branch to process (by default, the current branch)") -parser.add_option('-c', '--count', dest='count', type='int', +parser.add_argument('-c', '--count', dest='count', type=int, default=-1, help='Automatically create patches from top n commits') -parser.add_option('-e', '--end', type='int', default=0, +parser.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') -parser.add_option('-i', '--ignore-errors', action='store_true', +parser.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') -parser.add_option('-l', '--limit-cc', dest='limit', type='int', - default=None, help='Limit the cc list to LIMIT entries [default: %default]') -parser.add_option('-m', '--no-maintainers', action='store_false', +parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, + help='Limit the cc list to LIMIT entries [default: %(default)]') +parser.add_argument('-m', '--no-maintainers', action='store_false', dest='add_maintainers', default=True, help="Don't cc the file maintainers automatically") -parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', +parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', default=False, help="Do a dry run (create but don't email patches)") -parser.add_option('-p', '--project', default=project.DetectProject(), - help="Project name; affects default option values and " - "aliases [default: %default]") -parser.add_option('-r', '--in-reply-to', type='string', action='store', +parser.add_argument('-p', '--project', default=project.DetectProject(), + help="Project name; affects default option values and " + "aliases [default: %(default)]") +parser.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -parser.add_option('-s', '--start', dest='start', type='int', +parser.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') -parser.add_option('-t', '--ignore-bad-tags', action='store_true', - default=False, help='Ignore bad tags / aliases') -parser.add_option('-v', '--verbose', action='store_true', dest='verbose', +parser.add_argument('-t', '--ignore-bad-tags', action='store_true', + default=False, help='Ignore bad tags / aliases') +parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', default=False, help='Verbose output of errors and warnings') -parser.add_option('-T', '--thread', action='store_true', dest='thread', - default=False, help='Create patches as a single thread') -parser.add_option('--cc-cmd', dest='cc_cmd', type='string', action='store', +parser.add_argument('-T', '--thread', action='store_true', dest='thread', + default=False, help='Create patches as a single thread') +parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', default=None, help='Output cc list for patch file (used by git)') -parser.add_option('--no-binary', action='store_true', dest='ignore_binary', - default=False, - help="Do not output contents of changes in binary files") -parser.add_option('--no-check', action='store_false', dest='check_patch', - default=True, - help="Don't check for patch compliance") -parser.add_option('--no-tags', action='store_false', dest='process_tags', - default=True, help="Don't process subject tags as aliases") -parser.add_option('--smtp-server', type='str', - help="Specify the SMTP server to 'git send-email'") -parser.add_option('--test', action='store_true', dest='test', - default=False, help='run tests') - -parser.usage += """ - -Create patches from commits in a branch, check them and email them as -specified by tags you place in the commits. Use -n to do a dry run first.""" - +parser.add_argument('--no-binary', action='store_true', dest='ignore_binary', + default=False, + help="Do not output contents of changes in binary files") +parser.add_argument('--no-check', action='store_false', dest='check_patch', + default=True, + help="Don't check for patch compliance") +parser.add_argument('--no-tags', action='store_false', dest='process_tags', + default=True, help="Don't process subject tags as aliases") +parser.add_argument('--smtp-server', type=str, + help="Specify the SMTP server to 'git send-email'") +parser.add_argument('--test', action='store_true', dest='test', + default=False, help='run tests') + +parser.add_argument('patchfiles', nargs='*') # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). -(options, args) = parser.parse_args() -settings.Setup(gitutil, parser, options.project, '') -(options, args) = parser.parse_args() +argv = sys.argv[1:] +args = parser.parse_args(argv) +settings.Setup(gitutil, parser, args.project, '') +args = parser.parse_args(argv) if __name__ != "__main__": pass # Run our meagre tests -elif options.test: +elif args.test: import doctest from patman import func_test @@ -109,19 +108,19 @@ elif options.test: # Called from git with a patch filename as argument # Printout a list of additional CC recipients for this patch -elif options.cc_cmd: - fd = open(options.cc_cmd, 'r') +elif args.cc_cmd: + fd = open(args.cc_cmd, 'r') re_line = re.compile('(\S*) (.*)') for line in fd.readlines(): match = re_line.match(line) - if match and match.group(1) == args[0]: + if match and match.group(1) == args.patchfiles[0]: for cc in match.group(2).split('\0'): cc = cc.strip() if cc: print(cc) fd.close() -elif options.full_help: +elif args.full_help: pager = os.getenv('PAGER') if not pager: pager = 'more' @@ -131,4 +130,4 @@ elif options.full_help: # Process commits, produce patches files, check them, email them else: - control.send(options) + control.send(args) diff --git a/tools/patman/settings.py b/tools/patman/settings.py index 635561ac05..732bd40106 100644 --- a/tools/patman/settings.py +++ b/tools/patman/settings.py @@ -233,17 +233,19 @@ def _UpdateDefaults(parser, config): config: An instance of _ProjectConfigParser that we will query for settings. """ - defaults = parser.get_default_values() + defaults = parser.parse_known_args()[0] + defaults = vars(defaults) for name, val in config.items('settings'): - if hasattr(defaults, name): - default_val = getattr(defaults, name) + if name in defaults: + default_val = defaults[name] if isinstance(default_val, bool): val = config.getboolean('settings', name) elif isinstance(default_val, int): val = config.getint('settings', name) - parser.set_default(name, val) + defaults[name] = val else: print("WARNING: Unknown setting %s" % name) + parser.set_defaults(**defaults) def _ReadAliasFile(fname): """Read in the U-Boot git alias file if it exists. From patchwork Mon Jul 6 03:41:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240789 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:54 -0600 Subject: [RFC PATCH 07/16] patman: Allow different commands In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-8-sjg@chromium.org> At present patman only does one thing so does not have any comments. We want to add a few more command, so create a sub-parser for the default command ('send'). Signed-off-by: Simon Glass --- tools/patman/main.py | 77 +++++++++++++++++++++++--------------------- 1 file changed, 41 insertions(+), 36 deletions(-) diff --git a/tools/patman/main.py b/tools/patman/main.py index 34cad9a562..fee9bc848b 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -31,60 +31,65 @@ epilog = '''Create patches from commits in a branch, check them and email them as specified by tags you place in the commits. Use -n to do a dry run first.''' parser = ArgumentParser(epilog=epilog) -parser.add_argument('-H', '--full-help', action='store_true', dest='full_help', +subparsers = parser.add_subparsers(dest='cmd') +send = subparsers.add_parser('send') +send.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -parser.add_argument('-b', '--branch', type=str, +send.add_argument('-b', '--branch', type=str, help="Branch to process (by default, the current branch)") -parser.add_argument('-c', '--count', dest='count', type=int, +send.add_argument('-c', '--count', dest='count', type=int, default=-1, help='Automatically create patches from top n commits') -parser.add_argument('-e', '--end', type=int, default=0, +send.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') -parser.add_argument('-i', '--ignore-errors', action='store_true', +send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') -parser.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, - help='Limit the cc list to LIMIT entries [default: %(default)]') -parser.add_argument('-m', '--no-maintainers', action='store_false', +send.add_argument('-l', '--limit-cc', dest='limit', type=int, default=None, + help='Limit the cc list to LIMIT entries [default: %(default)s]') +send.add_argument('-m', '--no-maintainers', action='store_false', dest='add_maintainers', default=True, help="Don't cc the file maintainers automatically") -parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', +send.add_argument('-n', '--dry-run', action='store_true', dest='dry_run', default=False, help="Do a dry run (create but don't email patches)") -parser.add_argument('-p', '--project', default=project.DetectProject(), - help="Project name; affects default option values and " - "aliases [default: %(default)]") -parser.add_argument('-r', '--in-reply-to', type=str, action='store', +send.add_argument('-p', '--project', default=project.DetectProject(), + help="Project name; affects default option values and " + "aliases [default: %(default)s]") +send.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -parser.add_argument('-s', '--start', dest='start', type=int, +send.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') -parser.add_argument('-t', '--ignore-bad-tags', action='store_true', - default=False, help='Ignore bad tags / aliases') -parser.add_argument('-v', '--verbose', action='store_true', dest='verbose', +send.add_argument('-t', '--ignore-bad-tags', action='store_true', + default=False, help='Ignore bad tags / aliases') +send.add_argument('-v', '--verbose', action='store_true', dest='verbose', default=False, help='Verbose output of errors and warnings') -parser.add_argument('-T', '--thread', action='store_true', dest='thread', - default=False, help='Create patches as a single thread') -parser.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', +send.add_argument('-T', '--thread', action='store_true', dest='thread', + default=False, help='Create patches as a single thread') +send.add_argument('--cc-cmd', dest='cc_cmd', type=str, action='store', default=None, help='Output cc list for patch file (used by git)') -parser.add_argument('--no-binary', action='store_true', dest='ignore_binary', - default=False, - help="Do not output contents of changes in binary files") -parser.add_argument('--no-check', action='store_false', dest='check_patch', - default=True, - help="Don't check for patch compliance") -parser.add_argument('--no-tags', action='store_false', dest='process_tags', - default=True, help="Don't process subject tags as aliases") -parser.add_argument('--smtp-server', type=str, - help="Specify the SMTP server to 'git send-email'") -parser.add_argument('--test', action='store_true', dest='test', - default=False, help='run tests') - -parser.add_argument('patchfiles', nargs='*') +send.add_argument('--no-binary', action='store_true', dest='ignore_binary', + default=False, + help="Do not output contents of changes in binary files") +send.add_argument('--no-check', action='store_false', dest='check_patch', + default=True, + help="Don't check for patch compliance") +send.add_argument('--no-tags', action='store_false', dest='process_tags', + default=True, help="Don't process subject tags as aliases") +send.add_argument('--smtp-server', type=str, + help="Specify the SMTP server to 'git send-email'") +send.add_argument('--test', action='store_true', dest='test', + default=False, help='run tests') + +send.add_argument('patchfiles', nargs='*') # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). argv = sys.argv[1:] +if len(argv) < 1 or argv[0].startswith('-'): + argv = ['send'] + argv args = parser.parse_args(argv) -settings.Setup(gitutil, parser, args.project, '') -args = parser.parse_args(argv) +if hasattr(args, 'project'): + settings.Setup(gitutil, send, args.project, '') + args = parser.parse_args(argv) if __name__ != "__main__": pass From patchwork Mon Jul 6 03:41:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240790 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:55 -0600 Subject: [RFC PATCH 08/16] patman: Add a 'test' subcommand In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-9-sjg@chromium.org> At present we use --test to indicate that tests should be run. It is better to use a subcommand for list, like binman. Change it and adjust the existing code to fit under a 'send' subcommand, the default. Give this subcommand the same default arguments as the others. Signed-off-by: Simon Glass --- .azure-pipelines.yml | 2 +- .gitlab-ci.yml | 2 +- .travis.yml | 2 +- test/run | 2 +- tools/patman/main.py | 75 +++++++++++++++++++++------------------ tools/patman/test_util.py | 2 +- 6 files changed, 45 insertions(+), 40 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 9f88a539c0..45f0fabd6d 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -158,7 +158,7 @@ jobs: ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test ./tools/buildman/buildman -t ./tools/dtoc/dtoc -t - ./tools/patman/patman --test + ./tools/patman/patman test make O=${UBOOT_TRAVIS_BUILD_DIR} testconfig EOF cat build.sh diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index a685a7879d..c3d8ef6543 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -167,7 +167,7 @@ Run binman, buildman, dtoc, Kconfig and patman testsuites: ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test; ./tools/buildman/buildman -t; ./tools/dtoc/dtoc -t; - ./tools/patman/patman --test; + ./tools/patman/patman test; make testconfig Run tests for Nokia RX-51 (aka N900): diff --git a/.travis.yml b/.travis.yml index a042aa2c7d..4ce760d938 100644 --- a/.travis.yml +++ b/.travis.yml @@ -257,7 +257,7 @@ script: export PYTHONPATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc/pylibfdt"; export PATH="${UBOOT_TRAVIS_BUILD_DIR}/scripts/dtc:${PATH}"; ./tools/binman/binman --toolpath ${UBOOT_TRAVIS_BUILD_DIR}/tools test && - ./tools/patman/patman --test && + ./tools/patman/patman test && ./tools/buildman/buildman -t && ./tools/dtoc/dtoc -t && make testconfig; diff --git a/test/run b/test/run index 27331a8e40..de87e7530b 100755 --- a/test/run +++ b/test/run @@ -48,7 +48,7 @@ export DTC=${DTC_DIR}/dtc TOOLS_DIR=build-sandbox_spl/tools run_test "binman" ./tools/binman/binman --toolpath ${TOOLS_DIR} test -run_test "patman" ./tools/patman/patman --test +run_test "patman" ./tools/patman/patman test run_test "buildman" ./tools/buildman/buildman -t ${skip} run_test "fdt" ./tools/dtoc/test_fdt -t diff --git a/tools/patman/main.py b/tools/patman/main.py index fee9bc848b..77f187e769 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -27,6 +27,16 @@ from patman import terminal from patman import test_util from patman import test_checkpatch +def AddCommonArgs(parser): + parser.add_argument('-b', '--branch', type=str, + help="Branch to process (by default, the current branch)") + parser.add_argument('-c', '--count', dest='count', type=int, + default=-1, help='Automatically create patches from top n commits') + parser.add_argument('-e', '--end', type=int, default=0, + help='Commits to skip at end of patch list') + parser.add_argument('-s', '--start', dest='start', type=int, + default=0, help='Commit to start creating patches from (0 = HEAD)') + epilog = '''Create patches from commits in a branch, check them and email them as specified by tags you place in the commits. Use -n to do a dry run first.''' @@ -35,12 +45,6 @@ subparsers = parser.add_subparsers(dest='cmd') send = subparsers.add_parser('send') send.add_argument('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') -send.add_argument('-b', '--branch', type=str, - help="Branch to process (by default, the current branch)") -send.add_argument('-c', '--count', dest='count', type=int, - default=-1, help='Automatically create patches from top n commits') -send.add_argument('-e', '--end', type=int, default=0, - help='Commits to skip at end of patch list') send.add_argument('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') @@ -56,8 +60,6 @@ send.add_argument('-p', '--project', default=project.DetectProject(), "aliases [default: %(default)s]") send.add_argument('-r', '--in-reply-to', type=str, action='store', help="Message ID that this series is in reply to") -send.add_argument('-s', '--start', dest='start', type=int, - default=0, help='Commit to start creating patches from (0 = HEAD)') send.add_argument('-t', '--ignore-bad-tags', action='store_true', default=False, help='Ignore bad tags / aliases') send.add_argument('-v', '--verbose', action='store_true', dest='verbose', @@ -76,11 +78,13 @@ send.add_argument('--no-tags', action='store_false', dest='process_tags', default=True, help="Don't process subject tags as aliases") send.add_argument('--smtp-server', type=str, help="Specify the SMTP server to 'git send-email'") -send.add_argument('--test', action='store_true', dest='test', - default=False, help='run tests') +AddCommonArgs(send) send.add_argument('patchfiles', nargs='*') +test_parser = subparsers.add_parser('test', help='Run tests') +AddCommonArgs(test_parser) + # Parse options twice: first to get the project and second to handle # defaults properly (which depends on project). argv = sys.argv[1:] @@ -95,7 +99,7 @@ if __name__ != "__main__": pass # Run our meagre tests -elif args.test: +if args.cmd == 'test': import doctest from patman import func_test @@ -111,28 +115,29 @@ elif args.test: sys.exit(test_util.ReportResult('patman', None, result)) -# Called from git with a patch filename as argument -# Printout a list of additional CC recipients for this patch -elif args.cc_cmd: - fd = open(args.cc_cmd, 'r') - re_line = re.compile('(\S*) (.*)') - for line in fd.readlines(): - match = re_line.match(line) - if match and match.group(1) == args.patchfiles[0]: - for cc in match.group(2).split('\0'): - cc = cc.strip() - if cc: - print(cc) - fd.close() - -elif args.full_help: - pager = os.getenv('PAGER') - if not pager: - pager = 'more' - fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), - 'README') - command.Run(pager, fname) - # Process commits, produce patches files, check them, email them -else: - control.send(args) +elif args.cmd == 'send': + # Called from git with a patch filename as argument + # Printout a list of additional CC recipients for this patch + if args.cc_cmd: + fd = open(args.cc_cmd, 'r') + re_line = re.compile('(\S*) (.*)') + for line in fd.readlines(): + match = re_line.match(line) + if match and match.group(1) == args.patchfiles[0]: + for cc in match.group(2).split('\0'): + cc = cc.strip() + if cc: + print(cc) + fd.close() + + elif args.full_help: + pager = os.getenv('PAGER') + if not pager: + pager = 'more' + fname = os.path.join(os.path.dirname(os.path.realpath(sys.argv[0])), + 'README') + command.Run(pager, fname) + + else: + control.send(args) diff --git a/tools/patman/test_util.py b/tools/patman/test_util.py index 0827488f33..a87d3cc8f3 100644 --- a/tools/patman/test_util.py +++ b/tools/patman/test_util.py @@ -47,7 +47,7 @@ def RunTestCoverage(prog, filter_fname, exclude_list, build_dir, required=None): glob_list = [] glob_list += exclude_list glob_list += ['*libfdt.py', '*site-packages*', '*dist-packages*'] - test_cmd = 'test' if 'binman' in prog else '-t' + test_cmd = 'test' if 'binman' in prog or 'patman' in prog else '-t' prefix = '' if build_dir: prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir From patchwork Mon Jul 6 03:41:56 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240791 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:56 -0600 Subject: [RFC PATCH 09/16] patman: Allow disabling 'bright' mode with Print output In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-10-sjg@chromium.org> At present all text is marked bright, which makes it stand out on the terminal. Add a way to disable that, as is done with the Color class. Signed-off-by: Simon Glass --- tools/patman/terminal.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index c709438bdc..60dbce3ce1 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -122,7 +122,7 @@ def TrimAsciiLen(text, size): return out -def Print(text='', newline=True, colour=None, limit_to_line=False): +def Print(text='', newline=True, colour=None, limit_to_line=False, bright=True): """Handle a line of output to the terminal. In test mode this is recorded in a list. Otherwise it is output to the @@ -140,7 +140,7 @@ def Print(text='', newline=True, colour=None, limit_to_line=False): else: if colour: col = Color() - text = col.Color(colour, text) + text = col.Color(colour, text, bright=bright) if newline: print(text) last_print_len = None From patchwork Mon Jul 6 03:41:57 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240792 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:57 -0600 Subject: [RFC PATCH 10/16] patman: Support collecting response tags in Patchstream In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-11-sjg@chromium.org> Collect response tags such as 'Reviewed-by' while parsing the stream. This allows us to see what tags are present. Add a new 'Fixes' tag also, since this is now quite common. Signed-off-by: Simon Glass --- tools/patman/commit.py | 14 ++++++++++++++ tools/patman/patchstream.py | 21 ++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/tools/patman/commit.py b/tools/patman/commit.py index 48d0529c53..8d583c4ed3 100644 --- a/tools/patman/commit.py +++ b/tools/patman/commit.py @@ -2,6 +2,7 @@ # Copyright (c) 2011 The Chromium OS Authors. # +import collections import re # Separates a tag: at the beginning of the subject from the rest of it @@ -23,6 +24,9 @@ class Commit: notes: List of lines in the commit (not series) notes change_id: the Change-Id: tag that was stripped from this commit and can be used to generate the Message-Id. + rtags: Response tags (e.g. Reviewed-by) collected by the commit, dict: + key: rtag type (e.g. 'Reviewed-by') + value: Set of people who gave that rtag, each a name/email string """ def __init__(self, hash): self.hash = hash @@ -33,6 +37,7 @@ class Commit: self.signoff_set = set() self.notes = [] self.change_id = None + self.rtags = collections.defaultdict(set) def AddChange(self, version, info): """Add a new change line to the change list for a version. @@ -88,3 +93,12 @@ class Commit: return False self.signoff_set.add(signoff) return True + + def AddRtag(self, rtag_type, who): + """Add a response tag to a commit + + Args: + key: rtag type (e.g. 'Reviewed-by') + who: Person who gave that rtag, e.g. 'Fred Bloggs ' + """ + self.rtags[rtag_type].add(who) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 2ea8ebcc3f..0c68c86156 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -37,7 +37,7 @@ re_change_id = re.compile('^Change-Id: *(.*)') re_commit_tag = re.compile('^Commit-([a-z-]*): *(.*)') # Commit tags that we want to collect and keep -re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc): (.*)') +re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Patch-cc|Fixes): (.*)') # The start of a new commit in the git log re_commit = re.compile('^commit ([0-9a-f]*)$') @@ -112,6 +112,15 @@ class PatchStream: self.in_section = 'commit-' + name self.skip_blank = False + def AddCommitRtag(self, rtag_type, who): + """Add a response tag to the current commit + + Args: + key: rtag type (e.g. 'Reviewed-by') + who: Person who gave that rtag, e.g. 'Fred Bloggs ' + """ + self.commit.AddRtag(rtag_type, who) + def CloseCommit(self): """Save the current commit into our commit list, and reset our state""" if self.commit and self.is_log: @@ -346,12 +355,14 @@ class PatchStream: # Detect tags in the commit message elif tag_match: + rtag_type, who = tag_match.groups() + self.AddCommitRtag(rtag_type, who) # Remove Tested-by self, since few will take much notice - if (tag_match.group(1) == 'Tested-by' and - tag_match.group(2).find(os.getenv('USER') + '@') != -1): + if (rtag_type == 'Tested-by' and + who.find(os.getenv('USER') + '@') != -1): self.warn.append("Ignoring %s" % line) - elif tag_match.group(1) == 'Patch-cc': - self.commit.AddCc(tag_match.group(2).split(',')) + elif rtag_type == 'Patch-cc': + self.commit.AddCc(who.split(',')) else: out = [line] From patchwork Mon Jul 6 03:41:58 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240793 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:58 -0600 Subject: [RFC PATCH 11/16] patman: Allow linking a series with patchwork In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-12-sjg@chromium.org> Add a new Series-link tag to tell patman how to find the series in patchwork. Signed-off-by: Simon Glass --- tools/patman/README | 8 ++++++++ tools/patman/func_test.py | 1 + tools/patman/series.py | 2 +- 3 files changed, 10 insertions(+), 1 deletion(-) diff --git a/tools/patman/README b/tools/patman/README index 52b2cf70bd..7291e47d0c 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -187,6 +187,14 @@ Series-name: name patman does not yet use it, but it is convenient to put the branch name here to help you keep track of multiple upstreaming efforts. +Series-link: url + Set the URL of the series in patchwork. You can set this after you send + out the series and look in patchwork for the resulting series. The + URL you want is the one for the series itself, not any particular patch. + E.g. http://patchwork.ozlabs.org/project/uboot/list/?series=187331 + When this is set, patman can compare your local branch against patchwork + to see what new reviews your series has collected. + Cover-letter: This is the patch set title blah blah diff --git a/tools/patman/func_test.py b/tools/patman/func_test.py index 810af9c604..5eb777054a 100644 --- a/tools/patman/func_test.py +++ b/tools/patman/func_test.py @@ -392,6 +392,7 @@ Series for my board This series implements support for my glorious board. END +Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=183237 ''', 'serial.c', '''The code for the serial driver is here''') self.make_commit_with_file('bootm: Make it boot', ''' diff --git a/tools/patman/series.py b/tools/patman/series.py index 9f885c8987..edb1141fa8 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -16,7 +16,7 @@ from patman import tools # Series-xxx tags that we understand valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name', - 'cover_cc', 'process_log'] + 'cover_cc', 'process_log', 'link'] class Series(dict): """Holds information about a patch series, including all tags. From patchwork Mon Jul 6 03:41:59 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240794 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:41:59 -0600 Subject: [RFC PATCH 12/16] patman: Add a -D option to enable debugging In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-13-sjg@chromium.org> Most users don't want to see traceback errors. Add an option to enable them for debugging. Disable them by default. Signed-off-by: Simon Glass --- tools/patman/main.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/patman/main.py b/tools/patman/main.py index 77f187e769..b96000807e 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -10,6 +10,7 @@ from argparse import ArgumentParser import os import re import sys +import traceback import unittest if __name__ == "__main__": @@ -34,6 +35,8 @@ def AddCommonArgs(parser): default=-1, help='Automatically create patches from top n commits') parser.add_argument('-e', '--end', type=int, default=0, help='Commits to skip at end of patch list') + parser.add_argument('-D', '--debug', action='store_true', + help='Enabling debugging (provides a full traceback on error)') parser.add_argument('-s', '--start', dest='start', type=int, default=0, help='Commit to start creating patches from (0 = HEAD)') @@ -98,6 +101,9 @@ if hasattr(args, 'project'): if __name__ != "__main__": pass +if not args.debug: + sys.tracebacklimit = 0 + # Run our meagre tests if args.cmd == 'test': import doctest From patchwork Mon Jul 6 03:42:00 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240795 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:42:00 -0600 Subject: [RFC PATCH 13/16] patchstream: Support parsing of review snippets In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-14-sjg@chromium.org> Add support for parsing the contents of a patchwork 'patch' web page containing comments received from reviewers. This allows patman to show these comments in a simple 'snippets' format. A snippet is some quoted code plus some unquoted comments below it. Each review is from a unique person/email and can produce multiple snippets, one for each part of the code that attracts a comment. Signed-off-by: Simon Glass --- tools/patman/patchstream.py | 53 +++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index 0c68c86156..5e2b8e3986 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -2,10 +2,12 @@ # Copyright (c) 2011 The Chromium OS Authors. # +import collections import datetime import math import os import re +import queue import shutil import tempfile @@ -80,6 +82,10 @@ class PatchStream: self.state = STATE_MSG_HEADER # What state are we in? self.signoff = [] # Contents of signoff line self.commit = None # Current commit + self.snippets = [] # List of unquoted test blocks + self.recent_quoted = collections.deque([], 5) + self.recent_unquoted = queue.Queue() + self.was_quoted = None def AddToSeries(self, line, name, value): """Add a new Series-xxx tag. @@ -165,6 +171,40 @@ class PatchStream: self.commit.AddChange(self.change_version, change) self.change_lines = [] + def FinaliseSnippet(self): + """Finish off a snippet and add it to the list + + This is called when we get to the end of a snippet, i.e. the we enter + the next block of quoted text: + + This is a comment from someone. + + Something else + + > Now we have some code <----- end of snippet + > more code + + Now a comment about the above code + + This adds the snippet to our list + """ + quoted_lines = [] + while len(self.recent_quoted): + quoted_lines.append(self.recent_quoted.popleft()) + unquoted_lines = [] + valid = False + while not self.recent_unquoted.empty(): + text = self.recent_unquoted.get() + unquoted_lines.append(text) + if text: + valid = True + if valid: + lines = quoted_lines + unquoted_lines + if lines[0].startswith('On ') and lines[0].endswith('wrote:'): + lines = lines[1:] + if lines: + self.snippets.append(lines) + def ProcessLine(self, line): """Process a single line of a patch file or commit log @@ -384,6 +424,18 @@ class PatchStream: out = [line] self.linenum += 1 self.skip_blank = False + + # If this is quoted, keep recent lines + if self.linenum > 1 and line: + if line.startswith('>'): + if not self.was_quoted: + self.FinaliseSnippet() + self.recent_quoted.append(line) + self.was_quoted = True + else: + self.recent_unquoted.put(line) + self.was_quoted = False + if self.state == STATE_DIFFS: pass @@ -407,6 +459,7 @@ class PatchStream: def Finalize(self): """Close out processing of this patch stream""" + self.FinaliseSnippet() self.FinalizeChange() self.CloseCommit() if self.lines_after_test: From patchwork Mon Jul 6 03:42:01 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240796 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:42:01 -0600 Subject: [RFC PATCH 14/16] patman: Support checking for review tags in patchwork In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-15-sjg@chromium.org> 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. Signed-off-by: Simon Glass --- tools/patman/README | 34 ++++ tools/patman/control.py | 31 ++++ tools/patman/main.py | 17 ++ tools/patman/status.py | 334 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 416 insertions(+) create mode 100644 tools/patman/status.py diff --git a/tools/patman/README b/tools/patman/README index 7291e47d0c..a85974740f 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -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. @@ -345,6 +347,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 + + Reviewed-by: Bin Meng + 22 x86: mtrr: Restructure so command execution is in + Reviewed-by: Wolfgang Wallner + + Reviewed-by: Bin Meng +... + +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 ================= diff --git a/tools/patman/control.py b/tools/patman/control.py index e67867b845..3bb3c236e4 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -173,3 +173,34 @@ 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.GetMetaData(branch, start, count - end) + link = series.get('link') + if not link: + raise ValueError("Branch has no Series-link value") + + # Import this here to avoid failing on other commands if the dependencies + # are not present + from patman import status + status.check_patchwork_status(series, link, branch) diff --git a/tools/patman/main.py b/tools/patman/main.py index b96000807e..227eb3b228 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -88,6 +88,10 @@ send.add_argument('patchfiles', nargs='*') test_parser = subparsers.add_parser('test', help='Run tests') 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:] @@ -147,3 +151,16 @@ 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: + print('patman: %s: %s' % (type(e).__name__, e)) + if args.debug: + print() + traceback.print_exc() + ret_code = 1 + sys.exit(ret_code) diff --git a/tools/patman/status.py b/tools/patman/status.py new file mode 100644 index 0000000000..c2e98a5d11 --- /dev/null +++ b/tools/patman/status.py @@ -0,0 +1,334 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2020 Google LLC +# +# This talks to the patchwork service to figure out what patches have been +# reviewed and commented on. + +from bs4 import BeautifulSoup +import collections +import concurrent.futures +import io +import pygit2 +from itertools import repeat +import re +import requests +import time +from urllib.parse import urljoin + +from patman import commit +from patman import gitutil +from patman import patchstream +from patman import terminal +from patman.terminal import Color + +# 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('(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$') + +# This decodes the sequence string into a patch number and patch count +re_seq = re.compile('(\d+)/(\d+)') + +def to_int(vals): + """Convert a lsit of strings into integers, using 0 if not an integer + + Args: + vals: List of strings + + Returns: + 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: + seq: Sequence number within series (1=first) parsed from sequence string + count: Number of patches in series, parsed from sequence string + prefix: Prefix string or None + subject: Patch subject with [..] part removed (same as commit subject) + series: Series name + acked: Number of Acked-by rtags + fixes: Number of Fixes rtags + reviewed: Number of Reviewed-by rtags + tested: Number of Tested-by rtags + success: Success value + warning: Warning value + fail: Failure value + date: Date as a string + Submitter: Submitter name + Delegate: Deleted ID + state: Current state (e.g. 'New') + url: URL pointing to the patch on patchwork + """ + def __init__(self): + self.seq = None + self.count = None + self.prefix = None + self.subject = None + self.series = None + self.acked = None + self.fixes = None + self.reviewed = None + self.tested = None + self.success = None + self.warning = None + self.fail = None + self.date = None + self.submitter = None + self.delegate = None + self.state = None + self.url = None + + # These make us more like a dictionary + def __setattr__(self, name, value): + self[name] = value + + def __getattr__(self, name): + return self[name] + + def Add(self, name, value): + """Add information obtained from the web page + + This sets the object properties based on @name. + + Args: + name: Column name in lower-case, e.g. 'Series' or 'State' + value: Text obtained for this patch from that column, e.g. 'New' + """ + if name in self: + self[name] = value + elif name == 'patch': + mat = re_patch.search(value.strip()) + if not mat: + raise ValueError("Cannot parse subject '%s'" % value) + self.prefix = mat.group(1) + self.prefix, self.version, seq_info, self.subject = mat.groups()[3:] + mat_seq = re_seq.match(seq_info) + if not mat_seq: + raise ValueError("Cannot parse sequence spec '%s'" % seq_info) + self.seq = int(mat_seq.group(1)) + self.count = int(mat_seq.group(2)) + elif name == 'a/f/r/t': + self.acked, self.fixes, self.reviewed, self.tested = to_int( + value.split()) + elif name == 's/w/f': + self.success, self.warning, self.fail = to_int(value) + else: + print('Unknown field: %s\n' % name) + + def SetUrl(self, url): + """Sets the URL for a patch + + Args: + url: URL to set + """ + self.url = url + + +def FindResponses(url): + """Collect the response tags (rtags) to a patch found in patchwork + + Reads the URL and checks each comment for an rtags (e.g. Reviewed-by). + This does not look at the original patch (which have have rtags picked + up from previous rounds of reviews). It only looks at comments which add + new rtags. + + Args: + url: URL of the patch + + Returns: + Dict of rtags: + key: rtag type, e.g. 'Acked-by' + value: Set of people providing that rtag. Each member of the set + is a name/email, e.g. 'Bin Meng ' + """ + web_data = requests.get(url) + if web_data.status_code != 200: + raise ValueError("Could not read URL '%s'" % url) + soup = BeautifulSoup(web_data.text, 'html.parser') + + # The first comment is the patch itself, so skip that + comments = soup.findAll(class_='comment')[1:] + rtags = collections.defaultdict(set) + reviews = [] + + if comments: + for comment in comments: + ps = patchstream.PatchStream(None) + ps.commit = commit.Commit(None) + meta = comment.find(class_='meta') + content = comment.find(class_='content') + infd = io.StringIO(content.get_text()) + outfd = io.StringIO() + ps.ProcessStream(infd, outfd) + for response, people in ps.commit.rtags.items(): + rtags[response].update(people) + return rtags + +def CollectPatches(series, url): + """Collect patch information about a series from patchwork + + Reads the web page and collects information provided by patchwork about + the status of each patch. + + Args: + series: Series object corresponding to the local branch containing the + series + url: URL pointing to patchwork's view of this series, possibly an + earlier versions of it + + Returns: + List of patches sorted by sequence number, each a Patch object + """ + response = requests.get(url) + if response.status_code != 200: + raise ValueError("Could not read URL '%s'" % url) + soup = BeautifulSoup(response.text, 'html.parser') + + # Find the main table + tables = soup.findAll('table') + if len(tables) != 1: + raise ValueError('Failed to parse page (expected only one table)') + tab = tables[0] + + # Get all the rows, which are patches + rows = tab.tbody.findAll('tr') + count = len(rows) + num_commits = len(series.commits) + if count != num_commits: + print('Warning: Web page reports %d patches, series has %d' % + (count, num_commits)) + + # Get the column headers, which we use to figure out which column is which + hdrs = tab.thead.findAll('th') + cols = [] + for hdr in hdrs: + cols.append(hdr.get_text().strip()) + patches = [] + + # Work through each row (patch) one at a time, collecting the information + for row in rows: + patch = Patch() + for seq, col in enumerate(row.findAll('td')): + name = cols[seq].lower() + patch.Add(name, col.get_text()) + if name == 'patch': + patch.SetUrl(urljoin(url, col.a['href'])) + if patch.count != count: + raise ValueError("Patch %d '%s' has count of %d, expected %d" % + (patch.seq, patch.subject, patch.count, num_commits)) + patches.append(patch) + + # Sort patches by patch number + patches = sorted(patches, key=lambda x: x.seq) + return patches + +def FindNewResponses(new_rtag_list, seq, commit, patch): + """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: 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: Position in new_rtag_list to update + commit: Commit object for this commit + patch: Corresponding Patch object for this patch + """ + acked = commit.rtags['Acked-by'] + reviewed = commit.rtags['Reviewed-by'] + tested = commit.rtags['Tested-by'] + fixes = commit.rtags['Fixes'] + + # Figure out the number of new rtags by subtracting what we have in + # the commit from what patchwork's summary reports. + extra = 0 + extra += patch.acked - len(acked) + extra += patch.fixes - len(fixes) + extra += patch.reviewed - len(reviewed) + extra += patch.tested - len(tested) + + # If patchwork agrees with the commit, we don't need to read the web + # page, so save some time. If not, we need to add the new ones to the commit + new_rtags = collections.defaultdict(set) + if extra: + pw_rtags = FindResponses(patch.url) + base_rtags = commit.rtags + new_count = 0 + for tag, people in pw_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_count += 1 + new_rtag_list[seq] = new_rtags + +def ShowResponses(rtags, indent, is_new): + """Show rtags collected + + Args: + rtags: dict: + key: Response tag (e.g. 'Reviewed-by') + value: Set of people who gave that response, each a name/email string + indent: Indentation string to write before each line + is_new: True if this output should be highlighted + """ + 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, url, branch): + patches = CollectPatches(series, url) + col = terminal.Color() + count = len(patches) + new_rtag_list = [None] * count + + with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor: + futures = executor.map( + FindNewResponses, repeat(new_rtag_list), range(count), + series.commits, patches) + for fresponse in futures: + if fresponse: + raise fresponse.exception() + + num_to_add = 0 + for seq, patch in enumerate(patches): + terminal.Print('%3d %s' % (patch.seq, patch.subject[:50]), + colour=col.BLUE) + commit = series.commits[seq] + base_rtags = commit.rtags + new_rtags = new_rtag_list[seq] + + indent = ' ' * 2 + ShowResponses(base_rtags, indent, False) + num_to_add += ShowResponses(new_rtags, indent, True) + + print("%d new response%s available in patchwork" % + (num_to_add, 's' if num_to_add != 1 else '')) From patchwork Mon Jul 6 03:42:02 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240797 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:42:02 -0600 Subject: [RFC PATCH 15/16] patman: Support updating a branch with review tags In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-16-sjg@chromium.org> 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 --- tools/patman/README | 17 +++++++++++-- tools/patman/control.py | 9 ++++--- tools/patman/main.py | 7 +++++- tools/patman/status.py | 54 ++++++++++++++++++++++++++++++++++++++--- 4 files changed, 78 insertions(+), 9 deletions(-) diff --git a/tools/patman/README b/tools/patman/README index a85974740f..595a0d616b 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 @@ -378,6 +380,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 3bb3c236e4..b6ba0a56c0 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -174,11 +174,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) @@ -187,6 +187,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 @@ -203,4 +206,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, link, branch) + status.check_patchwork_status(series, link, branch, dest_branch, force) diff --git a/tools/patman/main.py b/tools/patman/main.py index 227eb3b228..c8fc035c10 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -90,6 +90,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 @@ -156,7 +160,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: print('patman: %s: %s' % (type(e).__name__, e)) if args.debug: diff --git a/tools/patman/status.py b/tools/patman/status.py index c2e98a5d11..71124d6e5e 100644 --- a/tools/patman/status.py +++ b/tools/patman/status.py @@ -304,7 +304,48 @@ def ShowResponses(rtags, indent, is_new): count += 1 return count -def check_patchwork_status(series, url, branch): +def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite): + if branch == dest_branch: + raise ValueError( + 'Destination branch must not be the same as the original branch') + repo = pygit2.Repository('.') + count = len(series.commits) + old_br = repo.branches[branch] + 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() + target = repo.revparse_single('%s~%d' % (branch, count)) + repo.branches.local.create(dest_branch, target) + + num_added = 0 + for seq in range(count): + basket = repo.branches.get(dest_branch) + cherry = repo.revparse_single('%s~%d' % (branch, count - seq - 1)) + + base = repo.merge_base(cherry.oid, basket.target) + base_tree = cherry.parents[0].tree + + index = repo.merge_trees(base_tree, basket, cherry) + tree_id = index.write_tree(repo) + + author = cherry.author + committer = cherry.committer + lines = [] + 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 + '\n'.join(lines) + + basket = repo.create_commit( + basket.name, cherry.author, cherry.committer, message, tree_id, + [basket.target]) + return num_added + +def check_patchwork_status(series, url, branch, dest_branch, force): patches = CollectPatches(series, url) col = terminal.Color() count = len(patches) @@ -330,5 +371,12 @@ def check_patchwork_status(series, url, branch): ShowResponses(base_rtags, indent, False) num_to_add += ShowResponses(new_rtags, indent, True) - print("%d new response%s available in patchwork" % - (num_to_add, 's' if num_to_add != 1 else '')) + 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 = CreateBranch(series, new_rtag_list, branch, + dest_branch, force) + print("%d response%s added from patchwork into new branch '%s'" % + (num_added, 's' if num_added != 1 else '', dest_branch)) From patchwork Mon Jul 6 03:42:03 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 240798 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Sun, 5 Jul 2020 21:42:03 -0600 Subject: [RFC PATCH 16/16] patman: Support listing comments from patchwork In-Reply-To: <20200706034203.2171077-1-sjg@chromium.org> References: <20200706034203.2171077-1-sjg@chromium.org> Message-ID: <20200706034203.2171077-17-sjg@chromium.org> While reviewing feedback it is helpful to see the review comments on the command line to check that each has been addressed. Add an option to support that. Update the workflow documentation to describe the new features. Signed-off-by: Simon Glass --- tools/patman/README | 36 +++++- tools/patman/control.py | 11 +- tools/patman/main.py | 5 +- tools/patman/status.py | 278 +++++++++++++++++++++++++++------------- 4 files changed, 227 insertions(+), 103 deletions(-) diff --git a/tools/patman/README b/tools/patman/README index 595a0d616b..0ce299d2c0 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -14,6 +14,7 @@ This tool is a Python script which: 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 +- lists comments received on a series 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, @@ -391,6 +392,8 @@ but has the new review tags in it. You can check that this worked with: which should show that there are no new responses compared to this new branch. +There is also a -C option to list the comments received for each patch. + Example Work Flow ================= @@ -475,17 +478,33 @@ people on the list don't see your secret info. Of course patches often attract comments and you need to make some updates. Let's say one person sent comments and you get an Acked-by: on one patch. Also, the patch on the list that you were waiting for has been merged, -so you can drop your wip commit. So you resync with upstream: +so you can drop your wip commit. + +Take a look on patchwork and find out the URL of the series. This will be +something like http://patchwork.ozlabs.org/project/uboot/list/?series=187331 +Add this to a tag in your top commit: + + Series-link: http://patchwork.ozlabs.org/project/uboot/list/?series=187331 + +You can use then patman to collect the Acked-by tag to the correct commit, +creating a new 'version 2' branch for us-cmd: + + patman status -d us-cmd2 + git checkout us-cmd2 + +You can look at the comments in Patchwork or with: + + patman status -C + +Then you can resync with upstream: git fetch origin (or whatever upstream is called) git rebase origin/master -and use git rebase -i to edit the commits, dropping the wip one. You add -the ack tag to one commit: - - Acked-by: Heiko Schocher +and use git rebase -i to edit the commits, dropping the wip one. -update the Series-cc: in the top commit: +Then update the Series-cc: in the top commit to add the person who reviewed +the v1 series: Series-cc: bfin, marex, Heiko Schocher @@ -524,7 +543,9 @@ so to send them: and it will create and send the version 2 series. -General points: + +General points +============== 1. When you change back to the us-cmd branch days or weeks later all your information is still there, safely stored in the commits. You don't need @@ -606,3 +627,4 @@ a bad thing. Simon Glass v1, v2, 19-Oct-11 revised v3 24-Nov-11 +revised v4 Independence Day 2020, with Patchwork integration diff --git a/tools/patman/control.py b/tools/patman/control.py index b6ba0a56c0..b7d23b58c1 100644 --- a/tools/patman/control.py +++ b/tools/patman/control.py @@ -174,11 +174,13 @@ def send(args): args.limit, args.dry_run, args.in_reply_to, args.thread, args.smtp_server) -def patchwork_status(branch, count, start, end, dest_branch, force): +def patchwork_status(branch, count, start, end, dest_branch, force, + show_comments): """Check the status of patches in patchwork This finds the series in patchwork using the Series-link tag, checks for new - review tags, displays then and creates a new branch with the review tags. + comments and review tags, displays then and creates a new branch with the + review tags. Args: branch (str): Branch to create patches from (None = current) @@ -190,6 +192,8 @@ def patchwork_status(branch, count, start, end, dest_branch, force): 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 + show_comments (bool): True to display snippets from the comments + provided by reviewers Raises: ValueError: if the branch has no Series-link value @@ -206,4 +210,5 @@ def patchwork_status(branch, count, start, end, dest_branch, force): # Import this here to avoid failing on other commands if the dependencies # are not present from patman import status - status.check_patchwork_status(series, link, branch, dest_branch, force) + status.check_patchwork_status(series, link, branch, dest_branch, force, +show_comments) diff --git a/tools/patman/main.py b/tools/patman/main.py index c8fc035c10..6e4e23763d 100755 --- a/tools/patman/main.py +++ b/tools/patman/main.py @@ -90,6 +90,8 @@ AddCommonArgs(test_parser) status = subparsers.add_parser('status', help='Check status of patches in patchwork') +status.add_argument('-C', '--show-comments', action='store_true', + help='Show comments from each patch') 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', @@ -161,7 +163,8 @@ elif args.cmd == 'status': ret_code = 0 try: control.patchwork_status(args.branch, args.count, args.start, args.end, - args.dest_branch, args.force) + args.dest_branch, args.force, + args.show_comments) except Exception as e: print('patman: %s: %s' % (type(e).__name__, e)) if args.debug: diff --git a/tools/patman/status.py b/tools/patman/status.py index 71124d6e5e..9e3cb97e72 100644 --- a/tools/patman/status.py +++ b/tools/patman/status.py @@ -2,44 +2,45 @@ # # Copyright 2020 Google LLC # -# This talks to the patchwork service to figure out what patches have been -# reviewed and commented on. -from bs4 import BeautifulSoup +"""Talks to the patchwork service to figure out what patches have been +reviewed and commented on. Provides a way to display review tags and comments. +Allows creation of a new branch based on the old but with the review tags +collected from patchwork. +""" + import collections import concurrent.futures import io -import pygit2 from itertools import repeat import re -import requests -import time from urllib.parse import urljoin +import pygit2 +import requests +from bs4 import BeautifulSoup from patman import commit -from patman import gitutil from patman import patchstream from patman import terminal -from patman.terminal import Color # 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('(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$') +RE_PATCH = re.compile(r'(\[(((.*),)?(.*),)?(.*)\]\s)?(.*)$') # This decodes the sequence string into a patch number and patch count -re_seq = re.compile('(\d+)/(\d+)') +RE_SEQ = re.compile(r'(\d+)/(\d+)') def to_int(vals): - """Convert a lsit of strings into integers, using 0 if not an integer + """Convert a list of strings into integers, using 0 if not an integer Args: - vals: List of strings + vals (list): List of strings Returns: - List of integers, one for each input string + list: List of integers, one for each input string """ out = [int(val) if val.isdigit() else 0 for val in vals] return out @@ -59,28 +60,33 @@ class Patch(dict): The other properties come from other columns in the display. Properties: - seq: Sequence number within series (1=first) parsed from sequence string - count: Number of patches in series, parsed from sequence string - prefix: Prefix string or None - subject: Patch subject with [..] part removed (same as commit subject) - series: Series name - acked: Number of Acked-by rtags - fixes: Number of Fixes rtags - reviewed: Number of Reviewed-by rtags - tested: Number of Tested-by rtags - success: Success value - warning: Warning value - fail: Failure value - date: Date as a string - Submitter: Submitter name - Delegate: Deleted ID - state: Current state (e.g. 'New') - url: URL pointing to the patch on patchwork + seq (int): Sequence number within series (1=first) parsed from sequence + string + count (int): Number of patches in series, parsed from sequence string + prefix (str): Prefix string or None (e.g. 'RFC') + version (str): Version string or None (e.g. 'v2') + subject (str): Patch subject with [..] part removed (same as commit + subject) + series (str): Series name + acked (int): Number of Acked-by rtags, or None if not known + fixes (int): Number of Fixes rtags, or None if not known + reviewed (int): Number of Reviewed-by rtags, or None if not known + tested (int): Number of Tested-by rtags, or None if not known + success (int): Success value, or None if not known + warning (int): Warning value, or None if not known + fail (int): Failure value, or None if not known + date (str): Date as a string + Submitter (str): Submitter name + Delegate (str): Delegate ID + state (str): Current state (e.g. 'New') + url (str): URL pointing to the patch on patchwork """ def __init__(self): + super().__init__() self.seq = None self.count = None self.prefix = None + self.version = None self.subject = None self.series = None self.acked = None @@ -103,24 +109,28 @@ class Patch(dict): def __getattr__(self, name): return self[name] - def Add(self, name, value): + def add(self, name, value): """Add information obtained from the web page This sets the object properties based on @name. Args: - name: Column name in lower-case, e.g. 'Series' or 'State' - value: Text obtained for this patch from that column, e.g. 'New' + name (str): Column name in lower-case, e.g. 'Series' or 'State' + value (str): Text obtained for this patch from that column, + e.g. 'New' + + Raises: + ValueError: if the name cannot be parsed or is not recognised """ if name in self: self[name] = value elif name == 'patch': - mat = re_patch.search(value.strip()) + mat = RE_PATCH.search(value.strip()) if not mat: raise ValueError("Cannot parse subject '%s'" % value) self.prefix = mat.group(1) self.prefix, self.version, seq_info, self.subject = mat.groups()[3:] - mat_seq = re_seq.match(seq_info) + mat_seq = RE_SEQ.match(seq_info) if not mat_seq: raise ValueError("Cannot parse sequence spec '%s'" % seq_info) self.seq = int(mat_seq.group(1)) @@ -133,16 +143,35 @@ class Patch(dict): else: print('Unknown field: %s\n' % name) - def SetUrl(self, url): + def set_url(self, url): """Sets the URL for a patch Args: - url: URL to set + url (str): URL to set """ self.url = url -def FindResponses(url): +class Review: + """Represents a single review email collected in Patchwork + + Patches can attract multiple reviews. Each consists of an author/date and + a variable number of 'snippets', which are groups of quoted and unquoted + text. + """ + def __init__(self, meta, snippets): + """Create new Review object + + Args: + meta (str): Text containing review author and date + snippets (list): List of snippets in th review, each a list of text + lines + """ + self.meta = ' : '.join([line for line in meta.splitlines() if line]) + self.snippets = snippets + + +def find_responses(url): """Collect the response tags (rtags) to a patch found in patchwork Reads the URL and checks each comment for an rtags (e.g. Reviewed-by). @@ -151,13 +180,18 @@ def FindResponses(url): new rtags. Args: - url: URL of the patch + url (str): URL of the patch Returns: - Dict of rtags: - key: rtag type, e.g. 'Acked-by' - value: Set of people providing that rtag. Each member of the set - is a name/email, e.g. 'Bin Meng ' + Tuple: + Dict of rtags: + key: rtag type, e.g. 'Acked-by' + value: Set of people providing that rtag. Each member of the set + is a name/email, e.g. 'Bin Meng ' + List of Review objects for the patch + + Raises: + ValueError: if the URL could not be read """ web_data = requests.get(url) if web_data.status_code != 200: @@ -171,31 +205,37 @@ def FindResponses(url): if comments: for comment in comments: - ps = patchstream.PatchStream(None) - ps.commit = commit.Commit(None) + pstrm = patchstream.PatchStream(None) + pstrm.commit = commit.Commit(None) meta = comment.find(class_='meta') content = comment.find(class_='content') infd = io.StringIO(content.get_text()) outfd = io.StringIO() - ps.ProcessStream(infd, outfd) - for response, people in ps.commit.rtags.items(): + pstrm.ProcessStream(infd, outfd) + for response, people in pstrm.commit.rtags.items(): rtags[response].update(people) - return rtags + if pstrm.snippets: + reviews.append(Review(meta.get_text(), pstrm.snippets)) + return rtags, reviews -def CollectPatches(series, url): +def collect_patches(series, url): """Collect patch information about a series from patchwork Reads the web page and collects information provided by patchwork about the status of each patch. Args: - series: Series object corresponding to the local branch containing the - series - url: URL pointing to patchwork's view of this series, possibly an + series (Series): Series object corresponding to the local branch + containing the series + url (str): URL pointing to patchwork's view of this series, possibly an earlier versions of it Returns: - List of patches sorted by sequence number, each a Patch object + 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 """ response = requests.get(url) if response.status_code != 200: @@ -228,11 +268,12 @@ def CollectPatches(series, url): patch = Patch() for seq, col in enumerate(row.findAll('td')): name = cols[seq].lower() - patch.Add(name, col.get_text()) + patch.add(name, col.get_text()) if name == 'patch': - patch.SetUrl(urljoin(url, col.a['href'])) + patch.set_url(urljoin(url, col.a['href'])) if patch.count != count: - raise ValueError("Patch %d '%s' has count of %d, expected %d" % + raise ValueError( + "Patch %d '%s' has count of %d, expected %d" % (patch.seq, patch.subject, patch.count, num_commits)) patches.append(patch) @@ -240,25 +281,31 @@ def CollectPatches(series, url): patches = sorted(patches, key=lambda x: x.seq) return patches -def FindNewResponses(new_rtag_list, seq, commit, patch): +def find_new_responses(new_rtag_list, review_list, seq, cmt, patch, + force_load): """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: New rtags are written to new_rtag_list[seq] + 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: Position in new_rtag_list to update - commit: Commit object for this commit - patch: Corresponding Patch object for this patch + review_list (list): New reviews are written to review_list[seq] + list, each a + List of reviews for the patch, each a Review + seq (int): Position in new_rtag_list to update + cmt (Commit): Commit object for this commit + patch (Patch): Corresponding Patch object for this patch + force_load (bool): True to always load the patch from patchwork, False + to only load it if patchwork has additional response tags """ - acked = commit.rtags['Acked-by'] - reviewed = commit.rtags['Reviewed-by'] - tested = commit.rtags['Tested-by'] - fixes = commit.rtags['Fixes'] + acked = cmt.rtags['Acked-by'] + reviewed = cmt.rtags['Reviewed-by'] + tested = cmt.rtags['Tested-by'] + fixes = cmt.rtags['Fixes'] # Figure out the number of new rtags by subtracting what we have in # the commit from what patchwork's summary reports. @@ -271,9 +318,10 @@ def FindNewResponses(new_rtag_list, seq, commit, patch): # If patchwork agrees with the commit, we don't need to read the web # page, so save some time. If not, we need to add the new ones to the commit new_rtags = collections.defaultdict(set) - if extra: - pw_rtags = FindResponses(patch.url) - base_rtags = commit.rtags + reviews = None + if extra or force_load: + pw_rtags, reviews = find_responses(patch.url) + base_rtags = cmt.rtags new_count = 0 for tag, people in pw_rtags.items(): for who in people: @@ -283,16 +331,20 @@ def FindNewResponses(new_rtag_list, seq, commit, patch): new_rtags[tag].add(who) new_count += 1 new_rtag_list[seq] = new_rtags + review_list[seq] = reviews -def ShowResponses(rtags, indent, is_new): +def show_responses(rtags, indent, is_new): """Show rtags collected Args: - rtags: dict: + 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: Indentation string to write before each line - is_new: True if this output should be highlighted + 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 @@ -304,13 +356,32 @@ def ShowResponses(rtags, indent, is_new): count += 1 return count -def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite): +def create_branch(series, new_rtag_list, branch, dest_branch, overwrite): + """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 + + 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') repo = pygit2.Repository('.') count = len(series.commits) - old_br = repo.branches[branch] new_br = repo.branches.get(dest_branch) if new_br: if not overwrite: @@ -322,17 +393,15 @@ def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite): num_added = 0 for seq in range(count): - basket = repo.branches.get(dest_branch) + parent = repo.branches.get(dest_branch) cherry = repo.revparse_single('%s~%d' % (branch, count - seq - 1)) - base = repo.merge_base(cherry.oid, basket.target) + repo.merge_base(cherry.oid, parent.target) base_tree = cherry.parents[0].tree - index = repo.merge_trees(base_tree, basket, cherry) + index = repo.merge_trees(base_tree, parent, cherry) tree_id = index.write_tree(repo) - author = cherry.author - committer = cherry.committer lines = [] for tag, people in new_rtag_list[seq].items(): for who in people: @@ -340,21 +409,37 @@ def CreateBranch(series, new_rtag_list, branch, dest_branch, overwrite): num_added += 1 message = cherry.message + '\n'.join(lines) - basket = repo.create_commit( - basket.name, cherry.author, cherry.committer, message, tree_id, - [basket.target]) + repo.create_commit( + parent.name, cherry.author, cherry.committer, message, tree_id, + [parent.target]) return num_added -def check_patchwork_status(series, url, branch, dest_branch, force): - patches = CollectPatches(series, url) +def check_patchwork_status(series, url, branch, dest_branch, force, + show_comments): + """Check the status of a series on Patchwork + + This finds review tags and comments for a series in Patchwork, displaying + them and optionally creating a new branch like the old but with the new + review tags. + + Args: + series (Series): Series object for the existing branch + url (str): URL pointing to the patch on patchwork + branch (str): Existing branch to update + dest_branch (str): Name of new branch to create + force (bool): True to force overwriting dest_branch if it exists + show_comments (bool): True to show the comments on each patch + """ + patches = collect_patches(series, url) col = terminal.Color() count = len(patches) new_rtag_list = [None] * count + review_list = [None] * count with concurrent.futures.ThreadPoolExecutor(max_workers=16) as executor: futures = executor.map( - FindNewResponses, repeat(new_rtag_list), range(count), - series.commits, patches) + find_new_responses, repeat(new_rtag_list), repeat(review_list), + range(count), series.commits, patches, repeat(show_comments)) for fresponse in futures: if fresponse: raise fresponse.exception() @@ -363,20 +448,29 @@ def check_patchwork_status(series, url, branch, dest_branch, force): for seq, patch in enumerate(patches): terminal.Print('%3d %s' % (patch.seq, patch.subject[:50]), colour=col.BLUE) - commit = series.commits[seq] - base_rtags = commit.rtags + cmt = series.commits[seq] + base_rtags = cmt.rtags new_rtags = new_rtag_list[seq] indent = ' ' * 2 - ShowResponses(base_rtags, indent, False) - num_to_add += ShowResponses(new_rtags, indent, True) + show_responses(base_rtags, indent, False) + num_to_add += show_responses(new_rtags, indent, True) + if show_comments: + for review in review_list[seq]: + terminal.Print('Review: %s' % review.meta, colour=col.RED) + for snippet in review.snippets: + for line in snippet: + quoted = line.startswith('>') + terminal.Print(' %s' % line, + colour=col.MAGENTA if quoted else None) + print() 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 = CreateBranch(series, new_rtag_list, branch, - dest_branch, force) + num_added = create_branch(series, new_rtag_list, branch, + dest_branch, force) print("%d response%s added from patchwork into new branch '%s'" % (num_added, 's' if num_added != 1 else '', dest_branch))