From 870bd567994b4455c9f68c3bc30fea6499fe5f54 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:04 -0600 Subject: patman: Fix 'warning' typo If no warnings are detected due to checkpatch having unexpected options, patman currently shows an error: TypeError: unsupported operand type(s) for +=: 'int' and 'property' Fix this by initing the variable correctly. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 795b519314..a2611a8a82 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -59,7 +59,7 @@ def CheckPatch(fname, verbose=False): 'stdout'] result = collections.namedtuple('CheckPatchResult', fields) result.ok = False - result.errors, result.warning, result.checks = 0, 0, 0 + result.errors, result.warnings, result.checks = 0, 0, 0 result.lines = 0 result.problems = [] chk = FindCheckPatch() -- cgit v1.2.3 From 7d5b5e8965b3f0bdf91665cf654b9eaeb9b30e52 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:05 -0600 Subject: patman: Support emacs mode with checkpatch If checkpatch is run in 'emacs' mode it shows the filename at the start of each line. Add support for this so that the warnings and errors are correctly detected. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index a2611a8a82..16d534b6ae 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -72,13 +72,17 @@ def CheckPatch(fname, verbose=False): # 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+)' + emacs_prefix = '(?:[0-9]{4}.*\.patch:[0-9]+: )?' + emacs_stats = '(?:[0-9]{4}.*\.patch )?' + re_stats = re.compile(emacs_stats + + 'total: (\\d+) errors, (\d+) warnings, (\d+)') + re_stats_full = re.compile(emacs_stats + + '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_warning = re.compile(emacs_prefix + 'WARNING:(?:[A-Z_]+:)? (.*)') re_check = re.compile('CHECK: (.*)') re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):') -- cgit v1.2.3 From 7175b085e1213c99fbc237483b33cd52ff00f891 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:06 -0600 Subject: patman: Don't try to process checkpatch lines twice Once we have determined what the line refers to there is no point in processing it further. Update the logic to continue to the next line in these cases. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 16d534b6ae..2cfa9778c6 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -91,9 +91,11 @@ def CheckPatch(fname, verbose=False): print(line) # A blank line indicates the end of a message - if not line and item: - result.problems.append(item) - item = {} + if not line: + if item: + result.problems.append(item) + item = {} + continue match = re_stats_full.match(line) if not match: match = re_stats.match(line) @@ -105,10 +107,13 @@ def CheckPatch(fname, verbose=False): result.lines = int(match.group(4)) else: result.lines = int(match.group(3)) + continue elif re_ok.match(line): result.ok = True + continue elif re_bad.match(line): result.ok = False + continue err_match = re_error.match(line) warn_match = re_warning.match(line) file_match = re_file.match(line) -- cgit v1.2.3 From 666eb15e923e93872dbca1b834ce123a327fc9b3 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:07 -0600 Subject: patman: Handle checkpatch output with notes and code If checkpatch is configured to output code we should ignore it. Similarly, notes should be ignored. Update the logic to handle these situations. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 2cfa9778c6..5426bb9e9e 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -85,7 +85,8 @@ def CheckPatch(fname, verbose=False): re_warning = re.compile(emacs_prefix + 'WARNING:(?:[A-Z_]+:)? (.*)') re_check = re.compile('CHECK: (.*)') re_file = re.compile('#\d+: FILE: ([^:]*):(\d+):') - + re_note = re.compile('NOTE: (.*)') + indent = ' ' * 6 for line in result.stdout.splitlines(): if verbose: print(line) @@ -96,6 +97,14 @@ def CheckPatch(fname, verbose=False): result.problems.append(item) item = {} continue + if re_note.match(line): + continue + # Skip lines which quote code + if line.startswith(indent): + continue + # Skip code quotes and # + if line.startswith('+') or line.startswith('#'): + continue match = re_stats_full.match(line) if not match: match = re_stats.match(line) -- cgit v1.2.3 From 37f3bb53b78d13f15634a976ebbfa36499d74290 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:08 -0600 Subject: patman: Support warnings in the patch subject Sometimes checkpatch outputs problems in the patch subject. Add support for parsing this output and reporting it correctly. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 5426bb9e9e..5ae450d771 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -127,6 +127,7 @@ def CheckPatch(fname, verbose=False): warn_match = re_warning.match(line) file_match = re_file.match(line) check_match = re_check.match(line) + subject_match = line.startswith('Subject:') if err_match: item['msg'] = err_match.group(1) item['type'] = 'error' @@ -139,6 +140,9 @@ def CheckPatch(fname, verbose=False): elif file_match: item['file'] = file_match.group(1) item['line'] = int(file_match.group(2)) + elif subject_match: + item['file'] = '' + item['line'] = None return result @@ -157,7 +161,8 @@ def GetWarningMsg(col, msg_type, fname, line, msg): msg_type = col.Color(col.RED, msg_type) elif msg_type == 'check': msg_type = col.Color(col.MAGENTA, msg_type) - return '%s:%d: %s: %s\n' % (fname, line, msg_type, msg) + line_str = '' if line is None else '%d' % line + return '%s:%s: %s: %s\n' % (fname, line_str, msg_type, msg) def CheckPatches(verbose, args): '''Run the checkpatch.pl script on each patch''' -- cgit v1.2.3 From 96daa41aab97f87f1a7b2d8bf00ee546db2ca156 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Wed, 6 May 2020 16:29:09 -0600 Subject: patman: Complain if a checkpatch line is not understood Rather than suffering in silence, output a warning if something about the checkpatch output cannot be understood. Signed-off-by: Simon Glass --- tools/patman/checkpatch.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'tools') diff --git a/tools/patman/checkpatch.py b/tools/patman/checkpatch.py index 5ae450d771..98c63af1dd 100644 --- a/tools/patman/checkpatch.py +++ b/tools/patman/checkpatch.py @@ -143,6 +143,8 @@ def CheckPatch(fname, verbose=False): elif subject_match: item['file'] = '' item['line'] = None + else: + print('bad line "%s", %d' % (line, len(line))) return result -- cgit v1.2.3 From 7664b03ffc51fa7563969c61fa28bf656af441bd Mon Sep 17 00:00:00 2001 From: Ovidiu Panait Date: Fri, 15 May 2020 09:30:12 +0300 Subject: buildman: Remove _of_#_ from results directory paths Currently, the following scenario will rebuild the first commit even though it is not really necessary - the commit sha or the position in the patchset did not change: $ git am $ tools/buildman/buildman -P -E -W -b master mx6 $ git am $ tools/buildman/buildman -P -E -W -b master mx6 <- will rebuild the first commit as well, even though nothing has changed about it. This is due to the fact that previous results directories get removed when the number of commits change. By removing the _of_#_ part of the directory path, the commits will be rebuilt only if the commit sha or the position in the patchset changes. Also, update the testcase to reflect this change. Signed-off-by: Ovidiu Panait --- tools/buildman/builder.py | 10 +++++----- tools/buildman/test.py | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) (limited to 'tools') diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index f8e71de427..f2756ea666 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -70,12 +70,12 @@ As an example, say we are building branch 'us-net' for boards 'sandbox' and like this: us-net/ base directory - 01_of_02_g4ed4ebc_net--Add-tftp-speed-/ + 01_g4ed4ebc_net--Add-tftp-speed-/ sandbox/ u-boot.bin seaboard/ u-boot.bin - 02_of_02_g4ed4ebc_net--Check-tftp-comp/ + 02_g4ed4ebc_net--Check-tftp-comp/ sandbox/ u-boot.bin seaboard/ @@ -487,8 +487,8 @@ class Builder: commit = self.commits[commit_upto] subject = commit.subject.translate(trans_valid_chars) # See _GetOutputSpaceRemovals() which parses this name - commit_dir = ('%02d_of_%02d_g%s_%s' % (commit_upto + 1, - self.commit_count, commit.hash, subject[:20])) + commit_dir = ('%02d_g%s_%s' % (commit_upto + 1, + commit.hash, subject[:20])) elif not self.no_subdirs: commit_dir = 'current' if not commit_dir: @@ -1599,7 +1599,7 @@ class Builder: for dirname in glob.glob(os.path.join(self.base_dir, '*')): if dirname not in dir_list: leaf = dirname[len(self.base_dir) + 1:] - m = re.match('[0-9]+_of_[0-9]+_g[0-9a-f]+_.*', leaf) + m = re.match('[0-9]+_g[0-9a-f]+_.*', leaf) if m: to_remove.append(dirname) return to_remove diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 40811ba9f9..82d25cfcaa 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -541,7 +541,7 @@ class TestBuild(unittest.TestCase): build.commits = self.commits build.commit_count = len(self.commits) subject = self.commits[1].subject.translate(builder.trans_valid_chars) - dirname ='/%02d_of_%02d_g%s_%s' % (2, build.commit_count, commits[1][0], + dirname ='/%02d_g%s_%s' % (2, build.commit_count, commits[1][0], subject[:20]) self.CheckDirs(build, dirname) @@ -609,9 +609,9 @@ class TestBuild(unittest.TestCase): base_dir = tempfile.mkdtemp() # Add various files that we want removed and left alone - to_remove = ['01_of_22_g0982734987_title', '102_of_222_g92bf_title', - '01_of_22_g2938abd8_title'] - to_leave = ['something_else', '01-something.patch', '01_of_22_another'] + to_remove = ['01_g0982734987_title', '102_g92bf_title', + '01_g2938abd8_title'] + to_leave = ['something_else', '01-something.patch', '01_another'] for name in to_remove + to_leave: _Touch(name) -- cgit v1.2.3