[U-Boot] [PATCH v2 0/7] Various patman fixes
Since checkpatch has changed in U-Boot, patman no longer detects errors properly. It seems like a good oppotunity to fix some of the bugs and annoying habits of patman that people have mentioned on the list.
If I have missed any bugs / problems in this sweep, please let me know.
Changes in v2: - Add comment about meaning of raise_on_error=False - Adjust to not allow any spaces in tags, change commit title - Remove 'ignore_errors' and just use 'raise_on_error' everywhere - Require sort, uniq tags to be comma-separated - Update code to remove match_count - Update commit message to add detail on what is changing - Update tests to check the 'checks' value, and add a new test - Use keyword args for raise_on_error - Use namedtuple to hold the return value from CheckPatch() function
Simon Glass (7): patman: Fix up checkpatch parsing to deal with 'CHECK' lines patman: Don't allow spaces in tags patman: Fix the comment in CheckTags to mention multiple tags patman: Provide option to ignore bad aliases patman: Add -a option to refrain from test-applying the patches patman: Add Series-process-log tag to sort/uniq change logs patman: Minor help message/README fixes
tools/patman/README | 11 ++++- tools/patman/checkpatch.py | 110 +++++++++++++++++++++++++++----------------- tools/patman/commit.py | 7 +-- tools/patman/gitutil.py | 65 ++++++++++++++++++-------- tools/patman/patchstream.py | 2 +- tools/patman/patman.py | 24 ++++++---- tools/patman/series.py | 19 ++++++-- tools/patman/test.py | 64 ++++++++++++++++++-------- 8 files changed, 203 insertions(+), 99 deletions(-)
checkpatch has a new type of warning, a 'CHECK'. At present patman fails with these, which makes it less than useful.
Add support for checks, making it backwards compatible with the old checkpatch.
At the same time, clean up formatting of the CheckPatches() output, fix erroneous "internal error" if multiple patches have warnings and be more robust to new types of problems.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Update code to remove match_count - Update commit message to add detail on what is changing - Update tests to check the 'checks' value, and add a new test - Use namedtuple to hold the return value from CheckPatch() function
tools/patman/checkpatch.py | 110 ++++++++++++++++++++++++++++----------------- tools/patman/test.py | 64 +++++++++++++++++--------- 2 files changed, 112 insertions(+), 62 deletions(-)
diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index d3a0477..83aaf71 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -19,6 +19,7 @@ # MA 02111-1307 USA #
+import collections import command import gitutil import os @@ -57,63 +58,86 @@ def CheckPatch(fname, verbose=False): """Run checkpatch.pl on a file.
Returns: - 4-tuple containing: - result: False=failure, True=ok + namedtuple containing: + ok: False=failure, True=ok problems: List of problems, each a dict: 'type'; error or warning 'msg': text message 'file' : filename 'line': line number + errors: Number of errors + warnings: Number of warnings + checks: Number of checks lines: Number of lines + stdout: Full output of checkpatch """ - result = False - error_count, warning_count, lines = 0, 0, 0 - problems = [] + fields = ['ok', 'problems', 'errors', 'warnings', 'checks', 'lines', + 'stdout'] + result = collections.namedtuple('CheckPatchResult', fields) + result.ok = False + result.errors, result.warning, result.checks = 0, 0, 0 + result.lines = 0 + result.problems = [] chk = FindCheckPatch() item = {} - stdout = command.Output(chk, '--no-tree', fname) + result.stdout = command.Output(chk, '--no-tree', fname) #pipe = subprocess.Popen(cmd, stdout=subprocess.PIPE) #stdout, stderr = pipe.communicate()
# total: 0 errors, 0 warnings, 159 lines checked + # or: + # total: 0 errors, 2 warnings, 7 checks, 473 lines checked re_stats = re.compile('total: (\d+) errors, (\d+) warnings, (\d+)') + re_stats_full = re.compile('total: (\d+) errors, (\d+) warnings, (\d+)' + ' checks, (\d+)') re_ok = re.compile('.*has no obvious style problems') re_bad = re.compile('.*has style problems, please review') re_error = re.compile('ERROR: (.*)') re_warning = re.compile('WARNING: (.*)') + re_check = re.compile('CHECK: (.*)') re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):')
- for line in stdout.splitlines(): + for line in result.stdout.splitlines(): if verbose: print line
# A blank line indicates the end of a message if not line and item: - problems.append(item) + result.problems.append(item) item = {} - match = re_stats.match(line) + match = re_stats_full.match(line) + if not match: + match = re_stats.match(line) if match: - error_count = int(match.group(1)) - warning_count = int(match.group(2)) - lines = int(match.group(3)) + result.errors = int(match.group(1)) + result.warnings = int(match.group(2)) + if len(match.groups()) == 4: + result.checks = int(match.group(3)) + result.lines = int(match.group(4)) + else: + result.lines = int(match.group(3)) elif re_ok.match(line): - result = True + result.ok = True elif re_bad.match(line): - result = False - match = re_error.match(line) - if match: - item['msg'] = match.group(1) + result.ok = False + err_match = re_error.match(line) + warn_match = re_warning.match(line) + file_match = re_file.match(line) + check_match = re_check.match(line) + if err_match: + item['msg'] = err_match.group(1) item['type'] = 'error' - match = re_warning.match(line) - if match: - item['msg'] = match.group(1) + elif warn_match: + item['msg'] = warn_match.group(1) item['type'] = 'warning' - match = re_file.match(line) - if match: - item['file'] = match.group(1) - item['line'] = int(match.group(2)) + elif check_match: + item['msg'] = check_match.group(1) + item['type'] = 'check' + elif file_match: + item['file'] = file_match.group(1) + item['line'] = int(file_match.group(2))
- return result, problems, error_count, warning_count, lines, stdout + return result
def GetWarningMsg(col, msg_type, fname, line, msg): '''Create a message for a given file/line @@ -128,37 +152,39 @@ def GetWarningMsg(col, msg_type, fname, line, msg): msg_type = col.Color(col.YELLOW, msg_type) elif msg_type == 'error': msg_type = col.Color(col.RED, msg_type) + elif msg_type == 'check': + msg_type = col.Color(col.MAGENTA, msg_type) return '%s: %s,%d: %s' % (msg_type, fname, line, msg)
def CheckPatches(verbose, args): '''Run the checkpatch.pl script on each patch''' - error_count = 0 - warning_count = 0 + error_count, warning_count, check_count = 0, 0, 0 col = terminal.Color()
for fname in args: - ok, problems, errors, warnings, lines, stdout = CheckPatch(fname, - verbose) - if not ok: - error_count += errors - warning_count += warnings - print '%d errors, %d warnings for %s:' % (errors, - warnings, fname) - if len(problems) != error_count + warning_count: + result = CheckPatch(fname, verbose) + if not result.ok: + error_count += result.errors + warning_count += result.warnings + check_count += result.checks + print '%d errors, %d warnings, %d checks for %s:' % (result.errors, + result.warnings, result.checks, col.Color(col.BLUE, fname)) + if (len(result.problems) != result.errors + result.warnings + + result.checks): print "Internal error: some problems lost" - for item in problems: - print GetWarningMsg(col, item['type'], + for item in result.problems: + print GetWarningMsg(col, item.get('type', '<unknown>'), item.get('file', '<unknown>'), - item.get('line', 0), item['msg']) + item.get('line', 0), item.get('msg', 'message')) + print #print stdout - if error_count != 0 or warning_count != 0: - str = 'checkpatch.pl found %d error(s), %d warning(s)' % ( - error_count, warning_count) + if error_count or warning_count or check_count: + str = 'checkpatch.pl found %d error(s), %d warning(s), %d checks(s)' color = col.GREEN if warning_count: color = col.YELLOW if error_count: color = col.RED - print col.Color(color, str) + print col.Color(color, str % (error_count, warning_count, check_count)) return False return True diff --git a/tools/patman/test.py b/tools/patman/test.py index f801ced..8cd2647 100644 --- a/tools/patman/test.py +++ b/tools/patman/test.py @@ -190,6 +190,11 @@ index 0000000..2234c87 + rec->time_us = (uint32_t)timer_get_us(); + rec->name = name; + } ++ if (!rec->name && ++ %ssomething_else) { ++ rec->time_us = (uint32_t)timer_get_us(); ++ rec->name = name; ++ } +%sreturn rec->time_us; +} -- @@ -197,15 +202,18 @@ index 0000000..2234c87 ''' signoff = 'Signed-off-by: Simon Glass sjg@chromium.org\n' tab = ' ' + indent = ' ' if data_type == 'good': pass elif data_type == 'no-signoff': signoff = '' elif data_type == 'spaces': tab = ' ' + elif data_type == 'indent': + indent = tab else: print 'not implemented' - return data % (signoff, tab, tab) + return data % (signoff, tab, indent, tab)
def SetupData(self, data_type): inhandle, inname = tempfile.mkstemp() @@ -215,33 +223,49 @@ index 0000000..2234c87 infd.close() return inname
- def testCheckpatch(self): + def testGood(self): """Test checkpatch operation""" inf = self.SetupData('good') - result, problems, err, warn, lines, stdout = checkpatch.CheckPatch(inf) - self.assertEqual(result, True) - self.assertEqual(problems, []) - self.assertEqual(err, 0) - self.assertEqual(warn, 0) - self.assertEqual(lines, 67) + result = checkpatch.CheckPatch(inf) + self.assertEqual(result.ok, True) + self.assertEqual(result.problems, []) + self.assertEqual(result.errors, 0) + self.assertEqual(result.warnings, 0) + self.assertEqual(result.checks, 0) + self.assertEqual(result.lines, 67) os.remove(inf)
+ def testNoSignoff(self): inf = self.SetupData('no-signoff') - result, problems, err, warn, lines, stdout = checkpatch.CheckPatch(inf) - self.assertEqual(result, False) - self.assertEqual(len(problems), 1) - self.assertEqual(err, 1) - self.assertEqual(warn, 0) - self.assertEqual(lines, 67) + result = checkpatch.CheckPatch(inf) + self.assertEqual(result.ok, False) + self.assertEqual(len(result.problems), 1) + self.assertEqual(result.errors, 1) + self.assertEqual(result.warnings, 0) + self.assertEqual(result.checks, 0) + self.assertEqual(result.lines, 67) os.remove(inf)
+ def testSpaces(self): inf = self.SetupData('spaces') - result, problems, err, warn, lines, stdout = checkpatch.CheckPatch(inf) - self.assertEqual(result, False) - self.assertEqual(len(problems), 2) - self.assertEqual(err, 0) - self.assertEqual(warn, 2) - self.assertEqual(lines, 67) + result = checkpatch.CheckPatch(inf) + self.assertEqual(result.ok, False) + self.assertEqual(len(result.problems), 1) + self.assertEqual(result.errors, 0) + self.assertEqual(result.warnings, 1) + self.assertEqual(result.checks, 0) + self.assertEqual(result.lines, 67) + os.remove(inf) + + def testIndent(self): + inf = self.SetupData('indent') + result = checkpatch.CheckPatch(inf) + self.assertEqual(result.ok, False) + self.assertEqual(len(result.problems), 1) + self.assertEqual(result.errors, 0) + self.assertEqual(result.warnings, 0) + self.assertEqual(result.checks, 1) + self.assertEqual(result.lines, 67) os.remove(inf)
Simon,
On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass sjg@chromium.org wrote:
checkpatch has a new type of warning, a 'CHECK'. At present patman fails with these, which makes it less than useful.
Add support for checks, making it backwards compatible with the old checkpatch.
At the same time, clean up formatting of the CheckPatches() output, fix erroneous "internal error" if multiple patches have warnings and be more robust to new types of problems.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Update code to remove match_count
- Update commit message to add detail on what is changing
- Update tests to check the 'checks' value, and add a new test
- Use namedtuple to hold the return value from CheckPatch() function
tools/patman/checkpatch.py | 110 ++++++++++++++++++++++++++++----------------- tools/patman/test.py | 64 +++++++++++++++++--------- 2 files changed, 112 insertions(+), 62 deletions(-)
Thanks for fixing and for fixing up the tests as well.
Reviewed-by: Doug Anderson dianders@chromium.org
At present something like:
Revert "arm: Add cache operations"
will try to use
Revert "arm
as a tag. Clearly this is wrong, so fix it.
If the revert is intended to be tagged, then the tag can come before the revert, perhaps. Alternatively the 'Cc' tag can be used in the commit messages.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Adjust to not allow any spaces in tags, change commit title
tools/patman/commit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/patman/commit.py b/tools/patman/commit.py index 7144e54..cbfbc46 100644 --- a/tools/patman/commit.py +++ b/tools/patman/commit.py @@ -22,7 +22,7 @@ import re
# Separates a tag: at the beginning of the subject from the rest of it -re_subject_tag = re.compile('([^:]*):\s*(.*)') +re_subject_tag = re.compile('([^:\s]*):\s*(.*)')
class Commit: """Holds information about a single commit/patch in the series.
Simon,
On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass sjg@chromium.org wrote:
At present something like:
Revert "arm: Add cache operations"
will try to use
Revert "arm
as a tag. Clearly this is wrong, so fix it.
If the revert is intended to be tagged, then the tag can come before the revert, perhaps. Alternatively the 'Cc' tag can be used in the commit messages.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Adjust to not allow any spaces in tags, change commit title
tools/patman/commit.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Doug Anderson dianders@chromium.org
This comment is less than helpful. Since multiple tags are supported, add an example of how multiple tags work.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Doug Anderson dianders@chromium.org --- Changes in v2: None
tools/patman/commit.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/tools/patman/commit.py b/tools/patman/commit.py index cbfbc46..468b50c 100644 --- a/tools/patman/commit.py +++ b/tools/patman/commit.py @@ -61,9 +61,10 @@ class Commit:
Subject tags look like this:
- propounder: Change the widget to propound correctly + propounder: fort: Change the widget to propound correctly
- Multiple tags are supported. The list is updated in self.tag + Here the tags are propounder and fort. Multiple tags are supported. + The list is updated in self.tag.
Returns: None if ok, else the name of a tag with no email alias
Often it happens that patches include tags which don't have aliases. It is annoying that patman fails in this case, and provides no option to continue other than adding empty tags to the .patman file.
Correct this by adding a '-t' option to ignore tags that don't exist. Print a warning instead.
Since running the tests is not a common operation, move this to --test instead, to reserve -t for this new option.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Add comment about meaning of raise_on_error=False - Remove 'ignore_errors' and just use 'raise_on_error' everywhere - Use keyword args for raise_on_error
tools/patman/gitutil.py | 65 +++++++++++++++++++++++++++++++++++-------------- tools/patman/patman.py | 10 +++++--- tools/patman/series.py | 10 +++++--- 3 files changed, 61 insertions(+), 24 deletions(-)
diff --git a/tools/patman/gitutil.py b/tools/patman/gitutil.py index c35d209..f70f851 100644 --- a/tools/patman/gitutil.py +++ b/tools/patman/gitutil.py @@ -159,7 +159,7 @@ def ApplyPatches(verbose, args, start_point): print stdout, stderr return error_count == 0
-def BuildEmailList(in_list, tag=None, alias=None): +def BuildEmailList(in_list, tag=None, alias=None, raise_on_error=True): """Build a list of email addresses based on an input list.
Takes a list of email addresses and aliases, and turns this into a list @@ -172,6 +172,9 @@ def BuildEmailList(in_list, tag=None, alias=None): Args: in_list: List of aliases/email addresses tag: Text to put before each address + alias: Alias dictionary + raise_on_error: True to raise an error when an alias fails to match, + False to just print a message.
Returns: List of email addresses @@ -193,7 +196,7 @@ def BuildEmailList(in_list, tag=None, alias=None): quote = '"' if tag and tag[0] == '-' else '' raw = [] for item in in_list: - raw += LookupEmail(item, alias) + raw += LookupEmail(item, alias, raise_on_error=raise_on_error) result = [] for item in raw: if not item in result: @@ -202,7 +205,7 @@ def BuildEmailList(in_list, tag=None, alias=None): return ['%s %s%s%s' % (tag, quote, email, quote) for email in result] return result
-def EmailPatches(series, cover_fname, args, dry_run, cc_fname, +def EmailPatches(series, cover_fname, args, dry_run, raise_on_error, cc_fname, self_only=False, alias=None, in_reply_to=None): """Email a patch series.
@@ -211,6 +214,8 @@ def EmailPatches(series, cover_fname, args, dry_run, cc_fname, cover_fname: filename of cover letter args: list of filenames of patch files dry_run: Just return the command that would be run + raise_on_error: True to raise an error when an alias fails to match, + False to just print a message. cc_fname: Filename of Cc file for per-commit Cc self_only: True to just email to yourself as a test in_reply_to: If set we'll pass this to git as --in-reply-to. @@ -233,20 +238,21 @@ def EmailPatches(series, cover_fname, args, dry_run, cc_fname, >>> series = series.Series() >>> series.to = ['fred'] >>> series.cc = ['mary'] - >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, 'cc-fname', False, \ - alias) + >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ + False, alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ "m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" cover p1 p2' - >>> EmailPatches(series, None, ['p1'], True, 'cc-fname', False, alias) + >>> EmailPatches(series, None, ['p1'], True, True, 'cc-fname', False, \ + alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ "m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" p1' >>> series.cc = ['all'] - >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, 'cc-fname', True, \ - alias) + >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ + True, alias) 'git send-email --annotate --to "this-is-me@me.com" --cc-cmd "./patman \ --cc-cmd cc-fname" cover p1 p2' - >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, 'cc-fname', False, \ - alias) + >>> EmailPatches(series, 'cover', ['p1', 'p2'], True, True, 'cc-fname', \ + False, alias) 'git send-email --annotate --to "f.bloggs@napier.co.nz" --cc \ "f.bloggs@napier.co.nz" --cc "j.bloggs@napier.co.nz" --cc \ "m.poppins@cloud.net" --cc-cmd "./patman --cc-cmd cc-fname" cover p1 p2' @@ -254,14 +260,14 @@ def EmailPatches(series, cover_fname, args, dry_run, cc_fname, # Restore argv[0] since we clobbered it. >>> sys.argv[0] = _old_argv0 """ - to = BuildEmailList(series.get('to'), '--to', alias) + to = BuildEmailList(series.get('to'), '--to', alias, raise_on_error) if not to: print ("No recipient, please add something like this to a commit\n" "Series-to: Fred Bloggs f.blogs@napier.co.nz") return - cc = BuildEmailList(series.get('cc'), '--cc', alias) + cc = BuildEmailList(series.get('cc'), '--cc', alias, raise_on_error) if self_only: - to = BuildEmailList([os.getenv('USER')], '--to', alias) + to = BuildEmailList([os.getenv('USER')], '--to', alias, raise_on_error) cc = [] cmd = ['git', 'send-email', '--annotate'] if in_reply_to: @@ -279,13 +285,16 @@ def EmailPatches(series, cover_fname, args, dry_run, cc_fname, return str
-def LookupEmail(lookup_name, alias=None, level=0): +def LookupEmail(lookup_name, alias=None, raise_on_error=True, level=0): """If an email address is an alias, look it up and return the full name
TODO: Why not just use git's own alias feature?
Args: lookup_name: Alias or email address to look up + alias: Dictionary containing aliases (None to use settings default) + raise_on_error: True to raise an error when an alias fails to match, + False to just print a message.
Returns: tuple: @@ -319,6 +328,15 @@ def LookupEmail(lookup_name, alias=None, level=0): Traceback (most recent call last): ... OSError: Recursive email alias at 'other' + >>> LookupEmail('odd', alias, raise_on_error=False) + \033[1;31mAlias 'odd' not found\033[0m + [] + >>> # In this case the loop part will effectively be ignored. + >>> LookupEmail('loop', alias, raise_on_error=False) + \033[1;31mRecursive email alias at 'other'\033[0m + \033[1;31mRecursive email alias at 'john'\033[0m + \033[1;31mRecursive email alias at 'mary'\033[0m + ['j.bloggs@napier.co.nz', 'm.poppins@cloud.net'] """ if not alias: alias = settings.alias @@ -327,16 +345,27 @@ def LookupEmail(lookup_name, alias=None, level=0): return [lookup_name]
lookup_name = lookup_name.lower() + col = terminal.Color()
+ out_list = [] if level > 10: - raise OSError, "Recursive email alias at '%s'" % lookup_name + msg = "Recursive email alias at '%s'" % lookup_name + if raise_on_error: + raise OSError, msg + else: + print col.Color(col.RED, msg) + return out_list
- out_list = [] if lookup_name: if not lookup_name in alias: - raise ValueError, "Alias '%s' not found" % lookup_name + msg = "Alias '%s' not found" % lookup_name + if raise_on_error: + raise ValueError, msg + else: + print col.Color(col.RED, msg) + return out_list for item in alias[lookup_name]: - todo = LookupEmail(item, alias, level + 1) + todo = LookupEmail(item, alias, raise_on_error, level + 1) for new_item in todo: if not new_item in out_list: out_list.append(new_item) diff --git a/tools/patman/patman.py b/tools/patman/patman.py index 377408d..023dceb 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -57,7 +57,9 @@ parser.add_option('-r', '--in-reply-to', type='string', action='store', help="Message ID that this series is in reply to") parser.add_option('-s', '--start', dest='start', type='int', default=0, help='Commit to start creating patches from (0 = HEAD)') -parser.add_option('-t', '--test', action='store_true', dest='test', +parser.add_option('-t', '--ignore-bad-tags', action='store_true', + default=False, help='Ignore bad tags / aliases') +parser.add_option('--test', action='store_true', dest='test', default=False, help='run tests') parser.add_option('-v', '--verbose', action='store_true', dest='verbose', default=False, help='Verbose output of errors and warnings') @@ -159,13 +161,15 @@ else: options.count + options.start): ok = False
- cc_file = series.MakeCcFile(options.process_tags, cover_fname) + cc_file = series.MakeCcFile(options.process_tags, cover_fname, + not options.ignore_bad_tags)
# Email the patches out (giving the user time to check / cancel) cmd = '' if ok or options.ignore_errors: cmd = gitutil.EmailPatches(series, cover_fname, args, - options.dry_run, cc_file, in_reply_to=options.in_reply_to) + options.dry_run, not options.ignore_bad_tags, cc_file, + in_reply_to=options.in_reply_to)
# For a dry run, just show our actions as a sanity check if options.dry_run: diff --git a/tools/patman/series.py b/tools/patman/series.py index 44ad931..eb5a00c 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -210,7 +210,7 @@ class Series(dict): str = 'Change log exists, but no version is set' print col.Color(col.RED, str)
- def MakeCcFile(self, process_tags, cover_fname): + def MakeCcFile(self, process_tags, cover_fname, raise_on_error): """Make a cc file for us to use for per-commit Cc automation
Also stores in self._generated_cc to make ShowActions() faster. @@ -218,6 +218,8 @@ class Series(dict): Args: process_tags: Process tags as if they were aliases cover_fname: If non-None the name of the cover letter. + raise_on_error: True to raise an error when an alias fails to match, + False to just print a message. Return: Filename of temp file created """ @@ -228,8 +230,10 @@ class Series(dict): for commit in self.commits: list = [] if process_tags: - list += gitutil.BuildEmailList(commit.tags) - list += gitutil.BuildEmailList(commit.cc_list) + list += gitutil.BuildEmailList(commit.tags, + raise_on_error=raise_on_error) + list += gitutil.BuildEmailList(commit.cc_list, + raise_on_error=raise_on_error) list += get_maintainer.GetMaintainer(commit.patch) all_ccs += list print >>fd, commit.patch, ', '.join(list)
Simon,
On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass sjg@chromium.org wrote:
Often it happens that patches include tags which don't have aliases. It is annoying that patman fails in this case, and provides no option to continue other than adding empty tags to the .patman file.
Correct this by adding a '-t' option to ignore tags that don't exist. Print a warning instead.
Since running the tests is not a common operation, move this to --test instead, to reserve -t for this new option.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Add comment about meaning of raise_on_error=False
- Remove 'ignore_errors' and just use 'raise_on_error' everywhere
- Use keyword args for raise_on_error
Reviewed-by: Doug Anderson dianders@chromium.org
Especially with the Linux kernel, it takes a long time (a minute or more) to test-apply the patches, so patman becomes significantly less useful. The only real problem that is found with this apply step is trailing spaces. Provide a -a option to skip this step, for those working with clean patches.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Doug Anderson dianders@chromium.org --- Changes in v2: None
tools/patman/patman.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index 023dceb..7a86b43 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -41,6 +41,9 @@ import test
parser = OptionParser() +parser.add_option('-a', '--no-apply', action='store_false', + dest='apply_patches', default=True, + help="Don't test-apply patches with git am") parser.add_option('-H', '--full-help', action='store_true', dest='full_help', default=False, help='Display the README file') parser.add_option('-c', '--count', dest='count', type='int', @@ -157,9 +160,10 @@ else: ok = checkpatch.CheckPatches(options.verbose, args) else: ok = True - if not gitutil.ApplyPatches(options.verbose, args, - options.count + options.start): - ok = False + if options.apply_patches: + if not gitutil.ApplyPatches(options.verbose, args, + options.count + options.start): + ok = False
cc_file = series.MakeCcFile(options.process_tags, cover_fname, not options.ignore_bad_tags)
For some series with lots of changes it is annoying that duplicate change log items are not caught. It is also helpful sometimes to sort the change logs.
Add a Series-process-log tag to enable this, which can be placed in a commit to control this.
The change to the Cc: line is to fix a checkpatch warning.
Signed-off-by: Simon Glass sjg@chromium.org --- Changes in v2: - Require sort, uniq tags to be comma-separated
tools/patman/README | 9 ++++++++- tools/patman/patchstream.py | 2 +- tools/patman/series.py | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/tools/patman/README b/tools/patman/README index 9922f2a..0bdaa63 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -225,9 +225,16 @@ Series-changes: n to update the log there and then, knowing that the script will do the rest.
-Cc: Their Name <email> + Cc: Their Name <email> This copies a single patch to another email address.
+Series-process-log: sort, uniq + This tells patman to sort and/or uniq the change logs. It is + assumed that each change log entry is only a single line long. + Use 'sort' to sort the entries, and 'uniq' to include only + unique entries. If omitted, no change log processing is done. + Separate each tag with a comma. + Various other tags are silently removed, like these Chrome OS and Gerrit tags:
diff --git a/tools/patman/patchstream.py b/tools/patman/patchstream.py index a4f2f31..9d8a918 100644 --- a/tools/patman/patchstream.py +++ b/tools/patman/patchstream.py @@ -46,7 +46,7 @@ re_cover = re.compile('^Cover-letter:') re_cover_cc = re.compile('^Cover-letter-cc: *(.*)')
# Patch series tag -re_series = re.compile('^Series-(\w*): *(.*)') +re_series = re.compile('^Series-([a-z-]*): *(.*)')
# Commit tags that we want to collect and keep re_tag = re.compile('^(Tested-by|Acked-by|Reviewed-by|Cc): (.*)') diff --git a/tools/patman/series.py b/tools/patman/series.py index eb5a00c..783b3dd 100644 --- a/tools/patman/series.py +++ b/tools/patman/series.py @@ -28,7 +28,7 @@ import terminal
# Series-xxx tags that we understand valid_series = ['to', 'cc', 'version', 'changes', 'prefix', 'notes', 'name', - 'cover-cc'] + 'cover-cc', 'process_log']
class Series(dict): """Holds information about a patch series, including all tags. @@ -167,15 +167,20 @@ class Series(dict): etc. """ final = [] + process_it = self.get('process_log', '').split(',') + process_it = [item.strip() for item in process_it] need_blank = False for change in sorted(self.changes, reverse=True): out = [] for this_commit, text in self.changes[change]: if commit and this_commit != commit: continue - out.append(text) + if 'uniq' not in process_it or text not in out: + out.append(text) line = 'Changes in v%d:' % change have_changes = len(out) > 0 + if 'sort' in process_it: + out = sorted(out) if have_changes: out.insert(0, line) else:
Simon,
On Tue, Mar 26, 2013 at 4:09 PM, Simon Glass sjg@chromium.org wrote:
For some series with lots of changes it is annoying that duplicate change log items are not caught. It is also helpful sometimes to sort the change logs.
Add a Series-process-log tag to enable this, which can be placed in a commit to control this.
The change to the Cc: line is to fix a checkpatch warning.
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v2:
- Require sort, uniq tags to be comma-separated
tools/patman/README | 9 ++++++++- tools/patman/patchstream.py | 2 +- tools/patman/series.py | 9 +++++++-- 3 files changed, 16 insertions(+), 4 deletions(-)
Reviewed-by: Doug Anderson dianders@chromium.org
A few of the help messages are not quite right, and there is a typo in the README. Fix these.
Signed-off-by: Simon Glass sjg@chromium.org Reviewed-by: Doug Anderson dianders@chromium.org --- Changes in v2: None
tools/patman/README | 2 +- tools/patman/patman.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/patman/README b/tools/patman/README index 0bdaa63..8cffcd1 100644 --- a/tools/patman/README +++ b/tools/patman/README @@ -68,7 +68,7 @@ will get a consistent result each time. How to configure it ===================
-For most cases of using patman for U-Boot developement patman will +For most cases of using patman for U-Boot development, patman will locate and use the file 'doc/git-mailrc' in your U-Boot directory. This contains most of the aliases you will need.
diff --git a/tools/patman/patman.py b/tools/patman/patman.py index 7a86b43..a8061a9 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -52,7 +52,7 @@ parser.add_option('-i', '--ignore-errors', action='store_true', dest='ignore_errors', default=False, help='Send patches email even if patch errors are found') parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', - default=False, help="Do a try run (create but don't email patches)") + 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]") @@ -77,7 +77,7 @@ parser.add_option('--no-tags', action='store_false', dest='process_tags', parser.usage = """patman [options]
Create patches from commits in a branch, check them and email them as -specified by tags you place in the commits. Use -n to """ +specified by tags you place in the commits. Use -n to do a dry run first."""
# Parse options twice: first to get the project and second to handle
participants (2)
-
Doug Anderson -
Simon Glass