From patchwork Thu Apr 9 21:08:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237603 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:53 -0600 Subject: [PATCH v2 26/28] buildman: Add an option to ignore migration warnings In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.26.Ic29734149ff51c5ab2d5d02ed274ea50ddc33372@changeid> These are becoming more common now. They cause boards to show warnings which can be mistaking for compiler warnings. Add a buildman option to ignore them. This option works only with the summary option (-s). It does not affect the build process. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to ignore migration warnings tools/buildman/README | 2 +- tools/buildman/builder.py | 12 +++- tools/buildman/cmdline.py | 5 +- tools/buildman/control.py | 3 +- tools/buildman/test.py | 120 ++++++++++++++++++++++++++++---------- 5 files changed, 107 insertions(+), 35 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index 0663ec33a5b..fe76d4ce0eb 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -1125,7 +1125,7 @@ warnings will produce success (since 129 is changed to 0). If there are both warnings and errors, errors win, so buildman returns 128. The -y option is provided (for use with -s) to ignore the bountiful device-tree -warnings. +warnings. Similarly, -Y tells buildman to ignore the migration warnings. How to change from MAKEALL diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 45569aa1f81..30ebe1d820a 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -306,6 +306,8 @@ class Builder: 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._re_migration_warning = re.compile(r'^={21} WARNING ={22}\n.*\n=+\n', + re.MULTILINE | re.DOTALL) self.queue = queue.Queue() self.out_queue = queue.Queue() @@ -338,7 +340,8 @@ class Builder: def SetDisplayOptions(self, show_errors=False, show_sizes=False, show_detail=False, show_bloat=False, list_error_boards=False, show_config=False, - show_environment=False, filter_dtb_warnings=False): + show_environment=False, filter_dtb_warnings=False, + filter_migration_warnings=False): """Setup display options for the builder. Args: @@ -351,6 +354,8 @@ class Builder: show_environment: Show environment deltas filter_dtb_warnings: Filter out any warnings from the device-tree compiler + filter_migration_warnings: Filter out any warnings about migrating + a board to driver model """ self._show_errors = show_errors self._show_sizes = show_sizes @@ -360,6 +365,7 @@ class Builder: self._show_config = show_config self._show_environment = show_environment self._filter_dtb_warnings = filter_dtb_warnings + self._filter_migration_warnings = filter_migration_warnings def _AddTimestamp(self): """Add a new timestamp to the list and record the build period. @@ -559,6 +565,10 @@ class Builder: New list with only interesting lines included """ out_lines = [] + if self._filter_migration_warnings: + text = '\n'.join(lines) + text = self._re_migration_warning.sub('', text) + lines = text.splitlines() for line in lines: if self.re_make_err.search(line): continue diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 8510c077f74..1377b9d2bec 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -116,7 +116,10 @@ def ParseArgs(): help='Specify a list of boards to exclude, separated by comma') parser.add_option('-y', '--filter-dtb-warnings', action='store_true', default=False, - help='Filter out device-tree-compiler warnings from output') + help='Filter out device-tree-compiler warnings from output') + parser.add_option('-Y', '--filter-migration-warnings', action='store_true', + default=False, + help='Filter out migration warnings from output') parser.usage += """ [list of target/arch/cpu/board/vendor/soc to build] diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 45d9ab73ceb..07f47a54454 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -353,7 +353,8 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, builder.SetDisplayOptions( options.show_errors, options.show_sizes, options.show_detail, options.show_bloat, options.list_error_boards, options.show_config, - options.show_environment, options.filter_dtb_warnings) + options.show_environment, options.filter_dtb_warnings, + options.filter_migration_warnings) if options.summary: builder.ShowSummary(commits, board_selected) else: diff --git a/tools/buildman/test.py b/tools/buildman/test.py index e6698ce86ca..d32b22653fb 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -36,6 +36,14 @@ main: /usr/sbin x86: i386 x86_64 ''' +migration = '''===================== WARNING ====================== +This board does not use CONFIG_DM. CONFIG_DM will be +compulsory starting with the v2020.01 release. +Failure to update may result in board removal. +See doc/driver-model/migration.rst for more info. +==================================================== +''' + errors = [ '''main.c: In function 'main_loop': main.c:260:6: warning: unused variable 'joe' [-Wunused-variable] @@ -79,13 +87,14 @@ make: *** [sub-make] Error 2 # hash, subject, return code, list of errors/warnings commits = [ - ['1234', 'upstream/master, ok', 0, []], + ['1234', 'upstream/master, migration warning', 0, []], ['5678', 'Second commit, a warning', 0, errors[0:1]], ['9012', 'Third commit, error', 1, errors[0:2]], ['3456', 'Fourth commit, warning', 0, [errors[0], errors[2]]], ['7890', 'Fifth commit, link errors', 1, [errors[0], errors[3]]], ['abcd', 'Sixth commit, fixes all errors', 0, []], - ['ef01', 'Seventh commit, check directory suppression', 1, [errors[4]]], + ['ef01', 'Seventh commit, fix migration, check directory suppression', 1, + [errors[4]]], ] boards = [ @@ -118,6 +127,8 @@ class TestBuild(unittest.TestCase): comm.subject = commit_info[1] comm.return_code = commit_info[2] comm.error_list = commit_info[3] + if sequence < 6: + comm.error_list += [migration] comm.sequence = sequence sequence += 1 self.commits.append(comm) @@ -162,6 +173,8 @@ class TestBuild(unittest.TestCase): result.return_code = commit.return_code result.stderr = (''.join(commit.error_list) % {'basedir' : self.base_dir + '/.bm-work/00/'}) + elif commit.sequence < 6: + result.stderr = migration result.combined = result.stdout + result.stderr return result @@ -214,7 +227,9 @@ class TestBuild(unittest.TestCase): terminal.EchoPrintTestLines() return iter(terminal.GetPrintTestLines()) - def _CheckOutput(self, lines, list_error_boards, filter_dtb_warnings): + def _CheckOutput(self, lines, list_error_boards=False, + filter_dtb_warnings=False, + filter_migration_warnings=False): """Check for expected output from the build summary Args: @@ -251,24 +266,38 @@ class TestBuild(unittest.TestCase): new_lines.append(expect) return '\n'.join(new_lines) + col = terminal.Color() + boards01234 = ('board0 board1 board2 board3 board4' + if list_error_boards else '') boards1234 = 'board1 board2 board3 board4' if list_error_boards else '' boards234 = 'board2 board3 board4' if list_error_boards else '' boards34 = 'board3 board4' if list_error_boards else '' boards4 = 'board4' if list_error_boards else '' - # Upstream commit: no errors + # Upstream commit: migration warnings only self.assertEqual(next(lines).text, '01: %s' % commits[0][1]) + if not filter_migration_warnings: + self.assertSummary(next(lines).text, 'arm', 'w+', + ['board0', 'board1'], outcome=OUTCOME_WARN) + self.assertSummary(next(lines).text, 'powerpc', 'w+', + ['board2', 'board3'], outcome=OUTCOME_WARN) + self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], + outcome=OUTCOME_WARN) + + self.assertEqual(next(lines).text, + add_line_prefix('+', boards01234, migration, col.RED)) + # Second commit: all archs should fail with warnings self.assertEqual(next(lines).text, '02: %s' % commits[1][1]) - col = terminal.Color() - self.assertSummary(next(lines).text, 'arm', 'w+', ['board1'], - outcome=OUTCOME_WARN) - self.assertSummary(next(lines).text, 'powerpc', 'w+', - ['board2', 'board3'], outcome=OUTCOME_WARN) - self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], - outcome=OUTCOME_WARN) + if filter_migration_warnings: + self.assertSummary(next(lines).text, 'arm', 'w+', + ['board1'], outcome=OUTCOME_WARN) + self.assertSummary(next(lines).text, 'powerpc', 'w+', + ['board2', 'board3'], outcome=OUTCOME_WARN) + self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], + outcome=OUTCOME_WARN) # Second commit: The warnings should be listed self.assertEqual(next(lines).text, @@ -276,8 +305,9 @@ class TestBuild(unittest.TestCase): # Third commit: Still fails self.assertEqual(next(lines).text, '03: %s' % commits[2][1]) - self.assertSummary(next(lines).text, 'arm', '', ['board1'], - outcome=OUTCOME_OK) + if filter_migration_warnings: + self.assertSummary(next(lines).text, 'arm', '', + ['board1'], outcome=OUTCOME_OK) self.assertSummary(next(lines).text, 'powerpc', '+', ['board2', 'board3']) self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) @@ -288,16 +318,20 @@ class TestBuild(unittest.TestCase): # Fourth commit: Compile errors are fixed, just have warning for board3 self.assertEqual(next(lines).text, '04: %s' % commits[3][1]) - 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(next(lines).text, expect) + if filter_migration_warnings: + 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(next(lines).text, expect) + else: + self.assertSummary(next(lines).text, 'powerpc', 'w+', + ['board2', 'board3'], outcome=OUTCOME_WARN) self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], - outcome=OUTCOME_WARN) + outcome=OUTCOME_WARN) # Compile error fixed self.assertEqual(next(lines).text, @@ -310,8 +344,9 @@ class TestBuild(unittest.TestCase): # Fifth commit self.assertEqual(next(lines).text, '05: %s' % commits[4][1]) - self.assertSummary(next(lines).text, 'powerpc', '', ['board3'], - outcome=OUTCOME_OK) + if filter_migration_warnings: + self.assertSummary(next(lines).text, 'powerpc', '', ['board3'], + outcome=OUTCOME_OK) self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) # The second line of errors[3] is a duplicate, so buildman will drop it @@ -328,8 +363,12 @@ class TestBuild(unittest.TestCase): # Sixth commit self.assertEqual(next(lines).text, '06: %s' % commits[5][1]) - self.assertSummary(next(lines).text, 'sandbox', '', ['board4'], - outcome=OUTCOME_OK) + if filter_migration_warnings: + self.assertSummary(next(lines).text, 'sandbox', '', ['board4'], + outcome=OUTCOME_OK) + else: + self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], + outcome=OUTCOME_WARN) # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') @@ -342,12 +381,24 @@ class TestBuild(unittest.TestCase): # Seventh commit self.assertEqual(next(lines).text, '07: %s' % commits[6][1]) - self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) + if filter_migration_warnings: + self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) + else: + self.assertSummary(next(lines).text, 'arm', '', ['board0', 'board1'], + outcome=OUTCOME_OK) + self.assertSummary(next(lines).text, 'powerpc', '', + ['board2', 'board3'], outcome=OUTCOME_OK) + self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) # Pick out the correct error lines expect_str = errors[4].rstrip().replace('%(basedir)s', '').split('\n') expect = expect_str[3:8] + [expect_str[-1]] expect = '\n'.join(expect) + if not filter_migration_warnings: + self.assertEqual( + next(lines).text, + add_line_prefix('-', boards01234, migration, col.GREEN)) + self.assertEqual(next(lines).text, add_line_prefix('+', boards4, expect, col.RED)) @@ -372,8 +423,7 @@ class TestBuild(unittest.TestCase): This does a line-by-line verification of the summary output. """ lines = self._SetupTest(show_errors=True, list_error_boards=True) - self._CheckOutput(lines, list_error_boards=True, - filter_dtb_warnings=False) + self._CheckOutput(lines, list_error_boards=True) def testFilterDtb(self): """Test output with --filter-dtb-warnings @@ -381,8 +431,16 @@ class TestBuild(unittest.TestCase): This does a line-by-line verification of the summary output. """ lines = self._SetupTest(show_errors=True, filter_dtb_warnings=True) - self._CheckOutput(lines, list_error_boards=False, - filter_dtb_warnings=True) + self._CheckOutput(lines, filter_dtb_warnings=True) + + def testFilterMigration(self): + """Test output with --filter-migration-warnings + + This does a line-by-line verification of the summary output. + """ + lines = self._SetupTest(show_errors=True, + filter_migration_warnings=True) + self._CheckOutput(lines, filter_migration_warnings=True) def _testGit(self): """Test basic builder operation by building a branch"""