From: Simon Glass <simon.glass@canonical.com> Move the _process_config() method from Builder class to a module-level function process_config() in cfgutil.py This function parses .config and autoconf.h files. It has no dependencies on Builder state other than a single parameter (squash_config_y). Update the tests to call the function directly from cfgutil instead of through a Builder instance. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 41 ++------------------------------------- tools/buildman/cfgutil.py | 40 ++++++++++++++++++++++++++++++++++++++ tools/buildman/test.py | 26 +++++++++---------------- 3 files changed, 51 insertions(+), 56 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 5531e496271..33ac9ab6e0a 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -20,7 +20,7 @@ import sys import threading from buildman import builderthread -from buildman.cfgutil import Config +from buildman.cfgutil import Config, process_config from u_boot_pylib import command from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -828,43 +828,6 @@ class Builder: sym[name] = sym.get(name, 0) + int(size, 16) return sym - def _process_config(self, fname): - """Read in a .config, autoconf.mk or autoconf.h file - - This function handles all config file types. It ignores comments and - any #defines which don't start with CONFIG_. - - Args: - fname: Filename to read - - Returns: - Dictionary: - key: Config name (e.g. CONFIG_DM) - value: Config value (e.g. 1) - """ - config = {} - if os.path.exists(fname): - with open(fname, encoding='utf-8') as fd: - for line in fd: - line = line.strip() - if line.startswith('#define'): - values = line[8:].split(' ', 1) - if len(values) > 1: - key, value = values - else: - key = values[0] - value = '1' if self.squash_config_y else '' - if not key.startswith('CONFIG_'): - continue - elif not line or line[0] in ['#', '*', '/']: - continue - else: - key, value = line.split('=', 1) - if self.squash_config_y and value == 'y': - value = '1' - config[key] = value - return config - def _process_environment(self, fname): """Read in a uboot.env file @@ -981,7 +944,7 @@ class Builder: output_dir = self.get_build_dir(commit_upto, target) for name in self.config_filenames: fname = os.path.join(output_dir, name) - config[name] = self._process_config(fname) + config[name] = process_config(fname, self.squash_config_y) if read_environment: output_dir = self.get_build_dir(commit_upto, target) diff --git a/tools/buildman/cfgutil.py b/tools/buildman/cfgutil.py index ad6dee17829..2d4c6799b5c 100644 --- a/tools/buildman/cfgutil.py +++ b/tools/buildman/cfgutil.py @@ -5,6 +5,7 @@ """Utility functions for dealing with Kconfig .config files""" +import os import re from u_boot_pylib import tools @@ -266,3 +267,42 @@ Failed adjustments: {content} ''' return None + + +def process_config(fname, squash_config_y): + """Read in a .config, autoconf.mk or autoconf.h file + + This function handles all config file types. It ignores comments and + any #defines which don't start with CONFIG_. + + Args: + fname (str): Filename to read + squash_config_y (bool): If True, replace 'y' values with '1' + + Returns: + dict: Dictionary with: + key: Config name (e.g. CONFIG_DM) + value: Config value (e.g. '1') + """ + config = {} + if os.path.exists(fname): + with open(fname, encoding='utf-8') as fd: + for line in fd: + line = line.strip() + if line.startswith('#define'): + values = line[8:].split(' ', 1) + if len(values) > 1: + key, value = values + else: + key = values[0] + value = '1' if squash_config_y else '' + if not key.startswith('CONFIG_'): + continue + elif not line or line[0] in ['#', '*', '/']: + continue + else: + key, value = line.split('=', 1) + if squash_config_y and value == 'y': + value = '1' + config[key] = value + return config diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 828d05781cc..bdf68ef841d 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -1323,9 +1323,7 @@ class TestBuilderFuncs(TestBuildBase): os.unlink(tmp_name) def test_process_config_defconfig(self): - """Test _process_config() with .config style file""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) - + """Test process_config() with .config style file""" config_data = '''# This is a comment CONFIG_OPTION_A=y CONFIG_OPTION_B="string" @@ -1338,7 +1336,7 @@ CONFIG_OPTION_C=123 tmp_name = tmp.name try: - config = build._process_config(tmp_name) + config = cfgutil.process_config(tmp_name, squash_config_y=False) self.assertEqual('y', config['CONFIG_OPTION_A']) self.assertEqual('"string"', config['CONFIG_OPTION_B']) @@ -1349,9 +1347,7 @@ CONFIG_OPTION_C=123 os.unlink(tmp_name) def test_process_config_autoconf_h(self): - """Test _process_config() with autoconf.h style file""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) - + """Test process_config() with autoconf.h style file""" config_data = '''/* Auto-generated header */ #define CONFIG_OPTION_A 1 #define CONFIG_OPTION_B "value" @@ -1364,7 +1360,7 @@ CONFIG_OPTION_C=123 tmp_name = tmp.name try: - config = build._process_config(tmp_name) + config = cfgutil.process_config(tmp_name, squash_config_y=False) self.assertEqual('1', config['CONFIG_OPTION_A']) self.assertEqual('"value"', config['CONFIG_OPTION_B']) @@ -1376,17 +1372,13 @@ CONFIG_OPTION_C=123 os.unlink(tmp_name) def test_process_config_nonexistent(self): - """Test _process_config() with non-existent file""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) - - config = build._process_config('/nonexistent/path/config') + """Test process_config() with non-existent file""" + config = cfgutil.process_config('/nonexistent/path/config', + squash_config_y=False) self.assertEqual({}, config) def test_process_config_squash_y(self): - """Test _process_config() with squash_config_y enabled""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) - build.squash_config_y = True - + """Test process_config() with squash_config_y enabled""" config_data = '''CONFIG_OPTION_A=y CONFIG_OPTION_B=n #define CONFIG_OPTION_C @@ -1396,7 +1388,7 @@ CONFIG_OPTION_B=n tmp_name = tmp.name try: - config = build._process_config(tmp_name) + config = cfgutil.process_config(tmp_name, squash_config_y=True) # y should be squashed to 1 self.assertEqual('1', config['CONFIG_OPTION_A']) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Extract the Outcome class from Builder to a new outcome.py module. This class records build outcomes (return code, errors, sizes, config, etc.) and is independent of the Builder class. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 38 +++++--------------------------------- tools/buildman/outcome.py | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 40 insertions(+), 33 deletions(-) create mode 100644 tools/buildman/outcome.py diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 33ac9ab6e0a..94552bf62ca 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -21,6 +21,7 @@ import threading from buildman import builderthread from buildman.cfgutil import Config, process_config +from buildman.outcome import Outcome from u_boot_pylib import command from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -230,35 +231,6 @@ class Builder: _ide: Produce output suitable for an Integrated Development Environment i.e. don't emit progress information and put errors on stderr """ - class Outcome: - """Records a build outcome for a single make invocation - - Public Members: - rc: Outcome value (OUTCOME_...) - err_lines: List of error lines or [] if none - sizes: Dictionary of image size information, keyed by filename - - Each value is itself a dictionary containing - values for 'text', 'data' and 'bss', being the integer - size in bytes of each section. - func_sizes: Dictionary keyed by filename - e.g. 'u-boot'. Each - value is itself a dictionary: - key: function name - value: Size of function in bytes - config: Dictionary keyed by filename - e.g. '.config'. Each - value is itself a dictionary: - key: config name - value: config value - environment: Dictionary keyed by environment variable, Each - value is the value of environment variable. - """ - def __init__(self, rc, err_lines, sizes, func_sizes, config, - environment): - self.rc = rc - self.err_lines = err_lines - self.sizes = sizes - self.func_sizes = func_sizes - self.config = config - self.environment = environment def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, gnu_make='make', checkout=True, show_unknown=True, step=1, @@ -951,10 +923,10 @@ class Builder: fname = os.path.join(output_dir, 'uboot.env') environment = self._process_environment(fname) - return Builder.Outcome(rc, err_lines, sizes, func_sizes, config, + return Outcome(rc, err_lines, sizes, func_sizes, config, environment) - return Builder.Outcome(OUTCOME_UNKNOWN, [], {}, {}, {}, {}) + return Outcome(OUTCOME_UNKNOWN, [], {}, {}, {}, {}) @staticmethod def _add_line(lines_summary, lines_boards, line, brd): @@ -1027,7 +999,7 @@ class Builder: tuple: Tuple containing: Dict containing boards which built this commit: key: board.target - value: Builder.Outcome object + value: Outcome object List containing a summary of error lines Dict keyed by error line, containing a list of the Board objects with that error @@ -1135,7 +1107,7 @@ class Builder: """ self._base_board_dict = {} for brd in board_selected: - self._base_board_dict[brd] = Builder.Outcome(0, [], [], {}, {}, {}) + self._base_board_dict[brd] = Outcome(0, [], [], {}, {}, {}) self._base_err_lines = [] self._base_warn_lines = [] self._base_err_line_boards = {} diff --git a/tools/buildman/outcome.py b/tools/buildman/outcome.py new file mode 100644 index 00000000000..4f8bc9a0bae --- /dev/null +++ b/tools/buildman/outcome.py @@ -0,0 +1,35 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2013 The Chromium OS Authors. + +"""Outcome class for buildman build results""" + + +class Outcome: + """Records a build outcome for a single make invocation + + Public Members: + rc: Outcome value (OUTCOME_...) + err_lines: List of error lines or [] if none + sizes: Dictionary of image size information, keyed by filename + - Each value is itself a dictionary containing + values for 'text', 'data' and 'bss', being the integer + size in bytes of each section. + func_sizes: Dictionary keyed by filename - e.g. 'u-boot'. Each + value is itself a dictionary: + key: function name + value: Size of function in bytes + config: Dictionary keyed by filename - e.g. '.config'. Each + value is itself a dictionary: + key: config name + value: config value + environment: Dictionary keyed by environment variable, Each + value is the value of environment variable. + """ + def __init__(self, rc, err_lines, sizes, func_sizes, config, + environment): + self.rc = rc + self.err_lines = err_lines + self.sizes = sizes + self.func_sizes = func_sizes + self.config = config + self.environment = environment -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the BoardStatus and ErrLine namedtuples from resulthandler.py to outcome.py, centralising all result-related types in one module. Update the imports in resulthandler.py and builder.py accordingly. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 20 +++----------------- tools/buildman/outcome.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 96976f513e8..dd1da92e0ea 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -21,8 +21,9 @@ import threading from buildman import builderthread from buildman.cfgutil import Config, process_config -from buildman.outcome import (Outcome, OUTCOME_OK, OUTCOME_WARNING, - OUTCOME_ERROR, OUTCOME_UNKNOWN) +from buildman.outcome import (BoardStatus, ErrLine, Outcome, + OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, + OUTCOME_UNKNOWN) from u_boot_pylib import command from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -121,21 +122,6 @@ u-boot/ source directory .git/ repository """ -# Holds information about a particular error line we are outputting -# 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,brds,errline') - -# Holds the outcome of classifying the boards: -# ok: List of boards fixed since last commit -# warn: List of boards with warnings since last commit -# err: List of new broken boards since last commit -# new: List of boards that didn't exist last time -# unknown: List of boards that were not built -BoardStatus = collections.namedtuple('BoardStatus', 'ok,warn,err,new,unknown') - # Translate a commit subject into a valid filename (and handle unicode) trans_valid_chars = str.maketrans('/: ', '---') diff --git a/tools/buildman/outcome.py b/tools/buildman/outcome.py index 373740cd94b..2cbed9a1aba 100644 --- a/tools/buildman/outcome.py +++ b/tools/buildman/outcome.py @@ -3,9 +3,26 @@ """Outcome class and constants for buildman build results""" +from collections import namedtuple + # Build-outcome codes OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = list(range(4)) +# Board status for display purposes +# ok: List of boards fixed since last commit +# warn: List of boards with warnings since last commit +# err: List of new broken boards since last commit +# new: List of boards that didn't exist last time +# unknown: List of boards that were not built +BoardStatus = namedtuple('BoardStatus', 'ok warn err new unknown') + +# Error line information for display +# char: Character representation: '+': error, '-': fixed error, 'w+': warning, +# 'w-' = fixed warning +# brds: List of Board objects which have line in the error/warning output +# errline: The text of the error line +ErrLine = namedtuple('ErrLine', 'char brds errline') + class Outcome: """Records a build outcome for a single make invocation -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add a col parameter to Builder to allow passing in a terminal.Color object instead of creating one internally. This makes the dependency explicit and allows for easier testing. Update control.py to create the Color object and pass it to Builder. Update all test files to pass col to Builder. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 5 +++-- tools/buildman/control.py | 5 ++++- tools/buildman/test.py | 35 ++++++++++++++++++++-------------- tools/buildman/test_builder.py | 21 +++++++++++++------- 4 files changed, 42 insertions(+), 24 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 00a3b2f165b..e52956cda0e 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -216,7 +216,7 @@ class Builder: """ def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, - gnu_make='make', checkout=True, step=1, + col, gnu_make='make', checkout=True, step=1, no_subdirs=False, full_path=False, verbose_build=False, mrproper=False, fallback_mrproper=False, per_board_out_dir=False, config_only=False, @@ -236,6 +236,7 @@ class Builder: git_dir: Git directory containing source repository num_threads: Number of builder threads to run num_jobs: Number of jobs to run at once (passed to make as -j) + col: terminal.Color object for coloured output gnu_make: the command name of GNU Make. checkout: True to check out source, False to skip that step. This is used for testing. @@ -335,7 +336,7 @@ class Builder: self._restarting_config = False self.warnings_as_errors = warnings_as_errors - self.col = terminal.Color() + self.col = col self.thread_exceptions = [] self.test_thread_exceptions = test_thread_exceptions diff --git a/tools/buildman/control.py b/tools/buildman/control.py index ddb8c2f145e..398441aa9ed 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -809,9 +809,12 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, if args.config_only and args.target: raise ValueError('Cannot use --config-only with --target') + # Create colour object for output + col = terminal.Color() + # Create a new builder with the selected args builder = Builder(toolchains, output_dir, git_dir, - args.threads, args.jobs, checkout=True, step=args.step, + args.threads, args.jobs, col=col, checkout=True, step=args.step, no_subdirs=args.no_subdirs, full_path=args.full_path, verbose_build=args.verbose_build, mrproper=args.mrproper, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index cf37b821bf7..6702fd9aad8 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -264,7 +264,7 @@ class TestBuildOutput(TestBuildBase): Iterator containing the output lines, each a PrintLine() object """ build = builder.Builder(self.toolchains, self.base_dir, None, threads, - 2, checkout=False) + 2, self._col, checkout=False) build.do_make = self.make board_selected = self.brds.get_selected_dict() @@ -332,7 +332,7 @@ class TestBuildOutput(TestBuildBase): filter_dtb_warnings: Adjust the check for output produced with the --filter-dtb-warnings flag """ - col = terminal.Color() + col = self._col boards01234 = ('board0 board1 board2 board3 board4' if list_error_boards else '') boards1234 = 'board1 board2 board3 board4' if list_error_boards else '' @@ -642,7 +642,7 @@ class TestBuild(TestBuildBase): def test_output_dir(self): """Test output-directory naming for a commit""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False) + self._col, checkout=False) build.commits = self.commits build.commit_count = len(self.commits) subject = self.commits[1].subject.translate(builder.trans_valid_chars) @@ -652,7 +652,7 @@ class TestBuild(TestBuildBase): def test_output_dir_current(self): """Test output-directory naming for current source""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False) + self._col, checkout=False) build.commits = None build.commit_count = 0 self.check_dirs(build, '/current') @@ -660,7 +660,7 @@ class TestBuild(TestBuildBase): def test_output_dir_no_subdirs(self): """Test output-directory naming without subdirectories""" build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2, - checkout=False, no_subdirs=True) + self._col, checkout=False, no_subdirs=True) build.commits = None build.commit_count = 0 self.check_dirs(build, '') @@ -759,7 +759,8 @@ class TestBuild(TestBuildBase): for name in to_remove + to_leave: _touch(name) - build = builder.Builder(self.toolchains, base_dir, None, 1, 2) + build = builder.Builder(self.toolchains, base_dir, None, 1, 2, + self._col) build.commits = self.commits build.commit_count = len(COMMITS) # pylint: disable=protected-access @@ -1003,7 +1004,7 @@ class TestBuildMisc(TestBuildBase): # Check a missing tool with self.assertRaises(ValueError) as exc: builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - dtc_skip=True) + self._col, dtc_skip=True) self.assertIn('Cannot find dtc', str(exc.exception)) # Create a fake tool to use @@ -1012,13 +1013,14 @@ class TestBuildMisc(TestBuildBase): os.chmod(dtc, 0o777) build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - dtc_skip=True) + self._col, dtc_skip=True) tch = self.toolchains.select('arm') env = build.make_environment(tch) self.assertIn(b'DTC', env) # Try the normal case, i.e. not skipping the dtc build - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) tch = self.toolchains.select('arm') env = build.make_environment(tch) self.assertNotIn(b'DTC', env) @@ -1146,7 +1148,8 @@ class TestBuilderFuncs(TestBuildBase): def test_read_func_sizes(self): """Test read_func_sizes() function""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Create test data simulating 'nm' output # NM_SYMBOL_TYPES = 'tTdDbBr' - text, data, bss, rodata @@ -1175,7 +1178,8 @@ class TestBuilderFuncs(TestBuildBase): def test_read_func_sizes_static(self): """Test read_func_sizes() with static function symbols""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Test static functions (have . in name after first char) nm_output = '''00000100 t func.1234 @@ -1196,7 +1200,8 @@ class TestBuilderFuncs(TestBuildBase): def test_process_environment(self): """Test _process_environment() function""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Environment file uses null-terminated strings env_data = 'bootcmd=run bootm\x00bootdelay=3\x00console=ttyS0\x00' @@ -1215,14 +1220,16 @@ class TestBuilderFuncs(TestBuildBase): def test_process_environment_nonexistent(self): """Test _process_environment() with non-existent file""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) env = build._process_environment('/nonexistent/path/uboot.env') self.assertEqual({}, env) def test_process_environment_invalid_lines(self): """Test _process_environment() handles invalid lines gracefully""" - build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) + build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, + self._col) # Include lines without '=' which should be ignored env_data = 'valid=value\x00invalid_no_equals\x00another=good\x00' diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index b92e0be18be..928be84167d 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -24,9 +24,10 @@ class TestPrintFuncSizeDetail(unittest.TestCase): def setUp(self): """Set up test fixtures""" # Create a minimal Builder for testing + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp', git_dir=None, num_threads=0, - num_jobs=1) + num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -153,9 +154,10 @@ class TestPrepareThread(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -267,9 +269,10 @@ class TestPrepareWorkingSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -477,9 +480,10 @@ class TestPrepareOutputSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) terminal.set_print_test_mode() def tearDown(self): @@ -560,9 +564,10 @@ class TestCheckOutputForLoop(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) # Reset state before each test self.builder._restarting_config = False self.builder._terminated = False @@ -639,9 +644,10 @@ class TestMake(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) @mock.patch('buildman.builder.command.run_one') def test_make_basic(self, mock_run_one): @@ -731,9 +737,10 @@ class TestPrintBuildSummary(unittest.TestCase): def setUp(self): """Set up test fixtures""" + self.col = terminal.Color() self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1) + num_threads=4, num_jobs=1, col=self.col) # Set a start time in the past (less than 1 second ago to avoid # duration output) self.builder._start_time = datetime.now() -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the size-display methods from Builder to ResultHandler: - print_size_summary(): Print architecture-level size summaries - print_size_detail(): Print board-level size details - print_func_size_detail(): Print function-level bloat analysis - calc_size_changes(): Calculate size changes across boards - calc_image_size_changes(): Calculate per-image size changes - colour_num(): Format numbers with colour Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 262 +----------------------------- tools/buildman/main.py | 1 + tools/buildman/resulthandler.py | 280 +++++++++++++++++++++++++++++++- tools/buildman/test_builder.py | 16 +- 4 files changed, 289 insertions(+), 270 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index db8d980e83e..4b6106b881b 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -24,6 +24,7 @@ from buildman.cfgutil import Config, process_config from buildman.outcome import (BoardStatus, DisplayOptions, ErrLine, Outcome, OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN) +from buildman.resulthandler import ResultHandler from u_boot_pylib import command from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -1040,21 +1041,6 @@ class Builder: else: arch_list[arch] += text - - def colour_num(self, num): - """Format a number with colour depending on its value - - Args: - num (int): Number to format - - Returns: - str: Formatted string (red if positive, green if negative/zero) - """ - color = self.col.RED if num > 0 else self.col.GREEN - if num == 0: - return '0' - return self.col.build(color, str(num)) - def reset_result_summary(self, board_selected): """Reset the results summary ready for use. @@ -1078,247 +1064,6 @@ class Builder: self._base_config = None self._base_environment = None - def print_func_size_detail(self, fname, old, new): - """Print detailed size information for each function - - Args: - fname (str): Filename to print (e.g. 'u-boot') - old (dict): Dictionary of old function sizes, keyed by function name - new (dict): Dictionary of new function sizes, keyed by function name - """ - grow, shrink, add, remove, up, down = 0, 0, 0, 0, 0, 0 - delta, common = [], {} - - for a in old: - if a in new: - common[a] = 1 - - for name in old: - if name not in common: - remove += 1 - down += old[name] - delta.append([-old[name], name]) - - for name in new: - if name not in common: - add += 1 - up += new[name] - delta.append([new[name], name]) - - for name in common: - diff = new.get(name, 0) - old.get(name, 0) - if diff > 0: - grow, up = grow + 1, up + diff - elif diff < 0: - shrink, down = shrink + 1, down - diff - delta.append([diff, name]) - - delta.sort() - delta.reverse() - - args = [add, -remove, grow, -shrink, up, -down, up - down] - if max(args) == 0 and min(args) == 0: - return - args = [self.colour_num(x) for x in args] - indent = ' ' * 15 - tprint(f'{indent}{self.col.build(self.col.YELLOW, fname)}: add: ' - f'{args[0]}/{args[1]}, grow: {args[2]}/{args[3]} bytes: ' - f'{args[4]}/{args[5]} ({args[6]})') - tprint(f'{indent} {"function":<38s} {"old":>7s} {"new":>7s} ' - f'{"delta":>7s}') - for diff, name in delta: - if diff: - color = self.col.RED if diff > 0 else self.col.GREEN - msg = (f'{indent} {name:<38s} {old.get(name, "-"):>7} ' - f'{new.get(name, "-"):>7} {diff:+7d}') - tprint(msg, colour=color) - - - def print_size_detail(self, target_list, show_bloat): - """Show details size information for each board - - Args: - target_list (list): List of targets, each a dict containing: - 'target': Target name - 'total_diff': Total difference in bytes across all areas - <part_name>: Difference for that part - show_bloat (bool): Show detail for each function - """ - targets_by_diff = sorted(target_list, reverse=True, - key=lambda x: x['_total_diff']) - for result in targets_by_diff: - printed_target = False - for name in sorted(result): - diff = result[name] - if name.startswith('_'): - continue - colour = self.col.RED if diff > 0 else self.col.GREEN - msg = f' {name} {diff:+d}' - if not printed_target: - tprint(f'{"":10s} {result["_target"]:<15s}:', - newline=False) - printed_target = True - tprint(msg, colour=colour, newline=False) - if printed_target: - tprint() - if show_bloat: - target = result['_target'] - outcome = result['_outcome'] - base_outcome = self._base_board_dict[target] - for fname in outcome.func_sizes: - self.print_func_size_detail(fname, - base_outcome.func_sizes[fname], - outcome.func_sizes[fname]) - - - @staticmethod - def _calc_image_size_changes(target, sizes, base_sizes): - """Calculate size changes for each image/part - - Args: - target (str): Target board name - sizes (dict): Dict of image sizes, keyed by image name - base_sizes (dict): Dict of base image sizes, keyed by image name - - Returns: - dict: Size changes, e.g.: - {'_target': 'snapper9g45', 'data': 5, 'u-boot-spl:text': -4} - meaning U-Boot data increased by 5 bytes, SPL text decreased - by 4 - """ - err = {'_target' : target} - for image in sizes: - if image in base_sizes: - base_image = base_sizes[image] - # Loop through the text, data, bss parts - for part in sorted(sizes[image]): - diff = sizes[image][part] - base_image[part] - if diff: - if image == 'u-boot': - name = part - else: - name = image + ':' + part - err[name] = diff - return err - - def _calc_size_changes(self, board_selected, board_dict): - """Calculate changes in size for different image parts - - The previous sizes are in Board.sizes, for each board - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - - Returns: - tuple: (arch_list, arch_count) where: - arch_list: dict keyed by arch name, containing a list of - size-change dicts - arch_count: dict keyed by arch name, containing the number of - boards for that arch - """ - arch_list = {} - arch_count = {} - for target in board_dict: - if target not in board_selected: - continue - base_sizes = self._base_board_dict[target].sizes - outcome = board_dict[target] - sizes = outcome.sizes - err = self._calc_image_size_changes(target, sizes, base_sizes) - arch = board_selected[target].arch - if not arch in arch_count: - arch_count[arch] = 1 - else: - arch_count[arch] += 1 - if not sizes: - pass # Only add to our list when we have some stats - elif not arch in arch_list: - arch_list[arch] = [err] - else: - arch_list[arch].append(err) - return arch_list, arch_count - - def print_size_summary(self, board_selected, board_dict, show_detail, - show_bloat): - """Print a summary of image sizes broken down by section. - - The summary takes the form of one line per architecture. The - line contains deltas for each of the sections (+ means the section - got bigger, - means smaller). The numbers are the average number - of bytes that a board in this section increased by. - - For example: - powerpc: (622 boards) text -0.0 - arm: (285 boards) text -0.0 - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - show_detail (bool): Show size delta detail for each board - show_bloat (bool): Show detail for each function - """ - arch_list, arch_count = self._calc_size_changes(board_selected, - board_dict) - - # We now have a list of image size changes sorted by arch - # Print out a summary of these - for arch, target_list in arch_list.items(): - # Get total difference for each type - totals = {} - for result in target_list: - total = 0 - for name, diff in result.items(): - if name.startswith('_'): - continue - total += diff - if name in totals: - totals[name] += diff - else: - totals[name] = diff - result['_total_diff'] = total - result['_outcome'] = board_dict[result['_target']] - - self._print_arch_size_summary(arch, target_list, arch_count, - totals, show_detail, show_bloat) - - def _print_arch_size_summary(self, arch, target_list, arch_count, totals, - show_detail, show_bloat): - """Print size summary for a single architecture - - Args: - arch (str): Architecture name - target_list (list): List of size-change dicts for this arch - arch_count (dict): Dict of arch name to board count - totals (dict): Dict of name to total size diff - show_detail (bool): Show size delta detail for each board - show_bloat (bool): Show detail for each function - """ - count = len(target_list) - printed_arch = False - for name in sorted(totals): - diff = totals[name] - if diff: - # Display the average difference in this name for this - # architecture - avg_diff = float(diff) / count - color = self.col.RED if avg_diff > 0 else self.col.GREEN - msg = f' {name} {avg_diff:+1.1f}' - if not printed_arch: - tprint(f'{arch:>10s}: (for {count}/{arch_count[arch]} ' - 'boards)', newline=False) - printed_arch = True - tprint(msg, colour=color, newline=False) - - if printed_arch: - tprint() - if show_detail: - self.print_size_detail(target_list, show_bloat) - def _classify_boards(self, board_selected, board_dict): """Classify boards into outcome categories @@ -1789,8 +1534,9 @@ class Builder: worse_err, better_warn, worse_warn) if show_sizes: - self.print_size_summary(board_selected, board_dict, show_detail, - show_bloat) + self._result_handler.print_size_summary( + board_selected, board_dict, self._base_board_dict, + show_detail, show_bloat) if show_environment and self._base_environment: self._show_environment_changes(board_selected, board_dict, diff --git a/tools/buildman/main.py b/tools/buildman/main.py index c8502c370f9..af289a46508 100755 --- a/tools/buildman/main.py +++ b/tools/buildman/main.py @@ -79,6 +79,7 @@ def run_test_coverage(): 'tools/buildman/builderthread.py', 'tools/buildman/cfgutil.py', 'tools/buildman/control.py', + 'tools/buildman/resulthandler.py', 'tools/buildman/toolchain.py']) diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index 86f95f3eea5..4c9b3c2f9cc 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -1,14 +1,23 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2013 The Chromium OS Authors. +# +# Bloat-o-meter code used here Copyright 2004 Matt Mackall <mpm@selenic.com> +# -"""Result handler for buildman build results""" +"""Result writer for buildman build results""" + +from u_boot_pylib.terminal import tprint class ResultHandler: - """Handles display of build results and summaries + """Handles display of build size results and summaries + + This class is responsible for displaying size information from builds, + including per-architecture summaries, per-board details, and per-function + bloat analysis. - This class is responsible for displaying build results, including - size information, errors, warnings, and configuration changes. + Attributes: + col: terminal.Color object for coloured output """ def __init__(self, col, opts): @@ -29,3 +38,266 @@ class ResultHandler: builder (Builder): Builder object to use for getting results """ self._builder = builder + + def colour_num(self, num): + """Format a number with colour depending on its value + + Args: + num (int): Number to format + + Returns: + str: Formatted string (red if positive, green if negative/zero) + """ + color = self._col.RED if num > 0 else self._col.GREEN + if num == 0: + return '0' + return self._col.build(color, str(num)) + + def print_func_size_detail(self, fname, old, new): + """Print detailed size information for each function + + Args: + fname (str): Filename to print (e.g. 'u-boot') + old (dict): Dictionary of old function sizes, keyed by function name + new (dict): Dictionary of new function sizes, keyed by function name + """ + grow, shrink, add, remove, up, down = 0, 0, 0, 0, 0, 0 + delta, common = [], {} + + for a in old: + if a in new: + common[a] = 1 + + for name in old: + if name not in common: + remove += 1 + down += old[name] + delta.append([-old[name], name]) + + for name in new: + if name not in common: + add += 1 + up += new[name] + delta.append([new[name], name]) + + for name in common: + diff = new.get(name, 0) - old.get(name, 0) + if diff > 0: + grow, up = grow + 1, up + diff + elif diff < 0: + shrink, down = shrink + 1, down - diff + delta.append([diff, name]) + + delta.sort() + delta.reverse() + + args = [add, -remove, grow, -shrink, up, -down, up - down] + if max(args) == 0 and min(args) == 0: + return + args = [self.colour_num(x) for x in args] + indent = ' ' * 15 + tprint(f'{indent}{self._col.build(self._col.YELLOW, fname)}: add: ' + f'{args[0]}/{args[1]}, grow: {args[2]}/{args[3]} bytes: ' + f'{args[4]}/{args[5]} ({args[6]})') + tprint(f'{indent} {"function":<38s} {"old":>7s} {"new":>7s} ' + f'{"delta":>7s}') + for diff, name in delta: + if diff: + color = self._col.RED if diff > 0 else self._col.GREEN + msg = (f'{indent} {name:<38s} {old.get(name, "-"):>7} ' + f'{new.get(name, "-"):>7} {diff:+7d}') + tprint(msg, colour=color) + + def print_size_detail(self, target_list, base_board_dict, board_dict, + show_bloat): + """Show detailed size information for each board + + Args: + target_list (list): List of targets, each a dict containing: + 'target': Target name + 'total_diff': Total difference in bytes across all areas + <part_name>: Difference for that part + base_board_dict (dict): Dict of base board outcomes + board_dict (dict): Dict of current board outcomes + show_bloat (bool): Show detail for each function + """ + targets_by_diff = sorted(target_list, reverse=True, + key=lambda x: x['_total_diff']) + for result in targets_by_diff: + printed_target = False + for name in sorted(result): + diff = result[name] + if name.startswith('_'): + continue + colour = self._col.RED if diff > 0 else self._col.GREEN + msg = f' {name} {diff:+d}' + if not printed_target: + tprint(f'{"":10s} {result["_target"]:<15s}:', + newline=False) + printed_target = True + tprint(msg, colour=colour, newline=False) + if printed_target: + tprint() + if show_bloat: + target = result['_target'] + outcome = board_dict[target] + base_outcome = base_board_dict[target] + for fname in outcome.func_sizes: + self.print_func_size_detail(fname, + base_outcome.func_sizes[fname], + outcome.func_sizes[fname]) + + @staticmethod + def calc_image_size_changes(target, sizes, base_sizes): + """Calculate size changes for each image/part + + Args: + target (str): Target board name + sizes (dict): Dict of image sizes, keyed by image name + base_sizes (dict): Dict of base image sizes, keyed by image name + + Returns: + dict: Size changes, e.g.: + {'_target': 'snapper9g45', 'data': 5, 'u-boot-spl:text': -4} + meaning U-Boot data increased by 5 bytes, SPL text decreased + by 4 + """ + err = {'_target' : target} + for image in sizes: + if image in base_sizes: + base_image = base_sizes[image] + # Loop through the text, data, bss parts + for part in sorted(sizes[image]): + diff = sizes[image][part] - base_image[part] + if diff: + if image == 'u-boot': + name = part + else: + name = image + ':' + part + err[name] = diff + return err + + def calc_size_changes(self, board_selected, board_dict, base_board_dict): + """Calculate changes in size for different image parts + + The previous sizes are in Board.sizes, for each board + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + base_board_dict (dict): Dict of base board outcomes + + Returns: + tuple: (arch_list, arch_count) where: + arch_list: dict keyed by arch name, containing a list of + size-change dicts + arch_count: dict keyed by arch name, containing the number of + boards for that arch + """ + arch_list = {} + arch_count = {} + for target in board_dict: + if target not in board_selected: + continue + base_sizes = base_board_dict[target].sizes + outcome = board_dict[target] + sizes = outcome.sizes + err = self.calc_image_size_changes(target, sizes, base_sizes) + arch = board_selected[target].arch + if not arch in arch_count: + arch_count[arch] = 1 + else: + arch_count[arch] += 1 + if not sizes: + pass # Only add to our list when we have some stats + elif not arch in arch_list: + arch_list[arch] = [err] + else: + arch_list[arch].append(err) + return arch_list, arch_count + + def print_size_summary(self, board_selected, board_dict, base_board_dict, + show_detail, show_bloat): + """Print a summary of image sizes broken down by section. + + The summary takes the form of one line per architecture. The + line contains deltas for each of the sections (+ means the section + got bigger, - means smaller). The numbers are the average number + of bytes that a board in this section increased by. + + For example: + powerpc: (622 boards) text -0.0 + arm: (285 boards) text -0.0 + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + base_board_dict (dict): Dict of base board outcomes + show_detail (bool): Show size delta detail for each board + show_bloat (bool): Show detail for each function + """ + arch_list, arch_count = self.calc_size_changes(board_selected, + board_dict, + base_board_dict) + + # We now have a list of image size changes sorted by arch + # Print out a summary of these + for arch, target_list in arch_list.items(): + # Get total difference for each type + totals = {} + for result in target_list: + total = 0 + for name, diff in result.items(): + if name.startswith('_'): + continue + total += diff + if name in totals: + totals[name] += diff + else: + totals[name] = diff + result['_total_diff'] = total + + self._print_arch_size_summary(arch, target_list, arch_count, + totals, base_board_dict, board_dict, + show_detail, show_bloat) + + def _print_arch_size_summary(self, arch, target_list, arch_count, totals, + base_board_dict, board_dict, + show_detail, show_bloat): + """Print size summary for a single architecture + + Args: + arch (str): Architecture name + target_list (list): List of size-change dicts for this arch + arch_count (dict): Dict of arch name to board count + totals (dict): Dict of name to total size diff + base_board_dict (dict): Dict of base board outcomes + board_dict (dict): Dict of current board outcomes + show_detail (bool): Show size delta detail for each board + show_bloat (bool): Show detail for each function + """ + count = len(target_list) + printed_arch = False + for name in sorted(totals): + diff = totals[name] + if diff: + # Display the average difference in this name for this + # architecture + avg_diff = float(diff) / count + color = self._col.RED if avg_diff > 0 else self._col.GREEN + msg = f' {name} {avg_diff:+1.1f}' + if not printed_arch: + tprint(f'{arch:>10s}: (for {count}/{arch_count[arch]} ' + 'boards)', newline=False) + printed_arch = True + tprint(msg, colour=color, newline=False) + + if printed_arch: + tprint() + if show_detail: + self.print_size_detail(target_list, base_board_dict, board_dict, + show_bloat) diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index 40132c1b46f..69e9e324c53 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -20,7 +20,7 @@ from u_boot_pylib import terminal class TestPrintFuncSizeDetail(unittest.TestCase): - """Tests for Builder.print_func_size_detail()""" + """Tests for ResultHandler.print_func_size_detail()""" def setUp(self): """Set up test fixtures""" @@ -46,7 +46,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100, 'func_b': 200} terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() # No output when there are no changes @@ -58,7 +58,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 150} terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() text = '\n'.join(line.text for line in lines) @@ -76,7 +76,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 150} terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() text = '\n'.join(line.text for line in lines) @@ -89,7 +89,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100, 'func_b': 200} terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() text = '\n'.join(line.text for line in lines) @@ -105,7 +105,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100} terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() text = '\n'.join(line.text for line in lines) @@ -129,7 +129,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', old, new) + self.result_handler.print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() text = '\n'.join(line.text for line in lines) @@ -148,7 +148,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): def test_empty_dicts(self): """Test with empty dictionaries""" terminal.get_print_test_lines() # Clear - self.builder.print_func_size_detail('u-boot', {}, {}) + self.result_handler.print_func_size_detail('u-boot', {}, {}) lines = terminal.get_print_test_lines() # No output when both dicts are empty -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Complete the extraction of display-related methods from builder.py to ResultHandler. Move the board classification and status methods: - classify_boards(): Categorise boards by outcome status - show_not_built(): Report boards that couldn't be built Move the BoardStatus namedtuple to resulthandler.py and import it into builder.py. The methods take OUTCOME constants as parameters to avoid circular imports. Update the tests to use ResultHandler.show_not_built() instead of the old Builder._show_not_built() method. This completes the ResultHandler extraction, reducing builder.py from ~2200 lines to ~1250 lines (removing ~950 lines of display code). Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 326 +------------------------------- tools/buildman/resulthandler.py | 326 ++++++++++++++++++++++++++++++++ tools/buildman/test_builder.py | 18 +- 3 files changed, 343 insertions(+), 327 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 4d3b96ab1a0..53f1c6bde98 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1034,289 +1034,6 @@ class Builder: self._base_config = None self._base_environment = None - def _classify_boards(self, board_selected, board_dict): - """Classify boards into outcome categories - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - - Returns: - BoardStatus: Named tuple containing lists of board targets - """ - ok = [] # List of boards fixed since last commit - warn = [] # List of boards with warnings since last commit - err = [] # List of new broken boards since last commit - new = [] # List of boards that didn't exist last time - unknown = [] # List of boards that were not built - - for target in board_dict: - if target not in board_selected: - continue - - # If the board was built last time, add its outcome to a list - if target in self._base_board_dict: - base_outcome = self._base_board_dict[target].rc - outcome = board_dict[target] - if outcome.rc == OUTCOME_UNKNOWN: - unknown.append(target) - elif outcome.rc < base_outcome: - if outcome.rc == OUTCOME_WARNING: - warn.append(target) - else: - ok.append(target) - elif outcome.rc > base_outcome: - if outcome.rc == OUTCOME_WARNING: - warn.append(target) - else: - err.append(target) - else: - new.append(target) - return BoardStatus(ok, warn, err, new, unknown) - - @staticmethod - def _calc_config(delta, name, config): - """Calculate configuration changes - - Args: - delta: Type of the delta, e.g. '+' - name: name of the file which changed (e.g. .config) - config: configuration change dictionary - key: config name - value: config value - Returns: - String containing the configuration changes which can be - printed - """ - out = '' - for key in sorted(config.keys()): - out += f'{key}={config[key]} ' - return f'{delta} {name}: {out}' - - @classmethod - def _add_config(cls, lines, name, config_plus, config_minus, config_change): - """Add changes in configuration to a list - - Args: - lines: list to add to - name: config file name - config_plus: configurations added, dictionary - key: config name - value: config value - config_minus: configurations removed, dictionary - key: config name - value: config value - config_change: configurations changed, dictionary - key: config name - value: config value - """ - if config_plus: - lines.append(cls._calc_config('+', name, config_plus)) - if config_minus: - lines.append(cls._calc_config('-', name, config_minus)) - if config_change: - lines.append(cls._calc_config('c', name, config_change)) - - def _output_config_info(self, lines): - """Output configuration change information - - Args: - lines: List of configuration change strings - """ - for line in lines: - if not line: - continue - col = None - if line[0] == '+': - col = self.col.GREEN - elif line[0] == '-': - col = self.col.RED - elif line[0] == 'c': - col = self.col.YELLOW - tprint(' ' + line, newline=True, colour=col) - - def _show_environment_changes(self, board_selected, board_dict, - environment): - """Show changes in environment variables - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - environment (dict): Dict of environment changes, keyed by - board.target - """ - lines = [] - for target in board_dict: - if target not in board_selected: - continue - - tbase = self._base_environment[target] - tenvironment = environment[target] - environment_plus = {} - environment_minus = {} - environment_change = {} - base = tbase.environment - for key, value in tenvironment.environment.items(): - if key not in base: - environment_plus[key] = value - for key, value in base.items(): - if key not in tenvironment.environment: - environment_minus[key] = value - for key, value in base.items(): - new_value = tenvironment.environment.get(key) - if new_value and value != new_value: - desc = f'{value} -> {new_value}' - environment_change[key] = desc - - self._add_config(lines, target, environment_plus, - environment_minus, environment_change) - self._output_config_info(lines) - - def _calc_config_changes(self, target, arch, config, arch_config_plus, - arch_config_minus, arch_config_change): - """Calculate configuration changes for a single target - - Args: - target (str): Target board name - arch (str): Architecture name - config (dict): Dict of config changes, keyed by board.target - arch_config_plus (dict): Dict to update with added configs by - arch - arch_config_minus (dict): Dict to update with removed configs by - arch - arch_config_change (dict): Dict to update with changed configs by - arch - - Returns: - str: Summary of config changes for this target - """ - all_config_plus = {} - all_config_minus = {} - all_config_change = {} - tbase = self._base_config[target] - tconfig = config[target] - lines = [] - for name in self.config_filenames: - if not tconfig.config[name]: - continue - config_plus = {} - config_minus = {} - config_change = {} - base = tbase.config[name] - for key, value in tconfig.config[name].items(): - if key not in base: - config_plus[key] = value - all_config_plus[key] = value - for key, value in base.items(): - if key not in tconfig.config[name]: - config_minus[key] = value - all_config_minus[key] = value - for key, value in base.items(): - new_value = tconfig.config[name].get(key) - if new_value and value != new_value: - desc = f'{value} -> {new_value}' - config_change[key] = desc - all_config_change[key] = desc - - arch_config_plus[arch][name].update(config_plus) - arch_config_minus[arch][name].update(config_minus) - arch_config_change[arch][name].update(config_change) - - self._add_config(lines, name, config_plus, config_minus, - config_change) - self._add_config(lines, 'all', all_config_plus, - all_config_minus, all_config_change) - return '\n'.join(lines) - - def _print_arch_config_summary(self, arch, arch_config_plus, - arch_config_minus, arch_config_change): - """Print configuration summary for a single architecture - - Args: - arch (str): Architecture name - arch_config_plus (dict): Dict of added configs by arch/filename - arch_config_minus (dict): Dict of removed configs by arch/filename - arch_config_change (dict): Dict of changed configs by arch/filename - """ - lines = [] - all_plus = {} - all_minus = {} - all_change = {} - for name in self.config_filenames: - all_plus.update(arch_config_plus[arch][name]) - all_minus.update(arch_config_minus[arch][name]) - all_change.update(arch_config_change[arch][name]) - self._add_config(lines, name, - arch_config_plus[arch][name], - arch_config_minus[arch][name], - arch_config_change[arch][name]) - self._add_config(lines, 'all', all_plus, all_minus, all_change) - if lines: - tprint(f'{arch}:') - self._output_config_info(lines) - - def _show_config_changes(self, board_selected, board_dict, config): - """Show changes in configuration - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - config (dict): Dict of config changes, keyed by board.target - """ - summary = {} - arch_config_plus = {} - arch_config_minus = {} - arch_config_change = {} - arch_list = [] - - for target in board_dict: - if target not in board_selected: - continue - arch = board_selected[target].arch - if arch not in arch_list: - arch_list.append(arch) - - for arch in arch_list: - arch_config_plus[arch] = {} - arch_config_minus[arch] = {} - arch_config_change[arch] = {} - for name in self.config_filenames: - arch_config_plus[arch][name] = {} - arch_config_minus[arch][name] = {} - arch_config_change[arch][name] = {} - - for target in board_dict: - if target not in board_selected: - continue - arch = board_selected[target].arch - summary[target] = self._calc_config_changes( - target, arch, config, arch_config_plus, arch_config_minus, - arch_config_change) - - lines_by_target = {} - for target, lines in summary.items(): - if lines in lines_by_target: - lines_by_target[lines].append(target) - else: - lines_by_target[lines] = [target] - - for arch in arch_list: - self._print_arch_config_summary(arch, arch_config_plus, - arch_config_minus, - arch_config_change) - - for lines, targets in lines_by_target.items(): - if not lines: - continue - tprint(f"{' '.join(sorted(targets))} :") - self._output_config_info(lines.split('\n')) - def print_result_summary(self, board_selected, board_dict, err_lines, err_line_boards, warn_lines, warn_line_boards, config, environment, show_sizes, show_detail, @@ -1407,7 +1124,8 @@ class Builder: better_lines.append(errline) return better_lines, worse_lines - brd_status = self._classify_boards(board_selected, board_dict) + brd_status = ResultHandler.classify_boards( + board_selected, board_dict, self._base_board_dict) # Get a list of errors and warnings that have appeared, and disappeared better_err, worse_err = _calc_error_delta(self._base_err_lines, @@ -1431,11 +1149,13 @@ class Builder: show_detail, show_bloat) if show_environment and self._base_environment: - self._show_environment_changes(board_selected, board_dict, - environment) + self._result_handler.show_environment_changes( + board_selected, board_dict, environment, self._base_environment) if show_config and self._base_config: - self._show_config_changes(board_selected, board_dict, config) + self._result_handler.show_config_changes( + board_selected, board_dict, config, self._base_config, + self.config_filenames) # Save our updated information for the next call to this function @@ -1447,37 +1167,7 @@ class Builder: self._base_config = config self._base_environment = environment - self._show_not_built(board_selected, board_dict) - - @staticmethod - def _show_not_built(board_selected, board_dict): - """Show boards that were not built - - This reports boards that couldn't be built due to toolchain issues. - These have OUTCOME_UNKNOWN (no result file) or OUTCOME_ERROR with - "Tool chain error" in the error lines. - - Args: - board_selected (dict): Dict of selected boards, keyed by target - board_dict (dict): Dict of boards that were built, keyed by target - """ - not_built = [] - for target in board_selected: - if target not in board_dict: - not_built.append(target) - else: - outcome = board_dict[target] - if outcome.rc == OUTCOME_UNKNOWN: - not_built.append(target) - elif outcome.rc == OUTCOME_ERROR: - # Check for toolchain error in the error lines - for line in outcome.err_lines: - if 'Tool chain error' in line: - not_built.append(target) - break - if not_built: - tprint(f"Boards not built ({len(not_built)}): " - f"{', '.join(not_built)}") + ResultHandler.show_not_built(board_selected, board_dict) def produce_result_summary(self, commit_upto, commits, board_selected): """Produce a summary of the results for a single commit diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index 56973d801bb..197f4d87003 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -8,6 +8,8 @@ import sys +from buildman.outcome import (BoardStatus, OUTCOME_OK, OUTCOME_WARNING, + OUTCOME_ERROR, OUTCOME_UNKNOWN) from u_boot_pylib.terminal import tprint @@ -419,3 +421,327 @@ class ResultHandler: outcome = board_dict[target] for line in outcome.err_lines: sys.stderr.write(line) + + @staticmethod + def calc_config(delta, name, config): + """Calculate configuration changes + + Args: + delta: Type of the delta, e.g. '+' + name: name of the file which changed (e.g. .config) + config: configuration change dictionary + key: config name + value: config value + Returns: + String containing the configuration changes which can be + printed + """ + out = '' + for key in sorted(config.keys()): + out += f'{key}={config[key]} ' + return f'{delta} {name}: {out}' + + @classmethod + def add_config(cls, lines, name, config_plus, config_minus, config_change): + """Add changes in configuration to a list + + Args: + lines: list to add to + name: config file name + config_plus: configurations added, dictionary + key: config name + value: config value + config_minus: configurations removed, dictionary + key: config name + value: config value + config_change: configurations changed, dictionary + key: config name + value: config value + """ + if config_plus: + lines.append(cls.calc_config('+', name, config_plus)) + if config_minus: + lines.append(cls.calc_config('-', name, config_minus)) + if config_change: + lines.append(cls.calc_config('c', name, config_change)) + + def output_config_info(self, lines): + """Output configuration change information + + Args: + lines: List of configuration change strings + """ + for line in lines: + if not line: + continue + col = None + if line[0] == '+': + col = self.col.GREEN + elif line[0] == '-': + col = self.col.RED + elif line[0] == 'c': + col = self.col.YELLOW + tprint(' ' + line, newline=True, colour=col) + + def show_environment_changes(self, board_selected, board_dict, + environment, base_environment): + """Show changes in environment variables + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + environment (dict): Dict of environment changes, keyed by + board.target + base_environment (dict): Dict of base environment, keyed by + board.target + """ + lines = [] + for target in board_dict: + if target not in board_selected: + continue + + tbase = base_environment[target] + tenvironment = environment[target] + environment_plus = {} + environment_minus = {} + environment_change = {} + base = tbase.environment + for key, value in tenvironment.environment.items(): + if key not in base: + environment_plus[key] = value + for key, value in base.items(): + if key not in tenvironment.environment: + environment_minus[key] = value + for key, value in base.items(): + new_value = tenvironment.environment.get(key) + if new_value and value != new_value: + desc = f'{value} -> {new_value}' + environment_change[key] = desc + + self.add_config(lines, target, environment_plus, + environment_minus, environment_change) + self.output_config_info(lines) + + def calc_config_changes(self, target, config, base_config, config_filenames, + arch, arch_config_plus, arch_config_minus, + arch_config_change): + """Calculate configuration changes for a single target + + Args: + target (str): Target board name + config (dict): Dict of config changes, keyed by board.target + base_config (dict): Dict of base config, keyed by board.target + config_filenames (list): List of config filenames to check + arch (str): Architecture name + arch_config_plus (dict): Dict to update with added configs by arch + arch_config_minus (dict): Dict to update with removed configs by + arch + arch_config_change (dict): Dict to update with changed configs by + arch + + Returns: + str: Summary of config changes for this target + """ + all_config_plus = {} + all_config_minus = {} + all_config_change = {} + tbase = base_config[target] + tconfig = config[target] + lines = [] + for name in config_filenames: + if not tconfig.config[name]: + continue + config_plus = {} + config_minus = {} + config_change = {} + base = tbase.config[name] + for key, value in tconfig.config[name].items(): + if key not in base: + config_plus[key] = value + all_config_plus[key] = value + for key, value in base.items(): + if key not in tconfig.config[name]: + config_minus[key] = value + all_config_minus[key] = value + for key, value in base.items(): + new_value = tconfig.config[name].get(key) + if new_value and value != new_value: + desc = f'{value} -> {new_value}' + config_change[key] = desc + all_config_change[key] = desc + + arch_config_plus[arch][name].update(config_plus) + arch_config_minus[arch][name].update(config_minus) + arch_config_change[arch][name].update(config_change) + + self.add_config(lines, name, config_plus, config_minus, + config_change) + self.add_config(lines, 'all', all_config_plus, + all_config_minus, all_config_change) + return '\n'.join(lines) + + def print_arch_config_summary(self, arch, arch_config_plus, + arch_config_minus, arch_config_change, + config_filenames): + """Print configuration summary for a single architecture + + Args: + arch (str): Architecture name + arch_config_plus (dict): Dict of added configs by arch/filename + arch_config_minus (dict): Dict of removed configs by arch/filename + arch_config_change (dict): Dict of changed configs by arch/filename + config_filenames (list): List of config filenames to check + """ + lines = [] + all_plus = {} + all_minus = {} + all_change = {} + for name in config_filenames: + all_plus.update(arch_config_plus[arch][name]) + all_minus.update(arch_config_minus[arch][name]) + all_change.update(arch_config_change[arch][name]) + self.add_config(lines, name, + arch_config_plus[arch][name], + arch_config_minus[arch][name], + arch_config_change[arch][name]) + self.add_config(lines, 'all', all_plus, all_minus, all_change) + if lines: + tprint(f'{arch}:') + self.output_config_info(lines) + + def show_config_changes(self, board_selected, board_dict, config, + base_config, config_filenames): + """Show changes in configuration + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + config (dict): Dict of config changes, keyed by board.target + base_config (dict): Dict of base config, keyed by board.target + config_filenames (list): List of config filenames to check + """ + summary = {} + arch_config_plus = {} + arch_config_minus = {} + arch_config_change = {} + arch_list = [] + + for target in board_dict: + if target not in board_selected: + continue + arch = board_selected[target].arch + if arch not in arch_list: + arch_list.append(arch) + + for arch in arch_list: + arch_config_plus[arch] = {} + arch_config_minus[arch] = {} + arch_config_change[arch] = {} + for name in config_filenames: + arch_config_plus[arch][name] = {} + arch_config_minus[arch][name] = {} + arch_config_change[arch][name] = {} + + for target in board_dict: + if target not in board_selected: + continue + arch = board_selected[target].arch + summary[target] = self.calc_config_changes( + target, config, base_config, config_filenames, arch, + arch_config_plus, arch_config_minus, arch_config_change) + + lines_by_target = {} + for target, lines in summary.items(): + if lines in lines_by_target: + lines_by_target[lines].append(target) + else: + lines_by_target[lines] = [target] + + for arch in arch_list: + self.print_arch_config_summary(arch, arch_config_plus, + arch_config_minus, + arch_config_change, config_filenames) + + for lines, targets in lines_by_target.items(): + if not lines: + continue + tprint(f"{' '.join(sorted(targets))} :") + self.output_config_info(lines.split('\n')) + + @staticmethod + def classify_boards(board_selected, board_dict, base_board_dict): + """Classify boards into outcome categories + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + base_board_dict (dict): Dict of base board outcomes + + Returns: + BoardStatus: Named tuple containing lists of board targets + """ + ok = [] # List of boards fixed since last commit + warn = [] # List of boards with warnings since last commit + err = [] # List of new broken boards since last commit + new = [] # List of boards that didn't exist last time + unknown = [] # List of boards that were not built + + for target in board_dict: + if target not in board_selected: + continue + + # If the board was built last time, add its outcome to a list + if target in base_board_dict: + base_outcome = base_board_dict[target].rc + outcome = board_dict[target] + if outcome.rc == OUTCOME_UNKNOWN: + unknown.append(target) + elif outcome.rc < base_outcome: + if outcome.rc == OUTCOME_WARNING: + warn.append(target) + else: + ok.append(target) + elif outcome.rc > base_outcome: + if outcome.rc == OUTCOME_WARNING: + warn.append(target) + else: + err.append(target) + else: + new.append(target) + return BoardStatus(ok, warn, err, new, unknown) + + @staticmethod + def show_not_built(board_selected, board_dict): + """Show boards that were not built + + This reports boards that couldn't be built due to toolchain issues. + These have OUTCOME_UNKNOWN (no result file) or OUTCOME_ERROR with + "Tool chain error" in the error lines. + + Args: + board_selected (dict): Dict of selected boards, keyed by target + board_dict (dict): Dict of boards that were built, keyed by target + """ + not_built = [] + for target in board_selected: + if target not in board_dict: + not_built.append(target) + else: + outcome = board_dict[target] + if outcome.rc == OUTCOME_UNKNOWN: + not_built.append(target) + elif outcome.rc == OUTCOME_ERROR: + # Check for toolchain error in the error lines + for line in outcome.err_lines: + if 'Tool chain error' in line: + not_built.append(target) + break + if not_built: + tprint(f"Boards not built ({len(not_built)}): " + f"{', '.join(not_built)}") diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index 69e9e324c53..6d75cfbc04f 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -364,7 +364,7 @@ class TestPrepareWorkingSpace(unittest.TestCase): class TestShowNotBuilt(unittest.TestCase): - """Tests for Builder._show_not_built()""" + """Tests for ResultHandler.show_not_built()""" def setUp(self): """Set up test fixtures""" @@ -382,8 +382,8 @@ class TestShowNotBuilt(unittest.TestCase): return outcome def _show_not_built(self, board_selected, board_dict): - """Helper to call Builder._show_not_built""" - builder.Builder._show_not_built(board_selected, board_dict) + """Helper to call ResultHandler.show_not_built""" + ResultHandler.show_not_built(board_selected, board_dict) def test_all_boards_built(self): """Test when all selected boards were built successfully""" @@ -394,7 +394,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() # No output when all boards were built @@ -410,7 +410,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() self.assertEqual(len(lines), 1) @@ -428,7 +428,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() self.assertEqual(len(lines), 1) @@ -446,7 +446,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() # Build errors are still "built", just with errors @@ -464,7 +464,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() # Only toolchain errors count as "not built" @@ -483,7 +483,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - builder.Builder._show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) lines = terminal.get_print_test_lines() self.assertEqual(len(lines), 1) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the print_result_summary() method and baseline state management from Builder to ResultHandler. This includes: - Baseline state attributes (_base_board_dict, _base_err_lines, etc.) - reset_result_summary() to initialise baseline state - print_result_summary() to display build result deltas - get_error_lines() to access the error line count Builder now delegates to ResultHandler for result display and baseline tracking. The OUTCOME_* constants are passed as parameters since they remain defined in builder.py. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 139 +++------------------------- tools/buildman/resulthandler.py | 154 ++++++++++++++++++++++++++++++-- 2 files changed, 161 insertions(+), 132 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index a2d4470b70c..412acfa9e4b 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -305,7 +305,6 @@ class Builder: self._next_delay_update = datetime.now() self._start_time = None self._step = step - self._error_lines = 0 self.no_subdirs = no_subdirs self.full_path = full_path self.verbose_build = verbose_build @@ -364,14 +363,7 @@ class Builder: self._timestamps = None self._verbose = False - # Attributes for result summaries - self._base_board_dict = {} - self._base_err_lines = [] - self._base_warn_lines = [] - self._base_err_line_boards = {} - self._base_warn_line_boards = {} - self._base_config = None - self._base_environment = None + # Note: baseline state for result summaries is now in ResultHandler self._setup_threads(mrproper, per_board_out_dir, test_thread_exceptions) @@ -580,7 +572,7 @@ class Builder: elif self._verbose: terminal.print_clear() boards_selected = {target : result.brd} - self.reset_result_summary(boards_selected) + self._result_handler.reset_result_summary(boards_selected) self.produce_result_summary(result.commit_upto, self.commits, boards_selected) else: @@ -1011,113 +1003,6 @@ class Builder: return (board_dict, err_lines_summary, err_lines_boards, warn_lines_summary, warn_lines_boards, config, environment) - def reset_result_summary(self, board_selected): - """Reset the results summary ready for use. - - Set up the base board list to be all those selected, and set the - error lines to empty. - - Following this, calls to print_result_summary() will use this - information to work out what has changed. - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - """ - self._base_board_dict = {} - for brd in board_selected: - self._base_board_dict[brd] = Outcome(0, [], [], {}, {}, {}) - self._base_err_lines = [] - self._base_warn_lines = [] - self._base_err_line_boards = {} - self._base_warn_line_boards = {} - self._base_config = None - self._base_environment = None - - def print_result_summary(self, board_selected, board_dict, err_lines, - err_line_boards, warn_lines, warn_line_boards, - config, environment, show_sizes, show_detail, - show_bloat, show_config, show_environment): - """Compare results with the base results and display delta. - - Only boards mentioned in board_selected will be considered. This - function is intended to be called repeatedly with the results of - each commit. It therefore shows a 'diff' between what it saw in - the last call and what it sees now. - - Args: - board_selected (dict): Dict containing boards to summarise, keyed - by board.target - board_dict (dict): Dict containing boards for which we built this - commit, keyed by board.target. The value is an Outcome object. - err_lines (list): A list of errors for this commit, or [] if there - is none, or we don't want to print errors - err_line_boards (dict): Dict keyed by error line, containing a list - of the Board objects with that error - warn_lines (list): A list of warnings for this commit, or [] if - there is none, or we don't want to print errors - warn_line_boards (dict): Dict keyed by warning line, containing a - list of the Board objects with that warning - config (dict): Dictionary keyed by filename - e.g. '.config'. Each - value is itself a dictionary: - key: config name - value: config value - environment (dict): Dictionary keyed by environment variable, Each - value is the value of environment variable. - show_sizes (bool): Show image size deltas - show_detail (bool): Show size delta detail for each board if - show_sizes - show_bloat (bool): Show detail for each function - show_config (bool): Show config changes - show_environment (bool): Show environment changes - """ - brd_status = ResultHandler.classify_boards( - board_selected, board_dict, self._base_board_dict) - - # Get a list of errors and warnings that have appeared, and disappeared - better_err, worse_err = ResultHandler.calc_error_delta( - self._base_err_lines, self._base_err_line_boards, err_lines, - err_line_boards, '', self._opts.list_error_boards) - better_warn, worse_warn = ResultHandler.calc_error_delta( - self._base_warn_lines, self._base_warn_line_boards, warn_lines, - warn_line_boards, 'w', self._opts.list_error_boards) - - # For the IDE mode, print out all the output - if self._opts.ide: - self._result_handler.print_ide_output(board_selected, board_dict) - - # Display results by arch - if not self._opts.ide: - self._error_lines += self._result_handler.display_arch_results( - board_selected, brd_status, better_err, worse_err, better_warn, - worse_warn, self._opts.show_unknown) - - if show_sizes: - self._result_handler.print_size_summary( - board_selected, board_dict, self._base_board_dict, - show_detail, show_bloat) - - if show_environment and self._base_environment: - self._result_handler.show_environment_changes( - board_selected, board_dict, environment, self._base_environment) - - if show_config and self._base_config: - self._result_handler.show_config_changes( - board_selected, board_dict, config, self._base_config, - self.config_filenames) - - - # Save our updated information for the next call to this function - self._base_board_dict = board_dict - self._base_err_lines = err_lines - self._base_warn_lines = warn_lines - self._base_err_line_boards = err_line_boards - self._base_warn_line_boards = warn_line_boards - self._base_config = config - self._base_environment = environment - - ResultHandler.show_not_built(board_selected, board_dict) - def produce_result_summary(self, commit_upto, commits, board_selected): """Produce a summary of the results for a single commit @@ -1135,11 +1020,14 @@ class Builder: if commits: msg = f'{commit_upto + 1:02d}: {commits[commit_upto].subject}' tprint(msg, colour=self.col.BLUE) - self.print_result_summary(board_selected, board_dict, - err_lines if self._opts.show_errors else [], err_line_boards, - warn_lines if self._opts.show_errors else [], warn_line_boards, - config, environment, self._opts.show_sizes, self._opts.show_detail, - self._opts.show_bloat, self._opts.show_config, self._opts.show_environment) + self._result_handler.print_result_summary( + board_selected, board_dict, + err_lines if self._opts.show_errors else [], err_line_boards, + warn_lines if self._opts.show_errors else [], warn_line_boards, + config, environment, self._opts.show_sizes, self._opts.show_detail, + self._opts.show_bloat, self._opts.show_config, self._opts.show_environment, + self._opts.show_unknown, self._opts.ide, self._opts.list_error_boards, + self.config_filenames) def show_summary(self, commits, board_selected): """Show a build summary for U-Boot for a given board list. @@ -1153,12 +1041,11 @@ class Builder: """ self.commit_count = len(commits) if commits else 1 self.commits = commits - self.reset_result_summary(board_selected) - self._error_lines = 0 + self._result_handler.reset_result_summary(board_selected) for commit_upto in range(0, self.commit_count, self._step): self.produce_result_summary(commit_upto, commits, board_selected) - if not self._error_lines: + if not self._result_handler.get_error_lines(): tprint('(no errors to report)', colour=self.col.GREEN) @@ -1328,7 +1215,7 @@ class Builder: self.commits = commits self._verbose = verbose - self.reset_result_summary(board_selected) + self._result_handler.reset_result_summary(board_selected) builderthread.mkdir(self.base_dir, parents = True) self._prepare_working_space(min(self.num_threads, len(board_selected)), commits is not None) diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index a2064986729..d1d6232ff97 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -8,8 +8,9 @@ import sys -from buildman.outcome import (BoardStatus, ErrLine, OUTCOME_OK, - OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN) +from buildman.outcome import (BoardStatus, ErrLine, Outcome, + OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, + OUTCOME_UNKNOWN) from u_boot_pylib.terminal import tprint @@ -18,10 +19,19 @@ class ResultHandler: This class is responsible for displaying size information from builds, including per-architecture summaries, per-board details, and per-function - bloat analysis. + bloat analysis. It also manages baseline state for comparing results + between commits. Attributes: col: terminal.Color object for coloured output + _base_board_dict: Last-summarised Dict of boards + _base_err_lines: Last-summarised list of errors + _base_warn_lines: Last-summarised list of warnings + _base_err_line_boards: Dict of error lines to boards + _base_warn_line_boards: Dict of warning lines to boards + _base_config: Last-summarised config + _base_environment: Last-summarised environment + _error_lines: Number of error lines output """ def __init__(self, col, opts): @@ -34,6 +44,16 @@ class ResultHandler: self._col = col self._opts = opts self._builder = None + self._error_lines = 0 + + # Baseline state for result comparisons + self._base_board_dict = {} + self._base_err_lines = [] + self._base_warn_lines = [] + self._base_err_line_boards = {} + self._base_warn_line_boards = {} + self._base_config = None + self._base_environment = None def set_builder(self, builder): """Set the builder for this result handler @@ -43,6 +63,128 @@ class ResultHandler: """ self._builder = builder + def reset_result_summary(self, board_selected): + """Reset the results summary ready for use. + + Set up the base board list to be all those selected, and set the + error lines to empty. + + Following this, calls to print_result_summary() will use this + information to work out what has changed. + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + """ + outcome_init = Outcome(0, [], [], {}, {}, {}) + self._base_board_dict = {} + for brd in board_selected: + self._base_board_dict[brd] = outcome_init + self._base_err_lines = [] + self._base_warn_lines = [] + self._base_err_line_boards = {} + self._base_warn_line_boards = {} + self._base_config = None + self._base_environment = None + self._error_lines = 0 + + def print_result_summary(self, board_selected, board_dict, err_lines, + err_line_boards, warn_lines, warn_line_boards, + config, environment, show_sizes, show_detail, + show_bloat, show_config, show_environment, + show_unknown, ide, list_error_boards, + config_filenames): + """Compare results with the base results and display delta. + + Only boards mentioned in board_selected will be considered. This + function is intended to be called repeatedly with the results of + each commit. It therefore shows a 'diff' between what it saw in + the last call and what it sees now. + + Args: + board_selected (dict): Dict containing boards to summarise, keyed + by board.target + board_dict (dict): Dict containing boards for which we built this + commit, keyed by board.target. The value is an Outcome object. + err_lines (list): A list of errors for this commit, or [] if there + is none, or we don't want to print errors + err_line_boards (dict): Dict keyed by error line, containing a list + of the Board objects with that error + warn_lines (list): A list of warnings for this commit, or [] if + there is none, or we don't want to print errors + warn_line_boards (dict): Dict keyed by warning line, containing a + list of the Board objects with that warning + config (dict): Dictionary keyed by filename - e.g. '.config'. Each + value is itself a dictionary: + key: config name + value: config value + environment (dict): Dictionary keyed by environment variable, Each + value is the value of environment variable. + show_sizes (bool): Show image size deltas + show_detail (bool): Show size delta detail for each board if + show_sizes + show_bloat (bool): Show detail for each function + show_config (bool): Show config changes + show_environment (bool): Show environment changes + show_unknown (bool): Show unknown boards in summary + ide (bool): IDE mode - output to stderr + list_error_boards (bool): Include board list with error lines + config_filenames (list): List of config filenames + """ + brd_status = self.classify_boards( + board_selected, board_dict, self._base_board_dict) + + # Get a list of errors and warnings that have appeared, and disappeared + better_err, worse_err = self.calc_error_delta( + self._base_err_lines, self._base_err_line_boards, err_lines, + err_line_boards, '', list_error_boards) + better_warn, worse_warn = self.calc_error_delta( + self._base_warn_lines, self._base_warn_line_boards, warn_lines, + warn_line_boards, 'w', list_error_boards) + + # For the IDE mode, print out all the output + if ide: + self.print_ide_output(board_selected, board_dict) + + # Display results by arch + if not ide: + self._error_lines += self.display_arch_results( + board_selected, brd_status, better_err, worse_err, better_warn, + worse_warn, show_unknown) + + if show_sizes: + self.print_size_summary( + board_selected, board_dict, self._base_board_dict, + show_detail, show_bloat) + + if show_environment and self._base_environment: + self.show_environment_changes( + board_selected, board_dict, environment, self._base_environment) + + if show_config and self._base_config: + self.show_config_changes( + board_selected, board_dict, config, self._base_config, + config_filenames) + + # Save our updated information for the next call to this function + self._base_board_dict = board_dict + self._base_err_lines = err_lines + self._base_warn_lines = warn_lines + self._base_err_line_boards = err_line_boards + self._base_warn_line_boards = warn_line_boards + self._base_config = config + self._base_environment = environment + + self.show_not_built(board_selected, board_dict) + + def get_error_lines(self): + """Get the number of error lines output + + Returns: + int: Number of error lines output + """ + return self._error_lines + def colour_num(self, num): """Format a number with colour depending on its value @@ -476,11 +618,11 @@ class ResultHandler: continue col = None if line[0] == '+': - col = self.col.GREEN + col = self._col.GREEN elif line[0] == '-': - col = self.col.RED + col = self._col.RED elif line[0] == 'c': - col = self.col.YELLOW + col = self._col.YELLOW tprint(' ' + line, newline=True, colour=col) def show_environment_changes(self, board_selected, board_dict, -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Move the summary-display methods from Builder to ResultHandler. These methods loop through commits and display results, which is display logic that belongs in ResultHandler. Builder now provides a callback (get_result_summary) that ResultHandler calls to gather build results from disk. This maintains the separation where Builder handles file I/O and ResultHandler handles display. Key changes: - Move show_summary() and produce_result_summary() to ResultHandler - Store config_filenames and result_getter as ResultHandler attributes - Add set_builder() method to ResultHandler for initialisation - Pass opts to ResultHandler constructor (required parameter) - Pass display_options to Builder constructor - Add result_handler property to Builder Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 55 +++-------------- tools/buildman/control.py | 4 +- tools/buildman/resulthandler.py | 101 ++++++++++++++++++++------------ tools/buildman/test.py | 2 +- 4 files changed, 75 insertions(+), 87 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 5df6f915285..2f072e34716 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -448,6 +448,11 @@ class Builder: self._filter_dtb_warnings = filter_dtb_warnings self._filter_migration_warnings = filter_migration_warnings + @property + def result_handler(self): + """Get the result handler for this builder""" + return self._result_handler + def _add_timestamp(self): """Add a new timestamp to the list and record the build period. @@ -573,8 +578,8 @@ class Builder: terminal.print_clear() boards_selected = {target : result.brd} self._result_handler.reset_result_summary(boards_selected) - self.produce_result_summary(result.commit_upto, self.commits, - boards_selected) + self._result_handler.produce_result_summary( + result.commit_upto, self.commits, boards_selected) else: target = '(starting)' @@ -1003,52 +1008,6 @@ class Builder: return (board_dict, err_lines_summary, err_lines_boards, warn_lines_summary, warn_lines_boards, config, environment) - def produce_result_summary(self, commit_upto, commits, board_selected): - """Produce a summary of the results for a single commit - - Args: - commit_upto (int): Commit number to summarise (0..self.count-1) - commits (list): List of commits being built - board_selected (dict): Dict containing boards to summarise - """ - (board_dict, err_lines, err_line_boards, warn_lines, - warn_line_boards, config, environment) = self.get_result_summary( - board_selected, commit_upto, - read_func_sizes=self._opts.show_bloat, - read_config=self._opts.show_config, - read_environment=self._opts.show_environment) - if commits: - msg = f'{commit_upto + 1:02d}: {commits[commit_upto].subject}' - tprint(msg, colour=self.col.BLUE) - self._result_handler.print_result_summary( - board_selected, board_dict, - err_lines if self._opts.show_errors else [], err_line_boards, - warn_lines if self._opts.show_errors else [], warn_line_boards, - config, environment, self._opts.show_sizes, self._opts.show_detail, - self._opts.show_bloat, self._opts.show_config, self._opts.show_environment, - self._opts.show_unknown, self._opts.ide, self._opts.list_error_boards, - self.config_filenames) - - def show_summary(self, commits, board_selected): - """Show a build summary for U-Boot for a given board list. - - Reset the result summary, then repeatedly call GetResultSummary on - each commit's results, then display the differences we see. - - Args: - commits (list): Commit objects to summarise - board_selected (dict): Dict containing boards to summarise - """ - self.commit_count = len(commits) if commits else 1 - self.commits = commits - self._result_handler.reset_result_summary(board_selected) - - for commit_upto in range(0, self.commit_count, self._step): - self.produce_result_summary(commit_upto, commits, board_selected) - if not self._result_handler.get_error_lines(): - tprint('(no errors to report)', colour=self.col.GREEN) - - def setup_build(self, board_selected, _commits): """Set up ready to start a build. diff --git a/tools/buildman/control.py b/tools/buildman/control.py index e225a1411ca..728389d9a8b 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -566,7 +566,9 @@ def run_builder(builder, commits, board_selected, display_options, args): builder.set_display_options( display_options, args.filter_dtb_warnings, args.filter_migration_warnings) if args.summary: - builder.show_summary(commits, board_selected) + builder.commits = commits + builder.result_handler.show_summary( + commits, board_selected, args.step) else: fail, warned, excs = builder.build_boards( commits, board_selected, args.keep_outputs, args.verbose, diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index 2c5488c7154..c0cf0ca331e 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -45,6 +45,8 @@ class ResultHandler: self._col = col self._opts = opts self._builder = None + self._config_filenames = None + self._result_getter = None self._error_lines = 0 # Baseline state for result comparisons @@ -63,6 +65,8 @@ class ResultHandler: builder (Builder): Builder object to use for getting results """ self._builder = builder + self._config_filenames = builder.config_filenames + self._result_getter = builder.get_result_summary def reset_result_summary(self, board_selected): """Reset the results summary ready for use. @@ -91,10 +95,7 @@ class ResultHandler: def print_result_summary(self, board_selected, board_dict, err_lines, err_line_boards, warn_lines, warn_line_boards, - config, environment, show_sizes, show_detail, - show_bloat, show_config, show_environment, - show_unknown, ide, list_error_boards, - config_filenames): + config, environment): """Compare results with the base results and display delta. Only boards mentioned in board_selected will be considered. This @@ -121,16 +122,6 @@ class ResultHandler: value: config value environment (dict): Dictionary keyed by environment variable, Each value is the value of environment variable. - show_sizes (bool): Show image size deltas - show_detail (bool): Show size delta detail for each board if - show_sizes - show_bloat (bool): Show detail for each function - show_config (bool): Show config changes - show_environment (bool): Show environment changes - show_unknown (bool): Show unknown boards in summary - ide (bool): IDE mode - output to stderr - list_error_boards (bool): Include board list with error lines - config_filenames (list): List of config filenames """ brd_status = self.classify_boards( board_selected, board_dict, self._base_board_dict) @@ -138,34 +129,33 @@ class ResultHandler: # Get a list of errors and warnings that have appeared, and disappeared better_err, worse_err = self.calc_error_delta( self._base_err_lines, self._base_err_line_boards, err_lines, - err_line_boards, '', list_error_boards) + err_line_boards, '', self._opts.list_error_boards) better_warn, worse_warn = self.calc_error_delta( self._base_warn_lines, self._base_warn_line_boards, warn_lines, - warn_line_boards, 'w', list_error_boards) + warn_line_boards, 'w', self._opts.list_error_boards) # For the IDE mode, print out all the output - if ide: + if self._opts.ide: self.print_ide_output(board_selected, board_dict) # Display results by arch - if not ide: + if not self._opts.ide: self._error_lines += self.display_arch_results( board_selected, brd_status, better_err, worse_err, better_warn, - worse_warn, show_unknown) + worse_warn, self._opts.show_unknown) - if show_sizes: + if self._opts.show_sizes: self.print_size_summary( board_selected, board_dict, self._base_board_dict, - show_detail, show_bloat) + self._opts.show_detail, self._opts.show_bloat) - if show_environment and self._base_environment: + if self._opts.show_environment and self._base_environment: self.show_environment_changes( board_selected, board_dict, environment, self._base_environment) - if show_config and self._base_config: + if self._opts.show_config and self._base_config: self.show_config_changes( - board_selected, board_dict, config, self._base_config, - config_filenames) + board_selected, board_dict, config, self._base_config) # Save our updated information for the next call to this function self._base_board_dict = board_dict @@ -186,6 +176,47 @@ class ResultHandler: """ return self._error_lines + def produce_result_summary(self, commit_upto, commits, board_selected): + """Produce a summary of the results for a single commit + + Args: + commit_upto (int): Commit number to summarise (0..count-1) + commits (list): List of commits being built + board_selected (dict): Dict containing boards to summarise + """ + (board_dict, err_lines, err_line_boards, warn_lines, + warn_line_boards, config, environment) = self._result_getter( + board_selected, commit_upto, self._opts.show_bloat, + self._opts.show_config, self._opts.show_environment) + if commits: + msg = f'{commit_upto + 1:02d}: {commits[commit_upto].subject}' + tprint(msg, colour=self._col.BLUE) + self.print_result_summary( + board_selected, board_dict, + err_lines if self._opts.show_errors else [], err_line_boards, + warn_lines if self._opts.show_errors else [], warn_line_boards, + config, environment) + + def show_summary(self, commits, board_selected, step): + """Show a build summary for U-Boot for a given board list. + + Reset the result summary, then repeatedly call produce_result_summary + on each commit's results, then display the differences we see. + + Args: + commits (list): Commit objects to summarise + board_selected (dict): Dict containing boards to summarise + step (int): Step size for iterating through commits + """ + commit_count = len(commits) if commits else 1 + self.reset_result_summary(board_selected) + + for commit_upto in range(0, commit_count, step): + self.produce_result_summary( + commit_upto, commits, board_selected) + if not self.get_error_lines(): + tprint('(no errors to report)', colour=self._col.GREEN) + def print_build_summary(self, count, already_done, kconfig_reconfig, start_time, thread_exceptions): """Print a summary of the build results @@ -706,7 +737,7 @@ class ResultHandler: environment_minus, environment_change) self.output_config_info(lines) - def calc_config_changes(self, target, config, base_config, config_filenames, + def calc_config_changes(self, target, config, base_config, arch, arch_config_plus, arch_config_minus, arch_config_change): """Calculate configuration changes for a single target @@ -715,7 +746,6 @@ class ResultHandler: target (str): Target board name config (dict): Dict of config changes, keyed by board.target base_config (dict): Dict of base config, keyed by board.target - config_filenames (list): List of config filenames to check arch (str): Architecture name arch_config_plus (dict): Dict to update with added configs by arch arch_config_minus (dict): Dict to update with removed configs by @@ -732,7 +762,7 @@ class ResultHandler: tbase = base_config[target] tconfig = config[target] lines = [] - for name in config_filenames: + for name in self._config_filenames: if not tconfig.config[name]: continue config_plus = {} @@ -765,8 +795,7 @@ class ResultHandler: return '\n'.join(lines) def print_arch_config_summary(self, arch, arch_config_plus, - arch_config_minus, arch_config_change, - config_filenames): + arch_config_minus, arch_config_change): """Print configuration summary for a single architecture Args: @@ -774,13 +803,12 @@ class ResultHandler: arch_config_plus (dict): Dict of added configs by arch/filename arch_config_minus (dict): Dict of removed configs by arch/filename arch_config_change (dict): Dict of changed configs by arch/filename - config_filenames (list): List of config filenames to check """ lines = [] all_plus = {} all_minus = {} all_change = {} - for name in config_filenames: + for name in self._config_filenames: all_plus.update(arch_config_plus[arch][name]) all_minus.update(arch_config_minus[arch][name]) all_change.update(arch_config_change[arch][name]) @@ -794,7 +822,7 @@ class ResultHandler: self.output_config_info(lines) def show_config_changes(self, board_selected, board_dict, config, - base_config, config_filenames): + base_config): """Show changes in configuration Args: @@ -804,7 +832,6 @@ class ResultHandler: commit, keyed by board.target. The value is an Outcome object. config (dict): Dict of config changes, keyed by board.target base_config (dict): Dict of base config, keyed by board.target - config_filenames (list): List of config filenames to check """ summary = {} arch_config_plus = {} @@ -823,7 +850,7 @@ class ResultHandler: arch_config_plus[arch] = {} arch_config_minus[arch] = {} arch_config_change[arch] = {} - for name in config_filenames: + for name in self._config_filenames: arch_config_plus[arch][name] = {} arch_config_minus[arch][name] = {} arch_config_change[arch][name] = {} @@ -833,7 +860,7 @@ class ResultHandler: continue arch = board_selected[target].arch summary[target] = self.calc_config_changes( - target, config, base_config, config_filenames, arch, + target, config, base_config, arch, arch_config_plus, arch_config_minus, arch_config_change) lines_by_target = {} @@ -846,7 +873,7 @@ class ResultHandler: for arch in arch_list: self.print_arch_config_summary(arch, arch_config_plus, arch_config_minus, - arch_config_change, config_filenames) + arch_config_change) for lines, targets in lines_by_target.items(): if not lines: diff --git a/tools/buildman/test.py b/tools/buildman/test.py index c84d616c38b..fdf3f5865ce 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -294,7 +294,7 @@ class TestBuildOutput(TestBuildBase): self.assertEqual(count, len(COMMITS) * len(BOARDS) + 3) build.set_display_options(opts, filter_dtb_warnings, filter_migration_warnings) - build.show_summary(self.commits, board_selected) + build._result_handler.show_summary(self.commits, board_selected, 1) if echo_lines: terminal.echo_print_test_lines() return iter(terminal.get_print_test_lines()) -- 2.43.0

