From: Simon Glass <simon.glass@canonical.com> Create a new ResultHandler class and set one up in control.py This will eventually hold all the result-display code currently in Builder so pass the display options to it. Pass the ResultHandler to the Builder constructor so it can use these methods. Update tests to pass ResultHandler when creating Builder instances. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 4 ++- tools/buildman/control.py | 34 +++++++++++-------- tools/buildman/resulthandler.py | 31 ++++++++++++++++++ tools/buildman/test.py | 45 +++++++++++++++---------- tools/buildman/test_builder.py | 58 ++++++++++++++++++++++++++++----- 5 files changed, 132 insertions(+), 40 deletions(-) create mode 100644 tools/buildman/resulthandler.py diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index e52956cda0e..db8d980e83e 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, - col, gnu_make='make', checkout=True, step=1, + col, result_handler, 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, @@ -234,6 +234,7 @@ class Builder: toolchains: Toolchains object to use for building base_dir: Base directory to use for builder git_dir: Git directory containing source repository + result_handler: ResultHandler object for displaying results 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 @@ -337,6 +338,7 @@ class Builder: self.warnings_as_errors = warnings_as_errors self.col = col + self._result_handler = result_handler self.thread_exceptions = [] self.test_thread_exceptions = test_thread_exceptions diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 398441aa9ed..e225a1411ca 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -21,6 +21,7 @@ from buildman import cfgutil from buildman import toolchain from buildman.builder import Builder from buildman.outcome import DisplayOptions +from buildman.resulthandler import ResultHandler from patman import patchstream import qconfig from u_boot_pylib import command @@ -535,7 +536,7 @@ def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col, return output_dir -def run_builder(builder, commits, board_selected, args): +def run_builder(builder, commits, board_selected, display_options, args): """Run the builder or show the summary Args: @@ -544,6 +545,8 @@ def run_builder(builder, commits, board_selected, args): board_selected (dict): Dict of selected boards: key: target name value: Board object + display_options (DisplayOptions): Named tuple containing display + settings args (Namespace): Namespace to use Returns: @@ -560,16 +563,6 @@ def run_builder(builder, commits, board_selected, args): tprint(get_action_summary(args.summary, commit_count, board_selected, args.threads, args.jobs)) - display_options = DisplayOptions( - show_errors=args.show_errors, - show_sizes=args.show_sizes, - show_detail=args.show_detail, - show_bloat=args.show_bloat, - show_config=args.show_config, - show_environment=args.show_environment, - show_unknown=args.show_unknown, - ide=args.ide, - list_error_boards=args.list_error_boards) builder.set_display_options( display_options, args.filter_dtb_warnings, args.filter_migration_warnings) if args.summary: @@ -809,12 +802,24 @@ 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 + # Create colour, display options and result handler objects col = terminal.Color() + display_options = DisplayOptions( + show_errors=args.show_errors, + show_sizes=args.show_sizes, + show_detail=args.show_detail, + show_bloat=args.show_bloat, + show_config=args.show_config, + show_environment=args.show_environment, + show_unknown=args.show_unknown, + ide=args.ide, + list_error_boards=args.list_error_boards) + result_handler = ResultHandler(col, display_options) # Create a new builder with the selected args builder = Builder(toolchains, output_dir, git_dir, - args.threads, args.jobs, col=col, checkout=True, step=args.step, + args.threads, args.jobs, col=col, + result_handler=result_handler, checkout=True, step=args.step, no_subdirs=args.no_subdirs, full_path=args.full_path, verbose_build=args.verbose_build, mrproper=args.mrproper, @@ -838,6 +843,7 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, force_reconfig = args.force_reconfig, in_tree = args.in_tree, force_config_on_failure=not args.quick, make_func=make_func, dtc_skip=args.dtc_skip, build_target=args.target) + result_handler.set_builder(builder) TEST_BUILDER = builder @@ -845,4 +851,4 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, wait_for_process_limit(args.process_limit) return run_builder(builder, series.commits if series else None, - brds.get_selected_dict(), args) + brds.get_selected_dict(), display_options, args) diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py new file mode 100644 index 00000000000..86f95f3eea5 --- /dev/null +++ b/tools/buildman/resulthandler.py @@ -0,0 +1,31 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2013 The Chromium OS Authors. + +"""Result handler for buildman build results""" + + +class ResultHandler: + """Handles display of build results and summaries + + This class is responsible for displaying build results, including + size information, errors, warnings, and configuration changes. + """ + + def __init__(self, col, opts): + """Create a new ResultHandler + + Args: + col: terminal.Color object for coloured output + opts (DisplayOptions): Options controlling what to display + """ + self._col = col + self._opts = opts + self._builder = None + + def set_builder(self, builder): + """Set the builder for this result handler + + Args: + builder (Builder): Builder object to use for getting results + """ + self._builder = builder diff --git a/tools/buildman/test.py b/tools/buildman/test.py index 6702fd9aad8..c84d616c38b 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -25,6 +25,7 @@ from buildman import cfgutil from buildman import control from buildman import toolchain from buildman.outcome import DisplayOptions +from buildman.resulthandler import ResultHandler from patman import commit from u_boot_pylib import command from u_boot_pylib import terminal @@ -196,6 +197,11 @@ class TestBuildBase(unittest.TestCase): # Avoid sending any output terminal.set_print_test_mode() self._col = terminal.Color() + self._opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self._result_handler = ResultHandler(self._col, self._opts) self.base_dir = tempfile.mkdtemp() if not os.path.isdir(self.base_dir): @@ -263,8 +269,13 @@ class TestBuildOutput(TestBuildBase): Returns: Iterator containing the output lines, each a PrintLine() object """ + opts = DisplayOptions( + show_errors=show_errors, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=list_error_boards) build = builder.Builder(self.toolchains, self.base_dir, None, threads, - 2, self._col, checkout=False) + 2, self._col, ResultHandler(self._col, opts), + checkout=False) build.do_make = self.make board_selected = self.brds.get_selected_dict() @@ -281,10 +292,6 @@ class TestBuildOutput(TestBuildBase): # We should get two starting messages, an update for every commit built # and a summary message self.assertEqual(count, len(COMMITS) * len(BOARDS) + 3) - opts = DisplayOptions( - show_errors=show_errors, show_sizes=False, show_detail=False, - show_bloat=False, show_config=False, show_environment=False, - show_unknown=False, ide=False, list_error_boards=list_error_boards) build.set_display_options(opts, filter_dtb_warnings, filter_migration_warnings) build.show_summary(self.commits, board_selected) @@ -642,7 +649,8 @@ 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, - self._col, checkout=False) + self._col, self._result_handler, + checkout=False) build.commits = self.commits build.commit_count = len(self.commits) subject = self.commits[1].subject.translate(builder.trans_valid_chars) @@ -652,7 +660,8 @@ 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, - self._col, checkout=False) + self._col, self._result_handler, + checkout=False) build.commits = None build.commit_count = 0 self.check_dirs(build, '/current') @@ -660,7 +669,8 @@ 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, - self._col, checkout=False, no_subdirs=True) + self._col, self._result_handler, + checkout=False, no_subdirs=True) build.commits = None build.commit_count = 0 self.check_dirs(build, '') @@ -760,7 +770,7 @@ class TestBuild(TestBuildBase): _touch(name) build = builder.Builder(self.toolchains, base_dir, None, 1, 2, - self._col) + self._col, self._result_handler) build.commits = self.commits build.commit_count = len(COMMITS) # pylint: disable=protected-access @@ -1004,7 +1014,7 @@ class TestBuildMisc(TestBuildBase): # Check a missing tool with self.assertRaises(ValueError) as exc: builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - self._col, dtc_skip=True) + self._col, self._result_handler, dtc_skip=True) self.assertIn('Cannot find dtc', str(exc.exception)) # Create a fake tool to use @@ -1013,14 +1023,15 @@ class TestBuildMisc(TestBuildBase): os.chmod(dtc, 0o777) build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - self._col, dtc_skip=True) + self._col, self._result_handler, + 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, - self._col) + self._col, self._result_handler) tch = self.toolchains.select('arm') env = build.make_environment(tch) self.assertNotIn(b'DTC', env) @@ -1149,7 +1160,7 @@ 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, - self._col) + self._col, self._result_handler) # Create test data simulating 'nm' output # NM_SYMBOL_TYPES = 'tTdDbBr' - text, data, bss, rodata @@ -1179,7 +1190,7 @@ 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, - self._col) + self._col, self._result_handler) # Test static functions (have . in name after first char) nm_output = '''00000100 t func.1234 @@ -1201,7 +1212,7 @@ class TestBuilderFuncs(TestBuildBase): def test_process_environment(self): """Test _process_environment() function""" build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2, - self._col) + self._col, self._result_handler) # Environment file uses null-terminated strings env_data = 'bootcmd=run bootm\x00bootdelay=3\x00console=ttyS0\x00' @@ -1221,7 +1232,7 @@ 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, - self._col) + self._col, self._result_handler) env = build._process_environment('/nonexistent/path/uboot.env') self.assertEqual({}, env) @@ -1229,7 +1240,7 @@ class TestBuilderFuncs(TestBuildBase): 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, - self._col) + self._col, self._result_handler) # 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 928be84167d..40132c1b46f 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -13,7 +13,8 @@ from unittest import mock from buildman import builder from buildman import builderthread from buildman.outcome import (OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, - OUTCOME_UNKNOWN) + OUTCOME_UNKNOWN, DisplayOptions) +from buildman.resulthandler import ResultHandler from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -25,9 +26,14 @@ class TestPrintFuncSizeDetail(unittest.TestCase): """Set up test fixtures""" # Create a minimal Builder for testing self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp', git_dir=None, num_threads=0, - num_jobs=1, col=self.col) + num_jobs=1, col=self.col, result_handler=self.result_handler) terminal.set_print_test_mode() def tearDown(self): @@ -155,9 +161,15 @@ class TestPrepareThread(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) terminal.set_print_test_mode() def tearDown(self): @@ -270,9 +282,15 @@ class TestPrepareWorkingSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) terminal.set_print_test_mode() def tearDown(self): @@ -481,9 +499,15 @@ class TestPrepareOutputSpace(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) terminal.set_print_test_mode() def tearDown(self): @@ -565,9 +589,15 @@ class TestCheckOutputForLoop(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) # Reset state before each test self.builder._restarting_config = False self.builder._terminated = False @@ -645,9 +675,15 @@ class TestMake(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) @mock.patch('buildman.builder.command.run_one') def test_make_basic(self, mock_run_one): @@ -738,9 +774,15 @@ class TestPrintBuildSummary(unittest.TestCase): def setUp(self): """Set up test fixtures""" self.col = terminal.Color() + opts = DisplayOptions( + show_errors=False, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=False, ide=False, list_error_boards=False) + self.result_handler = ResultHandler(self.col, opts) self.builder = builder.Builder( toolchains=None, base_dir='/tmp/test', git_dir='/src/repo', - num_threads=4, num_jobs=1, col=self.col) + num_threads=4, num_jobs=1, col=self.col, + result_handler=self.result_handler) # 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