* [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-04-01 23:16 ` Doug Anderson
2013-03-26 23:09 ` [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags Simon Glass
` (5 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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)
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines
2013-03-26 23:09 ` [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines Simon Glass
@ 2013-04-01 23:16 ` Doug Anderson
0 siblings, 0 replies; 12+ messages in thread
From: Doug Anderson @ 2013-04-01 23:16 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-04-01 23:22 ` Doug Anderson
2013-03-26 23:09 ` [U-Boot] [PATCH v2 3/7] patman: Fix the comment in CheckTags to mention multiple tags Simon Glass
` (4 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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.
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags
2013-03-26 23:09 ` [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags Simon Glass
@ 2013-04-01 23:22 ` Doug Anderson
0 siblings, 0 replies; 12+ messages in thread
From: Doug Anderson @ 2013-04-01 23:22 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 3/7] patman: Fix the comment in CheckTags to mention multiple tags
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 1/7] patman: Fix up checkpatch parsing to deal with 'CHECK' lines Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 2/7] patman: Don't allow spaces in tags Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases Simon Glass
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
` (2 preceding siblings ...)
2013-03-26 23:09 ` [U-Boot] [PATCH v2 3/7] patman: Fix the comment in CheckTags to mention multiple tags Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-04-02 0:03 ` Doug Anderson
2013-03-26 23:09 ` [U-Boot] [PATCH v2 5/7] patman: Add -a option to refrain from test-applying the patches Simon Glass
` (2 subsequent siblings)
6 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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 at napier.co.nz" --cc \
"m.poppins at 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 at napier.co.nz" --cc \
"m.poppins at 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 at 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 at napier.co.nz" --cc \
"f.bloggs at napier.co.nz" --cc "j.bloggs at napier.co.nz" --cc \
"m.poppins at 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 at napier.co.nz', 'm.poppins at 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)
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases
2013-03-26 23:09 ` [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases Simon Glass
@ 2013-04-02 0:03 ` Doug Anderson
0 siblings, 0 replies; 12+ messages in thread
From: Doug Anderson @ 2013-04-02 0:03 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 5/7] patman: Add -a option to refrain from test-applying the patches
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
` (3 preceding siblings ...)
2013-03-26 23:09 ` [U-Boot] [PATCH v2 4/7] patman: Provide option to ignore bad aliases Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs Simon Glass
2013-03-26 23:09 ` [U-Boot] [PATCH v2 7/7] patman: Minor help message/README fixes Simon Glass
6 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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)
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
` (4 preceding siblings ...)
2013-03-26 23:09 ` [U-Boot] [PATCH v2 5/7] patman: Add -a option to refrain from test-applying the patches Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
2013-04-02 0:06 ` Doug Anderson
2013-03-26 23:09 ` [U-Boot] [PATCH v2 7/7] patman: Minor help message/README fixes Simon Glass
6 siblings, 1 reply; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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:
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread* [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs
2013-03-26 23:09 ` [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs Simon Glass
@ 2013-04-02 0:06 ` Doug Anderson
0 siblings, 0 replies; 12+ messages in thread
From: Doug Anderson @ 2013-04-02 0:06 UTC (permalink / raw)
To: u-boot
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>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [U-Boot] [PATCH v2 7/7] patman: Minor help message/README fixes
2013-03-26 23:09 [U-Boot] [PATCH v2 0/7] Various patman fixes Simon Glass
` (5 preceding siblings ...)
2013-03-26 23:09 ` [U-Boot] [PATCH v2 6/7] patman: Add Series-process-log tag to sort/uniq change logs Simon Glass
@ 2013-03-26 23:09 ` Simon Glass
6 siblings, 0 replies; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
To: u-boot
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
--
1.8.1.3
^ permalink raw reply related [flat|nested] 12+ messages in thread