[PATCH 00/18] buildman: Split up the enormous Builder class
From: Simon Glass <simon.glass@canonical.com> This series extracts the ~800 lines of result-display code from builder.py into a new ResultHandler class. Builder handles building and file I/O, while ResultHandler handles all display of build results, progress, errors, sizes, and config changes. This is a complex and tedious refactor which is why have not done it for the past 5 years, but the time is now. Along the way, this: - Creates a DisplayOptions namedtuple for result-display settings - Adds col (terminal.Color) as a required Builder parameter - Creates a ResultHandler class with all display-related methods - Cleans up naming conventions with underscore prefixes for private members After this series, builder.py is reduced from ~2200 to ~1200 lines, making it more focused and maintainable. Unsurprisingly resulthandler.py ends up at just over 1000 lines. Simon Glass (18): buildman: Move Config class to cfgutil buildman: Move process_config() function to cfgutil buildman: Move cfgutil tests to test_cfgutil.py buildman: Move Outcome class to its own module buildman: Move OUTCOME_* constants to outcome module buildman: Move BoardStatus and ErrLine to outcome module buildman: Add a namedtuple for result-display settings buildman: Add col parameter to Builder buildman: Create a class for handling results buildman: Move size-display methods to ResultHandler buildman: Move error/warning display methods to ResultHandler buildman: Move board classification methods to ResultHandler buildman: Move error delta calculation to ResultHandler buildman: Move result summary methods to ResultHandler buildman: Move print_build_summary() to ResultHandler buildman: Move show_summary() and produce_result_summary() to ResultHandler buildman: Use underscore prefix for private methods buildman: Use underscore prefix for private member variables tools/buildman/builder.py | 1230 +++---------------------------- tools/buildman/cfgutil.py | 70 +- tools/buildman/control.py | 35 +- tools/buildman/main.py | 5 +- tools/buildman/outcome.py | 68 ++ tools/buildman/resulthandler.py | 1028 ++++++++++++++++++++++++++ tools/buildman/test.py | 290 ++------ tools/buildman/test_builder.py | 155 ++-- tools/buildman/test_cfgutil.py | 234 ++++++ 9 files changed, 1655 insertions(+), 1460 deletions(-) create mode 100644 tools/buildman/outcome.py create mode 100644 tools/buildman/resulthandler.py create mode 100644 tools/buildman/test_cfgutil.py -- 2.43.0 base-commit: c0b7ed1ad434eabf50bc8bdf7ef7f31423f4977d branch: bmq
From: Simon Glass <simon.glass@canonical.com> Move the Config class from builder.py to cfgutil.py as part of an effort to split up the large builder.py file. This class holds parsed configuration data for boards. It fits naturally in cfgutil.py alongside the other Kconfig utilities. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 27 +-------------------------- tools/buildman/cfgutil.py | 30 +++++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 27 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 3c0af07d624..5531e496271 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -20,6 +20,7 @@ import sys import threading from buildman import builderthread +from buildman.cfgutil import Config from u_boot_pylib import command from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -149,32 +150,6 @@ EXTRA_CONFIG_FILENAMES = [ 'autoconf.h', 'autoconf-spl.h','autoconf-tpl.h', ] -class Config: - """Holds information about configuration settings for a board.""" - def __init__(self, config_filename, target): - self.target = target - self.config = {} - for fname in config_filename: - self.config[fname] = {} - - def add(self, fname, key, value): - """Add a configuration value - - Args: - fname (str): Filename to add to (e.g. '.config') - key (str): Config key (e.g. 'CONFIG_DM') - value (str): Config value (e.g. 'y') - """ - self.config[fname][key] = value - - def __hash__(self): - val = 0 - for _, config in self.config.items(): - for key, value in config.items(): - print(key, value) - val = val ^ hash(key) & hash(value) - return val - class Environment: """Holds information about environment variables for a board.""" def __init__(self, target): diff --git a/tools/buildman/cfgutil.py b/tools/buildman/cfgutil.py index 5bc97d33595..ad6dee17829 100644 --- a/tools/buildman/cfgutil.py +++ b/tools/buildman/cfgutil.py @@ -3,12 +3,40 @@ # Written by Simon Glass <sjg@chromium.org> # -"""Utility functions for dealing with Kconfig .confing files""" +"""Utility functions for dealing with Kconfig .config files""" import re from u_boot_pylib import tools + +class Config: + """Holds information about configuration settings for a board.""" + def __init__(self, config_filename, target): + self.target = target + self.config = {} + for fname in config_filename: + self.config[fname] = {} + + def add(self, fname, key, value): + """Add a configuration value + + Args: + fname (str): Filename to add to (e.g. '.config') + key (str): Config key (e.g. 'CONFIG_DM') + value (str): Config value (e.g. 'y') + """ + self.config[fname][key] = value + + def __hash__(self): + val = 0 + for _, config in self.config.items(): + for key, value in config.items(): + print(key, value) + val = val ^ hash(key) & hash(value) + return val + + RE_LINE = re.compile(r'(# )?CONFIG_([A-Z0-9_]+)(=(.*)| is not set)') RE_CFG = re.compile(r'(~?)(CONFIG_)?([A-Z0-9_]+)(=.*)?') -- 2.43.0
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> Move the cfgutil-related tests from test.py to a new test_cfgutil.py file for better organisation: - TestAdjustCfg: Tests for adjust_cfg_line(), adjust_cfg_lines(), convert_list_to_dict(), and check_cfg_lines() - TestProcessConfig: Tests for process_config() This follows the pattern of other test files like test_builder.py and test_boards.py Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/main.py | 4 +- tools/buildman/test.py | 215 ------------------------------ tools/buildman/test_cfgutil.py | 234 +++++++++++++++++++++++++++++++++ 3 files changed, 237 insertions(+), 216 deletions(-) create mode 100644 tools/buildman/test_cfgutil.py diff --git a/tools/buildman/main.py b/tools/buildman/main.py index 5831dbf1222..c8502c370f9 100755 --- a/tools/buildman/main.py +++ b/tools/buildman/main.py @@ -40,6 +40,7 @@ def run_tests(skip_net_tests, debug, verbose, args): from buildman import test_boards from buildman import test_bsettings from buildman import test_builder + from buildman import test_cfgutil test_name = args.terms and args.terms[0] or None if skip_net_tests: @@ -50,7 +51,8 @@ def run_tests(skip_net_tests, debug, verbose, args): result = test_util.run_test_suites( 'buildman', debug, verbose, False, False, args.threads, test_name, [], [test.TestBuildOutput, test.TestBuildBoards, test.TestBuild, - test.TestBuildConfig, test.TestBuildMisc, test.TestBuilderFuncs, + test.TestBuildMisc, test.TestBuilderFuncs, + test_cfgutil.TestAdjustCfg, test_cfgutil.TestProcessConfig, func_test.TestFunctional, test_boards.TestBoards, test_bsettings.TestBsettings, test_builder.TestPrintFuncSizeDetail, diff --git a/tools/buildman/test.py b/tools/buildman/test.py index bdf68ef841d..b70a0012ca5 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -758,144 +758,6 @@ class TestBuild(TestBuildBase): self.assertEqual(expected, result) -class TestBuildConfig(TestBuildBase): - """Tests for config adjustment functionality""" - - def test_adjust_cfg_nop(self): - """check various adjustments of config that are nops""" - # enable an enabled CONFIG - self.assertEqual( - 'CONFIG_FRED=y', - cfgutil.adjust_cfg_line('CONFIG_FRED=y', {'FRED':'FRED'})[0]) - - # disable a disabled CONFIG - self.assertEqual( - '# CONFIG_FRED is not set', - cfgutil.adjust_cfg_line( - '# CONFIG_FRED is not set', {'FRED':'~FRED'})[0]) - - # use the adjust_cfg_lines() function - self.assertEqual( - ['CONFIG_FRED=y'], - cfgutil.adjust_cfg_lines(['CONFIG_FRED=y'], {'FRED':'FRED'})) - self.assertEqual( - ['# CONFIG_FRED is not set'], - cfgutil.adjust_cfg_lines(['CONFIG_FRED=y'], {'FRED':'~FRED'})) - - # handling an empty line - self.assertEqual('#', cfgutil.adjust_cfg_line('#', {'FRED':'~FRED'})[0]) - - def test_adjust_cfg(self): - """check various adjustments of config""" - # disable a CONFIG - self.assertEqual( - '# CONFIG_FRED is not set', - cfgutil.adjust_cfg_line('CONFIG_FRED=1' , {'FRED':'~FRED'})[0]) - - # enable a disabled CONFIG - self.assertEqual( - 'CONFIG_FRED=y', - cfgutil.adjust_cfg_line( - '# CONFIG_FRED is not set', {'FRED':'FRED'})[0]) - - # enable a CONFIG that doesn't exist - self.assertEqual( - ['CONFIG_FRED=y'], - cfgutil.adjust_cfg_lines([], {'FRED':'FRED'})) - - # disable a CONFIG that doesn't exist - self.assertEqual( - ['# CONFIG_FRED is not set'], - cfgutil.adjust_cfg_lines([], {'FRED':'~FRED'})) - - # disable a value CONFIG - self.assertEqual( - '# CONFIG_FRED is not set', - cfgutil.adjust_cfg_line('CONFIG_FRED="fred"' , {'FRED':'~FRED'})[0]) - - # setting a value CONFIG - self.assertEqual( - 'CONFIG_FRED="fred"', - cfgutil.adjust_cfg_line('# CONFIG_FRED is not set' , - {'FRED':'FRED="fred"'})[0]) - - # changing a value CONFIG - self.assertEqual( - 'CONFIG_FRED="fred"', - cfgutil.adjust_cfg_line('CONFIG_FRED="ernie"' , - {'FRED':'FRED="fred"'})[0]) - - # setting a value for a CONFIG that doesn't exist - self.assertEqual( - ['CONFIG_FRED="fred"'], - cfgutil.adjust_cfg_lines([], {'FRED':'FRED="fred"'})) - - def test_convert_adjust_cfg_list(self): - """Check conversion of the list of changes into a dict""" - self.assertEqual({}, cfgutil.convert_list_to_dict(None)) - - expect = { - 'FRED':'FRED', - 'MARY':'~MARY', - 'JOHN':'JOHN=0x123', - 'ALICE':'ALICE="alice"', - 'AMY':'AMY', - 'ABE':'~ABE', - 'MARK':'MARK=0x456', - 'ANNA':'ANNA="anna"', - } - actual = cfgutil.convert_list_to_dict( - ['FRED', '~MARY', 'JOHN=0x123', 'ALICE="alice"', - 'CONFIG_AMY', '~CONFIG_ABE', 'CONFIG_MARK=0x456', - 'CONFIG_ANNA="anna"']) - self.assertEqual(expect, actual) - - # Test comma-separated values - actual = cfgutil.convert_list_to_dict( - ['FRED,~MARY,JOHN=0x123', 'ALICE="alice"', - 'CONFIG_AMY,~CONFIG_ABE', 'CONFIG_MARK=0x456,CONFIG_ANNA="anna"']) - self.assertEqual(expect, actual) - - # Test mixed comma-separated and individual values - actual = cfgutil.convert_list_to_dict( - ['FRED,~MARY', 'JOHN=0x123', 'ALICE="alice",CONFIG_AMY', - '~CONFIG_ABE,CONFIG_MARK=0x456', 'CONFIG_ANNA="anna"']) - self.assertEqual(expect, actual) - - def test_check_cfg_file(self): - """Test check_cfg_file detects conflicts as expected""" - # Check failure to disable CONFIG - result = cfgutil.check_cfg_lines(['CONFIG_FRED=1'], {'FRED':'~FRED'}) - self.assertEqual([['~FRED', 'CONFIG_FRED=1']], result) - - result = cfgutil.check_cfg_lines( - ['CONFIG_FRED=1', 'CONFIG_MARY="mary"'], {'FRED':'~FRED'}) - self.assertEqual([['~FRED', 'CONFIG_FRED=1']], result) - - result = cfgutil.check_cfg_lines( - ['CONFIG_FRED=1', 'CONFIG_MARY="mary"'], {'MARY':'~MARY'}) - self.assertEqual([['~MARY', 'CONFIG_MARY="mary"']], result) - - # Check failure to enable CONFIG - result = cfgutil.check_cfg_lines( - ['# CONFIG_FRED is not set'], {'FRED':'FRED'}) - self.assertEqual([['FRED', '# CONFIG_FRED is not set']], result) - - # Check failure to set CONFIG value - result = cfgutil.check_cfg_lines( - ['# CONFIG_FRED is not set', 'CONFIG_MARY="not"'], - {'MARY':'MARY="mary"', 'FRED':'FRED'}) - self.assertEqual([ - ['FRED', '# CONFIG_FRED is not set'], - ['MARY="mary"', 'CONFIG_MARY="not"']], result) - - # Check failure to add CONFIG value - result = cfgutil.check_cfg_lines([], {'MARY':'MARY="mary"'}) - self.assertEqual([ - ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], - result) - - class TestBuildMisc(TestBuildBase): """Miscellaneous buildman tests""" @@ -1322,83 +1184,6 @@ class TestBuilderFuncs(TestBuildBase): finally: os.unlink(tmp_name) - def test_process_config_defconfig(self): - """Test process_config() with .config style file""" - config_data = '''# This is a comment -CONFIG_OPTION_A=y -CONFIG_OPTION_B="string" -CONFIG_OPTION_C=123 -# CONFIG_OPTION_D is not set -''' - with tempfile.NamedTemporaryFile(mode='w', delete=False, - suffix='.config') as tmp: - tmp.write(config_data) - tmp_name = tmp.name - - try: - config = cfgutil.process_config(tmp_name, squash_config_y=False) - - self.assertEqual('y', config['CONFIG_OPTION_A']) - self.assertEqual('"string"', config['CONFIG_OPTION_B']) - self.assertEqual('123', config['CONFIG_OPTION_C']) - # Comments should be ignored - self.assertNotIn('CONFIG_OPTION_D', config) - finally: - os.unlink(tmp_name) - - def test_process_config_autoconf_h(self): - """Test process_config() with autoconf.h style file""" - config_data = '''/* Auto-generated header */ -#define CONFIG_OPTION_A 1 -#define CONFIG_OPTION_B "value" -#define CONFIG_OPTION_C -#define NOT_CONFIG 1 -''' - with tempfile.NamedTemporaryFile(mode='w', delete=False, - suffix='.h') as tmp: - tmp.write(config_data) - tmp_name = tmp.name - - try: - config = cfgutil.process_config(tmp_name, squash_config_y=False) - - self.assertEqual('1', config['CONFIG_OPTION_A']) - self.assertEqual('"value"', config['CONFIG_OPTION_B']) - # #define without value gets empty string (squash_config_y=False) - self.assertEqual('', config['CONFIG_OPTION_C']) - # Non-CONFIG_ defines should be ignored - self.assertNotIn('NOT_CONFIG', config) - finally: - os.unlink(tmp_name) - - def test_process_config_nonexistent(self): - """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""" - config_data = '''CONFIG_OPTION_A=y -CONFIG_OPTION_B=n -#define CONFIG_OPTION_C -''' - with tempfile.NamedTemporaryFile(mode='w', delete=False) as tmp: - tmp.write(config_data) - tmp_name = tmp.name - - try: - config = cfgutil.process_config(tmp_name, squash_config_y=True) - - # y should be squashed to 1 - self.assertEqual('1', config['CONFIG_OPTION_A']) - # n should remain n - self.assertEqual('n', config['CONFIG_OPTION_B']) - # Empty #define should get '1' when squash_config_y is True - self.assertEqual('1', config['CONFIG_OPTION_C']) - finally: - os.unlink(tmp_name) - def test_process_environment(self): """Test _process_environment() function""" build = builder.Builder(self.toolchains, self.base_dir, None, 0, 2) diff --git a/tools/buildman/test_cfgutil.py b/tools/buildman/test_cfgutil.py new file mode 100644 index 00000000000..ba3f0468570 --- /dev/null +++ b/tools/buildman/test_cfgutil.py @@ -0,0 +1,234 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright (c) 2022 Google LLC +# + +"""Tests for cfgutil module""" + +import os +import tempfile +import unittest + +from buildman import cfgutil + + +class TestAdjustCfg(unittest.TestCase): + """Tests for config adjustment functions""" + + def test_adjust_cfg_nop(self): + """check various adjustments of config that are nops""" + # enable an enabled CONFIG + self.assertEqual( + 'CONFIG_FRED=y', + cfgutil.adjust_cfg_line('CONFIG_FRED=y', {'FRED':'FRED'})[0]) + + # disable a disabled CONFIG + self.assertEqual( + '# CONFIG_FRED is not set', + cfgutil.adjust_cfg_line( + '# CONFIG_FRED is not set', {'FRED':'~FRED'})[0]) + + # use the adjust_cfg_lines() function + self.assertEqual( + ['CONFIG_FRED=y'], + cfgutil.adjust_cfg_lines(['CONFIG_FRED=y'], {'FRED':'FRED'})) + self.assertEqual( + ['# CONFIG_FRED is not set'], + cfgutil.adjust_cfg_lines(['CONFIG_FRED=y'], {'FRED':'~FRED'})) + + # handling an empty line + self.assertEqual('#', cfgutil.adjust_cfg_line('#', {'FRED':'~FRED'})[0]) + + def test_adjust_cfg(self): + """check various adjustments of config""" + # disable a CONFIG + self.assertEqual( + '# CONFIG_FRED is not set', + cfgutil.adjust_cfg_line('CONFIG_FRED=1' , {'FRED':'~FRED'})[0]) + + # enable a disabled CONFIG + self.assertEqual( + 'CONFIG_FRED=y', + cfgutil.adjust_cfg_line( + '# CONFIG_FRED is not set', {'FRED':'FRED'})[0]) + + # enable a CONFIG that doesn't exist + self.assertEqual( + ['CONFIG_FRED=y'], + cfgutil.adjust_cfg_lines([], {'FRED':'FRED'})) + + # disable a CONFIG that doesn't exist + self.assertEqual( + ['# CONFIG_FRED is not set'], + cfgutil.adjust_cfg_lines([], {'FRED':'~FRED'})) + + # disable a value CONFIG + self.assertEqual( + '# CONFIG_FRED is not set', + cfgutil.adjust_cfg_line('CONFIG_FRED="fred"' , {'FRED':'~FRED'})[0]) + + # setting a value CONFIG + self.assertEqual( + 'CONFIG_FRED="fred"', + cfgutil.adjust_cfg_line('# CONFIG_FRED is not set' , + {'FRED':'FRED="fred"'})[0]) + + # changing a value CONFIG + self.assertEqual( + 'CONFIG_FRED="fred"', + cfgutil.adjust_cfg_line('CONFIG_FRED="ernie"' , + {'FRED':'FRED="fred"'})[0]) + + # setting a value for a CONFIG that doesn't exist + self.assertEqual( + ['CONFIG_FRED="fred"'], + cfgutil.adjust_cfg_lines([], {'FRED':'FRED="fred"'})) + + def test_convert_adjust_cfg_list(self): + """Check conversion of the list of changes into a dict""" + self.assertEqual({}, cfgutil.convert_list_to_dict(None)) + + expect = { + 'FRED':'FRED', + 'MARY':'~MARY', + 'JOHN':'JOHN=0x123', + 'ALICE':'ALICE="alice"', + 'AMY':'AMY', + 'ABE':'~ABE', + 'MARK':'MARK=0x456', + 'ANNA':'ANNA="anna"', + } + actual = cfgutil.convert_list_to_dict( + ['FRED', '~MARY', 'JOHN=0x123', 'ALICE="alice"', + 'CONFIG_AMY', '~CONFIG_ABE', 'CONFIG_MARK=0x456', + 'CONFIG_ANNA="anna"']) + self.assertEqual(expect, actual) + + # Test comma-separated values + actual = cfgutil.convert_list_to_dict( + ['FRED,~MARY,JOHN=0x123', 'ALICE="alice"', + 'CONFIG_AMY,~CONFIG_ABE', 'CONFIG_MARK=0x456,CONFIG_ANNA="anna"']) + self.assertEqual(expect, actual) + + # Test mixed comma-separated and individual values + actual = cfgutil.convert_list_to_dict( + ['FRED,~MARY', 'JOHN=0x123', 'ALICE="alice",CONFIG_AMY', + '~CONFIG_ABE,CONFIG_MARK=0x456', 'CONFIG_ANNA="anna"']) + self.assertEqual(expect, actual) + + def test_check_cfg_file(self): + """Test check_cfg_file detects conflicts as expected""" + # Check failure to disable CONFIG + result = cfgutil.check_cfg_lines(['CONFIG_FRED=1'], {'FRED':'~FRED'}) + self.assertEqual([['~FRED', 'CONFIG_FRED=1']], result) + + result = cfgutil.check_cfg_lines( + ['CONFIG_FRED=1', 'CONFIG_MARY="mary"'], {'FRED':'~FRED'}) + self.assertEqual([['~FRED', 'CONFIG_FRED=1']], result) + + result = cfgutil.check_cfg_lines( + ['CONFIG_FRED=1', 'CONFIG_MARY="mary"'], {'MARY':'~MARY'}) + self.assertEqual([['~MARY', 'CONFIG_MARY="mary"']], result) + + # Check failure to enable CONFIG + result = cfgutil.check_cfg_lines( + ['# CONFIG_FRED is not set'], {'FRED':'FRED'}) + self.assertEqual([['FRED', '# CONFIG_FRED is not set']], result) + + # Check failure to set CONFIG value + result = cfgutil.check_cfg_lines( + ['# CONFIG_FRED is not set', 'CONFIG_MARY="not"'], + {'MARY':'MARY="mary"', 'FRED':'FRED'}) + self.assertEqual([ + ['FRED', '# CONFIG_FRED is not set'], + ['MARY="mary"', 'CONFIG_MARY="not"']], result) + + # Check failure to add CONFIG value + result = cfgutil.check_cfg_lines([], {'MARY':'MARY="mary"'}) + self.assertEqual([ + ['MARY="mary"', 'Missing expected line: CONFIG_MARY="mary"']], + result) + + +class TestProcessConfig(unittest.TestCase): + """Tests for process_config() function""" + + def test_process_config_defconfig(self): + """Test process_config() with .config style file""" + config_data = '''# This is a comment +CONFIG_OPTION_A=y +CONFIG_OPTION_B="string" +CONFIG_OPTION_C=123 +# CONFIG_OPTION_D is not set +''' + with tempfile.NamedTemporaryFile(mode='w', delete=False, + suffix='.config') as tmp: + tmp.write(config_data) + tmp_name = tmp.name + + try: + config = cfgutil.process_config(tmp_name, squash_config_y=False) + + self.assertEqual('y', config['CONFIG_OPTION_A']) + self.assertEqual('"string"', config['CONFIG_OPTION_B']) + self.assertEqual('123', config['CONFIG_OPTION_C']) + # Comments should be ignored + self.assertNotIn('CONFIG_OPTION_D', config) + finally: + os.unlink(tmp_name) + + def test_process_config_autoconf_h(self): + """Test process_config() with autoconf.h style file""" + config_data = '''/* Auto-generated header */ +#define CONFIG_OPTION_A 1 +#define CONFIG_OPTION_B "value" +#define CONFIG_OPTION_C +#define NOT_CONFIG 1 +''' + with tempfile.NamedTemporaryFile(mode='w', delete=False, + suffix='.h') as tmp: + tmp.write(config_data) + tmp_name = tmp.name + + try: + config = cfgutil.process_config(tmp_name, squash_config_y=False) + + self.assertEqual('1', config['CONFIG_OPTION_A']) + self.assertEqual('"value"', config['CONFIG_OPTION_B']) + # #define without value gets empty string (squash_config_y=False) + self.assertEqual('', config['CONFIG_OPTION_C']) + # Non-CONFIG_ defines should be ignored + self.assertNotIn('NOT_CONFIG', config) + finally: + os.unlink(tmp_name) + + def test_process_config_nonexistent(self): + """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""" + config_data = '''CONFIG_OPTION_A=y +CONFIG_OPTION_B=n +#define CONFIG_OPTION_C +''' + with tempfile.NamedTemporaryFile(mode='w', delete=False) as tmp: + tmp.write(config_data) + tmp_name = tmp.name + + try: + config = cfgutil.process_config(tmp_name, squash_config_y=True) + + # y should be squashed to 1 + self.assertEqual('1', config['CONFIG_OPTION_A']) + # n should remain n + self.assertEqual('n', config['CONFIG_OPTION_B']) + # Empty #define should get '1' when squash_config_y is True + self.assertEqual('1', config['CONFIG_OPTION_C']) + finally: + os.unlink(tmp_name) + + +if __name__ == "__main__": + unittest.main() -- 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 OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, and OUTCOME_UNKNOWN constants from builder.py to outcome.py alongside the Outcome class. Update builder.py and test_builder.py to import from the new location. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 6 ++---- tools/buildman/outcome.py | 5 ++++- tools/buildman/test_builder.py | 32 +++++++++++++++++++------------- 3 files changed, 25 insertions(+), 18 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 94552bf62ca..96976f513e8 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -21,7 +21,8 @@ import threading from buildman import builderthread from buildman.cfgutil import Config, process_config -from buildman.outcome import Outcome +from buildman.outcome import (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 @@ -135,9 +136,6 @@ ErrLine = collections.namedtuple('ErrLine', 'char,brds,errline') # unknown: List of boards that were not built BoardStatus = collections.namedtuple('BoardStatus', 'ok,warn,err,new,unknown') -# Possible build outcomes -OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = list(range(4)) - # 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 4f8bc9a0bae..373740cd94b 100644 --- a/tools/buildman/outcome.py +++ b/tools/buildman/outcome.py @@ -1,7 +1,10 @@ # SPDX-License-Identifier: GPL-2.0+ # Copyright (c) 2013 The Chromium OS Authors. -"""Outcome class for buildman build results""" +"""Outcome class and constants for buildman build results""" + +# Build-outcome codes +OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = list(range(4)) class Outcome: diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index fd60767bca0..b92e0be18be 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -12,6 +12,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) from u_boot_pylib import gitutil from u_boot_pylib import terminal @@ -358,12 +360,16 @@ class TestShowNotBuilt(unittest.TestCase): outcome.err_lines = err_lines if err_lines else [] 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) + def test_all_boards_built(self): """Test when all selected boards were built successfully""" board_selected = {'board1': None, 'board2': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_OK), - 'board2': self._make_outcome(builder.OUTCOME_OK), + 'board1': self._make_outcome(OUTCOME_OK), + 'board2': self._make_outcome(OUTCOME_OK), } terminal.get_print_test_lines() # Clear @@ -377,9 +383,9 @@ class TestShowNotBuilt(unittest.TestCase): """Test when some boards have OUTCOME_UNKNOWN (e.g. missing toolchain)""" board_selected = {'board1': None, 'board2': None, 'board3': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_OK), - 'board2': self._make_outcome(builder.OUTCOME_UNKNOWN), - 'board3': self._make_outcome(builder.OUTCOME_UNKNOWN), + 'board1': self._make_outcome(OUTCOME_OK), + 'board2': self._make_outcome(OUTCOME_UNKNOWN), + 'board3': self._make_outcome(OUTCOME_UNKNOWN), } terminal.get_print_test_lines() # Clear @@ -396,8 +402,8 @@ class TestShowNotBuilt(unittest.TestCase): """Test when all boards have OUTCOME_UNKNOWN""" board_selected = {'board1': None, 'board2': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_UNKNOWN), - 'board2': self._make_outcome(builder.OUTCOME_UNKNOWN), + 'board1': self._make_outcome(OUTCOME_UNKNOWN), + 'board2': self._make_outcome(OUTCOME_UNKNOWN), } terminal.get_print_test_lines() # Clear @@ -413,8 +419,8 @@ class TestShowNotBuilt(unittest.TestCase): """Test that build errors (not toolchain) are not counted as 'not built'""" board_selected = {'board1': None, 'board2': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_OK), - 'board2': self._make_outcome(builder.OUTCOME_ERROR, + 'board1': self._make_outcome(OUTCOME_OK), + 'board2': self._make_outcome(OUTCOME_ERROR, ['error: some build error']), } @@ -429,10 +435,10 @@ class TestShowNotBuilt(unittest.TestCase): """Test that toolchain errors are counted as 'not built'""" board_selected = {'board1': None, 'board2': None, 'board3': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_OK), - 'board2': self._make_outcome(builder.OUTCOME_ERROR, + 'board1': self._make_outcome(OUTCOME_OK), + 'board2': self._make_outcome(OUTCOME_ERROR, ['Tool chain error for arm: not found']), - 'board3': self._make_outcome(builder.OUTCOME_ERROR, + 'board3': self._make_outcome(OUTCOME_ERROR, ['error: some build error']), } @@ -451,7 +457,7 @@ class TestShowNotBuilt(unittest.TestCase): """Test that boards missing from board_dict are counted as 'not built'""" board_selected = {'board1': None, 'board2': None, 'board3': None} board_dict = { - 'board1': self._make_outcome(builder.OUTCOME_OK), + 'board1': self._make_outcome(OUTCOME_OK), # board2 and board3 are not in board_dict } -- 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> Create a DisplayOptions namedtuple to group the display-related options (show_errors, show_sizes, show_detail, show_bloat, show_config, show_environment, show_unknown, ide, list_error_boards) that are passed between control, Builder, and ResultHandler. This simplifies method signatures significantly by replacing 9 boolean parameters with a single opts parameter. Builder stores these in self._opts and accesses them as self._opts.show_errors etc. The filter_dtb_warnings and filter_migration_warnings options remain as separate parameters to set_display_options() since they're only used by Builder, not ResultHandler. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 82 ++++++++++++++------------------------- tools/buildman/control.py | 18 ++++++--- tools/buildman/outcome.py | 13 +++++++ tools/buildman/test.py | 28 ++++++++----- 4 files changed, 74 insertions(+), 67 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index dd1da92e0ea..00a3b2f165b 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -21,7 +21,7 @@ import threading from buildman import builderthread from buildman.cfgutil import Config, process_config -from buildman.outcome import (BoardStatus, ErrLine, Outcome, +from buildman.outcome import (BoardStatus, DisplayOptions, ErrLine, Outcome, OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN) from u_boot_pylib import command @@ -202,7 +202,6 @@ class Builder: _complete_delay: Expected delay until completion (timedelta) _next_delay_update: Next time we plan to display a progress update (datatime) - _show_unknown: Show unknown boards (those not built) in summary _start_time: Start time for the build _timestamps: List of timestamps for the completion of the last last _timestamp_count builds. Each is a datetime object. @@ -217,7 +216,7 @@ class Builder: """ def __init__(self, toolchains, base_dir, git_dir, num_threads, num_jobs, - gnu_make='make', checkout=True, show_unknown=True, step=1, + 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, @@ -240,7 +239,6 @@ class Builder: gnu_make: the command name of GNU Make. checkout: True to check out source, False to skip that step. This is used for testing. - show_unknown: Show unknown boards (those not built) in summary step: 1 to process every commit, n to process every nth commit no_subdirs: Don't create subdirectories when building current source for a single board @@ -298,7 +296,6 @@ class Builder: self.kconfig_reconfig = 0 self.force_build = False self.git_dir = git_dir - self._show_unknown = show_unknown self._timestamp_count = 10 self._build_period_us = None self._complete_delay = None @@ -315,7 +312,6 @@ class Builder: self.work_in_output = work_in_output self.adjust_cfg = adjust_cfg self.allow_missing = allow_missing - self._ide = False self.no_lto = no_lto self.reproducible_builds = reproducible_builds self.force_build = force_build @@ -345,13 +341,10 @@ class Builder: self.test_thread_exceptions = test_thread_exceptions # Attributes used by set_display_options() - self._show_errors = False - self._show_sizes = False - self._show_detail = False - self._show_bloat = False - self._list_error_boards = False - self._show_config = False - self._show_environment = False + self._opts = DisplayOptions( + show_errors=True, show_sizes=False, show_detail=False, + show_bloat=False, show_config=False, show_environment=False, + show_unknown=True, ide=False, list_error_boards=False) self._filter_dtb_warnings = False self._filter_migration_warnings = False @@ -440,41 +433,24 @@ class Builder: env[b'DTC'] = tools.to_bytes(self.dtc) return env - def set_display_options(self, show_errors=False, show_sizes=False, - show_detail=False, show_bloat=False, - list_error_boards=False, show_config=False, - show_environment=False, filter_dtb_warnings=False, - filter_migration_warnings=False, ide=False): + def set_display_options(self, display_options, + filter_dtb_warnings=False, + filter_migration_warnings=False): """Setup display options for the builder. Args: - show_errors (bool): True to show summarised error/warning info - show_sizes (bool): Show size deltas - show_detail (bool): Show size delta detail for each board if - show_sizes - show_bloat (bool): Show detail for each function - list_error_boards (bool): Show the boards which caused each - error/warning - show_config (bool): Show config deltas - show_environment (bool): Show environment deltas + display_options (DisplayOptions): Named tuple containing display + settings (show_errors, show_sizes, show_detail, show_bloat, + show_config, show_environment, show_unknown, ide, + list_error_boards) filter_dtb_warnings (bool): Filter out any warnings from the device-tree compiler filter_migration_warnings (bool): Filter out any warnings about migrating a board to driver model - ide (bool): Create output that can be parsed by an IDE. There is - no '+' prefix on error lines and output on stderr stays on - stderr. """ - self._show_errors = show_errors - self._show_sizes = show_sizes - self._show_detail = show_detail - self._show_bloat = show_bloat - self._list_error_boards = list_error_boards - self._show_config = show_config - self._show_environment = show_environment + self._opts = display_options self._filter_dtb_warnings = filter_dtb_warnings self._filter_migration_warnings = filter_migration_warnings - self._ide = ide def _add_timestamp(self): """Add a new timestamp to the list and record the build period. @@ -594,7 +570,7 @@ class Builder: self.already_done += 1 if result.kconfig_reconfig: self.kconfig_reconfig += 1 - if self._ide: + if self._opts.ide: if result.stderr: sys.stderr.write(result.stderr) elif self._verbose: @@ -625,7 +601,7 @@ class Builder: line += f'{self._complete_delay} : ' line += target - if not self._ide: + if not self._opts.ide: terminal.print_clear() tprint(line, newline=False, limit_to_line=True) @@ -1662,7 +1638,7 @@ class Builder: better_warn: List of ErrLine for fixed warnings worse_warn: List of ErrLine for new warnings """ - if self._ide: + if self._opts.ide: return if not any((brd_status.ok, brd_status.warn, brd_status.err, brd_status.unknown, brd_status.new, worse_err, better_err, @@ -1677,7 +1653,7 @@ class Builder: self.col.RED) self.add_outcome(board_selected, arch_list, brd_status.new, '*', self.col.BLUE) - if self._show_unknown: + if self._opts.show_unknown: self.add_outcome(board_selected, arch_list, brd_status.unknown, '?', self.col.MAGENTA) for arch, target_list in arch_list.items(): @@ -1695,7 +1671,7 @@ class Builder: board_selected (dict): Dict of selected boards, keyed by target board_dict (dict): Dict of boards that were built, keyed by target """ - if not self._ide: + if not self._opts.ide: return for target in board_dict: if target not in board_selected: @@ -1753,7 +1729,7 @@ class Builder: """ brds = [] board_set = set() - if self._list_error_boards: + if self._opts.list_error_boards: for brd in line_boards[line]: if not brd in board_set: brds.append(brd) @@ -1873,17 +1849,17 @@ class Builder: (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._show_bloat, - read_config=self._show_config, - read_environment=self._show_environment) + 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.print_result_summary(board_selected, board_dict, - err_lines if self._show_errors else [], err_line_boards, - warn_lines if self._show_errors else [], warn_line_boards, - config, environment, self._show_sizes, self._show_detail, - self._show_bloat, self._show_config, self._show_environment) + 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) def show_summary(self, commits, board_selected): """Show a build summary for U-Boot for a given board list. @@ -2077,7 +2053,7 @@ class Builder: self._prepare_working_space(min(self.num_threads, len(board_selected)), commits is not None) self._prepare_output_space() - if not self._ide: + if not self._opts.ide: tprint('\rStarting build...', newline=False) self._start_time = datetime.now() self.setup_build(board_selected, commits) @@ -2107,7 +2083,7 @@ class Builder: # Wait until we have processed all output self.out_queue.join() - if not self._ide: + if not self._opts.ide: self._print_build_summary() return (self.fail, self.warned, self.thread_exceptions) diff --git a/tools/buildman/control.py b/tools/buildman/control.py index 052c9fa4343..ddb8c2f145e 100644 --- a/tools/buildman/control.py +++ b/tools/buildman/control.py @@ -20,6 +20,7 @@ from buildman import bsettings from buildman import cfgutil from buildman import toolchain from buildman.builder import Builder +from buildman.outcome import DisplayOptions from patman import patchstream import qconfig from u_boot_pylib import command @@ -559,10 +560,18 @@ 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( - args.show_errors, args.show_sizes, args.show_detail, args.show_bloat, - args.list_error_boards, args.show_config, args.show_environment, - args.filter_dtb_warnings, args.filter_migration_warnings, args.ide) + display_options, args.filter_dtb_warnings, args.filter_migration_warnings) if args.summary: builder.show_summary(commits, board_selected) else: @@ -802,8 +811,7 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None, # Create a new builder with the selected args builder = Builder(toolchains, output_dir, git_dir, - args.threads, args.jobs, checkout=True, - show_unknown=args.show_unknown, step=args.step, + args.threads, args.jobs, 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/outcome.py b/tools/buildman/outcome.py index 2cbed9a1aba..18c97e523a0 100644 --- a/tools/buildman/outcome.py +++ b/tools/buildman/outcome.py @@ -16,6 +16,19 @@ OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN = list(range(4)) # unknown: List of boards that were not built BoardStatus = namedtuple('BoardStatus', 'ok warn err new unknown') +# Display options for result output +DisplayOptions = namedtuple('DisplayOptions', [ + 'show_errors', # Show error/warning lines + 'show_sizes', # Show size deltas + 'show_detail', # Show size delta detail for each board + 'show_bloat', # Show detail for each function + 'show_config', # Show config changes + 'show_environment', # Show environment changes + 'show_unknown', # Show unknown boards in summary + 'ide', # IDE mode - output to stderr + 'list_error_boards', # Include board list with error lines +]) + # Error line information for display # char: Character representation: '+': error, '-': fixed error, 'w+': warning, # 'w-' = fixed warning diff --git a/tools/buildman/test.py b/tools/buildman/test.py index b70a0012ca5..cf37b821bf7 100644 --- a/tools/buildman/test.py +++ b/tools/buildman/test.py @@ -24,6 +24,7 @@ from buildman import builderthread from buildman import cfgutil from buildman import control from buildman import toolchain +from buildman.outcome import DisplayOptions from patman import commit from u_boot_pylib import command from u_boot_pylib import terminal @@ -245,20 +246,25 @@ class TestBuildOutput(TestBuildBase): expect += col.build(expected_colour, f' {brd}') self.assertEqual(text, expect) - def _setup_test(self, echo_lines=False, threads=1, **kwdisplay_args): + def _setup_test(self, echo_lines=False, threads=1, + show_errors=False, list_error_boards=False, + filter_dtb_warnings=False, filter_migration_warnings=False): """Set up the test by running a build and summary Args: echo_lines: True to echo lines to the terminal to aid test development - kwdisplay_args: Dict of arguments to pass to - Builder.SetDisplayOptions() + threads: Number of threads to use + show_errors: Show errors in summary + list_error_boards: List boards with errors + filter_dtb_warnings: Filter device-tree warnings + filter_migration_warnings: Filter migration warnings Returns: Iterator containing the output lines, each a PrintLine() object """ build = builder.Builder(self.toolchains, self.base_dir, None, threads, - 2, checkout=False, show_unknown=False) + 2, checkout=False) build.do_make = self.make board_selected = self.brds.get_selected_dict() @@ -275,7 +281,12 @@ 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) - build.set_display_options(**kwdisplay_args) + 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) if echo_lines: terminal.echo_print_test_lines() @@ -631,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, show_unknown=False) + checkout=False) build.commits = self.commits build.commit_count = len(self.commits) subject = self.commits[1].subject.translate(builder.trans_valid_chars) @@ -641,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, show_unknown=False) + checkout=False) build.commits = None build.commit_count = 0 self.check_dirs(build, '/current') @@ -649,8 +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, show_unknown=False, - no_subdirs=True) + checkout=False, no_subdirs=True) build.commits = None build.commit_count = 0 self.check_dirs(build, '') -- 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> 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
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> Continue extracting display methods from builder.py to ResultHandler. Move the error and warning display methods: - add_outcome(): Format board status by architecture - output_err_lines(): Display formatted error/warning lines - display_arch_results(): Show board status by architecture - print_ide_output(): Output errors to stderr for IDE The methods now return an error line count instead of updating a Builder attribute directly, making them more pure and testable. Update print_result_summary() to use the ResultHandler methods and track the error_lines counter locally. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 120 ++------------------------------ tools/buildman/resulthandler.py | 118 +++++++++++++++++++++++++++++++ 2 files changed, 124 insertions(+), 114 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 4b6106b881b..4d3b96ab1a0 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1011,36 +1011,6 @@ class Builder: return (board_dict, err_lines_summary, err_lines_boards, warn_lines_summary, warn_lines_boards, config, environment) - def add_outcome(self, board_dict, arch_list, changes, char, color): - """Add an output to our list of outcomes for each architecture - - This simple function adds failing boards (changes) to the - relevant architecture string, so we can print the results out - sorted by architecture. - - Args: - board_dict (dict): Dict containing all boards - arch_list (dict): Dict keyed by arch name. Value is a string - containing a list of board names which failed for that arch. - changes (list): List of boards to add to arch_list - char (str): Character to display for this board - color (int): terminal.Colour object - """ - done_arch = {} - for target in changes: - if target in board_dict: - arch = board_dict[target].arch - else: - arch = 'unknown' - text = self.col.build(color, ' ' + target) - if arch not in done_arch: - text = f' {self.col.build(color, char)} {text}' - done_arch[arch] = True - if arch not in arch_list: - arch_list[arch] = text - else: - arch_list[arch] += text - def reset_result_summary(self, board_selected): """Reset the results summary ready for use. @@ -1347,87 +1317,6 @@ class Builder: tprint(f"{' '.join(sorted(targets))} :") self._output_config_info(lines.split('\n')) - def _output_err_lines(self, err_lines, colour): - """Output the line of error/warning lines, if not empty - - Also increments self._error_lines if err_lines not empty - - Args: - err_lines: List of ErrLine objects, each an error or warning - line, possibly including a list of boards with that - error/warning - colour: Colour to use for output - """ - if err_lines: - out_list = [] - for line in err_lines: - names = [brd.target for brd in line.brds] - board_str = ' '.join(names) if names else '' - if board_str: - out = self.col.build(colour, line.char + '(') - out += self.col.build(self.col.MAGENTA, board_str, - bright=False) - out += self.col.build(colour, f') {line.errline}') - else: - out = self.col.build(colour, line.char + line.errline) - out_list.append(out) - tprint('\n'.join(out_list)) - self._error_lines += 1 - - def _display_arch_results(self, board_selected, brd_status, better_err, - worse_err, better_warn, worse_warn): - """Display results by architecture - - Args: - board_selected (dict): Dict containing boards to summarise - brd_status (BoardStatus): Named tuple with board classifications - better_err: List of ErrLine for fixed errors - worse_err: List of ErrLine for new errors - better_warn: List of ErrLine for fixed warnings - worse_warn: List of ErrLine for new warnings - """ - if self._opts.ide: - return - if not any((brd_status.ok, brd_status.warn, brd_status.err, - brd_status.unknown, brd_status.new, worse_err, better_err, - worse_warn, better_warn)): - return - arch_list = {} - self.add_outcome(board_selected, arch_list, brd_status.ok, '', - self.col.GREEN) - self.add_outcome(board_selected, arch_list, brd_status.warn, 'w+', - self.col.YELLOW) - self.add_outcome(board_selected, arch_list, brd_status.err, '+', - self.col.RED) - self.add_outcome(board_selected, arch_list, brd_status.new, '*', - self.col.BLUE) - if self._opts.show_unknown: - self.add_outcome(board_selected, arch_list, brd_status.unknown, - '?', self.col.MAGENTA) - for arch, target_list in arch_list.items(): - tprint(f'{arch:>10s}: {target_list}') - self._error_lines += 1 - self._output_err_lines(better_err, colour=self.col.GREEN) - self._output_err_lines(worse_err, colour=self.col.RED) - self._output_err_lines(better_warn, colour=self.col.CYAN) - self._output_err_lines(worse_warn, colour=self.col.YELLOW) - - def _print_ide_output(self, board_selected, board_dict): - """Print output for IDE mode - - Args: - board_selected (dict): Dict of selected boards, keyed by target - board_dict (dict): Dict of boards that were built, keyed by target - """ - if not self._opts.ide: - return - for target in board_dict: - if target not in board_selected: - continue - outcome = board_dict[target] - for line in outcome.err_lines: - sys.stderr.write(line) - 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, @@ -1527,11 +1416,14 @@ class Builder: self._base_warn_line_boards, warn_lines, warn_line_boards, 'w') # For the IDE mode, print out all the output - self._print_ide_output(board_selected, board_dict) + if self._opts.ide: + self._result_handler.print_ide_output(board_selected, board_dict) # Display results by arch - self._display_arch_results(board_selected, brd_status, better_err, - worse_err, better_warn, worse_warn) + 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( diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index 4c9b3c2f9cc..56973d801bb 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -6,6 +6,8 @@ """Result writer for buildman build results""" +import sys + from u_boot_pylib.terminal import tprint @@ -301,3 +303,119 @@ class ResultHandler: if show_detail: self.print_size_detail(target_list, base_board_dict, board_dict, show_bloat) + + def add_outcome(self, board_dict, arch_list, changes, char, color): + """Add an output to our list of outcomes for each architecture + + This simple function adds failing boards (changes) to the + relevant architecture string, so we can print the results out + sorted by architecture. + + Args: + board_dict (dict): Dict containing all boards + arch_list (dict): Dict keyed by arch name. Value is a string + containing a list of board names which failed for that arch. + changes (list): List of boards to add to arch_list + char (str): Character to display for this board + color (int): terminal.Colour object + """ + done_arch = {} + for target in changes: + if target in board_dict: + arch = board_dict[target].arch + else: + arch = 'unknown' + text = self._col.build(color, ' ' + target) + if arch not in done_arch: + text = f' {self._col.build(color, char)} {text}' + done_arch[arch] = True + if arch not in arch_list: + arch_list[arch] = text + else: + arch_list[arch] += text + + def output_err_lines(self, err_lines, colour): + """Output the line of error/warning lines, if not empty + + Args: + err_lines: List of ErrLine objects, each an error or warning + line, possibly including a list of boards with that + error/warning + colour: Colour to use for output + + Returns: + int: 1 if any lines were output, 0 otherwise + """ + if err_lines: + out_list = [] + for line in err_lines: + names = [brd.target for brd in line.brds] + board_str = ' '.join(names) if names else '' + if board_str: + out = self._col.build(colour, line.char + '(') + out += self._col.build(self._col.MAGENTA, board_str, + bright=False) + out += self._col.build(colour, f') {line.errline}') + else: + out = self._col.build(colour, line.char + line.errline) + out_list.append(out) + tprint('\n'.join(out_list)) + return 1 + return 0 + + def display_arch_results(self, board_selected, brd_status, better_err, + worse_err, better_warn, worse_warn, show_unknown): + """Display results by architecture + + Args: + board_selected (dict): Dict containing boards to summarise + brd_status (BoardStatus): Named tuple with board classifications + better_err: List of ErrLine for fixed errors + worse_err: List of ErrLine for new errors + better_warn: List of ErrLine for fixed warnings + worse_warn: List of ErrLine for new warnings + show_unknown (bool): Whether to show unknown boards + + Returns: + int: Number of error lines output + """ + error_lines = 0 + if not any((brd_status.ok, brd_status.warn, brd_status.err, + brd_status.unknown, brd_status.new, worse_err, better_err, + worse_warn, better_warn)): + return error_lines + arch_list = {} + self.add_outcome(board_selected, arch_list, brd_status.ok, '', + self._col.GREEN) + self.add_outcome(board_selected, arch_list, brd_status.warn, 'w+', + self._col.YELLOW) + self.add_outcome(board_selected, arch_list, brd_status.err, '+', + self._col.RED) + self.add_outcome(board_selected, arch_list, brd_status.new, '*', + self._col.BLUE) + if show_unknown: + self.add_outcome(board_selected, arch_list, brd_status.unknown, + '?', self._col.MAGENTA) + for arch, target_list in arch_list.items(): + tprint(f'{arch:>10s}: {target_list}') + error_lines += 1 + error_lines += self.output_err_lines(better_err, colour=self._col.GREEN) + error_lines += self.output_err_lines(worse_err, colour=self._col.RED) + error_lines += self.output_err_lines(better_warn, colour=self._col.CYAN) + error_lines += self.output_err_lines(worse_warn, colour=self._col.YELLOW) + return error_lines + + @staticmethod + def print_ide_output(board_selected, board_dict): + """Print output for IDE mode + + Args: + board_selected (dict): Dict of selected boards, keyed by target + board_dict (dict): Dict of boards that were built, keyed by target + """ + for target in board_dict: + if target not in board_selected: + continue + outcome = board_dict[target] + for line in outcome.err_lines: + sys.stderr.write(line) -- 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 _calc_error_delta() function and its helper _board_list() from the nested functions in print_result_summary() to ResultHandler. Also move the ErrLine namedtuple from builder.py to resulthandler.py since it is used by calc_error_delta(). This continues the extraction of display-related code from builder.py into the ResultHandler class. The list_error_boards parameter is now passed explicitly instead of being accessed via self._list_error_boards. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 63 +++--------------------------- tools/buildman/resulthandler.py | 68 ++++++++++++++++++++++++++++++++- 2 files changed, 72 insertions(+), 59 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 53f1c6bde98..a2d4470b70c 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1071,67 +1071,16 @@ class Builder: show_config (bool): Show config changes show_environment (bool): Show environment changes """ - def _board_list(line, line_boards): - """Helper function to get a line of boards containing a line - - Args: - line: Error line to search for - line_boards: boards to search, each a Board - Return: - List of boards with that error line, or [] if the user has not - requested such a list - """ - brds = [] - board_set = set() - if self._opts.list_error_boards: - for brd in line_boards[line]: - if not brd in board_set: - brds.append(brd) - board_set.add(brd) - return brds - - def _calc_error_delta(base_lines, base_line_boards, lines, line_boards, - char): - """Calculate the required output based on changes in errors - - Args: - base_lines: List of errors/warnings for previous commit - base_line_boards: Dict keyed by error line, containing a list - of the Board objects with that error in the previous commit - lines: List of errors/warning for this commit, each a str - line_boards: Dict keyed by error line, containing a list - of the Board objects with that error in this commit - char: Character representing error ('') or warning ('w'). The - broken ('+') or fixed ('-') characters are added in this - function - - Returns: - Tuple - List of ErrLine objects for 'better' lines - List of ErrLine objects for 'worse' lines - """ - better_lines = [] - worse_lines = [] - for line in lines: - if line not in base_lines: - errline = ErrLine( - char + '+', _board_list(line, line_boards), line) - worse_lines.append(errline) - for line in base_lines: - if line not in lines: - errline = ErrLine(char + '-', - _board_list(line, base_line_boards), line) - better_lines.append(errline) - return better_lines, worse_lines - 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, - self._base_err_line_boards, err_lines, err_line_boards, '') - better_warn, worse_warn = _calc_error_delta(self._base_warn_lines, - self._base_warn_line_boards, warn_lines, warn_line_boards, 'w') + 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: diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index 197f4d87003..a2064986729 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -8,8 +8,8 @@ import sys -from buildman.outcome import (BoardStatus, OUTCOME_OK, OUTCOME_WARNING, - OUTCOME_ERROR, OUTCOME_UNKNOWN) +from buildman.outcome import (BoardStatus, ErrLine, OUTCOME_OK, + OUTCOME_WARNING, OUTCOME_ERROR, OUTCOME_UNKNOWN) from u_boot_pylib.terminal import tprint @@ -745,3 +745,67 @@ class ResultHandler: if not_built: tprint(f"Boards not built ({len(not_built)}): " f"{', '.join(not_built)}") + + @staticmethod + def _board_list(line, line_boards, list_error_boards): + """Get a list of boards containing a particular error/warning line + + Args: + line (str): Error line to search for + line_boards (dict): Dict keyed by line, containing list of Board + objects with that line + list_error_boards (bool): True to return the board list, False to + return empty list + + Returns: + list: List of Board objects with that error line, or [] if + list_error_boards is False + """ + brds = [] + board_set = set() + if list_error_boards: + for brd in line_boards[line]: + if brd not in board_set: + brds.append(brd) + board_set.add(brd) + return brds + + @classmethod + def calc_error_delta(cls, base_lines, base_line_boards, lines, line_boards, + char, list_error_boards): + """Calculate the required output based on changes in errors + + Args: + base_lines (list): List of errors/warnings for previous commit + base_line_boards (dict): Dict keyed by error line, containing a + list of the Board objects with that error in the previous + commit + lines (list): List of errors/warning for this commit, each a str + line_boards (dict): Dict keyed by error line, containing a list + of the Board objects with that error in this commit + char (str): Character representing error ('') or warning ('w'). The + broken ('+') or fixed ('-') characters are added in this + function + list_error_boards (bool): True to include board list in output + + Returns: + tuple: (better_lines, worse_lines) where each is a list of + ErrLine objects + """ + better_lines = [] + worse_lines = [] + for line in lines: + if line not in base_lines: + errline = ErrLine( + char + '+', + cls._board_list(line, line_boards, list_error_boards), + line) + worse_lines.append(errline) + for line in base_lines: + if line not in lines: + errline = ErrLine( + char + '-', + cls._board_list(line, base_line_boards, list_error_boards), + line) + better_lines.append(errline) + return better_lines, worse_lines -- 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 build-completion, summary display from Builder to ResultHandler. This method prints the final build statistics including total builds, previously done, newly built, kconfig reconfigs, duration, rate, and any thread exceptions. The method now takes all required values as parameters rather than accessing Builder instance attributes. Update the tests to call the ResultHandler method directly with explicit parameters. Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 35 ++------------------- tools/buildman/resulthandler.py | 40 ++++++++++++++++++++++++ tools/buildman/test_builder.py | 54 ++++++++------------------------- 3 files changed, 55 insertions(+), 74 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 412acfa9e4b..5df6f915285 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -1251,37 +1251,8 @@ class Builder: # Wait until we have processed all output self.out_queue.join() if not self._opts.ide: - self._print_build_summary() + self._result_handler.print_build_summary( + self.count, self.already_done, self.kconfig_reconfig, + self._start_time, self.thread_exceptions) return (self.fail, self.warned, self.thread_exceptions) - - def _print_build_summary(self): - """Print a summary of the build results - - Show the number of boards built, how many were already done, duration - and build rate. Also show any thread exceptions that occurred. - """ - tprint() - - msg = f'Completed: {self.count} total built' - if self.already_done or self.kconfig_reconfig: - parts = [] - if self.already_done: - parts.append(f'{self.already_done} previously') - if self.already_done != self.count: - parts.append(f'{self.count - self.already_done} newly') - if self.kconfig_reconfig: - parts.append(f'{self.kconfig_reconfig} reconfig') - msg += ' (' + ', '.join(parts) + ')' - duration = datetime.now() - self._start_time - if duration > timedelta(microseconds=1000000): - if duration.microseconds >= 500000: - duration = duration + timedelta(seconds=1) - duration -= timedelta(microseconds=duration.microseconds) - rate = float(self.count) / duration.total_seconds() - msg += f', duration {duration}, rate {rate:1.2f}' - tprint(msg) - if self.thread_exceptions: - tprint( - f'Failed: {len(self.thread_exceptions)} thread exceptions', - colour=self.col.RED) diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index d1d6232ff97..2c5488c7154 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -6,6 +6,7 @@ """Result writer for buildman build results""" +from datetime import datetime, timedelta import sys from buildman.outcome import (BoardStatus, ErrLine, Outcome, @@ -185,6 +186,45 @@ class ResultHandler: """ return self._error_lines + def print_build_summary(self, count, already_done, kconfig_reconfig, + start_time, thread_exceptions): + """Print a summary of the build results + + Show the number of boards built, how many were already done, duration + and build rate. Also show any thread exceptions that occurred. + + Args: + count (int): Total number of builds + already_done (int): Number of builds already completed previously + kconfig_reconfig (int): Number of builds triggered by Kconfig changes + start_time (datetime): When the build started + thread_exceptions (list): List of thread exceptions that occurred + """ + tprint() + + msg = f'Completed: {count} total built' + if already_done or kconfig_reconfig: + parts = [] + if already_done: + parts.append(f'{already_done} previously') + if already_done != count: + parts.append(f'{count - already_done} newly') + if kconfig_reconfig: + parts.append(f'{kconfig_reconfig} reconfig') + msg += ' (' + ', '.join(parts) + ')' + duration = datetime.now() - start_time + if duration > timedelta(microseconds=1000000): + if duration.microseconds >= 500000: + duration = duration + timedelta(seconds=1) + duration -= timedelta(microseconds=duration.microseconds) + rate = float(count) / duration.total_seconds() + msg += f', duration {duration}, rate {rate:1.2f}' + tprint(msg) + if thread_exceptions: + tprint( + f'Failed: {len(thread_exceptions)} thread exceptions', + colour=self._col.RED) + def colour_num(self, num): """Format a number with colour depending on its value diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index 6d75cfbc04f..88d859d145c 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -769,7 +769,7 @@ class TestMake(unittest.TestCase): class TestPrintBuildSummary(unittest.TestCase): - """Tests for Builder._print_build_summary()""" + """Tests for ResultHandler.print_build_summary()""" def setUp(self): """Set up test fixtures""" @@ -785,8 +785,7 @@ class TestPrintBuildSummary(unittest.TestCase): 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() - self.builder.thread_exceptions = [] + self.start_time = datetime.now() terminal.set_print_test_mode() def tearDown(self): @@ -795,12 +794,8 @@ class TestPrintBuildSummary(unittest.TestCase): def test_basic_count(self): """Test basic completed message with just count""" - self.builder.count = 10 - self.builder.already_done = 0 - self.builder.kconfig_reconfig = 0 - terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(10, 0, 0, self.start_time, []) lines = terminal.get_print_test_lines() # First line is blank, second is the message @@ -811,12 +806,8 @@ class TestPrintBuildSummary(unittest.TestCase): def test_all_previously_done(self): """Test message when all builds were already done""" - self.builder.count = 5 - self.builder.already_done = 5 - self.builder.kconfig_reconfig = 0 - terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(5, 5, 0, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('5 previously', lines[1].text) @@ -824,12 +815,8 @@ class TestPrintBuildSummary(unittest.TestCase): def test_some_newly_built(self): """Test message with some previously done and some new""" - self.builder.count = 10 - self.builder.already_done = 6 - self.builder.kconfig_reconfig = 0 - terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(10, 6, 0, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('6 previously', lines[1].text) @@ -837,68 +824,51 @@ class TestPrintBuildSummary(unittest.TestCase): def test_with_kconfig_reconfig(self): """Test message with kconfig reconfigurations""" - self.builder.count = 8 - self.builder.already_done = 0 - self.builder.kconfig_reconfig = 3 - terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(8, 0, 3, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('3 reconfig', lines[1].text) def test_thread_exceptions(self): """Test message with thread exceptions""" - self.builder.count = 5 - self.builder.already_done = 0 - self.builder.kconfig_reconfig = 0 - self.builder.thread_exceptions = [Exception('err1'), Exception('err2')] + exceptions = [Exception('err1'), Exception('err2')] terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(5, 0, 0, self.start_time, exceptions) lines = terminal.get_print_test_lines() self.assertEqual(len(lines), 3) self.assertIn('Failed: 2 thread exceptions', lines[2].text) - @mock.patch('buildman.builder.datetime') + @mock.patch('buildman.resulthandler.datetime') def test_duration_and_rate(self, mock_datetime): """Test message includes duration and rate for long builds""" - self.builder.count = 100 - self.builder.already_done = 0 - self.builder.kconfig_reconfig = 0 - # Mock datetime to simulate a 10 second build start_time = datetime(2024, 1, 1, 12, 0, 0) end_time = datetime(2024, 1, 1, 12, 0, 10) - self.builder._start_time = start_time mock_datetime.now.return_value = end_time mock_datetime.side_effect = lambda *args, **kwargs: datetime(*args, **kwargs) terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(100, 0, 0, start_time, []) lines = terminal.get_print_test_lines() self.assertIn('duration', lines[1].text) self.assertIn('rate', lines[1].text) self.assertIn('10.00', lines[1].text) # 100 boards / 10 seconds - @mock.patch('buildman.builder.datetime') + @mock.patch('buildman.resulthandler.datetime') def test_duration_rounds_up(self, mock_datetime): """Test duration rounds up when microseconds >= 500000""" - self.builder.count = 100 - self.builder.already_done = 0 - self.builder.kconfig_reconfig = 0 - # Mock datetime to simulate a 10.6 second build (should round to 11) start_time = datetime(2024, 1, 1, 12, 0, 0) end_time = datetime(2024, 1, 1, 12, 0, 10, 600000) # 10.6 seconds - self.builder._start_time = start_time mock_datetime.now.return_value = end_time mock_datetime.side_effect = lambda *args, **kwargs: datetime(*args, **kwargs) terminal.get_print_test_lines() # Clear - self.builder._print_build_summary() + self.result_handler.print_build_summary(100, 0, 0, start_time, []) lines = terminal.get_print_test_lines() # Duration should be rounded up to 11 seconds -- 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 methods in builder.py and resulthandler.py to follow Python naming conventions. This makes it clearer which methods are internal implementation details vs. public API. builder.py: - signal_handler, filter_errors, setup_build, select_commit resulthandler.py: - Most methods except set_builder, reset_result_summary, produce_result_summary, show_summary, and print_build_summary Co-developed-by: Claude Opus 4.5 <noreply@anthropic.com> Signed-off-by: Simon Glass <simon.glass@canonical.com> --- tools/buildman/builder.py | 14 ++-- tools/buildman/resulthandler.py | 122 ++++++++++++++++---------------- tools/buildman/test_builder.py | 112 +++++++++++------------------ 3 files changed, 111 insertions(+), 137 deletions(-) diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py index 2f072e34716..21816391773 100644 --- a/tools/buildman/builder.py +++ b/tools/buildman/builder.py @@ -372,7 +372,7 @@ class Builder: self.re_make_err = re.compile('|'.join(ignore_lines)) # Handle existing graceful with SIGINT / Ctrl-C - signal.signal(signal.SIGINT, self.signal_handler) + signal.signal(signal.SIGINT, self._signal_handler) def _setup_threads(self, mrproper, per_board_out_dir, test_thread_exceptions): @@ -409,7 +409,7 @@ class Builder: """Get rid of all threads created by the builder""" self.threads.clear() - def signal_handler(self, _signum, _frame): + def _signal_handler(self, _signum, _frame): """Handle a signal by exiting""" sys.exit(1) @@ -482,7 +482,7 @@ class Builder: self._timestamps.popleft() count -= 1 - def select_commit(self, commit, checkout=True): + def _select_commit(self, commit, checkout=True): """Checkout the selected commit for this build Args: @@ -714,7 +714,7 @@ class Builder: output_dir = self.get_build_dir(commit_upto, target) return os.path.join(output_dir, 'err') - def filter_errors(self, lines): + def _filter_errors(self, lines): """Filter out errors in which we have no interest We should probably use map(). @@ -812,7 +812,7 @@ class Builder: err_file = self.get_err_file(commit_upto, target) if os.path.exists(err_file): with open(err_file, 'r', encoding='utf-8') as fd: - err_lines = self.filter_errors(fd.readlines()) + err_lines = self._filter_errors(fd.readlines()) # Decide whether the build was ok, failed or created warnings if return_code: @@ -1008,7 +1008,7 @@ class Builder: return (board_dict, err_lines_summary, err_lines_boards, warn_lines_summary, warn_lines_boards, config, environment) - def setup_build(self, board_selected, _commits): + def _setup_build(self, board_selected, _commits): """Set up ready to start a build. Args: @@ -1182,7 +1182,7 @@ class Builder: if not self._opts.ide: tprint('\rStarting build...', newline=False) self._start_time = datetime.now() - self.setup_build(board_selected, commits) + self._setup_build(board_selected, commits) self.process_result(None) self.thread_exceptions = [] # Create jobs to build all commits for each board diff --git a/tools/buildman/resulthandler.py b/tools/buildman/resulthandler.py index c0cf0ca331e..d6c5e3224b9 100644 --- a/tools/buildman/resulthandler.py +++ b/tools/buildman/resulthandler.py @@ -74,7 +74,7 @@ class ResultHandler: 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 + Following this, calls to _print_result_summary() will use this information to work out what has changed. Args: @@ -93,7 +93,7 @@ class ResultHandler: self._base_environment = None self._error_lines = 0 - def print_result_summary(self, board_selected, board_dict, err_lines, + def _print_result_summary(self, board_selected, board_dict, err_lines, err_line_boards, warn_lines, warn_line_boards, config, environment): """Compare results with the base results and display delta. @@ -123,38 +123,38 @@ class ResultHandler: environment (dict): Dictionary keyed by environment variable, Each value is the value of environment variable. """ - brd_status = self.classify_boards( + 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( + better_err, worse_err = self._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 = self.calc_error_delta( + better_warn, worse_warn = self._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.print_ide_output(board_selected, board_dict) + self._print_ide_output(board_selected, board_dict) # Display results by arch if not self._opts.ide: - self._error_lines += self.display_arch_results( + self._error_lines += self._display_arch_results( board_selected, brd_status, better_err, worse_err, better_warn, worse_warn, self._opts.show_unknown) if self._opts.show_sizes: - self.print_size_summary( + self._print_size_summary( board_selected, board_dict, self._base_board_dict, self._opts.show_detail, self._opts.show_bloat) if self._opts.show_environment and self._base_environment: - self.show_environment_changes( + self._show_environment_changes( board_selected, board_dict, environment, self._base_environment) if self._opts.show_config and self._base_config: - self.show_config_changes( + self._show_config_changes( board_selected, board_dict, config, self._base_config) # Save our updated information for the next call to this function @@ -166,9 +166,9 @@ class ResultHandler: self._base_config = config self._base_environment = environment - self.show_not_built(board_selected, board_dict) + self._show_not_built(board_selected, board_dict) - def get_error_lines(self): + def _get_error_lines(self): """Get the number of error lines output Returns: @@ -191,7 +191,7 @@ class ResultHandler: if commits: msg = f'{commit_upto + 1:02d}: {commits[commit_upto].subject}' tprint(msg, colour=self._col.BLUE) - self.print_result_summary( + 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, @@ -214,7 +214,7 @@ class ResultHandler: for commit_upto in range(0, commit_count, step): self.produce_result_summary( commit_upto, commits, board_selected) - if not self.get_error_lines(): + 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, @@ -256,7 +256,7 @@ class ResultHandler: f'Failed: {len(thread_exceptions)} thread exceptions', colour=self._col.RED) - def colour_num(self, num): + def _colour_num(self, num): """Format a number with colour depending on its value Args: @@ -270,7 +270,7 @@ class ResultHandler: return '0' return self._col.build(color, str(num)) - def print_func_size_detail(self, fname, old, new): + def _print_func_size_detail(self, fname, old, new): """Print detailed size information for each function Args: @@ -311,7 +311,7 @@ class ResultHandler: 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] + 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: ' @@ -325,7 +325,7 @@ class ResultHandler: f'{new.get(name, "-"):>7} {diff:+7d}') tprint(msg, colour=color) - def print_size_detail(self, target_list, base_board_dict, board_dict, + def _print_size_detail(self, target_list, base_board_dict, board_dict, show_bloat): """Show detailed size information for each board @@ -360,12 +360,12 @@ class ResultHandler: outcome = board_dict[target] base_outcome = base_board_dict[target] for fname in outcome.func_sizes: - self.print_func_size_detail(fname, + 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): + def _calc_image_size_changes(target, sizes, base_sizes): """Calculate size changes for each image/part Args: @@ -394,7 +394,7 @@ class ResultHandler: err[name] = diff return err - def calc_size_changes(self, board_selected, board_dict, base_board_dict): + 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 @@ -421,7 +421,7 @@ class ResultHandler: base_sizes = base_board_dict[target].sizes outcome = board_dict[target] sizes = outcome.sizes - err = self.calc_image_size_changes(target, sizes, base_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 @@ -435,7 +435,7 @@ class ResultHandler: arch_list[arch].append(err) return arch_list, arch_count - def print_size_summary(self, board_selected, board_dict, base_board_dict, + 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. @@ -457,7 +457,7 @@ class ResultHandler: 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, + arch_list, arch_count = self._calc_size_changes(board_selected, board_dict, base_board_dict) @@ -516,10 +516,10 @@ class ResultHandler: if printed_arch: tprint() if show_detail: - self.print_size_detail(target_list, base_board_dict, board_dict, + self._print_size_detail(target_list, base_board_dict, board_dict, show_bloat) - def add_outcome(self, board_dict, arch_list, changes, char, color): + def _add_outcome(self, board_dict, arch_list, changes, char, color): """Add an output to our list of outcomes for each architecture This simple function adds failing boards (changes) to the @@ -549,7 +549,7 @@ class ResultHandler: else: arch_list[arch] += text - def output_err_lines(self, err_lines, colour): + def _output_err_lines(self, err_lines, colour): """Output the line of error/warning lines, if not empty Args: @@ -578,7 +578,7 @@ class ResultHandler: return 1 return 0 - def display_arch_results(self, board_selected, brd_status, better_err, + def _display_arch_results(self, board_selected, brd_status, better_err, worse_err, better_warn, worse_warn, show_unknown): """Display results by architecture @@ -600,28 +600,28 @@ class ResultHandler: worse_warn, better_warn)): return error_lines arch_list = {} - self.add_outcome(board_selected, arch_list, brd_status.ok, '', + self._add_outcome(board_selected, arch_list, brd_status.ok, '', self._col.GREEN) - self.add_outcome(board_selected, arch_list, brd_status.warn, 'w+', + self._add_outcome(board_selected, arch_list, brd_status.warn, 'w+', self._col.YELLOW) - self.add_outcome(board_selected, arch_list, brd_status.err, '+', + self._add_outcome(board_selected, arch_list, brd_status.err, '+', self._col.RED) - self.add_outcome(board_selected, arch_list, brd_status.new, '*', + self._add_outcome(board_selected, arch_list, brd_status.new, '*', self._col.BLUE) if show_unknown: - self.add_outcome(board_selected, arch_list, brd_status.unknown, + self._add_outcome(board_selected, arch_list, brd_status.unknown, '?', self._col.MAGENTA) for arch, target_list in arch_list.items(): tprint(f'{arch:>10s}: {target_list}') error_lines += 1 - error_lines += self.output_err_lines(better_err, colour=self._col.GREEN) - error_lines += self.output_err_lines(worse_err, colour=self._col.RED) - error_lines += self.output_err_lines(better_warn, colour=self._col.CYAN) - error_lines += self.output_err_lines(worse_warn, colour=self._col.YELLOW) + error_lines += self._output_err_lines(better_err, colour=self._col.GREEN) + error_lines += self._output_err_lines(worse_err, colour=self._col.RED) + error_lines += self._output_err_lines(better_warn, colour=self._col.CYAN) + error_lines += self._output_err_lines(worse_warn, colour=self._col.YELLOW) return error_lines @staticmethod - def print_ide_output(board_selected, board_dict): + def _print_ide_output(board_selected, board_dict): """Print output for IDE mode Args: @@ -636,7 +636,7 @@ class ResultHandler: sys.stderr.write(line) @staticmethod - def calc_config(delta, name, config): + def _calc_config(delta, name, config): """Calculate configuration changes Args: @@ -655,7 +655,7 @@ class ResultHandler: return f'{delta} {name}: {out}' @classmethod - def add_config(cls, lines, name, config_plus, config_minus, config_change): + def _add_config(cls, lines, name, config_plus, config_minus, config_change): """Add changes in configuration to a list Args: @@ -672,13 +672,13 @@ class ResultHandler: value: config value """ if config_plus: - lines.append(cls.calc_config('+', name, config_plus)) + lines.append(cls._calc_config('+', name, config_plus)) if config_minus: - lines.append(cls.calc_config('-', name, config_minus)) + lines.append(cls._calc_config('-', name, config_minus)) if config_change: - lines.append(cls.calc_config('c', name, config_change)) + lines.append(cls._calc_config('c', name, config_change)) - def output_config_info(self, lines): + def _output_config_info(self, lines): """Output configuration change information Args: @@ -696,7 +696,7 @@ class ResultHandler: col = self._col.YELLOW tprint(' ' + line, newline=True, colour=col) - def show_environment_changes(self, board_selected, board_dict, + def _show_environment_changes(self, board_selected, board_dict, environment, base_environment): """Show changes in environment variables @@ -733,11 +733,11 @@ class ResultHandler: desc = f'{value} -> {new_value}' environment_change[key] = desc - self.add_config(lines, target, environment_plus, + self._add_config(lines, target, environment_plus, environment_minus, environment_change) - self.output_config_info(lines) + self._output_config_info(lines) - def calc_config_changes(self, target, config, base_config, + 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 @@ -788,13 +788,13 @@ class ResultHandler: 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, + self._add_config(lines, name, config_plus, config_minus, config_change) - self.add_config(lines, 'all', all_config_plus, + 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, + def _print_arch_config_summary(self, arch, arch_config_plus, arch_config_minus, arch_config_change): """Print configuration summary for a single architecture @@ -812,16 +812,16 @@ class ResultHandler: 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, + 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) + self._add_config(lines, 'all', all_plus, all_minus, all_change) if lines: tprint(f'{arch}:') - self.output_config_info(lines) + self._output_config_info(lines) - def show_config_changes(self, board_selected, board_dict, config, + def _show_config_changes(self, board_selected, board_dict, config, base_config): """Show changes in configuration @@ -859,7 +859,7 @@ class ResultHandler: if target not in board_selected: continue arch = board_selected[target].arch - summary[target] = self.calc_config_changes( + summary[target] = self.__calc_config_changes( target, config, base_config, arch, arch_config_plus, arch_config_minus, arch_config_change) @@ -871,7 +871,7 @@ class ResultHandler: lines_by_target[lines] = [target] for arch in arch_list: - self.print_arch_config_summary(arch, arch_config_plus, + self._print_arch_config_summary(arch, arch_config_plus, arch_config_minus, arch_config_change) @@ -879,10 +879,10 @@ class ResultHandler: if not lines: continue tprint(f"{' '.join(sorted(targets))} :") - self.output_config_info(lines.split('\n')) + self._output_config_info(lines.split('\n')) @staticmethod - def classify_boards(board_selected, board_dict, base_board_dict): + def _classify_boards(board_selected, board_dict, base_board_dict): """Classify boards into outcome categories Args: @@ -926,7 +926,7 @@ class ResultHandler: return BoardStatus(ok, warn, err, new, unknown) @staticmethod - def show_not_built(board_selected, board_dict): + 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. @@ -980,7 +980,7 @@ class ResultHandler: return brds @classmethod - def calc_error_delta(cls, base_lines, base_line_boards, lines, line_boards, + def _calc_error_delta(cls, base_lines, base_line_boards, lines, line_boards, char, list_error_boards): """Calculate the required output based on changes in errors diff --git a/tools/buildman/test_builder.py b/tools/buildman/test_builder.py index 88d859d145c..ec1e0301a6e 100644 --- a/tools/buildman/test_builder.py +++ b/tools/buildman/test_builder.py @@ -12,28 +12,31 @@ from unittest import mock from buildman import builder from buildman import builderthread -from buildman.outcome import (OUTCOME_OK, OUTCOME_WARNING, OUTCOME_ERROR, - OUTCOME_UNKNOWN, DisplayOptions) +from buildman.outcome import (DisplayOptions, OUTCOME_OK, OUTCOME_WARNING, + OUTCOME_ERROR, OUTCOME_UNKNOWN) from buildman.resulthandler import ResultHandler from u_boot_pylib import gitutil from u_boot_pylib import terminal +# Default display options for tests +DEFAULT_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) + class TestPrintFuncSizeDetail(unittest.TestCase): - """Tests for ResultHandler.print_func_size_detail()""" + """Tests for ResultHandler._print_func_size_detail()""" def setUp(self): """Set up test fixtures""" - # Create a minimal Builder for testing + # Create a minimal Builder for testing (provides _result_handler) 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.result_handler = ResultHandler(self.col, DEFAULT_OPTS) self.builder = builder.Builder( toolchains=None, base_dir='/tmp', git_dir=None, num_threads=0, num_jobs=1, col=self.col, result_handler=self.result_handler) + self.writer = self.builder._result_handler terminal.set_print_test_mode() def tearDown(self): @@ -46,7 +49,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100, 'func_b': 200} terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._print_func_size_detail('u-boot', old, new) lines = terminal.get_print_test_lines() # No output when there are no changes @@ -58,7 +61,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 150} terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._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 +79,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 150} terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._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 +92,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100, 'func_b': 200} terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._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 +108,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): new = {'func_a': 100} terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._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 +132,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', old, new) + self.writer._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 +151,7 @@ class TestPrintFuncSizeDetail(unittest.TestCase): def test_empty_dicts(self): """Test with empty dictionaries""" terminal.get_print_test_lines() # Clear - self.result_handler.print_func_size_detail('u-boot', {}, {}) + self.writer._print_func_size_detail('u-boot', {}, {}) lines = terminal.get_print_test_lines() # No output when both dicts are empty @@ -161,15 +164,10 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) terminal.set_print_test_mode() def tearDown(self): @@ -282,15 +280,10 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) terminal.set_print_test_mode() def tearDown(self): @@ -364,7 +357,7 @@ class TestPrepareWorkingSpace(unittest.TestCase): class TestShowNotBuilt(unittest.TestCase): - """Tests for ResultHandler.show_not_built()""" + """Tests for ResultHandler._show_not_built()""" def setUp(self): """Set up test fixtures""" @@ -381,9 +374,9 @@ class TestShowNotBuilt(unittest.TestCase): outcome.err_lines = err_lines if err_lines else [] return outcome - def _show_not_built(self, board_selected, board_dict): - """Helper to call ResultHandler.show_not_built""" - ResultHandler.show_not_built(board_selected, board_dict) + def __show_not_built(self, 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 +387,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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 +403,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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 +421,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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 +439,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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 +457,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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 +476,7 @@ class TestShowNotBuilt(unittest.TestCase): } terminal.get_print_test_lines() # Clear - self._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) @@ -499,15 +492,10 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) terminal.set_print_test_mode() def tearDown(self): @@ -589,15 +577,10 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) # Reset state before each test self.builder._restarting_config = False self.builder._terminated = False @@ -675,15 +658,10 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) @mock.patch('buildman.builder.command.run_one') def test_make_basic(self, mock_run_one): @@ -774,15 +752,11 @@ 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, - result_handler=self.result_handler) + result_handler=ResultHandler(self.col, DEFAULT_OPTS)) + self.handler = self.builder._result_handler # Set a start time in the past (less than 1 second ago to avoid # duration output) self.start_time = datetime.now() @@ -795,7 +769,7 @@ class TestPrintBuildSummary(unittest.TestCase): def test_basic_count(self): """Test basic completed message with just count""" terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(10, 0, 0, self.start_time, []) + self.handler.print_build_summary(10, 0, 0, self.start_time, []) lines = terminal.get_print_test_lines() # First line is blank, second is the message @@ -807,7 +781,7 @@ class TestPrintBuildSummary(unittest.TestCase): def test_all_previously_done(self): """Test message when all builds were already done""" terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(5, 5, 0, self.start_time, []) + self.handler.print_build_summary(5, 5, 0, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('5 previously', lines[1].text) @@ -816,7 +790,7 @@ class TestPrintBuildSummary(unittest.TestCase): def test_some_newly_built(self): """Test message with some previously done and some new""" terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(10, 6, 0, self.start_time, []) + self.handler.print_build_summary(10, 6, 0, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('6 previously', lines[1].text) @@ -825,7 +799,7 @@ class TestPrintBuildSummary(unittest.TestCase): def test_with_kconfig_reconfig(self): """Test message with kconfig reconfigurations""" terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(8, 0, 3, self.start_time, []) + self.handler.print_build_summary(8, 0, 3, self.start_time, []) lines = terminal.get_print_test_lines() self.assertIn('3 reconfig', lines[1].text) @@ -835,7 +809,7 @@ class TestPrintBuildSummary(unittest.TestCase): exceptions = [Exception('err1'), Exception('err2')] terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(5, 0, 0, self.start_time, exceptions) + self.handler.print_build_summary(5, 0, 0, self.start_time, exceptions) lines = terminal.get_print_test_lines() self.assertEqual(len(lines), 3) @@ -851,7 +825,7 @@ class TestPrintBuildSummary(unittest.TestCase): mock_datetime.side_effect = lambda *args, **kwargs: datetime(*args, **kwargs) terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(100, 0, 0, start_time, []) + self.handler.print_build_summary(100, 0, 0, start_time, []) lines = terminal.get_print_test_lines() self.assertIn('duration', lines[1].text) @@ -868,7 +842,7 @@ class TestPrintBuildSummary(unittest.TestCase): mock_datetime.side_effect = lambda *args, **kwargs: datetime(*args, **kwargs) terminal.get_print_test_lines() # Clear - self.result_handler.print_build_summary(100, 0, 0, start_time, []) + self.handler.print_build_summary(100, 0, 0, start_time, []) lines = terminal.get_print_test_lines() # Duration should be rounded up to 11 seconds -- 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
participants (1)
-
Simon Glass