From d29fe6e2d2cbcbfd1bfaaf886818d84edde0fc0d Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 26 Mar 2013 13:09:39 +0000 Subject: [PATCH] patman: Fix up checkpatch parsing to deal with 'CHECK' lines 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 Reviewed-by: Doug Anderson --- 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 d3a0477bbf..83aaf710ba 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', ''), item.get('file', ''), - 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 f801cedc7b..8cd26471c3 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 \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) -- 2.30.2