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