From 40232c91d70f4f2066408cabf1afba1deb190df6 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 16:02:10 -0700 Subject: buildman: Only print toolchain probing with -v At present --list-tool-chains prints a lot of information about the toolchain-probing process. This is generally not very interesting. Update buildman to print this only if --list-tool-chains is given with -v. Signed-off-by: Simon Glass --- tools/buildman/cmdline.py | 2 +- tools/buildman/control.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'tools') diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 49a8a13118..93d09ca08d 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -66,7 +66,7 @@ def ParseArgs(): parser.add_option('-l', '--list-error-boards', action='store_true', default=False, help='Show a list of boards next to each error/warning') parser.add_option('--list-tool-chains', action='store_true', default=False, - help='List available tool chains') + help='List available tool chains (use -v to see probing detail)') parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run', default=False, help="Do a dry run (describe actions, but do nothing)") parser.add_option('-N', '--no-subdirs', action='store_true', dest='no_subdirs', diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 96f8ccfe07..c900211510 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -164,7 +164,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, if no_toolchains: toolchains.GetSettings() - toolchains.Scan(options.list_tool_chains) + toolchains.Scan(options.list_tool_chains and options.verbose) if options.list_tool_chains: toolchains.List() print -- cgit v1.2.1 From 2d48333e447e8cc5a2bd3eaed6be657925cbee16 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 16:02:11 -0700 Subject: buildman: Detect dtc warnings At present messages from the device-tree compiler like this: arch/arm/dts/socfpga_arria10_socdk_sdmmc.dtb: Warning (avoid_unnecessary_addr_size): /clocks: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property are detected as errors since they don't match the gcc warning regex. Add a new one for dtc to fix this. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 4 +++- tools/buildman/test.py | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 05f8299541..fc80705a45 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -290,6 +290,7 @@ class Builder: self._re_function = re.compile('(.*): In function.*') self._re_files = re.compile('In file included from.*') self._re_warning = re.compile('(.*):(\d*):(\d*): warning: .*') + self._re_dtb_warning = re.compile('(.*): Warning .*') self._re_note = re.compile('(.*):(\d*):(\d*): note: this is the location of the previous.*') self.queue = Queue.Queue() @@ -788,7 +789,8 @@ class Builder: self._re_files.match(line)): last_func = line else: - is_warning = self._re_warning.match(line) + is_warning = (self._re_warning.match(line) or + self._re_dtb_warning.match(line)) is_note = self._re_note.match(line) if is_warning or (last_was_warning and is_note): if last_func: diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 61a462655f..5c82c50793 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -46,8 +46,9 @@ make[1]: *** [main.o] Error 1 make: *** [common/libcommon.o] Error 2 Make failed ''', - '''main.c: In function 'main_loop3': -main.c:280:6: warning: unused variable 'mary' [-Wunused-variable] + '''arch/arm/dts/socfpga_arria10_socdk_sdmmc.dtb: Warning \ +(avoid_unnecessary_addr_size): /clocks: unnecessary #address-cells/#size-cells \ +without "ranges" or child "reg" property ''', '''powerpc-linux-ld: warning: dot moved backwards before `.bss' powerpc-linux-ld: warning: dot moved backwards before `.bss' -- cgit v1.2.1 From 4cf2b221c6f283aa0fb646cf637ae08fc90dd6d2 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 16:02:12 -0700 Subject: buildman: Rename the good, better, worse variables At present we don't distinguish between errors and warnings when printing the architecture summary. Rename the variables to better describe their purpose. 'Worse' at present means we got an error, so use that as the name. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'tools') diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index fc80705a45..d9d86ef6a3 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1196,10 +1196,10 @@ class Builder: Print(' ' + line, newline=True, colour=col) - better = [] # List of boards fixed since last commit - worse = [] # List of new broken boards since last commit - new = [] # List of boards that didn't exist last time - unknown = [] # List of boards that were not built + ok_boards = [] # List of boards fixed since last commit + err_boards = [] # List of new broken boards since last commit + new_boards = [] # List of boards that didn't exist last time + unknown_boards = [] # List of boards that were not built for target in board_dict: if target not in board_selected: @@ -1210,13 +1210,13 @@ class Builder: base_outcome = self._base_board_dict[target].rc outcome = board_dict[target] if outcome.rc == OUTCOME_UNKNOWN: - unknown.append(target) + unknown_boards.append(target) elif outcome.rc < base_outcome: - better.append(target) + ok_boards.append(target) elif outcome.rc > base_outcome: - worse.append(target) + err_boards.append(target) else: - new.append(target) + new_boards.append(target) # Get a list of errors that have appeared, and disappeared better_err, worse_err = _CalcErrorDelta(self._base_err_lines, @@ -1225,16 +1225,16 @@ class Builder: self._base_warn_line_boards, warn_lines, warn_line_boards, 'w') # Display results by arch - if (better or worse or unknown or new or worse_err or better_err - or worse_warn or better_warn): + if any((ok_boards, err_boards, unknown_boards, new_boards, worse_err, + better_err, worse_warn, better_warn)): arch_list = {} - self.AddOutcome(board_selected, arch_list, better, '', + self.AddOutcome(board_selected, arch_list, ok_boards, '', self.col.GREEN) - self.AddOutcome(board_selected, arch_list, worse, '+', + self.AddOutcome(board_selected, arch_list, err_boards, '+', self.col.RED) - self.AddOutcome(board_selected, arch_list, new, '*', self.col.BLUE) + self.AddOutcome(board_selected, arch_list, new_boards, '*', self.col.BLUE) if self._show_unknown: - self.AddOutcome(board_selected, arch_list, unknown, '?', + self.AddOutcome(board_selected, arch_list, unknown_boards, '?', self.col.MAGENTA) for arch, target_list in arch_list.iteritems(): Print('%10s: %s' % (arch, target_list)) -- cgit v1.2.1 From 6af7101b75d0e30cf7ed7d1f57fdb68ed5f8ffa0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 16:02:13 -0700 Subject: buildman: Show boards with warning with w+ At present we should boards with warnings in the same way as those with errors. This is not ideal. Add a new 'warn' state and show these listed in yellow to match the actual warning lines printing with -e. Signed-off-by: Simon Glass --- tools/buildman/builder.py | 17 ++++++++++---- tools/buildman/test.py | 56 ++++++++++++++++++++++++++++++++++------------- 2 files changed, 54 insertions(+), 19 deletions(-) (limited to 'tools') diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index d9d86ef6a3..6a6c83bf33 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1197,6 +1197,7 @@ class Builder: ok_boards = [] # List of boards fixed since last commit + warn_boards = [] # List of boards with warnings since last commit err_boards = [] # List of new broken boards since last commit new_boards = [] # List of boards that didn't exist last time unknown_boards = [] # List of boards that were not built @@ -1212,9 +1213,15 @@ class Builder: if outcome.rc == OUTCOME_UNKNOWN: unknown_boards.append(target) elif outcome.rc < base_outcome: - ok_boards.append(target) + if outcome.rc == OUTCOME_WARNING: + warn_boards.append(target) + else: + ok_boards.append(target) elif outcome.rc > base_outcome: - err_boards.append(target) + if outcome.rc == OUTCOME_WARNING: + warn_boards.append(target) + else: + err_boards.append(target) else: new_boards.append(target) @@ -1225,11 +1232,13 @@ class Builder: self._base_warn_line_boards, warn_lines, warn_line_boards, 'w') # Display results by arch - if any((ok_boards, err_boards, unknown_boards, new_boards, worse_err, - better_err, worse_warn, better_warn)): + if any((ok_boards, warn_boards, err_boards, unknown_boards, new_boards, + worse_err, better_err, worse_warn, better_warn)): arch_list = {} self.AddOutcome(board_selected, arch_list, ok_boards, '', self.col.GREEN) + self.AddOutcome(board_selected, arch_list, warn_boards, 'w+', + self.col.YELLOW) self.AddOutcome(board_selected, arch_list, err_boards, '+', self.col.RED) self.AddOutcome(board_selected, arch_list, new_boards, '*', self.col.BLUE) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 5c82c50793..de02f61be6 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -97,6 +97,8 @@ boards = [ BASE_DIR = 'base' +OUTCOME_OK, OUTCOME_WARN, OUTCOME_ERR = range(3) + class Options: """Class that holds build options""" pass @@ -166,9 +168,10 @@ class TestBuild(unittest.TestCase): result.combined = result.stdout + result.stderr return result - def assertSummary(self, text, arch, plus, boards, ok=False): + def assertSummary(self, text, arch, plus, boards, outcome=OUTCOME_ERR): col = self._col - expected_colour = col.GREEN if ok else col.RED + expected_colour = (col.GREEN if outcome == OUTCOME_OK else + col.YELLOW if outcome == OUTCOME_WARN else col.RED) expect = '%10s: ' % arch # TODO(sjg@chromium.org): If plus is '', we shouldn't need this expect += ' ' + col.Color(expected_colour, plus) @@ -192,6 +195,8 @@ class TestBuild(unittest.TestCase): build.do_make = self.Make board_selected = self.boards.GetSelectedDict() + # Build the boards for the pre-defined commits and warnings/errors + # associated with each. This calls our Make() to inject the fake output. build.BuildBoards(self.commits, board_selected, keep_outputs=False, verbose=False) lines = terminal.GetPrintTestLines() @@ -207,33 +212,49 @@ class TestBuild(unittest.TestCase): build.ShowSummary(self.commits, board_selected) #terminal.EchoPrintTestLines() lines = terminal.GetPrintTestLines() + + # Upstream commit: no errors self.assertEqual(lines[0].text, '01: %s' % commits[0][1]) + + # Second commit: all archs should fail with warnings self.assertEqual(lines[1].text, '02: %s' % commits[1][1]) - # We expect all archs to fail col = terminal.Color() - self.assertSummary(lines[2].text, 'sandbox', '+', ['board4']) - self.assertSummary(lines[3].text, 'arm', '+', ['board1']) - self.assertSummary(lines[4].text, 'powerpc', '+', ['board2', 'board3']) - - # Now we should have the compiler warning + self.assertSummary(lines[2].text, 'sandbox', 'w+', ['board4'], + outcome=OUTCOME_WARN) + self.assertSummary(lines[3].text, 'arm', 'w+', ['board1'], + outcome=OUTCOME_WARN) + self.assertSummary(lines[4].text, 'powerpc', 'w+', ['board2', 'board3'], + outcome=OUTCOME_WARN) + + # Second commit: The warnings should be listed self.assertEqual(lines[5].text, 'w+%s' % errors[0].rstrip().replace('\n', '\nw+')) self.assertEqual(lines[5].colour, col.MAGENTA) + # Third commit: Still fails self.assertEqual(lines[6].text, '03: %s' % commits[2][1]) self.assertSummary(lines[7].text, 'sandbox', '+', ['board4']) - self.assertSummary(lines[8].text, 'arm', '', ['board1'], ok=True) + self.assertSummary(lines[8].text, 'arm', '', ['board1'], + outcome=OUTCOME_OK) self.assertSummary(lines[9].text, 'powerpc', '+', ['board2', 'board3']) - # Compiler error + # Expect a compiler error self.assertEqual(lines[10].text, '+%s' % errors[1].rstrip().replace('\n', '\n+')) + # Fourth commit: Compile errors are fixed, just have warning for board3 self.assertEqual(lines[11].text, '04: %s' % commits[3][1]) - self.assertSummary(lines[12].text, 'sandbox', '', ['board4'], ok=True) - self.assertSummary(lines[13].text, 'powerpc', '', ['board2', 'board3'], - ok=True) + self.assertSummary(lines[12].text, 'sandbox', 'w+', ['board4'], + outcome=OUTCOME_WARN) + expect = '%10s: ' % 'powerpc' + expect += ' ' + col.Color(col.GREEN, '') + expect += ' ' + expect += col.Color(col.GREEN, ' %s' % 'board2') + expect += ' ' + col.Color(col.YELLOW, 'w+') + expect += ' ' + expect += col.Color(col.YELLOW, ' %s' % 'board3') + self.assertEqual(lines[13].text, expect) # Compile error fixed self.assertEqual(lines[14].text, '-%s' % @@ -244,9 +265,11 @@ class TestBuild(unittest.TestCase): errors[2].rstrip().replace('\n', '\nw+')) self.assertEqual(lines[15].colour, col.MAGENTA) + # Fifth commit self.assertEqual(lines[16].text, '05: %s' % commits[4][1]) self.assertSummary(lines[17].text, 'sandbox', '+', ['board4']) - self.assertSummary(lines[18].text, 'powerpc', '', ['board3'], ok=True) + self.assertSummary(lines[18].text, 'powerpc', '', ['board3'], + outcome=OUTCOME_OK) # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') @@ -257,8 +280,10 @@ class TestBuild(unittest.TestCase): self.assertEqual(lines[20].text, 'w-%s' % errors[2].rstrip().replace('\n', '\nw-')) + # Sixth commit self.assertEqual(lines[21].text, '06: %s' % commits[5][1]) - self.assertSummary(lines[22].text, 'sandbox', '', ['board4'], ok=True) + self.assertSummary(lines[22].text, 'sandbox', '', ['board4'], + outcome=OUTCOME_OK) # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') @@ -269,6 +294,7 @@ class TestBuild(unittest.TestCase): self.assertEqual(lines[24].text, 'w-%s' % errors[0].rstrip().replace('\n', '\nw-')) + # Seventh commit self.assertEqual(lines[25].text, '07: %s' % commits[6][1]) self.assertSummary(lines[26].text, 'sandbox', '+', ['board4']) -- cgit v1.2.1 From a3c005506ac87f3684603fc40ada98070027aaa0 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 15:21:31 -0700 Subject: binman: Add a way to enable debugging from the build When the build fails due to something wrong in binman it is sometimes useful to get a full backtrace showing the location of the failure. Add a BINMAN_DEBUG environment variable to support this along with some documentation. Signed-off-by: Simon Glass --- tools/binman/README | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'tools') diff --git a/tools/binman/README b/tools/binman/README index b64dedf2eb..04ed2b799c 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -723,6 +723,12 @@ If you need to specify a particular device-tree compiler to use, you can define the DTC environment variable. This can be useful when the system dtc is too old. +To enable a full backtrace and other debugging features in binman, pass +BINMAN_DEBUG=1 to your build: + + make sandbox_defconfig + make BINMAN_DEBUG=1 + History / Credits ----------------- -- cgit v1.2.1 From 26cc8fccc61a6846e763c3cf36c28bc547308bbe Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 15:21:32 -0700 Subject: binman: Drop an unnecessary comma in blob handling This comma is not needed. Drop it. Signed-off-by: Simon Glass --- tools/binman/etype/blob.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'tools') diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 642a0e482a..ae80bbee53 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -60,7 +60,7 @@ class Entry_blob(Entry): except AttributeError: data = lz4.compress(data) ''' - data = tools.Run('lz4', '-c', self._pathname, ) + data = tools.Run('lz4', '-c', self._pathname) self.SetContents(data) return True -- cgit v1.2.1 From 4b6dbaa3073adb24f8c67f89d0f70dbcf00808b9 Mon Sep 17 00:00:00 2001 From: Simon Glass Date: Tue, 6 Nov 2018 15:21:33 -0700 Subject: binman: Set the pathname correctly for ELF files At present, stripped files don't have the right pathname which means that blob compression cannot be used. Fix this. Signed-off-by: Simon Glass --- tools/binman/etype/u_boot_elf.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'tools') diff --git a/tools/binman/etype/u_boot_elf.py b/tools/binman/etype/u_boot_elf.py index 134b6cc15b..f83860dc0a 100644 --- a/tools/binman/etype/u_boot_elf.py +++ b/tools/binman/etype/u_boot_elf.py @@ -30,9 +30,8 @@ class Entry_u_boot_elf(Entry_blob): out_fname = tools.GetOutputFilename('%s.stripped' % uniq) tools.WriteFile(out_fname, tools.ReadFile(self._pathname)) tools.Run('strip', out_fname) - self.SetContents(tools.ReadFile(out_fname)) - else: - self.SetContents(tools.ReadFile(self._pathname)) + self._pathname = out_fname + Entry_blob.ReadBlobContents(self) return True def GetDefaultFilename(self): -- cgit v1.2.1