From: Simon Glass <simon.glass@canonical.com> Add underscore prefix to private member variables in builder.py and resulthandler.py to follow Python naming conventions. builder.py: - col, upto, warned, commit, threads, num_threads, already_done, re_make_err resulthandler.py: - col, opts, config_filenames, result_getter Members accessed from outside their class (builderthread.py, control.py, func_test.py) are left as public. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 99 +++++++++++++++++---------------- tools/buildman/resulthandler.py | 20 +++++-- tools/buildman/test.py | 1 + 3 files changed, 66 insertions(+), 54 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 21816391773..f2cbad53c30 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -154,13 +154,12 @@ class Environment: class Builder: """Class for building U-Boot for a particular commit. - Public members: (many should ->private) - already_done: Number of builds already completed - kconfig_reconfig: Number of builds triggered by Kconfig changes + Public members: base_dir: Base directory to use for builder checkout: True to check out source, False to skip that step. This is used for testing. - col: terminal.Color() object + commit_count: Number of commits being built + commits: List of commits being built count: Total number of commits to build, which is the number of commits multiplied by the number of boards do_make: Method to call to invoke Make @@ -172,48 +171,52 @@ 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 + gnu_make: Command name for GNU Make + in_tree: Build U-Boot in-tree instead of specifying an output + directory separate from the source code. This option is really + only useful for testing in-tree builds. + kconfig_reconfig: Number of builds triggered by Kconfig changes 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 queue: Queue of jobs to run - threads: List of active threads - toolchains: Toolchains object to use for building - upto: Current commit number we are building (0.count-1) - warned: Number of builds that produced at least one warning force_reconfig: Reconfigure U-Boot on each comiit. This disables incremental building, where buildman reconfigures on the first commit for a baord, and then just does an incremental build for the following commits. In fact buildman will reconfigure and retry for any failing commits, so generally the only effect of this option is to slow things down. - in_tree: Build U-Boot in-tree instead of specifying an output - directory separate from the source code. This option is really - only useful for testing in-tree builds. + thread_exceptions: List of exceptions raised by thread jobs + toolchains: Toolchains object to use for building work_in_output: Use the output directory as the work directory and don't write to a separate output directory. - thread_exceptions: List of exceptions raised by thread jobs no_lto (bool): True to set the NO_LTO flag when building reproducible_builds (bool): True to set SOURCE_DATE_EPOCH=0 for builds Private members: - _base_board_dict: Last-summarised Dict of boards - _base_err_lines: Last-summarised list of errors - _base_warn_lines: Last-summarised list of warnings + _already_done: Number of builds already completed _build_period_us: Time taken for a single build (float object). + _col: terminal.Color() object + _commit: Current commit being built _complete_delay: Expected delay until completion (timedelta) _next_delay_update: Next time we plan to display a progress update (datatime) + _num_threads: Number of builder threads to run + _opts: DisplayOptions for result output + _re_make_err: Compiled regex for make error detection + _restarting_config: True if 'Restart config' is detected in output + _result_handler: ResultHandler for displaying results + _single_builder: BuilderThread object for the singer builder, if + threading is not being used _start_time: Start time for the build + _step: Step value for processing commits (1=all, 2=every other, etc.) + _terminated: Thread was terminated due to an error + _threads: List of active threads _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. + _upto: Current commit number we are building (0.count-1) + _warned: Number of builds that produced at least one warning _working_dir: Base working directory containing all threads - _single_builder: BuilderThread object for the singer builder, if - threading is not being used - _terminated: Thread was terminated due to an error - _restarting_config: True if 'Restart config' is detected in output - _ide: Produce output suitable for an Integrated Development Environment - i.e. don't emit progress information and put errors on stderr """ def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, @@ -289,13 +292,13 @@ class Builder: self._working_dir = base_dir else: self._working_dir = os.path.join(base_dir, '.bm-work') - self.threads = [] + self._threads = [] self.do_make = make_func or self.make self.gnu_make = gnu_make self.checkout = checkout - self.num_threads = num_threads + self._num_threads = num_threads self.num_jobs = num_jobs - self.already_done = 0 + self._already_done = 0 self.kconfig_reconfig = 0 self.force_build = False self.git_dir = git_dir @@ -353,9 +356,9 @@ class Builder: # Attributes set by other methods self._build_period = None - self.commit = None - self.upto = 0 - self.warned = 0 + self._commit = None + self._upto = 0 + self._warned = 0 self.fail = 0 self.commit_count = 0 self.commits = None @@ -369,7 +372,7 @@ class Builder: ignore_lines = ['(make.*Waiting for unfinished)', '(Segmentation fault)'] - self.re_make_err = re.compile('|'.join(ignore_lines)) + self._re_make_err = re.compile('|'.join(ignore_lines)) # Handle existing graceful with SIGINT / Ctrl-C signal.signal(signal.SIGINT, self._signal_handler) @@ -385,29 +388,29 @@ class Builder: test_thread_exceptions (bool): True to make threads raise an exception instead of reporting their result (for tests) """ - if self.num_threads: + if self._num_threads: self._single_builder = None self.queue = queue.Queue() self.out_queue = queue.Queue() - for i in range(self.num_threads): + for i in range(self._num_threads): t = builderthread.BuilderThread( self, i, mrproper, per_board_out_dir, test_exception=test_thread_exceptions) t.daemon = True t.start() - self.threads.append(t) + self._threads.append(t) t = builderthread.ResultThread(self) t.daemon = True t.start() - self.threads.append(t) + self._threads.append(t) else: self._single_builder = builderthread.BuilderThread( self, -1, mrproper, per_board_out_dir) def __del__(self): """Get rid of all threads created by the builder""" - self.threads.clear() + self._threads.clear() def _signal_handler(self, _signum, _frame): """Handle a signal by exiting""" @@ -471,7 +474,7 @@ class Builder: self._next_delay_update = now + timedelta(seconds=2) if seconds > 0: self._build_period = float(seconds) / count - todo = self.count - self.upto + todo = self.count - self._upto self._complete_delay = timedelta(microseconds= self._build_period * todo * 1000000) # Round it @@ -489,7 +492,7 @@ class Builder: commit (Commit): Commit object that is being built checkout (bool): True to checkout the commit """ - self.commit = commit + self._commit = commit if checkout and self.checkout: gitutil.checkout(commit.hash) @@ -562,13 +565,13 @@ class Builder: if result: target = result.brd.target - self.upto += 1 + self._upto += 1 if result.return_code != 0: self.fail += 1 elif result.stderr: - self.warned += 1 + self._warned += 1 if result.already_done: - self.already_done += 1 + self._already_done += 1 if result.kconfig_reconfig: self.kconfig_reconfig += 1 if self._opts.ide: @@ -584,13 +587,13 @@ class Builder: target = '(starting)' # Display separate counts for ok, warned and fail - ok = self.upto - self.warned - self.fail + ok = self._upto - self._warned - self.fail line = '\r' + self.col.build(self.col.GREEN, f'{ok:5d}') - line += self.col.build(self.col.YELLOW, f'{self.warned:5d}') + line += self.col.build(self.col.YELLOW, f'{self._warned:5d}') line += self.col.build(self.col.RED, f'{self.fail:5d}') line += f' /{self.count:<5d} ' - remaining = self.count - self.upto + remaining = self.count - self._upto if remaining: line += self.col.build(self.col.MAGENTA, f' -{remaining:<5d} ') else: @@ -1017,7 +1020,7 @@ class Builder: # First work out how many commits we will build count = (self.commit_count + self._step - 1) // self._step self.count = len(board_selected) * count - self.upto = self.warned = self.fail = 0 + self._upto = self._warned = self.fail = 0 self._timestamps = collections.deque() def get_thread_dir(self, thread_num): @@ -1176,7 +1179,7 @@ class Builder: self._result_handler.reset_result_summary(board_selected) builderthread.mkdir(self.base_dir, parents = True) - self._prepare_working_space(min(self.num_threads, len(board_selected)), + self._prepare_working_space(min(self._num_threads, len(board_selected)), commits is not None) self._prepare_output_space() if not self._opts.ide: @@ -1195,12 +1198,12 @@ class Builder: job.adjust_cfg = self.adjust_cfg job.fragments = fragments job.step = self._step - if self.num_threads: + if self._num_threads: self.queue.put(job) else: self._single_builder.run_job(job) - if self.num_threads: + if self._num_threads: term = threading.Thread(target=self.queue.join) term.daemon = True term.start() @@ -1211,7 +1214,7 @@ class Builder: self.out_queue.join() if not self._opts.ide: self._result_handler.print_build_summary( - self.count, self.already_done, self.kconfig_reconfig, + self.count, self._already_done, self.kconfig_reconfig, self._start_time, self.thread_exceptions) - return (self.fail, self.warned, self.thread_exceptions) + return (self.fail, self._warned, self.thread_exceptions) diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index d6c5e3224b9..fca8e7d6ba1 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -23,16 +23,19 @@ class ResultHandler: bloat analysis. It also manages baseline state for comparing results between commits. - Attributes: - col: terminal.Color object for coloured output + Private members: _base_board_dict: Last-summarised Dict of boards - _base_err_lines: Last-summarised list of errors - _base_warn_lines: Last-summarised list of warnings - _base_err_line_boards: Dict of error lines to boards - _base_warn_line_boards: Dict of warning lines to boards _base_config: Last-summarised config _base_environment: Last-summarised environment + _base_err_line_boards: Dict of error lines to boards + _base_err_lines: Last-summarised list of errors + _base_warn_line_boards: Dict of warning lines to boards + _base_warn_lines: Last-summarised list of warnings + _col: terminal.Color object for coloured output + _config_filenames: List of config filenames to track _error_lines: Number of error lines output + _opts: DisplayOptions for result output + _result_getter: Callback to get result summary data """ def __init__(self, col, opts): @@ -58,6 +61,11 @@ class ResultHandler: self._base_config = None self._base_environment = None + @property + def opts(self): + """Get the display options""" + return self._opts + def set_builder(self, builder): """Set the builder for this result handler diff --git a/tools/buildman/test.py b/tools/buildman/test.py index fdf3f5865ce..b217b907176 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -276,6 +276,7 @@ class TestBuildOutput(TestBuildBase): build = builder.Builder(self.toolchains, self.base_dir, None, threads, 2, self._col, ResultHandler(self._col, opts), checkout=False) + build._result_handler.set_builder(build) build.do_make = self.make board_selected = self.brds.get_selected_dict() -- 2.43.0