All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v2 0/7] Various patman fixes
@ 2013-03-26 23:09 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
                   ` (6 more replies)
  0 siblings, 7 replies; 12+ messages in thread
From: Simon Glass @ 2013-03-26 23:09 UTC (permalink / raw)
  To: u-boot

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

-- 
1.8.1.3

^ permalink raw reply	[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 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 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 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 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 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

* [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 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 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 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

end of thread, other threads:[~2013-04-02  0:06 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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
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
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
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 ` [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
2013-03-26 23:09 ` [U-Boot] [PATCH v2 7/7] patman: Minor help message/README fixes Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.