[Concept,09/18] buildman: Create a class for handling results

Message ID 20260110200828.168672-10-sjg@u-boot.org
State New
Headers
Series buildman: Split up the enormous Builder class |

Commit Message

Simon Glass Jan. 10, 2026, 8:08 p.m. UTC
  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
  

Patch

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()