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 sjg@chromium.org ---

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 """

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 sjg@chromium.org ---

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@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@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 bcrump@whataroa.nz +''') + 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):

Add a -b option to allow patches to be created from a branch other than the current one.

Signed-off-by: Simon Glass sjg@chromium.org ---

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

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 sjg@chromium.org ---

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')

Convert from OptionParser to ArgumentParser to match binman. With this we can easily add sub-commands.

Signed-off-by: Simon Glass sjg@chromium.org ---

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.

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 sjg@chromium.org ---

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

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 sjg@chromium.org ---

.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

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 sjg@chromium.org ---

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

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 sjg@chromium.org ---

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 fred@bloggs.org' + """ + 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 fred@bloggs.org' + """ + 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]

Add a new Series-link tag to tell patman how to find the series in patchwork.

Signed-off-by: Simon Glass sjg@chromium.org ---

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.

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 sjg@chromium.org ---

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:

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

Signed-off-by: Simon Glass sjg@chromium.org ---

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))