From patchwork Thu Apr 9 21:08:28 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237579 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:28 -0600 Subject: [PATCH v2 01/28] buildman: Refactor error-line output int a function In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.1.I181c89e3a9cb207bb06cabb19ca6c47caf43d176@changeid> Reduce the amount of repeated code by creating an _OutputErrLines() function to hold this code. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 70c55c588a7..621147696ed 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1209,6 +1209,20 @@ class Builder: col = self.col.YELLOW Print(' ' + line, newline=True, colour=col) + def _OutputErrLines(err_lines, colour): + """Output the line of error/warning lines, if not empty + + Also increments self._error_lines if err_lines not empty + + Args: + err_lines: List of strings, each an error or warning line, + possibly including a list of boards with that error/warning + colour: Colour to use for output + """ + if err_lines: + Print('\n'.join(err_lines), colour=colour) + self._error_lines += 1 + ok_boards = [] # List of boards fixed since last commit warn_boards = [] # List of boards with warnings since last commit @@ -1239,7 +1253,7 @@ class Builder: else: new_boards.append(target) - # Get a list of errors that have appeared, and disappeared + # Get a list of errors and warnings that have appeared, and disappeared better_err, worse_err = _CalcErrorDelta(self._base_err_lines, self._base_err_line_boards, err_lines, err_line_boards, '') better_warn, worse_warn = _CalcErrorDelta(self._base_warn_lines, @@ -1262,18 +1276,10 @@ class Builder: for arch, target_list in arch_list.items(): Print('%10s: %s' % (arch, target_list)) self._error_lines += 1 - if better_err: - Print('\n'.join(better_err), colour=self.col.GREEN) - self._error_lines += 1 - if worse_err: - Print('\n'.join(worse_err), colour=self.col.RED) - self._error_lines += 1 - if better_warn: - Print('\n'.join(better_warn), colour=self.col.CYAN) - self._error_lines += 1 - if worse_warn: - Print('\n'.join(worse_warn), colour=self.col.MAGENTA) - self._error_lines += 1 + _OutputErrLines(better_err, colour=self.col.GREEN) + _OutputErrLines(worse_err, colour=self.col.RED) + _OutputErrLines(better_warn, colour=self.col.CYAN) + _OutputErrLines(worse_warn, colour=self.col.MAGENTA) if show_sizes: self.PrintSizeSummary(board_selected, board_dict, show_detail, From patchwork Thu Apr 9 21:08:29 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237580 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:29 -0600 Subject: [PATCH v2 02/28] buildman: Add test coverage for error/warning colour In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.2.I1cd5b6438dbc4153816c940764d5f5ba0cf95e48@changeid> Buildman should output the right colours for each error/warning line. Some of these checks are missing. Add them. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 2aaedf44ac7..8e2b07fb438 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -235,6 +235,7 @@ class TestBuild(unittest.TestCase): # Expect a compiler error self.assertEqual(lines[10].text, '+%s' % errors[1].rstrip().replace('\n', '\n+')) + self.assertEqual(lines[10].colour, col.RED) # Fourth commit: Compile errors are fixed, just have warning for board3 self.assertEqual(lines[11].text, '04: %s' % commits[3][1]) @@ -269,9 +270,11 @@ class TestBuild(unittest.TestCase): expect = [expect[0]] + expect[2:] self.assertEqual(lines[19].text, '+%s' % '\n'.join(expect).replace('\n', '\n+')) + self.assertEqual(lines[19].colour, col.RED) self.assertEqual(lines[20].text, 'w-%s' % errors[2].rstrip().replace('\n', '\nw-')) + self.assertEqual(lines[20].colour, col.CYAN) # Sixth commit self.assertEqual(lines[21].text, '06: %s' % commits[5][1]) @@ -283,9 +286,11 @@ class TestBuild(unittest.TestCase): expect = [expect[0]] + expect[2:] self.assertEqual(lines[23].text, '-%s' % '\n'.join(expect).replace('\n', '\n-')) + self.assertEqual(lines[23].colour, col.GREEN) self.assertEqual(lines[24].text, 'w-%s' % errors[0].rstrip().replace('\n', '\nw-')) + self.assertEqual(lines[24].colour, col.CYAN) # Seventh commit self.assertEqual(lines[25].text, '07: %s' % commits[6][1]) @@ -296,11 +301,13 @@ class TestBuild(unittest.TestCase): expect = expect_str[3:8] + [expect_str[-1]] self.assertEqual(lines[27].text, '+%s' % '\n'.join(expect).replace('\n', '\n+')) + self.assertEqual(lines[27].colour, col.RED) # Now the warnings lines expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] self.assertEqual(lines[28].text, 'w+%s' % '\n'.join(expect).replace('\n', '\nw+')) + self.assertEqual(lines[28].colour, col.MAGENTA) self.assertEqual(len(lines), 29) shutil.rmtree(base_dir) From patchwork Thu Apr 9 21:08:30 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237581 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:30 -0600 Subject: [PATCH v2 03/28] buildman: Use an iterator to check test output In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.3.I5c7aff1e4cf2ca0dbb09b4df934be91a93d66f93@changeid> Rather than using the absolute array index, use an interator to work through the expected output lines. This is easier to follow. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/test.py | 94 +++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 42 deletions(-) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 8e2b07fb438..84dd608127e 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -204,41 +204,44 @@ class TestBuild(unittest.TestCase): build.SetDisplayOptions(show_errors=True); build.ShowSummary(self.commits, board_selected) #terminal.EchoPrintTestLines() - lines = terminal.GetPrintTestLines() + lines = iter(terminal.GetPrintTestLines()) # Upstream commit: no errors - self.assertEqual(lines[0].text, '01: %s' % commits[0][1]) + self.assertEqual(next(lines).text, '01: %s' % commits[0][1]) # Second commit: all archs should fail with warnings - self.assertEqual(lines[1].text, '02: %s' % commits[1][1]) + self.assertEqual(next(lines).text, '02: %s' % commits[1][1]) col = terminal.Color() - self.assertSummary(lines[2].text, 'arm', 'w+', ['board1'], - outcome=OUTCOME_WARN) - self.assertSummary(lines[3].text, 'powerpc', 'w+', ['board2', 'board3'], + self.assertSummary(next(lines).text, 'arm', 'w+', ['board1'], outcome=OUTCOME_WARN) - self.assertSummary(lines[4].text, 'sandbox', 'w+', ['board4'], + 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(lines[5].text, 'w+%s' % + line = next(lines) + self.assertEqual(line.text, 'w+%s' % errors[0].rstrip().replace('\n', '\nw+')) - self.assertEqual(lines[5].colour, col.MAGENTA) + self.assertEqual(line.colour, col.MAGENTA) # Third commit: Still fails - self.assertEqual(lines[6].text, '03: %s' % commits[2][1]) - self.assertSummary(lines[7].text, 'arm', '', ['board1'], + self.assertEqual(next(lines).text, '03: %s' % commits[2][1]) + self.assertSummary(next(lines).text, 'arm', '', ['board1'], outcome=OUTCOME_OK) - self.assertSummary(lines[8].text, 'powerpc', '+', ['board2', 'board3']) - self.assertSummary(lines[9].text, 'sandbox', '+', ['board4']) + self.assertSummary(next(lines).text, 'powerpc', '+', + ['board2', 'board3']) + self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) # Expect a compiler error - self.assertEqual(lines[10].text, '+%s' % + line = next(lines) + self.assertEqual(line.text, '+%s' % errors[1].rstrip().replace('\n', '\n+')) - self.assertEqual(lines[10].colour, col.RED) + self.assertEqual(line.colour, col.RED) # Fourth commit: Compile errors are fixed, just have warning for board3 - self.assertEqual(lines[11].text, '04: %s' % commits[3][1]) + self.assertEqual(next(lines).text, '04: %s' % commits[3][1]) expect = '%10s: ' % 'powerpc' expect += ' ' + col.Color(col.GREEN, '') expect += ' ' @@ -246,70 +249,77 @@ class TestBuild(unittest.TestCase): expect += ' ' + col.Color(col.YELLOW, 'w+') expect += ' ' expect += col.Color(col.YELLOW, ' %s' % 'board3') - self.assertEqual(lines[12].text, expect) - self.assertSummary(lines[13].text, 'sandbox', 'w+', ['board4'], + self.assertEqual(next(lines).text, expect) + self.assertSummary(next(lines).text, 'sandbox', 'w+', ['board4'], outcome=OUTCOME_WARN) # Compile error fixed - self.assertEqual(lines[14].text, '-%s' % + line = next(lines) + self.assertEqual(line.text, '-%s' % errors[1].rstrip().replace('\n', '\n-')) - self.assertEqual(lines[14].colour, col.GREEN) + self.assertEqual(line.colour, col.GREEN) - self.assertEqual(lines[15].text, 'w+%s' % + line = next(lines) + self.assertEqual(line.text, 'w+%s' % errors[2].rstrip().replace('\n', '\nw+')) - self.assertEqual(lines[15].colour, col.MAGENTA) + self.assertEqual(line.colour, col.MAGENTA) # Fifth commit - self.assertEqual(lines[16].text, '05: %s' % commits[4][1]) - self.assertSummary(lines[17].text, 'powerpc', '', ['board3'], + self.assertEqual(next(lines).text, '05: %s' % commits[4][1]) + self.assertSummary(next(lines).text, 'powerpc', '', ['board3'], outcome=OUTCOME_OK) - self.assertSummary(lines[18].text, 'sandbox', '+', ['board4']) + self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') expect = [expect[0]] + expect[2:] - self.assertEqual(lines[19].text, '+%s' % + line = next(lines) + self.assertEqual(line.text, '+%s' % '\n'.join(expect).replace('\n', '\n+')) - self.assertEqual(lines[19].colour, col.RED) + self.assertEqual(line.colour, col.RED) - self.assertEqual(lines[20].text, 'w-%s' % + line = next(lines) + self.assertEqual(line.text, 'w-%s' % errors[2].rstrip().replace('\n', '\nw-')) - self.assertEqual(lines[20].colour, col.CYAN) + self.assertEqual(line.colour, col.CYAN) # Sixth commit - self.assertEqual(lines[21].text, '06: %s' % commits[5][1]) - self.assertSummary(lines[22].text, 'sandbox', '', ['board4'], + self.assertEqual(next(lines).text, '06: %s' % commits[5][1]) + self.assertSummary(next(lines).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') expect = [expect[0]] + expect[2:] - self.assertEqual(lines[23].text, '-%s' % + line = next(lines) + self.assertEqual(line.text, '-%s' % '\n'.join(expect).replace('\n', '\n-')) - self.assertEqual(lines[23].colour, col.GREEN) + self.assertEqual(line.colour, col.GREEN) - self.assertEqual(lines[24].text, 'w-%s' % + line = next(lines) + self.assertEqual(line.text, 'w-%s' % errors[0].rstrip().replace('\n', '\nw-')) - self.assertEqual(lines[24].colour, col.CYAN) + self.assertEqual(line.colour, col.CYAN) # Seventh commit - self.assertEqual(lines[25].text, '07: %s' % commits[6][1]) - self.assertSummary(lines[26].text, 'sandbox', '+', ['board4']) + self.assertEqual(next(lines).text, '07: %s' % commits[6][1]) + 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]] - self.assertEqual(lines[27].text, '+%s' % + line = next(lines) + self.assertEqual(line.text, '+%s' % '\n'.join(expect).replace('\n', '\n+')) - self.assertEqual(lines[27].colour, col.RED) + self.assertEqual(line.colour, col.RED) # Now the warnings lines expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] - self.assertEqual(lines[28].text, 'w+%s' % + line = next(lines) + self.assertEqual(line.text, 'w+%s' % '\n'.join(expect).replace('\n', '\nw+')) - self.assertEqual(lines[28].colour, col.MAGENTA) + self.assertEqual(line.colour, col.MAGENTA) - self.assertEqual(len(lines), 29) shutil.rmtree(base_dir) def _testGit(self): From patchwork Thu Apr 9 21:08:31 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237582 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:31 -0600 Subject: [PATCH v2 04/28] buildman: Create temp directory in test setup In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.4.I814dcaf2bd35d6ca55c0e74c7218fcdd188b49ec@changeid> Rather than having a few tests handle this themselves, create the temporary directory in the setUp() method and remove it in tearDown(). This will make it easier to add more tests. Only testOutput and testGit() actually need it, but it doesn't add to the test time noticeably to do this for all tests in this file. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/test.py | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 84dd608127e..b2f7e1edf76 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -143,9 +143,14 @@ class TestBuild(unittest.TestCase): terminal.SetPrintTestMode() self._col = terminal.Color() - def Make(self, commit, brd, stage, *args, **kwargs): - global base_dir + self.base_dir = tempfile.mkdtemp() + if not os.path.isdir(self.base_dir): + os.mkdir(self.base_dir) + + def tearDown(self): + shutil.rmtree(self.base_dir) + def Make(self, commit, brd, stage, *args, **kwargs): result = command.CommandResult() boardnum = int(brd.target[-1]) result.return_code = 0 @@ -156,7 +161,7 @@ class TestBuild(unittest.TestCase): boardnum == 4 and commit.sequence == 6): result.return_code = commit.return_code result.stderr = (''.join(commit.error_list) - % {'basedir' : base_dir + '/.bm-work/00/'}) + % {'basedir' : self.base_dir + '/.bm-work/00/'}) result.combined = result.stdout + result.stderr return result @@ -178,12 +183,7 @@ class TestBuild(unittest.TestCase): This does a line-by-line verification of the summary output. """ - global base_dir - - base_dir = tempfile.mkdtemp() - if not os.path.isdir(base_dir): - os.mkdir(base_dir) - build = builder.Builder(self.toolchains, base_dir, None, 1, 2, + build = builder.Builder(self.toolchains, self.base_dir, None, 1, 2, checkout=False, show_unknown=False) build.do_make = self.Make board_selected = self.boards.GetSelectedDict() @@ -320,19 +320,14 @@ class TestBuild(unittest.TestCase): '\n'.join(expect).replace('\n', '\nw+')) self.assertEqual(line.colour, col.MAGENTA) - shutil.rmtree(base_dir) - def _testGit(self): """Test basic builder operation by building a branch""" - base_dir = tempfile.mkdtemp() - if not os.path.isdir(base_dir): - os.mkdir(base_dir) options = Options() options.git = os.getcwd() options.summary = False options.jobs = None options.dry_run = False - #options.git = os.path.join(base_dir, 'repo') + #options.git = os.path.join(self.base_dir, 'repo') options.branch = 'test-buildman' options.force_build = False options.list_tool_chains = False @@ -345,7 +340,6 @@ class TestBuild(unittest.TestCase): options.keep_outputs = False args = ['tegra20'] control.DoBuildman(options, args) - shutil.rmtree(base_dir) def testBoardSingle(self): """Test single board selection""" From patchwork Thu Apr 9 21:08:32 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237583 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:32 -0600 Subject: [PATCH v2 05/28] buildman: Split out testOutput() into separate functions In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.5.I7f0722cbddc85907c76f3603e5c53a4462c3f4f6@changeid> We want to add a few more tests similar to testOutput(). Split its logic into a function which runs buildman to get the output and another which checks the output. This will make it easier to reuse the code. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/test.py | 34 ++++++++++++++++++++++++++++------ 1 file changed, 28 insertions(+), 6 deletions(-) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index b2f7e1edf76..c9c7a05ca61 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -178,10 +178,17 @@ class TestBuild(unittest.TestCase): expect += col.Color(expected_colour, ' %s' % board) self.assertEqual(text, expect) - def testOutput(self): - """Test basic builder operation and output + def _SetupTest(self, echo_lines=False, **kwdisplay_args): + """Set up the test by running a build and summary - This does a line-by-line verification of the summary output. + Args: + echo_lines: True to echo lines to the terminal to aid test + development + kwdisplay_args: Dict of arguemnts to pass to + Builder.SetDisplayOptions() + + Returns: + Iterator containing the output lines, each a PrintLine() object """ build = builder.Builder(self.toolchains, self.base_dir, None, 1, 2, checkout=False, show_unknown=False) @@ -201,11 +208,18 @@ class TestBuild(unittest.TestCase): # We should get two starting messages, then an update for every commit # built. self.assertEqual(count, len(commits) * len(boards) + 2) - build.SetDisplayOptions(show_errors=True); + build.SetDisplayOptions(**kwdisplay_args); build.ShowSummary(self.commits, board_selected) - #terminal.EchoPrintTestLines() - lines = iter(terminal.GetPrintTestLines()) + if echo_lines: + terminal.EchoPrintTestLines() + return iter(terminal.GetPrintTestLines()) + + def _CheckOutput(self, lines): + """Check for expected output from the build summary + Args: + lines: Iterator containing the lines returned from the summary + """ # Upstream commit: no errors self.assertEqual(next(lines).text, '01: %s' % commits[0][1]) @@ -320,6 +334,14 @@ class TestBuild(unittest.TestCase): '\n'.join(expect).replace('\n', '\nw+')) self.assertEqual(line.colour, col.MAGENTA) + def testOutput(self): + """Test basic builder operation and output + + This does a line-by-line verification of the summary output. + """ + lines = self._SetupTest(show_errors=True) + self._CheckOutput(lines) + def _testGit(self): """Test basic builder operation by building a branch""" options = Options() From patchwork Thu Apr 9 21:08:33 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237585 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:33 -0600 Subject: [PATCH v2 06/28] buildman: Add a test helper for creating a line prefix In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.6.I8b66c9426751450927d017e85a33c890c9185cb4@changeid> The split/join code is repeated in a lot of places. Add a function to handle this. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/test.py | 51 +++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index c9c7a05ca61..1377035fbb4 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -220,6 +220,22 @@ class TestBuild(unittest.TestCase): Args: lines: Iterator containing the lines returned from the summary """ + def add_line_prefix(prefix, error_str): + """Add a prefix to each line of a string + + The training \n in error_str is removed before processing + + Args: + prefix: String prefix to add + error_str: Error string containing the lines + + Returns: + New string where each line has the prefix added + """ + lines = error_str.strip().splitlines() + new_lines = [prefix + line for line in lines] + return '\n'.join(new_lines) + # Upstream commit: no errors self.assertEqual(next(lines).text, '01: %s' % commits[0][1]) @@ -236,8 +252,8 @@ class TestBuild(unittest.TestCase): # Second commit: The warnings should be listed line = next(lines) - self.assertEqual(line.text, 'w+%s' % - errors[0].rstrip().replace('\n', '\nw+')) + + self.assertEqual(line.text, add_line_prefix('w+', errors[0])) self.assertEqual(line.colour, col.MAGENTA) # Third commit: Still fails @@ -250,8 +266,7 @@ class TestBuild(unittest.TestCase): # Expect a compiler error line = next(lines) - self.assertEqual(line.text, '+%s' % - errors[1].rstrip().replace('\n', '\n+')) + self.assertEqual(line.text, add_line_prefix('+', errors[1])) self.assertEqual(line.colour, col.RED) # Fourth commit: Compile errors are fixed, just have warning for board3 @@ -269,13 +284,11 @@ class TestBuild(unittest.TestCase): # Compile error fixed line = next(lines) - self.assertEqual(line.text, '-%s' % - errors[1].rstrip().replace('\n', '\n-')) + self.assertEqual(line.text, add_line_prefix('-', errors[1])) self.assertEqual(line.colour, col.GREEN) line = next(lines) - self.assertEqual(line.text, 'w+%s' % - errors[2].rstrip().replace('\n', '\nw+')) + self.assertEqual(line.text, add_line_prefix('w+', errors[2])) self.assertEqual(line.colour, col.MAGENTA) # Fifth commit @@ -287,14 +300,13 @@ class TestBuild(unittest.TestCase): # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') expect = [expect[0]] + expect[2:] + expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, '+%s' % - '\n'.join(expect).replace('\n', '\n+')) + self.assertEqual(line.text, add_line_prefix('+', expect)) self.assertEqual(line.colour, col.RED) line = next(lines) - self.assertEqual(line.text, 'w-%s' % - errors[2].rstrip().replace('\n', '\nw-')) + self.assertEqual(line.text, add_line_prefix('w-', errors[2])) self.assertEqual(line.colour, col.CYAN) # Sixth commit @@ -305,14 +317,13 @@ class TestBuild(unittest.TestCase): # The second line of errors[3] is a duplicate, so buildman will drop it expect = errors[3].rstrip().split('\n') expect = [expect[0]] + expect[2:] + expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, '-%s' % - '\n'.join(expect).replace('\n', '\n-')) + self.assertEqual(line.text, add_line_prefix('-', expect)) self.assertEqual(line.colour, col.GREEN) line = next(lines) - self.assertEqual(line.text, 'w-%s' % - errors[0].rstrip().replace('\n', '\nw-')) + self.assertEqual(line.text, add_line_prefix('w-', errors[0])) self.assertEqual(line.colour, col.CYAN) # Seventh commit @@ -322,16 +333,16 @@ class TestBuild(unittest.TestCase): # 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) line = next(lines) - self.assertEqual(line.text, '+%s' % - '\n'.join(expect).replace('\n', '\n+')) + self.assertEqual(line.text, add_line_prefix('+', expect)) self.assertEqual(line.colour, col.RED) # Now the warnings lines expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] + expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, 'w+%s' % - '\n'.join(expect).replace('\n', '\nw+')) + self.assertEqual(line.text, add_line_prefix('w+', expect)) self.assertEqual(line.colour, col.MAGENTA) def testOutput(self): From patchwork Thu Apr 9 21:08:34 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237589 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:34 -0600 Subject: [PATCH v2 07/28] buildman: Test the output with --list-error-boards In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.7.I571a331de630a5dcc7b659f464531e65d55aa94b@changeid> Add a test to cover this flag, which adds the name of each board to each error/warning line. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: - Move the 'boards' variables to the start of _CheckOutput() tools/buildman/test.py | 46 +++++++++++++++++++++++++++++------------- 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 1377035fbb4..b1435b45c03 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -214,13 +214,15 @@ class TestBuild(unittest.TestCase): terminal.EchoPrintTestLines() return iter(terminal.GetPrintTestLines()) - def _CheckOutput(self, lines): + def _CheckOutput(self, lines, list_error_boards): """Check for expected output from the build summary Args: lines: Iterator containing the lines returned from the summary + list_error_boards: Adjust the check for output produced with the + --list-error-boards flag """ - def add_line_prefix(prefix, error_str): + def add_line_prefix(prefix, boards, error_str): """Add a prefix to each line of a string The training \n in error_str is removed before processing @@ -232,10 +234,17 @@ class TestBuild(unittest.TestCase): Returns: New string where each line has the prefix added """ + if boards: + boards = '(%s) ' % boards lines = error_str.strip().splitlines() - new_lines = [prefix + line for line in lines] + new_lines = [prefix + boards + line for line in lines] return '\n'.join(new_lines) + 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 self.assertEqual(next(lines).text, '01: %s' % commits[0][1]) @@ -253,7 +262,8 @@ class TestBuild(unittest.TestCase): # Second commit: The warnings should be listed line = next(lines) - self.assertEqual(line.text, add_line_prefix('w+', errors[0])) + self.assertEqual(line.text, + add_line_prefix('w+', boards1234, errors[0])) self.assertEqual(line.colour, col.MAGENTA) # Third commit: Still fails @@ -266,7 +276,7 @@ class TestBuild(unittest.TestCase): # Expect a compiler error line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', errors[1])) + self.assertEqual(line.text, add_line_prefix('+', boards234, errors[1])) self.assertEqual(line.colour, col.RED) # Fourth commit: Compile errors are fixed, just have warning for board3 @@ -284,11 +294,11 @@ class TestBuild(unittest.TestCase): # Compile error fixed line = next(lines) - self.assertEqual(line.text, add_line_prefix('-', errors[1])) + self.assertEqual(line.text, add_line_prefix('-', boards234, errors[1])) self.assertEqual(line.colour, col.GREEN) line = next(lines) - self.assertEqual(line.text, add_line_prefix('w+', errors[2])) + self.assertEqual(line.text, add_line_prefix('w+', boards34, errors[2])) self.assertEqual(line.colour, col.MAGENTA) # Fifth commit @@ -302,11 +312,11 @@ class TestBuild(unittest.TestCase): expect = [expect[0]] + expect[2:] expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', expect)) + self.assertEqual(line.text, add_line_prefix('+', boards4, expect)) self.assertEqual(line.colour, col.RED) line = next(lines) - self.assertEqual(line.text, add_line_prefix('w-', errors[2])) + self.assertEqual(line.text, add_line_prefix('w-', boards34, errors[2])) self.assertEqual(line.colour, col.CYAN) # Sixth commit @@ -319,11 +329,11 @@ class TestBuild(unittest.TestCase): expect = [expect[0]] + expect[2:] expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, add_line_prefix('-', expect)) + self.assertEqual(line.text, add_line_prefix('-', boards4, expect)) self.assertEqual(line.colour, col.GREEN) line = next(lines) - self.assertEqual(line.text, add_line_prefix('w-', errors[0])) + self.assertEqual(line.text, add_line_prefix('w-', boards4, errors[0])) self.assertEqual(line.colour, col.CYAN) # Seventh commit @@ -335,14 +345,14 @@ class TestBuild(unittest.TestCase): expect = expect_str[3:8] + [expect_str[-1]] expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', expect)) + self.assertEqual(line.text, add_line_prefix('+', boards4, expect)) self.assertEqual(line.colour, col.RED) # Now the warnings lines expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] expect = '\n'.join(expect) line = next(lines) - self.assertEqual(line.text, add_line_prefix('w+', expect)) + self.assertEqual(line.text, add_line_prefix('w+', boards4, expect)) self.assertEqual(line.colour, col.MAGENTA) def testOutput(self): @@ -351,7 +361,15 @@ class TestBuild(unittest.TestCase): This does a line-by-line verification of the summary output. """ lines = self._SetupTest(show_errors=True) - self._CheckOutput(lines) + self._CheckOutput(lines, list_error_boards=False) + + def testErrorBoards(self): + """Test output with --list-error-boards + + 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) def _testGit(self): """Test basic builder operation by building a branch""" From patchwork Thu Apr 9 21:08:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237584 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:35 -0600 Subject: [PATCH v2 08/28] buildman: Use yellow consistently for warning lines In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.8.Ifbcc1c7085c9545bd1533d4b0685145aacd5a7f0@changeid> At present warnings are shown in yellow in the summary (-s) but magenta in the detail listing (-e). Use yellow in both. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 2 +- tools/buildman/test.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 621147696ed..01d8bf46e45 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1279,7 +1279,7 @@ class Builder: _OutputErrLines(better_err, colour=self.col.GREEN) _OutputErrLines(worse_err, colour=self.col.RED) _OutputErrLines(better_warn, colour=self.col.CYAN) - _OutputErrLines(worse_warn, colour=self.col.MAGENTA) + _OutputErrLines(worse_warn, colour=self.col.YELLOW) if show_sizes: self.PrintSizeSummary(board_selected, board_dict, show_detail, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index b1435b45c03..c494a158be6 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -264,7 +264,7 @@ class TestBuild(unittest.TestCase): self.assertEqual(line.text, add_line_prefix('w+', boards1234, errors[0])) - self.assertEqual(line.colour, col.MAGENTA) + self.assertEqual(line.colour, col.YELLOW) # Third commit: Still fails self.assertEqual(next(lines).text, '03: %s' % commits[2][1]) @@ -299,7 +299,7 @@ class TestBuild(unittest.TestCase): line = next(lines) self.assertEqual(line.text, add_line_prefix('w+', boards34, errors[2])) - self.assertEqual(line.colour, col.MAGENTA) + self.assertEqual(line.colour, col.YELLOW) # Fifth commit self.assertEqual(next(lines).text, '05: %s' % commits[4][1]) @@ -353,7 +353,7 @@ class TestBuild(unittest.TestCase): expect = '\n'.join(expect) line = next(lines) self.assertEqual(line.text, add_line_prefix('w+', boards4, expect)) - self.assertEqual(line.colour, col.MAGENTA) + self.assertEqual(line.colour, col.YELLOW) def testOutput(self): """Test basic builder operation and output From patchwork Thu Apr 9 21:08:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237586 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:36 -0600 Subject: [PATCH v2 09/28] buildman: Use an object to hold error lines In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.9.I27020486c709ec681fbee7c99799ade05cfbcfe0@changeid> At present the string for each error line is created in _CalcErrorDelta() and used to create the summary output. This is inflexible since all the information (error/warning character, error line, list of boards with that error line) is munged together in a string. Create an object to hold this information and only convert it to a string when printing the actual output. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 69 +++++++++++++++++++++++++++++---------- 1 file changed, 52 insertions(+), 17 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 01d8bf46e45..238b711dec4 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -24,7 +24,6 @@ import terminal from terminal import Print import toolchain - """ Theory of Operation @@ -91,6 +90,15 @@ u-boot/ source directory .git/ repository """ +"""Holds information about a particular error line we are outputing + + char: Character representation: '+': error, '-': fixed error, 'w+': warning, + 'w-' = fixed warning + boards: List of Board objects which have line in the error/warning output + errline: The text of the error line +""" +ErrLine = collections.namedtuple('ErrLine', 'char,boards,errline') + # Possible build outcomes OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = list(range(4)) @@ -1128,32 +1136,52 @@ class Builder: Args: line: Error line to search for + line_boards: boards to search, each a Board Return: - String containing a list of boards with that error line, or - '' if the user has not requested such a list + List of boards with that error line, or [] if the user has not + requested such a list """ + boards = [] + board_set = set() if self._list_error_boards: - names = [] for board in line_boards[line]: - if not board.target in names: - names.append(board.target) - names_str = '(%s) ' % ','.join(names) - else: - names_str = '' - return names_str + if not board in board_set: + boards.append(board) + board_set.add(board) + return boards def _CalcErrorDelta(base_lines, base_line_boards, lines, line_boards, char): + """Calculate the required output based on changes in errors + + Args: + base_lines: List of errors/warnings for previous commit + base_line_boards: Dict keyed by error line, containing a list + of the Board objects with that error in the previous commit + lines: List of errors/warning for this commit, each a str + line_boards: Dict keyed by error line, containing a list + of the Board objects with that error in this commit + char: Character representing error ('') or warning ('w'). The + broken ('+') or fixed ('-') characters are added in this + function + + Returns: + Tuple + List of ErrLine objects for 'better' lines + List of ErrLine objects for 'worse' lines + """ better_lines = [] worse_lines = [] for line in lines: if line not in base_lines: - worse_lines.append(char + '+' + - _BoardList(line, line_boards) + line) + errline = ErrLine(char + '+', _BoardList(line, line_boards), + line) + worse_lines.append(errline) for line in base_lines: if line not in lines: - better_lines.append(char + '-' + - _BoardList(line, base_line_boards) + line) + errline = ErrLine(char + '-', + _BoardList(line, base_line_boards), line) + better_lines.append(errline) return better_lines, worse_lines def _CalcConfig(delta, name, config): @@ -1215,12 +1243,19 @@ class Builder: Also increments self._error_lines if err_lines not empty Args: - err_lines: List of strings, each an error or warning line, - possibly including a list of boards with that error/warning + err_lines: List of ErrLine objects, each an error or warning + line, possibly including a list of boards with that + error/warning colour: Colour to use for output """ if err_lines: - Print('\n'.join(err_lines), colour=colour) + out = [] + for line in err_lines: + boards = '' + names = [board.target for board in line.boards] + boards = '(%s) ' % ','.join(names) if names else '' + out.append('%s%s%s' % (line.char, boards, line.errline)) + Print('\n'.join(out), colour=colour) self._error_lines += 1 From patchwork Thu Apr 9 21:08:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237587 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:37 -0600 Subject: [PATCH v2 10/28] buildman: Show the list of boards in magenta In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.10.I6a438c17db351ba63e0d2fa4cc3d309cd2391763@changeid> It is quite hard to see the list of board for each error line since the colour is the same as the actual error line. Show the board list in magenta so that it is easier to distinguish them. There is no point in checking the colour of the overall line, since there are now multiple colours. So drop those tests. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 15 ++++++--- tools/buildman/test.py | 70 ++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 238b711dec4..7cbb1a6f628 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1249,13 +1249,20 @@ class Builder: colour: Colour to use for output """ if err_lines: - out = [] + out_list = [] for line in err_lines: boards = '' names = [board.target for board in line.boards] - boards = '(%s) ' % ','.join(names) if names else '' - out.append('%s%s%s' % (line.char, boards, line.errline)) - Print('\n'.join(out), colour=colour) + board_str = ','.join(names) if names else '' + if board_str: + out = self.col.Color(colour, line.char + '(') + out += self.col.Color(self.col.MAGENTA, board_str, + bright=False) + out += self.col.Color(colour, ') %s' % line.errline) + else: + out = self.col.Color(colour, line.char + line.errline) + out_list.append(out) + Print('\n'.join(out_list)) self._error_lines += 1 diff --git a/tools/buildman/test.py b/tools/buildman/test.py index c494a158be6..a64372dee36 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -222,7 +222,7 @@ class TestBuild(unittest.TestCase): list_error_boards: Adjust the check for output produced with the --list-error-boards flag """ - def add_line_prefix(prefix, boards, error_str): + def add_line_prefix(prefix, boards, error_str, colour): """Add a prefix to each line of a string The training \n in error_str is removed before processing @@ -230,14 +230,23 @@ class TestBuild(unittest.TestCase): Args: prefix: String prefix to add error_str: Error string containing the lines + colour: Expected colour for the line. Note that the board list, + if present, always appears in magenta Returns: New string where each line has the prefix added """ - if boards: - boards = '(%s) ' % boards lines = error_str.strip().splitlines() - new_lines = [prefix + boards + line for line in lines] + new_lines = [] + for line in lines: + if boards: + expect = self._col.Color(colour, prefix + '(') + expect += self._col.Color(self._col.MAGENTA, boards, + bright=False) + expect += self._col.Color(colour, ') %s' % line) + else: + expect = self._col.Color(colour, prefix + line) + new_lines.append(expect) return '\n'.join(new_lines) boards1234 = 'board1,board2,board3,board4' if list_error_boards else '' @@ -260,11 +269,8 @@ class TestBuild(unittest.TestCase): outcome=OUTCOME_WARN) # Second commit: The warnings should be listed - line = next(lines) - - self.assertEqual(line.text, - add_line_prefix('w+', boards1234, errors[0])) - self.assertEqual(line.colour, col.YELLOW) + self.assertEqual(next(lines).text, + add_line_prefix('w+', boards1234, errors[0], col.YELLOW)) # Third commit: Still fails self.assertEqual(next(lines).text, '03: %s' % commits[2][1]) @@ -275,9 +281,8 @@ class TestBuild(unittest.TestCase): self.assertSummary(next(lines).text, 'sandbox', '+', ['board4']) # Expect a compiler error - line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', boards234, errors[1])) - self.assertEqual(line.colour, col.RED) + self.assertEqual(next(lines).text, + add_line_prefix('+', boards234, errors[1], col.RED)) # Fourth commit: Compile errors are fixed, just have warning for board3 self.assertEqual(next(lines).text, '04: %s' % commits[3][1]) @@ -293,13 +298,11 @@ class TestBuild(unittest.TestCase): outcome=OUTCOME_WARN) # Compile error fixed - line = next(lines) - self.assertEqual(line.text, add_line_prefix('-', boards234, errors[1])) - self.assertEqual(line.colour, col.GREEN) + self.assertEqual(next(lines).text, + add_line_prefix('-', boards234, errors[1], col.GREEN)) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('w+', boards34, errors[2])) - self.assertEqual(line.colour, col.YELLOW) + self.assertEqual(next(lines).text, + add_line_prefix('w+', boards34, errors[2], col.YELLOW)) # Fifth commit self.assertEqual(next(lines).text, '05: %s' % commits[4][1]) @@ -311,13 +314,11 @@ class TestBuild(unittest.TestCase): expect = errors[3].rstrip().split('\n') expect = [expect[0]] + expect[2:] expect = '\n'.join(expect) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', boards4, expect)) - self.assertEqual(line.colour, col.RED) + self.assertEqual(next(lines).text, + add_line_prefix('+', boards4, expect, col.RED)) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('w-', boards34, errors[2])) - self.assertEqual(line.colour, col.CYAN) + self.assertEqual(next(lines).text, + add_line_prefix('w-', boards34, errors[2], col.CYAN)) # Sixth commit self.assertEqual(next(lines).text, '06: %s' % commits[5][1]) @@ -328,13 +329,10 @@ class TestBuild(unittest.TestCase): expect = errors[3].rstrip().split('\n') expect = [expect[0]] + expect[2:] expect = '\n'.join(expect) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('-', boards4, expect)) - self.assertEqual(line.colour, col.GREEN) - - line = next(lines) - self.assertEqual(line.text, add_line_prefix('w-', boards4, errors[0])) - self.assertEqual(line.colour, col.CYAN) + self.assertEqual(next(lines).text, + add_line_prefix('-', boards4, expect, col.GREEN)) + self.assertEqual(next(lines).text, + add_line_prefix('w-', boards4, errors[0], col.CYAN)) # Seventh commit self.assertEqual(next(lines).text, '07: %s' % commits[6][1]) @@ -344,16 +342,14 @@ class TestBuild(unittest.TestCase): expect_str = errors[4].rstrip().replace('%(basedir)s', '').split('\n') expect = expect_str[3:8] + [expect_str[-1]] expect = '\n'.join(expect) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('+', boards4, expect)) - self.assertEqual(line.colour, col.RED) + self.assertEqual(next(lines).text, + add_line_prefix('+', boards4, expect, col.RED)) # Now the warnings lines expect = [expect_str[0]] + expect_str[10:12] + [expect_str[9]] expect = '\n'.join(expect) - line = next(lines) - self.assertEqual(line.text, add_line_prefix('w+', boards4, expect)) - self.assertEqual(line.colour, col.YELLOW) + self.assertEqual(next(lines).text, + add_line_prefix('w+', boards4, expect, col.YELLOW)) def testOutput(self): """Test basic builder operation and output From patchwork Thu Apr 9 21:08:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237588 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:38 -0600 Subject: [PATCH v2 11/28] buildman: Use spaces in the board list In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.11.I39b6c927505684d4425384d0ede4c77a51bb65fb@changeid> At present the board names shown with -l are separated by commas. This makes it hard to double-click to select a particular board. Also it is not possible to select all boards and paste them as arguments to a subsequent buildman run, since buildman requires spaces to separate the list on the command line, not commas. Change the output format to use spaces instead of commas. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to use spaces in the board list tools/buildman/builder.py | 2 +- tools/buildman/test.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 7cbb1a6f628..e895ab73d8d 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1253,7 +1253,7 @@ class Builder: for line in err_lines: boards = '' names = [board.target for board in line.boards] - board_str = ','.join(names) if names else '' + board_str = ' '.join(names) if names else '' if board_str: out = self.col.Color(colour, line.char + '(') out += self.col.Color(self.col.MAGENTA, board_str, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index a64372dee36..a495df03036 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -249,9 +249,9 @@ class TestBuild(unittest.TestCase): new_lines.append(expect) return '\n'.join(new_lines) - 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 '' + 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 From patchwork Thu Apr 9 21:08:39 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237590 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:39 -0600 Subject: [PATCH v2 12/28] patman: Update flushing Print() for Python 3 In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.12.I9d8a5afe3f0d9fccfb1f768d754f98b58c365e33@changeid> This does not seem to work on Python 3. Update the code to use the built-in support. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/patman/terminal.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index 7a3b658b00e..6541fa8f410 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -53,11 +53,10 @@ def Print(text='', newline=True, colour=None): if colour: col = Color() text = col.Color(colour, text) - print(text, end='') if newline: - print() + print(text) else: - sys.stdout.flush() + print(text, end='', flush=True) def SetPrintTestMode(): """Go into test mode, where all printing is recorded""" From patchwork Thu Apr 9 21:08:40 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237592 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:40 -0600 Subject: [PATCH v2 13/28] patman: Support erasing a previously unfinished text line In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.13.Ibccb893865ead03f67a73ab6419a7927d4afd2a9@changeid> When printing progress it is useful to print a message and leave the cursor at the end of the line until the operation is finished. When it is finished, the line needs to be erased so a new line can start in its place. Add a function to handle clearing a line previously written by terminal.Print() Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/patman/patman.py | 2 +- tools/patman/terminal.py | 47 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/tools/patman/patman.py b/tools/patman/patman.py index cf53e532ddf..7f4ac9aef48 100755 --- a/tools/patman/patman.py +++ b/tools/patman/patman.py @@ -93,7 +93,7 @@ elif options.test: suite = unittest.TestLoader().loadTestsFromTestCase(module) suite.run(result) - for module in ['gitutil', 'settings']: + for module in ['gitutil', 'settings', 'terminal']: suite = doctest.DocTestSuite(module) suite.run(result) diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index 6541fa8f410..c7693eb57ad 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -10,6 +10,7 @@ This module handles terminal interaction including ANSI color codes. from __future__ import print_function import os +import re import sys # Selection of when we want our output to be colored @@ -19,6 +20,13 @@ COLOR_IF_TERMINAL, COLOR_ALWAYS, COLOR_NEVER = range(3) print_test_mode = False print_test_list = [] +# The length of the last line printed without a newline +last_print_len = None + +# credit: +# stackoverflow.com/questions/14693701/how-can-i-remove-the-ansi-escape-sequences-from-a-string-in-python +ansi_escape = re.compile(r'\x1b(?:[@-Z\\-_]|\[[0-?]*[ -/]*[@-~])') + class PrintLine: """A line of text output @@ -36,6 +44,33 @@ class PrintLine: return 'newline=%s, colour=%s, text=%s' % (self.newline, self.colour, self.text) +def CalcAsciiLen(text): + """Calculate the length of a string, ignoring any ANSI sequences + + Args: + text: Text to check + + Returns: + Length of text, after skipping ANSI sequences + + >>> col = Color(COLOR_ALWAYS) + >>> text = col.Color(Color.RED, 'abc') + >>> len(text) + 14 + >>> CalcAsciiLen(text) + 3 + >>> + >>> text += 'def' + >>> CalcAsciiLen(text) + 6 + >>> text += col.Color(Color.RED, 'abc') + >>> CalcAsciiLen(text) + 9 + """ + result = ansi_escape.sub('', text) + return len(result) + + def Print(text='', newline=True, colour=None): """Handle a line of output to the terminal. @@ -47,6 +82,8 @@ def Print(text='', newline=True, colour=None): newline: True to add a new line at the end of the text colour: Colour to use for the text """ + global last_print_len + if print_test_mode: print_test_list.append(PrintLine(text, newline, colour)) else: @@ -55,8 +92,18 @@ def Print(text='', newline=True, colour=None): text = col.Color(colour, text) if newline: print(text) + last_print_len = None else: print(text, end='', flush=True) + last_print_len = CalcAsciiLen(text) + +def PrintClear(): + """Clear a previously line that was printed with no newline""" + global last_print_len + + if last_print_len: + print('\r%s\r' % (' '* last_print_len), end='', flush=True) + last_print_len = None def SetPrintTestMode(): """Go into test mode, where all printing is recorded""" From patchwork Thu Apr 9 21:08:41 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237591 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:41 -0600 Subject: [PATCH v2 14/28] patman: Support limiting output to a single line In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.14.Icc97d7478a3a877ddb620428e02f8d83321695b0@changeid> When outputing a progress line we don't want it to go past the end of the current terminal line, or it will not be possible to erase it. Add an option to Print() which allows limiting the output to the terminal width. Since ANSI sequences do not take up space on the terminal, these are removed. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/patman/terminal.py | 58 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/tools/patman/terminal.py b/tools/patman/terminal.py index c7693eb57ad..5c9e3eea20c 100644 --- a/tools/patman/terminal.py +++ b/tools/patman/terminal.py @@ -11,6 +11,7 @@ from __future__ import print_function import os import re +import shutil import sys # Selection of when we want our output to be colored @@ -47,6 +48,9 @@ class PrintLine: def CalcAsciiLen(text): """Calculate the length of a string, ignoring any ANSI sequences + When displayed on a terminal, ANSI sequences don't take any space, so we + need to ignore them when calculating the length of a string. + Args: text: Text to check @@ -70,8 +74,57 @@ def CalcAsciiLen(text): result = ansi_escape.sub('', text) return len(result) +def TrimAsciiLen(text, size): + """Trim a string containing ANSI sequences to the given ASCII length + + The string is trimmed with ANSI sequences being ignored for the length + calculation. + + >>> col = Color(COLOR_ALWAYS) + >>> text = col.Color(Color.RED, 'abc') + >>> len(text) + 14 + >>> CalcAsciiLen(TrimAsciiLen(text, 4)) + 3 + >>> CalcAsciiLen(TrimAsciiLen(text, 2)) + 2 + >>> text += 'def' + >>> CalcAsciiLen(TrimAsciiLen(text, 4)) + 4 + >>> text += col.Color(Color.RED, 'ghi') + >>> CalcAsciiLen(TrimAsciiLen(text, 7)) + 7 + """ + if CalcAsciiLen(text) < size: + return text + pos = 0 + out = '' + left = size + + # Work through each ANSI sequence in turn + for m in ansi_escape.finditer(text): + # Find the text before the sequence and add it to our string, making + # sure it doesn't overflow + before = text[pos:m.start()] + toadd = before[:left] + out += toadd + + # Figure out how much non-ANSI space we have left + left -= len(toadd) + + # Add the ANSI sequence and move to the position immediately after it + out += m.group() + pos = m.start() + len(m.group()) + + # Deal with text after the last ANSI sequence + after = text[pos:] + toadd = after[:left] + out += toadd + + return out + -def Print(text='', newline=True, colour=None): +def Print(text='', newline=True, colour=None, limit_to_line=False): """Handle a line of output to the terminal. In test mode this is recorded in a list. Otherwise it is output to the @@ -94,6 +147,9 @@ def Print(text='', newline=True, colour=None): print(text) last_print_len = None else: + if limit_to_line: + cols = shutil.get_terminal_size().columns + text = TrimAsciiLen(text, cols) print(text, end='', flush=True) last_print_len = CalcAsciiLen(text) From patchwork Thu Apr 9 21:08:42 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237593 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:42 -0600 Subject: [PATCH v2 15/28] buildman: Drop the line-clearing code in Builder In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.15.Ic80ce13fd286e5b336a9094283791b4b434b1773@changeid> The new feature in terminal can be used by buildman. Update the Builder class accordingly. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index e895ab73d8d..60e854bb49a 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -162,8 +162,6 @@ class Builder: force_build_failures: If a previously-built build (i.e. built on a previous run of buildman) is marked as failed, rebuild it. git_dir: Git directory containing source repository - last_line_len: Length of the last line we printed (used for erasing - it with new progress information) num_jobs: Number of jobs to run at once (passed to make as -j) num_threads: Number of builder threads to run out_queue: Queue of results to process @@ -317,7 +315,6 @@ class Builder: t.start() self.threads.append(t) - self.last_line_len = 0 t = builderthread.ResultThread(self) t.setDaemon(True) t.start() @@ -388,22 +385,6 @@ class Builder: self._timestamps.popleft() count -= 1 - def ClearLine(self, length): - """Clear any characters on the current line - - Make way for a new line of length 'length', by outputting enough - spaces to clear out the old line. Then remember the new length for - next time. - - Args: - length: Length of new line, in characters - """ - if length < self.last_line_len: - Print(' ' * (self.last_line_len - length), newline=False) - Print('\r', newline=False) - self.last_line_len = length - sys.stdout.flush() - def SelectCommit(self, commit, checkout=True): """Checkout the selected commit for this build """ @@ -449,8 +430,7 @@ class Builder: if result.already_done: self.already_done += 1 if self._verbose: - Print('\r', newline=False) - self.ClearLine(0) + terminal.PrintClear() boards_selected = {target : result.brd} self.ResetResultSummary(boards_selected) self.ProduceResultSummary(result.commit_upto, self.commits, @@ -477,9 +457,8 @@ class Builder: self.commit_count) name += target + terminal.PrintClear() Print(line + name, newline=False) - length = 16 + len(name) - self.ClearLine(length) def _GetOutputDir(self, commit_upto): """Get the name of the output directory for a commit number @@ -1559,7 +1538,7 @@ class Builder: Print('\rCloning repo for thread %d' % thread_num, newline=False) gitutil.Clone(src_dir, thread_dir) - Print('\r%s\r' % (' ' * 30), newline=False) + terminal.PrintClear() def _PrepareWorkingSpace(self, max_threads, setup_git): """Prepare the working directory for use. @@ -1660,5 +1639,4 @@ class Builder: # Wait until we have processed all output self.out_queue.join() Print() - self.ClearLine(0) return (self.fail, self.warned) From patchwork Thu Apr 9 21:08:43 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237595 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:43 -0600 Subject: [PATCH v2 16/28] buildman: Show a message when fetching a repo In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.16.Id46378e9383a2e8760b31d9345fe6a6bab0cb4bc@changeid> Fetching updated versions of a repo can take time. At present buildman gives no indication that it is doing this. Add a message to explain the delay. Tidy up a few other messages while we are here. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 60e854bb49a..ccf46ac5f67 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1533,7 +1533,10 @@ class Builder: if setup_git and self.git_dir: src_dir = os.path.abspath(self.git_dir) if os.path.exists(git_dir): + Print('\rFetching repo for thread %d' % thread_num, + newline=False) gitutil.Fetch(git_dir, thread_dir) + terminal.PrintClear() else: Print('\rCloning repo for thread %d' % thread_num, newline=False) @@ -1591,7 +1594,7 @@ class Builder: newline=False) for dirname in to_remove: shutil.rmtree(dirname) - Print('done') + terminal.PrintClear() def BuildBoards(self, commits, board_selected, keep_outputs, verbose): """Build all commits for a list of boards From patchwork Thu Apr 9 21:08:44 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237594 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:44 -0600 Subject: [PATCH v2 17/28] buildman: Drop unused output code In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.17.I9d481e6c0c694b5242e0bdcb47dce5eb3007358b@changeid> The commit counter is a hangover from when buildman processed each board for a commit. Now buildman processes each commit for a board, so this output is never triggered. Delete it. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index ccf46ac5f67..ae8eee16e35 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -450,11 +450,6 @@ class Builder: self._AddTimestamp() if self._complete_delay: name += '%s : ' % self._complete_delay - # When building all boards for a commit, we can print a commit - # progress message. - if result and result.commit_upto is None: - name += 'commit %2d/%-3d' % (self.commit_upto + 1, - self.commit_count) name += target terminal.PrintClear() From patchwork Thu Apr 9 21:08:45 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237596 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:45 -0600 Subject: [PATCH v2 18/28] buildman: Show the number of builds remaining In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.18.I0b22b46f575d45f1b67c61b54d69117e11abdaed@changeid> It is nice to see the actual number of builds remaining to complete. Add this in the progress message, using a different colour. Drop the unnecessary 'name' variable while we are here. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index ae8eee16e35..b9caf6cf9c1 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -444,16 +444,21 @@ class Builder: line += self.col.Color(self.col.YELLOW, '%5d' % self.warned) line += self.col.Color(self.col.RED, '%5d' % self.fail) - name = ' /%-5d ' % self.count + line += ' /%-5d ' % self.count + remaining = self.count - self.upto + if remaining: + line += self.col.Color(self.col.MAGENTA, ' -%-5d ' % remaining) + else: + line += ' ' * 8 # Add our current completion time estimate self._AddTimestamp() if self._complete_delay: - name += '%s : ' % self._complete_delay + line += '%s : ' % self._complete_delay - name += target + line += target terminal.PrintClear() - Print(line + name, newline=False) + Print(line, newline=False) def _GetOutputDir(self, commit_upto): """Get the name of the output directory for a commit number From patchwork Thu Apr 9 21:08:46 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237598 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:46 -0600 Subject: [PATCH v2 19/28] buildman: Limit the length of progress messages In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.19.I8eddf71be429d9f2fb7ce2143092623d29898aaa@changeid> If a progress message is longer than the terminal line it will scroll the terminal. Limit the messages to the terminal width. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index b9caf6cf9c1..73753eb4bc9 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -458,7 +458,7 @@ class Builder: line += target terminal.PrintClear() - Print(line, newline=False) + Print(line, newline=False, limit_to_line=True) def _GetOutputDir(self, commit_upto): """Get the name of the output directory for a commit number From patchwork Thu Apr 9 21:08:47 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237597 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:47 -0600 Subject: [PATCH v2 20/28] buildman: Show a summary of the build result In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.20.Ib67b1b74e8bc38990eac6aefbf1174812e060346@changeid> When buildman finishes it leaves the last summary line visible, which shows the number of successful builds, builds with warnings and builds with errors. It is useful also to see how many builds were done in total along with the time taken. Show these on a separate line before buildman finishes. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/builder.py | 17 +++++++++++++++++ tools/buildman/test.py | 6 +++--- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 73753eb4bc9..6819e5b40da 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -192,6 +192,7 @@ class Builder: _next_delay_update: Next time we plan to display a progress update (datatime) _show_unknown: Show unknown boards (those not built) in summary + _start_time: Start time for the build _timestamps: List of timestamps for the completion of the last last _timestamp_count builds. Each is a datetime object. _timestamp_count: Number of timestamps to keep in our list. @@ -281,6 +282,7 @@ class Builder: self._build_period_us = None self._complete_delay = None self._next_delay_update = datetime.now() + self._start_time = datetime.now() self.force_config_on_failure = True self.force_build_failures = False self.force_reconfig = False @@ -1642,4 +1644,19 @@ class Builder: # Wait until we have processed all output self.out_queue.join() Print() + + msg = 'Completed: %d total built' % self.count + if self.already_done: + msg += ' (%d previously' % self.already_done + if self.already_done != self.count: + msg += ', %d newly' % (self.count - self.already_done) + msg += ')' + duration = datetime.now() - self._start_time + if duration > timedelta(microseconds=1000000): + if duration.microseconds >= 500000: + duration = duration + timedelta(seconds=1) + duration = duration - timedelta(microseconds=duration.microseconds) + msg += ', duration %s' % duration + Print(msg) + return (self.fail, self.warned) diff --git a/tools/buildman/test.py b/tools/buildman/test.py index a495df03036..fb861e16a1b 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -205,9 +205,9 @@ class TestBuild(unittest.TestCase): if line.text.strip(): count += 1 - # We should get two starting messages, then an update for every commit - # built. - self.assertEqual(count, len(commits) * len(boards) + 2) + # We should get two starting messages, an update for every commit built + # and a summary message + self.assertEqual(count, len(commits) * len(boards) + 3) build.SetDisplayOptions(**kwdisplay_args); build.ShowSummary(self.commits, board_selected) if echo_lines: From patchwork Thu Apr 9 21:08:48 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237599 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:48 -0600 Subject: [PATCH v2 21/28] buildman: Update the 'theory of operation' a little In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.21.Ib73f5c254b06a9d8c7019d86197aa685f6bcb90f@changeid> Make a few updates to this important section of the documentation, to make things clearer. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/README | 69 ++++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 30 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index 4cf01141574..86f5dfe501f 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -51,23 +51,25 @@ Theory of Operation Buildman is a builder. It is not make, although it runs make. It does not produce any useful output on the terminal while building, except for -progress information (except with -v, see below). All the output (errors, -warnings and binaries if you ask for them) is stored in output -directories, which you can look at while the build is progressing, or when -it is finished. +progress information (but see -v below). All the output (errors, warnings and +binaries if you ask for them) is stored in output directories, which you can +look at from a separate 'buildman -s' instance while the build is progressing, +or when it is finished. Buildman is designed to build entire git branches, i.e. muliple commits. It -can be run repeatedly on the same branch. In this case it will automatically -rebuild commits which have changed (and remove its old results for that -commit). It is possible to build a branch for one board, then later build it -for another board. If you want buildman to re-build a commit it has already -built (e.g. because of a toolchain update), use the -f flag. +can be run repeatedly on the same branch after making changes to commits on +that branch. In this case it will automatically rebuild commits which have +changed (and remove its old results for that commit). It is possible to build +a branch for one board, then later build it for another board. This adds to +the output, so now you have results for two boards. If you want buildman to +re-build a commit it has already built (e.g. because of a toolchain update), +use the -f flag. Buildman produces a concise summary of which boards succeeded and failed. It shows which commit introduced which board failure using a simple -red/green colour coding. Full error information can be requested, in which -case it is de-duped and displayed against the commit that introduced the -error. An example workflow is below. +red/green colour coding (with yellow/cyan for warnings). Full error +information can be requested, in which case it is de-duped and displayed +against the commit that introduced the error. An example workflow is below. Buildman stores image size information and can report changes in image size from commit to commit. An example of this is below. @@ -75,16 +77,20 @@ from commit to commit. An example of this is below. Buildman starts multiple threads, and each thread builds for one board at a time. A thread starts at the first commit, configures the source for your board and builds it. Then it checks out the next commit and does an -incremental build. Eventually the thread reaches the last commit and stops. -If errors or warnings are found along the way, the thread will reconfigure -after every commit, and your build will be very slow. This is because a -file that produces just a warning would not normally be rebuilt in an -incremental build. +incremental build (i.e. not using 'make xxx_defconfig' unless you use -C). +Eventually the thread reaches the last commit and stops. If a commit causes +an error or warning, buildman will try it again after reconfiguring (but see +-Q). Thus some commits may be built twice, with the first result silently +discarded. Lots of errors and warnings will causes lots of reconfigures and your +build will be very slow. This is because a file that produces just a warning +would not normally be rebuilt in an incremental build. Once a thread finishes +building all the commits for a board, it starts on the commits for another +board. Buildman works in an entirely separate place from your U-Boot repository. It creates a separate working directory for each thread, and puts the output files in the working directory, organised by commit name and board -name, in a two-level hierarchy. +name, in a two-level hierarchy (but see -P). Buildman is invoked in your U-Boot directory, the one with the .git directory. It clones this repository into a copy for each thread, and the @@ -92,20 +98,23 @@ threads do not affect the state of your git repository. Any checkouts done by the thread affect only the working directory for that thread. Buildman automatically selects the correct tool chain for each board. You -must supply suitable tool chains, but buildman takes care of selecting the -right one. +must supply suitable tool chains (see --fetch-arch), but buildman takes care +of selecting the right one. Buildman generally builds a branch (with the -b flag), and in this case -builds the upstream commit as well, for comparison. It cannot build -individual commits at present, unless (maybe) you point it at an empty -branch. Put all your commits in a branch, set the branch's upstream to a -valid value, and all will be well. Otherwise buildman will perform random -actions. Use -n to check what the random actions might be. - -If you just want to build the current source tree, leave off the -b flag -and add -e. This will display results and errors as they happen. You can -still look at them later using -se. Note that buildman will assume that the -source has changed, and will build all specified boards in this case. +builds the upstream commit as well, for comparison. So even if you have one +commit in your branch, two commits will be built. Put all your commits in a +branch, set the branch's upstream to a valid value, and all will be well. +Otherwise buildman will perform random actions. Use -n to check what the +random actions might be. + +Buildman effectively has two modes: without -s it builds, with -s it +summarises the results of previous (or active) builds. + +If you just want to build the current source tree, leave off the -b flag. +This will display results and errors as they happen. You can still look at +them later using -se. Note that buildman will assume that the source has +changed, and will build all specified boards in this case. Buildman is optimised for building many commits at once, for many boards. On multi-core machines, Buildman is fast because it uses most of the From patchwork Thu Apr 9 21:08:49 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237600 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:49 -0600 Subject: [PATCH v2 22/28] buildman: Add the abbreviation for --boards In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.22.Idd7cd7127181859708e5a1b7cc9632b1a7a444e2@changeid> This option may be frequency used, so mention that it can be abbreviated to --bo Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/README | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index 86f5dfe501f..9bf4383f9f7 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -151,9 +151,9 @@ You can also use -x to specifically exclude some boards. For example: means to build all arm boards except nvidia, freescale and anything ending with 'ball'. -For building specific boards you can use the --boards option, which takes a -comma-separated list of board target names and be used multiple times on -the command line: +For building specific boards you can use the --boards (or --bo) option, which +takes a comma-separated list of board target names and be used multiple times +on the command line: buildman --boards sandbox,snow --boards From patchwork Thu Apr 9 21:08:50 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237601 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:50 -0600 Subject: [PATCH v2 23/28] buildman: Update workflow documentation with more detail In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.23.I9bd0866fbf29c84b114f02f77c0892bd8e91924c@changeid> Make a few additions and change some wording in the workflow documentation. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/README | 53 +++++++++++++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 17 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index 9bf4383f9f7..ca0d1f64460 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -501,6 +501,8 @@ If it can't detect the upstream branch, try checking out the branch, and doing something like 'git branch --set-upstream-to upstream/master' or something similar. Buildman will try to guess a suitable upstream branch if it can't find one (you will see a message like" Guessing upstream as ...). +You can also use the -c option to manually specify the number of commits to +build. As an example: @@ -551,12 +553,13 @@ Buildman will set up some working directories, and get started. After a minute or so it will settle down to a steady pace, with a display like this: Building 18 commits for 1059 boards (4 threads, 1 job per thread) - 528 36 124 /19062 1:13:30 : SIMPC8313_SP + 528 36 124 /19062 -18374 1:13:30 : SIMPC8313_SP This means that it is building 19062 board/commit combinations. So far it has managed to successfully build 528. Another 36 have built with warnings, -and 124 more didn't build at all. Buildman expects to complete the process -in around an hour and a quarter. Use this time to buy a faster computer. +and 124 more didn't build at all. It has 18374 builds left to complete. +Buildman expects to complete the process in around an hour and a quarter. +Use this time to buy a faster computer. To find out how the build went, ask for a summary with -s. You can do this @@ -588,32 +591,32 @@ $ ./tools/buildman/buildman -b lcd9b -s This shows which commits have succeeded and which have failed. In this case the build is still in progress so many boards are not built yet (use -u to -see which ones). But still we can see a few failures. The galaxy5200_LOWBOOT +see which ones). But already we can see a few failures. The galaxy5200_LOWBOOT never builds correctly. This could be a problem with our toolchain, or it could be a bug in the upstream. The good news is that we probably don't need to blame our commits. The bad news is that our commits are not tested on that board. -Commit 12 broke lubbock. That's what the '+ lubbock' means. The failure -is never fixed by a later commit, or you would see lubbock again, in green, -without the +. +Commit 12 broke lubbock. That's what the '+ lubbock', in red, means. The +failure is never fixed by a later commit, or you would see lubbock again, in +green, without the +. To see the actual error: -$ ./tools/buildman/buildman -b -se lubbock +$ ./tools/buildman/buildman -b -se ... 12: lcd: Add support for flushing LCD fb from dcache after update arm: + lubbock +common/libcommon.o: In function `lcd_sync': -+/u-boot/lcd9b/.bm-work/00/common/lcd.c:120: undefined reference to `flush_dcache_range' ++common/lcd.c:120: undefined reference to `flush_dcache_range' +arm-none-linux-gnueabi-ld: BFD (Sourcery G++ Lite 2010q1-202) 2.19.51.20090709 assertion fail /scratch/julian/2010q1-release-linux-lite/obj/binutils-src-2010q1-202-arm-none-linux-gnueabi-i686-pc-linux-gnu/bfd/elf32-arm.c:12572 -+make: *** [/u-boot/lcd9b/.bm-work/00/build/u-boot] Error 139 ++make: *** [build/u-boot] Error 139 13: tegra: Align LCD frame buffer to section boundary 14: tegra: Support control of cache settings for LCD 15: tegra: fdt: Add LCD definitions for Seaboard 16: lcd: Add CONFIG_CONSOLE_SCROLL_LINES option to speed console --/u-boot/lcd9b/.bm-work/00/common/lcd.c:120: undefined reference to `flush_dcache_range' -+/u-boot/lcd9b/.bm-work/00/common/lcd.c:125: undefined reference to `flush_dcache_range' +-common/lcd.c:120: undefined reference to `flush_dcache_range' ++common/lcd.c:125: undefined reference to `flush_dcache_range' 17: tegra: Enable display/lcd support on Seaboard 18: wip @@ -621,6 +624,21 @@ So the problem is in lcd.c, due to missing cache operations. This information should be enough to work out what that commit is doing to break these boards. (In this case pxa did not have cache operations defined). +Note that if there were other boards with errors, the above command would +show their errors also. Each line is shown only once. So if lubbock and snow +produce the same error, we just see: + +12: lcd: Add support for flushing LCD fb from dcache after update + arm: + lubbock snow ++common/libcommon.o: In function `lcd_sync': ++common/lcd.c:120: undefined reference to `flush_dcache_range' ++arm-none-linux-gnueabi-ld: BFD (Sourcery G++ Lite 2010q1-202) 2.19.51.20090709 assertion fail /scratch/julian/2010q1-release-linux-lite/obj/binutils-src-2010q1-202-arm-none-linux-gnueabi-i686-pc-linux-gnu/bfd/elf32-arm.c:12572 ++make: *** [build/u-boot] Error 139 + +But if you did want to see just the errors for lubbock, use: + +$ ./tools/buildman/buildman -b -se lubbock + If you see error lines marked with '-', that means that the errors were fixed by that commit. Sometimes commits can be in the wrong order, so that a breakage is introduced for a few commits and fixed by later commits. This @@ -631,13 +649,14 @@ At commit 16, the error moves: you can see that the old error at line 120 is fixed, but there is a new one at line 126. This is probably only because we added some code and moved the broken line further down the file. -If many boards have the same error, then -e will display the error only -once. This makes the output as concise as possible. To see which boards have -each error, use -l. So it is safe to omit the board name - you will not get -lots of repeated output for every board. +As mentioned, if many boards have the same error, then -e will display the +error only once. This makes the output as concise as possible. To see which +boards have each error, use -l. So it is safe to omit the board name - you +will not get lots of repeated output for every board. Buildman tries to distinguish warnings from errors, and shows warning lines -separately with a 'w' prefix. +separately with a 'w' prefix. Warnings introduced show as yellow. Warnings +fixed show as cyan. The full build output in this case is available in: From patchwork Thu Apr 9 21:08:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237602 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:51 -0600 Subject: [PATCH v2 24/28] buildman: Make -I the default In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.24.I382759809727016f00b7861565dc22d55d3528ad@changeid> At present buildman defaults to running 'mrproper' on every thread before it starts building commits for each board. This can add a delay of about 5 seconds to the start of the process, since the tools and other invariants must be rebuilt. In particular, a build without '-b', to build current source, runs much slower without -I, since any existing build is removed, thus losing the possibility of an incremental build. Partly this behaviour was to avoid strange build-system problems caused by running 'make defconfig' for one board and then one with a different architecture. But these problems were fixed quite a while ago. The -I option (which disabled mrproper) was introduced four years ago and does not seem to cause any problems with builds. So make -I the default and deprecate the option. To allow use of 'mrproper', add a new -m flag. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: None tools/buildman/README | 13 ++++++------- tools/buildman/builder.py | 7 +++---- tools/buildman/builderthread.py | 6 +++--- tools/buildman/cmdline.py | 5 ++++- tools/buildman/control.py | 6 +++++- tools/buildman/func_test.py | 28 ++++++++++++++++++++-------- 6 files changed, 41 insertions(+), 24 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index ca0d1f64460..f299b0c2972 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -958,12 +958,11 @@ will build commits in us-buildman that are not in upstream/master. Building Faster =============== -By default, buildman executes 'make mrproper' prior to building the first -commit for each board. This causes everything to be built from scratch. If you -trust the build system's incremental build capabilities, you can pass the -I -flag to skip the 'make mproper' invocation, which will reduce the amount of -work 'make' does, and hence speed up the build. This flag will speed up any -buildman invocation, since it reduces the amount of work done on any build. +By default, buildman doesn't execute 'make mrproper' prior to building the +first commit for each board. This reduces the amount of work 'make' does, and +hence speeds up the build. To force use of 'make mrproper', use -the -m flag. +This flag will slow down any buildman invocation, since it increases the amount +of work done on any build. One possible application of buildman is as part of a continual edit, build, edit, build, ... cycle; repeatedly applying buildman to the same change or @@ -994,7 +993,7 @@ Combining all of these options together yields the command-line shown below. This will provide the quickest possible feedback regarding the current content of the source tree, thus allowing rapid tested evolution of the code. - SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -I -P tegra + SOURCE_DATE_EPOCH=0 ./tools/buildman/buildman -P tegra Checking configuration diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 6819e5b40da..597a03ffb08 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -231,7 +231,7 @@ class Builder: def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, - incremental=False, per_board_out_dir=False, + mrproper=False, per_board_out_dir=False, config_only=False, squash_config_y=False, warnings_as_errors=False, work_in_output=False): """Create a new Builder object @@ -252,8 +252,7 @@ class Builder: full_path: Return the full path in CROSS_COMPILE and don't set PATH verbose_build: Run build with V=1 and don't use 'make -s' - incremental: Always perform incremental builds; don't run make - mrproper when configuring + mrproper: Always run 'make mrproper' when configuring per_board_out_dir: Build in a separate persistent directory per board rather than a thread-specific directory config_only: Only configure each build, don't build it @@ -311,7 +310,7 @@ class Builder: self.queue = queue.Queue() self.out_queue = queue.Queue() for i in range(self.num_threads): - t = builderthread.BuilderThread(self, i, incremental, + t = builderthread.BuilderThread(self, i, mrproper, per_board_out_dir) t.setDaemon(True) t.start() diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py index 7561f399428..fc6e1ab25da 100644 --- a/tools/buildman/builderthread.py +++ b/tools/buildman/builderthread.py @@ -90,12 +90,12 @@ class BuilderThread(threading.Thread): thread_num: Our thread number (0-n-1), used to decide on a temporary directory """ - def __init__(self, builder, thread_num, incremental, per_board_out_dir): + def __init__(self, builder, thread_num, mrproper, per_board_out_dir): """Set up a new builder thread""" threading.Thread.__init__(self) self.builder = builder self.thread_num = thread_num - self.incremental = incremental + self.mrproper = mrproper self.per_board_out_dir = per_board_out_dir def Make(self, commit, brd, stage, cwd, *args, **kwargs): @@ -243,7 +243,7 @@ class BuilderThread(threading.Thread): # If we need to reconfigure, do that now if do_config: config_out = '' - if not self.incremental: + if self.mrproper: result = self.Make(commit, brd, 'mrproper', cwd, 'mrproper', *args, env=env) config_out += result.combined diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 17ea015a956..0178c6e8845 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -55,8 +55,9 @@ def ParseArgs(): parser.add_option('-i', '--in-tree', dest='in_tree', action='store_true', default=False, help='Build in the source tree instead of a separate directory') + # -I will be removed after April 2021 parser.add_option('-I', '--incremental', action='store_true', - default=False, help='Do not run make mrproper (when reconfiguring)') + default=False, help='Deprecated, does nothing. See -m') parser.add_option('-j', '--jobs', dest='jobs', type='int', default=None, help='Number of jobs to run at once (passed to make)') parser.add_option('-k', '--keep-outputs', action='store_true', @@ -69,6 +70,8 @@ def ParseArgs(): 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 (use -v to see probing detail)') + parser.add_option('-m', '--mrproper', action='store_true', + default=False, help="Run 'make mrproper before reconfiguring") 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 5ddc598c952..384e62dbc56 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -172,6 +172,10 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, print() return 0 + if options.incremental: + print(col.Color(col.RED, + 'Warning: -I has been removed. See documentation')) + # Work out what subset of the boards we are building if not boards: if not os.path.exists(options.output_dir): @@ -309,7 +313,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, show_unknown=options.show_unknown, step=options.step, no_subdirs=options.no_subdirs, full_path=options.full_path, verbose_build=options.verbose_build, - incremental=options.incremental, + mrproper=options.mrproper, per_board_out_dir=options.per_board_out_dir, config_only=options.config_only, squash_config_y=not options.preserve_config_y, diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py index 2a256a92639..b9e347ecb01 100644 --- a/tools/buildman/func_test.py +++ b/tools/buildman/func_test.py @@ -476,15 +476,15 @@ class TestFunctional(unittest.TestCase): self._RunControl('-b', TEST_BRANCH, '-c2', '-o', self._output_dir) self.assertEqual(self._builder.count, 2 * len(boards)) self.assertEqual(self._builder.fail, 0) - # Each board has a mrproper, config, and then one make per commit - self.assertEqual(self._make_calls, len(boards) * (2 + 2)) + # Each board has a config, and then one make per commit + self.assertEqual(self._make_calls, len(boards) * (1 + 2)) def testIncremental(self): """Test building a branch twice - the second time should do nothing""" self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir) # Each board has a mrproper, config, and then one make per commit - self.assertEqual(self._make_calls, len(boards) * (self._commits + 2)) + self.assertEqual(self._make_calls, len(boards) * (self._commits + 1)) self._make_calls = 0 self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir, clean_dir=False) self.assertEqual(self._make_calls, 0) @@ -496,14 +496,26 @@ class TestFunctional(unittest.TestCase): self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir) self._make_calls = 0 self._RunControl('-b', TEST_BRANCH, '-f', '-o', self._output_dir, clean_dir=False) - # Each board has a mrproper, config, and then one make per commit - self.assertEqual(self._make_calls, len(boards) * (self._commits + 2)) + # Each board has a config and one make per commit + self.assertEqual(self._make_calls, len(boards) * (self._commits + 1)) def testForceReconfigure(self): """The -f flag should force a rebuild""" self._RunControl('-b', TEST_BRANCH, '-C', '-o', self._output_dir) - # Each commit has a mrproper, config and make - self.assertEqual(self._make_calls, len(boards) * self._commits * 3) + # Each commit has a config and make + self.assertEqual(self._make_calls, len(boards) * self._commits * 2) + + def testForceReconfigure(self): + """The -f flag should force a rebuild""" + self._RunControl('-b', TEST_BRANCH, '-C', '-o', self._output_dir) + # Each commit has a config and make + self.assertEqual(self._make_calls, len(boards) * self._commits * 2) + + def testMrproper(self): + """The -f flag should force a rebuild""" + self._RunControl('-b', TEST_BRANCH, '-m', '-o', self._output_dir) + # Each board has a mkproper, config and then one make per commit + self.assertEqual(self._make_calls, len(boards) * (self._commits + 2)) def testErrors(self): """Test handling of build errors""" @@ -525,7 +537,7 @@ class TestFunctional(unittest.TestCase): self._RunControl('-b', TEST_BRANCH, '-o', self._output_dir, '-F', clean_dir=False) self.assertEqual(self._builder.count, self._total_builds) self.assertEqual(self._builder.fail, 0) - self.assertEqual(self._make_calls, 3) + self.assertEqual(self._make_calls, 2) def testBranchWithSlash(self): """Test building a branch with a '/' in the name""" From patchwork Thu Apr 9 21:08:52 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237605 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:52 -0600 Subject: [PATCH v2 25/28] buildman: Add an option to ignore device-tree warnings In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150840.v2.25.I1a7849b8623c5083ff547814647e5e86022bcec4@changeid> Unfortunately the plague of device-tree warnings has not lifted. These warnings infiltrate almost every build, adding noise and confusion. 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: - Drop use of temporary 'line' variable tools/buildman/README | 3 +++ tools/buildman/builder.py | 27 +++++++++++++++++---------- tools/buildman/cmdline.py | 3 +++ tools/buildman/control.py | 11 +++++------ tools/buildman/test.py | 31 ++++++++++++++++++++++++------- 5 files changed, 52 insertions(+), 23 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index f299b0c2972..0663ec33a5b 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -1124,6 +1124,9 @@ 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. + How to change from MAKEALL ========================== diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 597a03ffb08..45569aa1f81 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -338,16 +338,19 @@ 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): + show_environment=False, filter_dtb_warnings=False): """Setup display options for the builder. - show_errors: True to show summarised error/warning info - show_sizes: Show size deltas - show_detail: Show size delta detail for each board if show_sizes - show_bloat: Show detail for each function - list_error_boards: Show the boards which caused each error/warning - show_config: Show config deltas - show_environment: Show environment deltas + Args: + show_errors: True to show summarised error/warning info + show_sizes: Show size deltas + show_detail: Show size delta detail for each board if show_sizes + show_bloat: Show detail for each function + list_error_boards: Show the boards which caused each error/warning + show_config: Show config deltas + show_environment: Show environment deltas + filter_dtb_warnings: Filter out any warnings from the device-tree + compiler """ self._show_errors = show_errors self._show_sizes = show_sizes @@ -356,6 +359,7 @@ class Builder: self._list_error_boards = list_error_boards self._show_config = show_config self._show_environment = show_environment + self._filter_dtb_warnings = filter_dtb_warnings def _AddTimestamp(self): """Add a new timestamp to the list and record the build period. @@ -556,8 +560,11 @@ class Builder: """ out_lines = [] for line in lines: - if not self.re_make_err.search(line): - out_lines.append(line) + if self.re_make_err.search(line): + continue + if self._filter_dtb_warnings and self._re_dtb_warning.search(line): + continue + out_lines.append(line) return out_lines def ReadFuncSizes(self, fname, fd): diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py index 0178c6e8845..8510c077f74 100644 --- a/tools/buildman/cmdline.py +++ b/tools/buildman/cmdline.py @@ -114,6 +114,9 @@ def ParseArgs(): parser.add_option('-x', '--exclude', dest='exclude', type='string', action='append', 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') 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 384e62dbc56..45d9ab73ceb 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -345,16 +345,15 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, commits = None Print(GetActionSummary(options.summary, commits, board_selected, - options)) + options)) # We can't show function sizes without board details at present if options.show_bloat: options.show_detail = True - builder.SetDisplayOptions(options.show_errors, options.show_sizes, - options.show_detail, options.show_bloat, - options.list_error_boards, - options.show_config, - options.show_environment) + 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) if options.summary: builder.ShowSummary(commits, board_selected) else: diff --git a/tools/buildman/test.py b/tools/buildman/test.py index fb861e16a1b..e6698ce86ca 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -214,13 +214,15 @@ class TestBuild(unittest.TestCase): terminal.EchoPrintTestLines() return iter(terminal.GetPrintTestLines()) - def _CheckOutput(self, lines, list_error_boards): + def _CheckOutput(self, lines, list_error_boards, filter_dtb_warnings): """Check for expected output from the build summary Args: lines: Iterator containing the lines returned from the summary list_error_boards: Adjust the check for output produced with the --list-error-boards flag + filter_dtb_warnings: Adjust the check for output produced with the + --filter-dtb-warnings flag """ def add_line_prefix(prefix, boards, error_str, colour): """Add a prefix to each line of a string @@ -301,8 +303,10 @@ class TestBuild(unittest.TestCase): self.assertEqual(next(lines).text, add_line_prefix('-', boards234, errors[1], col.GREEN)) - self.assertEqual(next(lines).text, - add_line_prefix('w+', boards34, errors[2], col.YELLOW)) + if not filter_dtb_warnings: + self.assertEqual( + next(lines).text, + add_line_prefix('w+', boards34, errors[2], col.YELLOW)) # Fifth commit self.assertEqual(next(lines).text, '05: %s' % commits[4][1]) @@ -317,8 +321,10 @@ class TestBuild(unittest.TestCase): self.assertEqual(next(lines).text, add_line_prefix('+', boards4, expect, col.RED)) - self.assertEqual(next(lines).text, - add_line_prefix('w-', boards34, errors[2], col.CYAN)) + if not filter_dtb_warnings: + self.assertEqual( + next(lines).text, + add_line_prefix('w-', boards34, errors[2], col.CYAN)) # Sixth commit self.assertEqual(next(lines).text, '06: %s' % commits[5][1]) @@ -357,7 +363,8 @@ class TestBuild(unittest.TestCase): This does a line-by-line verification of the summary output. """ lines = self._SetupTest(show_errors=True) - self._CheckOutput(lines, list_error_boards=False) + self._CheckOutput(lines, list_error_boards=False, + filter_dtb_warnings=False) def testErrorBoards(self): """Test output with --list-error-boards @@ -365,7 +372,17 @@ 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) + self._CheckOutput(lines, list_error_boards=True, + filter_dtb_warnings=False) + + def testFilterDtb(self): + """Test output with --filter-dtb-warnings + + 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) def _testGit(self): """Test basic builder operation by building a branch""" 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""" From patchwork Thu Apr 9 21:08:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237606 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:54 -0600 Subject: [PATCH v2 27/28] buildman: Update the TODO items In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409210856.160952-2-sjg@chromium.org> A few of these have been done. Drop those and add some new ideas. Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: - Add patch to udpate TODO items tools/buildman/README | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index fe76d4ce0eb..cb565b551f8 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -1232,12 +1232,16 @@ Some options you might like are: TODO ==== -This has mostly be written in my spare time as a response to my difficulties -in testing large series of patches. Apart from tidying up there is quite a -bit of scope for improvement. Things like better error diffs and easier -access to log files. Also it would be nice if buildman could 'hunt' for -problems, perhaps by building a few boards for each arch, or checking -commits for changed files and building only boards which use those files. +Many improvements have been made over the years. There is still quite a bit of +scope for more though, e.g.: + +- easier access to log files +- 'hunting' for problems, perhaps by building a few boards for each arch, or + checking commits for changed files and building only boards which use those + files +- using the same git repo for all threads instead of cloning it. Currently + it uses about 500MB per thread, so on a 64-thread machine this is 32GB for + the build. Credits @@ -1253,3 +1257,4 @@ sjg at chromium.org Halloween 2012 Updated 12-12-12 Updated 23-02-13 +Updated 09-04-20 From patchwork Thu Apr 9 21:08:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Glass X-Patchwork-Id: 237604 List-Id: U-Boot discussion From: sjg at chromium.org (Simon Glass) Date: Thu, 9 Apr 2020 15:08:55 -0600 Subject: [PATCH v2 28/28] buildman: Change the exit codes In-Reply-To: <20200409210856.160952-1-sjg@chromium.org> References: <20200409210856.160952-1-sjg@chromium.org> Message-ID: <20200409150841.v2.28.I0537c150284b8c2f8dae5bb03de8f5be2c8cb1d3@changeid> The current exit codes of 128 and 129 are useful in that they do not conflict with those returned by tools, but they are not actually valid. It seems better to pick some codes which work with 'bit bisect run'. Update them to 100 (for errors) and 101 (for warnings). Signed-off-by: Simon Glass Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to change the exit codes tools/buildman/README | 12 ++++++------ tools/buildman/control.py | 4 ++-- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/tools/buildman/README b/tools/buildman/README index cb565b551f8..f3a0dc7288d 100644 --- a/tools/buildman/README +++ b/tools/buildman/README @@ -1114,15 +1114,15 @@ with -E, e.g. the migration warnings: When doing builds, Buildman's return code will reflect the overall result: 0 (success) No errors or warnings found - 128 Errors found - 129 Warnings found (only if no -W) + 100 Errors found + 101 Warnings found (only if no -W) -You can use -W to tell Buildman to return 0 (success) instead of 129 when +You can use -W to tell Buildman to return 0 (success) instead of 101 when warnings are found. Note that it can be useful to combine -E and -W. This means -that all compiler warnings will produce failures (code 128) and all other -warnings will produce success (since 129 is changed to 0). +that all compiler warnings will produce failures (code 100) and all other +warnings will produce success (since 101 is changed to 0). -If there are both warnings and errors, errors win, so buildman returns 128. +If there are both warnings and errors, errors win, so buildman returns 100. The -y option is provided (for use with -s) to ignore the bountiful device-tree warnings. Similarly, -Y tells buildman to ignore the migration warnings. diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 07f47a54454..30c030fd16e 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -361,7 +361,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, boards=None, fail, warned = builder.BuildBoards(commits, board_selected, options.keep_outputs, options.verbose) if fail: - return 128 + return 100 elif warned and not options.ignore_warnings: - return 129 + return 101 return 0