[PATCH v2 00/60] buildman: Refactor code and correct some pylint warnings

[PATCH v2 00/60] buildman: Refactor code and correct some pylint warnings

[Simon Glass]

The buildman code has grown considerable since it was originally written.
In places it can be quite hard to understanding and work with.

This series improves things, particularly in the control and builder
parts, splitting functions into smaller sizes.

More work remains in buildthead._write_result() and other places.

Changes in v2:
- Correct operation of -A
- Use snake case for tests
- Add new patch to print the architecture for a board
- Drop patch to move -A logic up a little (since it breaks it)

Simon Glass (60):
  buildman: Tidy up pylint warnings in main
  buildman: Convert camel case in control.py
  buildman: Fix most pylint warnings in control
  buildman: Move full-help processing to main
  buildman: Move series calculations into a separate function
  buildman: Move fetch-arch code into a separate function
  buildman: Add a test for the -A option
  buildman: Drop use of builder in show_actions()
  buildman: Move dry-run handling higher in do_buildman()
  buildman: Move board-selection code into a function
  bulidman: Move more code to determine_series()
  buildman: Move Boards-object code into a function
  bulidman: Move toolchain handling to a function
  bulldman: Set up output_dir earlier
  buildman: Move output-file setup into one place
  buildman: Pass option values to get_action_summary()
  buildman: Pass option values to show_actions()
  buildman: Build option-adjusting into a function
  buildman: Move counting of commits into a function
  buildman: Move setting up the output dir into a function
  buildman: Move commit numbering into determine_series()
  buildman: Avoid too many returns in do_buildman()
  buildman: Move remaining builder properties to constructor
  buildman: Tweak commits and show_bloat
  buildman: Moving running of the builder into a function
  buildman: Drop some unnecessary variables
  buildman: Adjust show_toolchain_prefix() to not return
  buildman: Move checking for make into run_builder()
  buildman: Move getting the adjust_cfg into run_builder()
  buildman: Use get_alow_missing() directly to avoid var
  buildman: Create a function to get number of built commits
  buildman: Convert camel case in cmdline.py
  buildman: Correct most pylint warnings in cmdline
  buildman: Add a test for --boards
  buildman: Convert to argparse
  buildman: Convert camel case in bsettings.py
  buildman: Convert camel case in builder.py
  buildman: Split parser creation in two
  buildman: Convert camel case in builderthread.py
  buildman: Correct most pylint warnings in builderthread
  buildman: Export _get_output_dir() to avoid warnings
  buildman: Correct invalid use of out_dir variable
  buildman: Drop unnecessary assignment of config_out
  buildman: Start a function to set up the make arguments
  buildman: Move setting of toolchain arguments to _build_args()
  buildman: Move more things into _build_args()
  buildman: Convert config_out to string IO
  buildman: Move reconfigure code into its own function
  buildman: Move bulid code into its own function
  buildman: Move reading of the done file into a function
  buildman: Move code to remove old outputs
  buildman: Move code to decide output dirs
  buildman: Move checkout code to a separate function
  buildman: Create a function to handle config and build
  buildman: Avoid passing result into _read_done_file()
  buildman: Tidy up reporting of a toolchain error
  buildman: Tidy up some comments in builderthread
  buildman: Move copy_files() out ot BuilderThread class
  buildman: Add a way to print the architecture for a board
  buildman: Enable test coverage

 tools/buildman/bsettings.py     |  14 +-
 tools/buildman/builder.py       | 262 ++++++-----
 tools/buildman/builderthread.py | 652 ++++++++++++++++----------
 tools/buildman/buildman.rst     |   2 +-
 tools/buildman/cmdline.py       | 172 ++++---
 tools/buildman/control.py       | 780 +++++++++++++++++++++-----------
 tools/buildman/func_test.py     |  58 ++-
 tools/buildman/main.py          |  59 ++-
 tools/buildman/test.py          |  28 +-
 tools/buildman/toolchain.py     |  14 +-
 tools/moveconfig.py             |   2 +-
 tools/u_boot_pylib/test_util.py |  10 +-
 12 files changed, 1277 insertions(+), 776 deletions(-)

-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 01/60] buildman: Tidy up pylint warnings in main

[Simon Glass]

Tidy up the various pylint warnings in module 'main'.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/main.py | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index a2ffbc9073e7..ca1beb098292 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -6,29 +6,21 @@
 
 """See README for more information"""
 
-import doctest
-import multiprocessing
 import os
-import re
 import sys
 
 # Bring in the patman libraries
+# pylint: disable=C0413
 our_path = os.path.dirname(os.path.realpath(__file__))
 sys.path.insert(1, os.path.join(our_path, '..'))
 
 # Our modules
-from buildman import board
 from buildman import bsettings
-from buildman import builder
 from buildman import cmdline
 from buildman import control
-from buildman import toolchain
-from patman import patchstream
-from patman import gitutil
-from u_boot_pylib import terminal
 from u_boot_pylib import test_util
 
-def RunTests(skip_net_tests, verbose, args):
+def run_tests(skip_net_tests, verbose, args):
     """Run the buildman tests
 
     Args:
@@ -36,9 +28,11 @@ def RunTests(skip_net_tests, verbose, args):
         verbosity (int): Verbosity level to use (0-4)
         args (list of str): List of tests to run, empty to run all
     """
+    # These imports are here since tests are not available when buildman is
+    # installed as a Python module
+    # pylint: disable=C0415
     from buildman import func_test
     from buildman import test
-    import doctest
 
     test_name = args and args[0] or None
     if skip_net_tests:
@@ -54,6 +48,11 @@ def RunTests(skip_net_tests, verbose, args):
     return (0 if result.wasSuccessful() else 1)
 
 def run_buildman():
+    """Run bulidman
+
+    This is the main program. It collects arguments and runs either the tests or
+    the control module.
+    """
     options, args = cmdline.ParseArgs()
 
     if not options.debug:
@@ -61,7 +60,7 @@ def run_buildman():
 
     # Run our meagre tests
     if cmdline.HAS_TESTS and options.test:
-        RunTests(options.skip_net_tests, options.verbose, args)
+        run_tests(options.skip_net_tests, options.verbose, args)
 
     # Build selected commits for selected boards
     else:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 02/60] buildman: Convert camel case in control.py

[Simon Glass]

Convert this file to snake case and update all files which use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py   | 31 ++++++++++++++++++-------------
 tools/buildman/func_test.py |  7 ++++---
 tools/buildman/main.py      |  2 +-
 tools/buildman/test.py      |  2 +-
 4 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b8286e185411..bfb02834e8fb 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -2,6 +2,11 @@
 # Copyright (c) 2013 The Chromium OS Authors.
 #
 
+"""Control module for buildman
+
+This holds the main control logic for buildman, when not running tests.
+"""
+
 import multiprocessing
 try:
     import importlib.resources
@@ -25,11 +30,11 @@ from u_boot_pylib import terminal
 from u_boot_pylib import tools
 from u_boot_pylib.terminal import tprint
 
-def GetPlural(count):
+def get_plural(count):
     """Returns a plural 's' if count is not 1"""
     return 's' if count != 1 else ''
 
-def GetActionSummary(is_summary, commits, selected, options):
+def get_action_summary(is_summary, commits, selected, options):
     """Return a string summarising the intended action.
 
     Returns:
@@ -38,18 +43,18 @@ def GetActionSummary(is_summary, commits, selected, options):
     if commits:
         count = len(commits)
         count = (count + options.step - 1) // options.step
-        commit_str = '%d commit%s' % (count, GetPlural(count))
+        commit_str = '%d commit%s' % (count, get_plural(count))
     else:
         commit_str = 'current source'
     str = '%s %s for %d boards' % (
         'Summary of' if is_summary else 'Building', commit_str,
         len(selected))
     str += ' (%d thread%s, %d job%s per thread)' % (options.threads,
-            GetPlural(options.threads), options.jobs, GetPlural(options.jobs))
+            get_plural(options.threads), options.jobs, get_plural(options.jobs))
     return str
 
-def ShowActions(series, why_selected, boards_selected, builder, options,
-                board_warnings):
+def show_actions(series, why_selected, boards_selected, builder, options,
+                 board_warnings):
     """Display a list of actions that we would take, if not a dry run.
 
     Args:
@@ -72,7 +77,7 @@ def ShowActions(series, why_selected, boards_selected, builder, options,
         commits = series.commits
     else:
         commits = None
-    print(GetActionSummary(False, commits, boards_selected,
+    print(get_action_summary(False, commits, boards_selected,
             options))
     print('Build directory: %s' % builder.base_dir)
     if commits:
@@ -92,7 +97,7 @@ def ShowActions(series, why_selected, boards_selected, builder, options,
         for warning in board_warnings:
             print(col.build(col.YELLOW, warning))
 
-def ShowToolchainPrefix(brds, toolchains):
+def show_toolchain_prefix(brds, toolchains):
     """Show information about a the tool chain used by one or more boards
 
     The function checks that all boards use the same toolchain, then prints
@@ -133,8 +138,8 @@ def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
         allow_missing = False
     return allow_missing
 
-def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
-               clean_dir=False, test_thread_exceptions=False):
+def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
+                clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
 
     Args:
@@ -237,7 +242,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
         sys.exit(col.build(col.RED, 'No matching boards found'))
 
     if options.print_prefix:
-        err = ShowToolchainPrefix(brds, toolchains)
+        err = show_toolchain_prefix(brds, toolchains)
         if err:
             sys.exit(col.build(col.RED, err))
         return 0
@@ -373,7 +378,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # For a dry run, just show our actions as a sanity check
     if options.dry_run:
-        ShowActions(series, why_selected, selected, builder, options,
+        show_actions(series, why_selected, selected, builder, options,
                     board_warnings)
     else:
         builder.force_build = options.force_build
@@ -393,7 +398,7 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
             commits = None
 
         if not options.ide:
-            tprint(GetActionSummary(options.summary, commits, board_selected,
+            tprint(get_action_summary(options.summary, commits, board_selected,
                                     options))
 
         # We can't show function sizes without board details at present
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 08b2714b7be2..052153043b21 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -247,9 +247,10 @@ class TestFunctional(unittest.TestCase):
         options, args = cmdline.ParseArgs()
         if brds == False:
             brds = self._boards
-        result = control.DoBuildman(options, args, toolchains=self._toolchains,
-                make_func=self._HandleMake, brds=brds, clean_dir=clean_dir,
-                test_thread_exceptions=test_thread_exceptions)
+        result = control.do_buildman(
+            options, args, toolchains=self._toolchains,
+            make_func=self._HandleMake, brds=brds, clean_dir=clean_dir,
+            test_thread_exceptions=test_thread_exceptions)
         if get_builder:
             self._builder = control.builder
         return result
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index ca1beb098292..9c84e16e7df0 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -65,7 +65,7 @@ def run_buildman():
     # Build selected commits for selected boards
     else:
         bsettings.Setup(options.config_file)
-        ret_code = control.DoBuildman(options, args)
+        ret_code = control.do_buildman(options, args)
         sys.exit(ret_code)
 
 
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 9fa6445b798f..7eb25aa80eba 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -465,7 +465,7 @@ class TestBuild(unittest.TestCase):
         options.show_errors = False
         options.keep_outputs = False
         args = ['tegra20']
-        control.DoBuildman(options, args)
+        control.do_buildman(options, args)
 
     def testBoardSingle(self):
         """Test single board selection"""
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 03/60] buildman: Fix most pylint warnings in control

[Simon Glass]

Tidy up the easier-to-fix pylint warnings in module 'control'.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py   | 119 +++++++++++++++++++++---------------
 tools/buildman/func_test.py |   2 +-
 2 files changed, 70 insertions(+), 51 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index bfb02834e8fb..c890f778f501 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -15,7 +15,6 @@ except ImportError:
     import importlib_resources
 import os
 import shutil
-import subprocess
 import sys
 
 from buildman import boards
@@ -30,6 +29,8 @@ from u_boot_pylib import terminal
 from u_boot_pylib import tools
 from u_boot_pylib.terminal import tprint
 
+TEST_BUILDER = None
+
 def get_plural(count):
     """Returns a plural 's' if count is not 1"""
     return 's' if count != 1 else ''
@@ -43,17 +44,17 @@ def get_action_summary(is_summary, commits, selected, options):
     if commits:
         count = len(commits)
         count = (count + options.step - 1) // options.step
-        commit_str = '%d commit%s' % (count, get_plural(count))
+        commit_str = f'{count} commit{get_plural(count)}'
     else:
         commit_str = 'current source'
-    str = '%s %s for %d boards' % (
-        'Summary of' if is_summary else 'Building', commit_str,
-        len(selected))
-    str += ' (%d thread%s, %d job%s per thread)' % (options.threads,
-            get_plural(options.threads), options.jobs, get_plural(options.jobs))
-    return str
-
-def show_actions(series, why_selected, boards_selected, builder, options,
+    msg = (f"{'Summary of' if is_summary else 'Building'} "
+           f'{commit_str} for {len(selected)} boards')
+    msg += (f' ({options.threads} thread{get_plural(options.threads)}, '
+            f'{options.jobs} job{get_plural(options.jobs)} per thread)')
+    return msg
+
+# pylint: disable=R0913
+def show_actions(series, why_selected, boards_selected, bldr, options,
                  board_warnings):
     """Display a list of actions that we would take, if not a dry run.
 
@@ -66,7 +67,7 @@ def show_actions(series, why_selected, boards_selected, builder, options,
                 the value would be a list of board names.
         boards_selected: Dict of selected boards, key is target name,
                 value is Board object
-        builder: The builder that will be used to build the commits
+        bldr: The builder that will be used to build the commits
         options: Command line options object
         board_warnings: List of warnings obtained from board selected
     """
@@ -79,7 +80,7 @@ def show_actions(series, why_selected, boards_selected, builder, options,
         commits = None
     print(get_action_summary(False, commits, boards_selected,
             options))
-    print('Build directory: %s' % builder.base_dir)
+    print(f'Build directory: {bldr.base_dir}')
     if commits:
         for upto in range(0, len(series.commits), options.step):
             commit = series.commits[upto]
@@ -88,11 +89,11 @@ def show_actions(series, why_selected, boards_selected, builder, options,
     print()
     for arg in why_selected:
         if arg != 'all':
-            print(arg, ': %d boards' % len(why_selected[arg]))
+            print(arg, f': {len(why_selected[arg])} boards')
             if options.verbose:
-                print('   %s' % ' '.join(why_selected[arg]))
-    print(('Total boards to build for each commit: %d\n' %
-            len(why_selected['all'])))
+                print(f"   {' '.join(why_selected[arg])}")
+    print('Total boards to build for each '
+          f"commit: {len(why_selected['all'])}\n")
     if board_warnings:
         for warning in board_warnings:
             print(col.build(col.YELLOW, warning))
@@ -116,12 +117,26 @@ def show_toolchain_prefix(brds, toolchains):
         tc_set.add(toolchains.Select(brd.arch))
     if len(tc_set) != 1:
         return 'Supplied boards must share one toolchain'
-        return False
-    tc = tc_set.pop()
-    print(tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
+    tchain = tc_set.pop()
+    print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
     return None
 
 def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
+    """Figure out whether to allow external blobs
+
+    Uses the allow-missing setting and the provided arguments to decide whether
+    missing external blobs should be allowed
+
+    Args:
+        opt_allow (bool): True if --allow-missing flag is set
+        opt_no_allow (bool): True if --no-allow-missing flag is set
+        num_selected (int): Number of selected board
+        has_branch (bool): True if a git branch (to build) has been provided
+
+    Returns:
+        bool: True to allow missing external blobs, False to produce an error if
+            external blobs are used
+    """
     allow_missing = False
     am_setting = bsettings.GetGlobalItemValue('allow-missing')
     if am_setting:
@@ -159,7 +174,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             raise an exception instead of reporting their result. This simulates
             a failure in the code somewhere
     """
-    global builder
+    # Used so testing can obtain the builder: pylint: disable=W0603
+    global TEST_BUILDER
 
     if options.full_help:
         with importlib.resources.path('buildman', 'README.rst') as readme:
@@ -178,22 +194,23 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     if options.fetch_arch:
         if options.fetch_arch == 'list':
             sorted_list = toolchains.ListArchs()
-            print(col.build(col.BLUE, 'Available architectures: %s\n' %
-                            ' '.join(sorted_list)))
-            return 0
-        else:
-            fetch_arch = options.fetch_arch
-            if fetch_arch == 'all':
-                fetch_arch = ','.join(toolchains.ListArchs())
-                print(col.build(col.CYAN, '\nDownloading toolchains: %s' %
-                                fetch_arch))
-            for arch in fetch_arch.split(','):
-                print()
-                ret = toolchains.FetchAndInstall(arch)
-                if ret:
-                    return ret
+            print(col.build(
+                col.BLUE,
+                f"Available architectures: {' '.join(sorted_list)}\n"))
             return 0
 
+        fetch_arch = options.fetch_arch
+        if fetch_arch == 'all':
+            fetch_arch = ','.join(toolchains.ListArchs())
+            print(col.build(col.CYAN,
+                            f'\nDownloading toolchains: {fetch_arch}'))
+        for arch in fetch_arch.split(','):
+            print()
+            ret = toolchains.FetchAndInstall(arch)
+            if ret:
+                return ret
+        return 0
+
     if no_toolchains:
         toolchains.GetSettings()
         toolchains.Scan(options.list_tool_chains and options.verbose)
@@ -216,12 +233,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             board_file = options.regen_board_list
 
         brds = boards.Boards()
-        ok = brds.ensure_board_list(board_file,
-                                    options.threads or multiprocessing.cpu_count(),
-                                    force=options.regen_board_list,
-                                    quiet=not options.verbose)
+        okay = brds.ensure_board_list(
+            board_file,
+            options.threads or multiprocessing.cpu_count(),
+            force=options.regen_board_list,
+            quiet=not options.verbose)
         if options.regen_board_list:
-            return 0 if ok else 2
+            return 0 if okay else 2
         brds.read_boards(board_file)
 
     exclude = []
@@ -231,14 +249,14 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     if options.boards:
         requested_boards = []
-        for b in options.boards:
-            requested_boards += b.split(',')
+        for brd in options.boards:
+            requested_boards += brd.split(',')
     else:
         requested_boards = None
     why_selected, board_warnings = brds.select_boards(args, exclude,
                                                       requested_boards)
     selected = brds.get_selected()
-    if not len(selected):
+    if not selected:
         sys.exit(col.build(col.RED, 'No matching boards found'))
 
     if options.print_prefix:
@@ -265,15 +283,15 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             if count is None:
                 sys.exit(col.build(col.RED, msg))
             elif count == 0:
-                sys.exit(col.build(col.RED, "Range '%s' has no commits" %
-                                   options.branch))
+                sys.exit(col.build(col.RED,
+                                   f"Range '{options.branch}' has no commits"))
             if msg:
                 print(col.build(col.YELLOW, msg))
             count += 1   # Build upstream commit also
 
     if not count:
-        msg = ("No commits found to process in branch '%s': "
-               "set branch's upstream or use -c flag" % options.branch)
+        msg = (f"No commits found to process in branch '{options.branch}': "
+               "set branch's upstream or use -c flag")
         sys.exit(col.build(col.RED, msg))
     if options.work_in_output:
         if len(selected) != 1:
@@ -372,6 +390,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             adjust_cfg=adjust_cfg,
             allow_missing=allow_missing, no_lto=options.no_lto,
             reproducible_builds=options.reproducible_builds)
+    TEST_BUILDER = builder
     builder.force_config_on_failure = not options.quick
     if make_func:
         builder.do_make = make_func
@@ -392,8 +411,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         if series:
             commits = series.commits
             # Number the commits for test purposes
-            for commit in range(len(commits)):
-                commits[commit].sequence = commit
+            for i, commit in enumerate(commits):
+                commit.sequence = i
         else:
             commits = None
 
@@ -416,8 +435,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 commits, board_selected, options.keep_outputs, options.verbose)
             if excs:
                 return 102
-            elif fail:
+            if fail:
                 return 100
-            elif warned and not options.ignore_warnings:
+            if warned and not options.ignore_warnings:
                 return 101
     return 0
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 052153043b21..d9b4c41ec68c 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -252,7 +252,7 @@ class TestFunctional(unittest.TestCase):
             make_func=self._HandleMake, brds=brds, clean_dir=clean_dir,
             test_thread_exceptions=test_thread_exceptions)
         if get_builder:
-            self._builder = control.builder
+            self._builder = control.TEST_BUILDER
         return result
 
     def testFullHelp(self):
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 04/60] buildman: Move full-help processing to main

[Simon Glass]

This does not need any of the control features. Move it out of main to
reduce the size of the do_buildman() function.

For Python 3.6 the -H feature will not work, but this does not seem to be
a huge problem, as it dates from 2016.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 11 -----------
 tools/buildman/main.py    |  9 +++++++++
 2 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c890f778f501..b27a6b08477f 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -8,11 +8,6 @@ This holds the main control logic for buildman, when not running tests.
 """
 
 import multiprocessing
-try:
-    import importlib.resources
-except ImportError:
-    # for Python 3.6
-    import importlib_resources
 import os
 import shutil
 import sys
@@ -26,7 +21,6 @@ from patman import gitutil
 from patman import patchstream
 from u_boot_pylib import command
 from u_boot_pylib import terminal
-from u_boot_pylib import tools
 from u_boot_pylib.terminal import tprint
 
 TEST_BUILDER = None
@@ -177,11 +171,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     # Used so testing can obtain the builder: pylint: disable=W0603
     global TEST_BUILDER
 
-    if options.full_help:
-        with importlib.resources.path('buildman', 'README.rst') as readme:
-            tools.print_full_help(str(readme))
-        return 0
-
     gitutil.setup()
     col = terminal.Color()
 
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 9c84e16e7df0..2253401709a8 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -6,6 +6,11 @@
 
 """See README for more information"""
 
+try:
+    from importlib.resources import files
+except ImportError:
+    # for Python 3.6
+    import importlib_resources
 import os
 import sys
 
@@ -19,6 +24,7 @@ from buildman import bsettings
 from buildman import cmdline
 from buildman import control
 from u_boot_pylib import test_util
+from u_boot_pylib import tools
 
 def run_tests(skip_net_tests, verbose, args):
     """Run the buildman tests
@@ -62,6 +68,9 @@ def run_buildman():
     if cmdline.HAS_TESTS and options.test:
         run_tests(options.skip_net_tests, options.verbose, args)
 
+    elif options.full_help:
+        tools.print_full_help(str(files('buildman').joinpath('README.rst')))
+
     # Build selected commits for selected boards
     else:
         bsettings.Setup(options.config_file)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 05/60] buildman: Move series calculations into a separate function

[Simon Glass]

Reduce the size of the do_buildman() function a little by moving the code
that figures out the series into a separate function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 95 +++++++++++++++++++++++----------------
 1 file changed, 56 insertions(+), 39 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b27a6b08477f..7a81c2ced8b3 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -147,6 +147,51 @@ def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
         allow_missing = False
     return allow_missing
 
+
+def determine_series(count, has_range, branch, git_dir):
+    """Determine the series which is to be built, if any
+
+    Args:
+        count (int): Number of commits in branch
+        has_range (bool): True if a range of commits ('xx..yy') is being built
+        branch (str): Name of branch to build, or None if none
+        git_dir (str): Git directory to use, e.g. './.git'
+
+    Returns:
+        Series: Series to build, or None for none
+
+    Read the metadata from the commits. First look at the upstream commit,
+    then the ones in the branch. We would like to do something like
+    upstream/master~..branch but that isn't possible if upstream/master is
+    a merge commit (it will list all the commits that form part of the
+    merge)
+
+    Conflicting tags are not a problem for buildman, since it does not use
+    them. For example, Series-version is not useful for buildman. On the
+    other hand conflicting tags will cause an error. So allow later tags
+    to overwrite earlier ones by setting allow_overwrite=True
+    """
+    if branch:
+        if count == -1:
+            if has_range:
+                range_expr = branch
+            else:
+                range_expr = gitutil.get_range_in_branch(git_dir, branch)
+            upstream_commit = gitutil.get_upstream(git_dir, branch)
+            series = patchstream.get_metadata_for_list(upstream_commit,
+                git_dir, 1, series=None, allow_overwrite=True)
+
+            series = patchstream.get_metadata_for_list(range_expr,
+                    git_dir, None, series, allow_overwrite=True)
+        else:
+            # Honour the count
+            series = patchstream.get_metadata_for_list(branch,
+                    git_dir, count, series=None, allow_overwrite=True)
+    else:
+        series = None
+    return series
+
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -174,7 +219,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     gitutil.setup()
     col = terminal.Color()
 
-    options.git_dir = os.path.join(options.git, '.git')
+    git_dir = os.path.join(options.git, '.git')
 
     no_toolchains = toolchains is None
     if no_toolchains:
@@ -264,11 +309,11 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             count = 1
         else:
             if has_range:
-                count, msg = gitutil.count_commits_in_range(options.git_dir,
-                                                         options.branch)
+                count, msg = gitutil.count_commits_in_range(git_dir,
+                                                            options.branch)
             else:
-                count, msg = gitutil.count_commits_in_branch(options.git_dir,
-                                                          options.branch)
+                count, msg = gitutil.count_commits_in_branch(git_dir,
+                                                             options.branch)
             if count is None:
                 sys.exit(col.build(col.RED, msg))
             elif count == 0:
@@ -290,39 +335,11 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             sys.exit(col.build(col.RED,
                                '-w can only be used with a single commit'))
 
-    # Read the metadata from the commits. First look at the upstream commit,
-    # then the ones in the branch. We would like to do something like
-    # upstream/master~..branch but that isn't possible if upstream/master is
-    # a merge commit (it will list all the commits that form part of the
-    # merge)
-    # Conflicting tags are not a problem for buildman, since it does not use
-    # them. For example, Series-version is not useful for buildman. On the
-    # other hand conflicting tags will cause an error. So allow later tags
-    # to overwrite earlier ones by setting allow_overwrite=True
-    if options.branch:
-        if count == -1:
-            if has_range:
-                range_expr = options.branch
-            else:
-                range_expr = gitutil.get_range_in_branch(options.git_dir,
-                                                      options.branch)
-            upstream_commit = gitutil.get_upstream(options.git_dir,
-                                                  options.branch)
-            series = patchstream.get_metadata_for_list(upstream_commit,
-                options.git_dir, 1, series=None, allow_overwrite=True)
-
-            series = patchstream.get_metadata_for_list(range_expr,
-                    options.git_dir, None, series, allow_overwrite=True)
-        else:
-            # Honour the count
-            series = patchstream.get_metadata_for_list(options.branch,
-                    options.git_dir, count, series=None, allow_overwrite=True)
-    else:
-        series = None
-        if not options.dry_run:
-            options.verbose = True
-            if not options.summary:
-                options.show_errors = True
+    series = determine_series(count, has_range, options.branch, git_dir)
+    if not series and not options.dry_run:
+        options.verbose = True
+        if not options.summary:
+            options.show_errors = True
 
     # By default we have one thread per CPU. But if there are not enough jobs
     # we can have fewer threads and use a high '-j' value for make.
@@ -364,7 +381,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         else:
             adjust_cfg['LOCALVERSION_AUTO'] = '~'
 
-    builder = Builder(toolchains, output_dir, options.git_dir,
+    builder = Builder(toolchains, output_dir, git_dir,
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step,
             no_subdirs=options.no_subdirs, full_path=options.full_path,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 06/60] buildman: Move fetch-arch code into a separate function

[Simon Glass]

Reduce the size of the do_buildman() function a little by moving the code
that handles --fetch-arch into a separate function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 49 +++++++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7a81c2ced8b3..fd136e54baf9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -192,6 +192,36 @@ def determine_series(count, has_range, branch, git_dir):
     return series
 
 
+def do_fetch_arch(toolchains, col, fetch_arch):
+    """Handle the --fetch-arch option
+
+    Args:
+        toolchains (Toolchains): Tool chains to use
+        col (terminal.Color): Color object to build
+        fetch_arch (str): Argument passed to the --fetch-arch option
+
+    Returns:
+        int: Return code for buildman
+    """
+    if fetch_arch == 'list':
+        sorted_list = toolchains.ListArchs()
+        print(col.build(
+            col.BLUE,
+            f"Available architectures: {' '.join(sorted_list)}\n"))
+        return 0
+
+    if fetch_arch == 'all':
+        fetch_arch = ','.join(toolchains.ListArchs())
+        print(col.build(col.CYAN,
+                        f'\nDownloading toolchains: {fetch_arch}'))
+    for arch in fetch_arch.split(','):
+        print()
+        ret = toolchains.FetchAndInstall(arch)
+        if ret:
+            return ret
+    return 0
+
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -226,24 +256,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         toolchains = toolchain.Toolchains(options.override_toolchain)
 
     if options.fetch_arch:
-        if options.fetch_arch == 'list':
-            sorted_list = toolchains.ListArchs()
-            print(col.build(
-                col.BLUE,
-                f"Available architectures: {' '.join(sorted_list)}\n"))
-            return 0
-
-        fetch_arch = options.fetch_arch
-        if fetch_arch == 'all':
-            fetch_arch = ','.join(toolchains.ListArchs())
-            print(col.build(col.CYAN,
-                            f'\nDownloading toolchains: {fetch_arch}'))
-        for arch in fetch_arch.split(','):
-            print()
-            ret = toolchains.FetchAndInstall(arch)
-            if ret:
-                return ret
-        return 0
+        return do_fetch_arch(toolchains, col, options.fetch_arch)
 
     if no_toolchains:
         toolchains.GetSettings()
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 07/60] buildman: Add a test for the -A option

[Simon Glass]

This lacks a test at present. Add one.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/func_test.py | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index d9b4c41ec68c..5dbbb53b6572 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -785,10 +785,9 @@ CONFIG_LOCALVERSION=y
 ''', cfg_data)
         self.assertIn('Not dropping LOCALVERSION_AUTO', stdout.getvalue())
 
-    def testRegenBoards(self):
-        """Test that we can regenerate the boards.cfg file"""
-        outfile = os.path.join(self._output_dir, 'test-boards.cfg')
-        if os.path.exists(outfile):
-            os.remove(outfile)
-        result = self._RunControl('-R', outfile, brds=None, get_builder=False)
-        self.assertTrue(os.path.exists(outfile))
+    def test_print_prefix(self):
+        """Test that we can print the toolchain prefix"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            result = self._RunControl('-A', 'board0')
+        self.assertEqual('arm-\n', stdout.getvalue())
+        self.assertEqual('', stderr.getvalue())
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 08/60] buildman: Drop use of builder in show_actions()

[Simon Glass]

This function only needs the output directory from the builder. This is
passed into the builder, so just pass the same value to show_actions().
The avoids needing a builder to call show_actions().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index fd136e54baf9..7beb5bce6444 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -48,7 +48,7 @@ def get_action_summary(is_summary, commits, selected, options):
     return msg
 
 # pylint: disable=R0913
-def show_actions(series, why_selected, boards_selected, bldr, options,
+def show_actions(series, why_selected, boards_selected, output_dir, options,
                  board_warnings):
     """Display a list of actions that we would take, if not a dry run.
 
@@ -61,7 +61,7 @@ def show_actions(series, why_selected, boards_selected, bldr, options,
                 the value would be a list of board names.
         boards_selected: Dict of selected boards, key is target name,
                 value is Board object
-        bldr: The builder that will be used to build the commits
+        output_dir (str): Output directory for builder
         options: Command line options object
         board_warnings: List of warnings obtained from board selected
     """
@@ -74,7 +74,7 @@ def show_actions(series, why_selected, boards_selected, bldr, options,
         commits = None
     print(get_action_summary(False, commits, boards_selected,
             options))
-    print(f'Build directory: {bldr.base_dir}')
+    print(f'Build directory: {output_dir}')
     if commits:
         for upto in range(0, len(series.commits), options.step):
             commit = series.commits[upto]
@@ -416,7 +416,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # For a dry run, just show our actions as a sanity check
     if options.dry_run:
-        show_actions(series, why_selected, selected, builder, options,
+        show_actions(series, why_selected, selected, output_dir, options,
                     board_warnings)
     else:
         builder.force_build = options.force_build
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 09/60] buildman: Move dry-run handling higher in do_buildman()

[Simon Glass]

Move this up above where the builder is created, since it no-longer makes
use of the builder.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 88 ++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 43 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7beb5bce6444..c45a28cce3c4 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -384,6 +384,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             output_dir = os.path.join(options.output_dir, dirname)
         if clean_dir and os.path.exists(output_dir):
             shutil.rmtree(output_dir)
+
+    # For a dry run, just show our actions as a sanity check
+    if options.dry_run:
+        show_actions(series, why_selected, selected, output_dir, options,
+                    board_warnings)
+        return 0
+
     adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
 
     # Drop LOCALVERSION_AUTO since it changes the version string on every commit
@@ -414,48 +421,43 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     if make_func:
         builder.do_make = make_func
 
-    # For a dry run, just show our actions as a sanity check
-    if options.dry_run:
-        show_actions(series, why_selected, selected, output_dir, options,
-                    board_warnings)
+    builder.force_build = options.force_build
+    builder.force_build_failures = options.force_build_failures
+    builder.force_reconfig = options.force_reconfig
+    builder.in_tree = options.in_tree
+
+    # Work out which boards to build
+    board_selected = brds.get_selected_dict()
+
+    if series:
+        commits = series.commits
+        # Number the commits for test purposes
+        for i, commit in enumerate(commits):
+            commit.sequence = i
     else:
-        builder.force_build = options.force_build
-        builder.force_build_failures = options.force_build_failures
-        builder.force_reconfig = options.force_reconfig
-        builder.in_tree = options.in_tree
-
-        # Work out which boards to build
-        board_selected = brds.get_selected_dict()
-
-        if series:
-            commits = series.commits
-            # Number the commits for test purposes
-            for i, commit in enumerate(commits):
-                commit.sequence = i
-        else:
-            commits = None
-
-        if not options.ide:
-            tprint(get_action_summary(options.summary, commits, board_selected,
-                                    options))
-
-        # We can't show function sizes without board details at present
-        if options.show_bloat:
-            options.show_detail = True
-        builder.SetDisplayOptions(
-            options.show_errors, options.show_sizes, options.show_detail,
-            options.show_bloat, options.list_error_boards, options.show_config,
-            options.show_environment, options.filter_dtb_warnings,
-            options.filter_migration_warnings, options.ide)
-        if options.summary:
-            builder.ShowSummary(commits, board_selected)
-        else:
-            fail, warned, excs = builder.BuildBoards(
-                commits, board_selected, options.keep_outputs, options.verbose)
-            if excs:
-                return 102
-            if fail:
-                return 100
-            if warned and not options.ignore_warnings:
-                return 101
+        commits = None
+
+    if not options.ide:
+        tprint(get_action_summary(options.summary, commits, board_selected,
+                                options))
+
+    # We can't show function sizes without board details at present
+    if options.show_bloat:
+        options.show_detail = True
+    builder.SetDisplayOptions(
+        options.show_errors, options.show_sizes, options.show_detail,
+        options.show_bloat, options.list_error_boards, options.show_config,
+        options.show_environment, options.filter_dtb_warnings,
+        options.filter_migration_warnings, options.ide)
+    if options.summary:
+        builder.ShowSummary(commits, board_selected)
+    else:
+        fail, warned, excs = builder.BuildBoards(
+            commits, board_selected, options.keep_outputs, options.verbose)
+        if excs:
+            return 102
+        if fail:
+            return 100
+        if warned and not options.ignore_warnings:
+            return 101
     return 0
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 10/60] buildman: Move board-selection code into a function

[Simon Glass]

Create a new determine_boards() function to hold the code which selects
which boards to build.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 59 ++++++++++++++++++++++++++++-----------
 1 file changed, 43 insertions(+), 16 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c45a28cce3c4..b94e066d1bdf 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -222,6 +222,47 @@ def do_fetch_arch(toolchains, col, fetch_arch):
     return 0
 
 
+def determine_boards(brds, args, col, opt_boards, exclude):
+    """Determine which boards to build
+
+    Each element of args and exclude can refer to a board name, arch or SoC
+
+    Args:
+        brds (Boards): Boards object
+        args (list of str): Arguments describing boards to build
+        col (Terminal.Color): Color object
+        opt_boards (list of str): Specific boards to build, or None for all
+        exclude (list of str): Arguments describing boards to exclude
+
+    Returns:
+        tuple:
+            list of Board: List of Board objects that are marked selected
+            why_selected: Dictionary where each key is a buildman argument
+                    provided by the user, and the value is the list of boards
+                    brought in by that argument. For example, 'arm' might bring
+                    in 400 boards, so in this case the key would be 'arm' and
+                    the value would be a list of board names.
+            board_warnings: List of warnings obtained from board selected
+    """
+    exclude = []
+    if exclude:
+        for arg in exclude:
+            exclude += arg.split(',')
+
+    if opt_boards:
+        requested_boards = []
+        for brd in opt_boards:
+            requested_boards += brd.split(',')
+    else:
+        requested_boards = None
+    why_selected, board_warnings = brds.select_boards(args, exclude,
+                                                      requested_boards)
+    selected = brds.get_selected()
+    if not selected:
+        sys.exit(col.build(col.RED, 'No matching boards found'))
+    return selected, why_selected, board_warnings
+
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -289,22 +330,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             return 0 if okay else 2
         brds.read_boards(board_file)
 
-    exclude = []
-    if options.exclude:
-        for arg in options.exclude:
-            exclude += arg.split(',')
-
-    if options.boards:
-        requested_boards = []
-        for brd in options.boards:
-            requested_boards += brd.split(',')
-    else:
-        requested_boards = None
-    why_selected, board_warnings = brds.select_boards(args, exclude,
-                                                      requested_boards)
-    selected = brds.get_selected()
-    if not selected:
-        sys.exit(col.build(col.RED, 'No matching boards found'))
+    selected, why_selected, board_warnings = determine_boards(
+        brds, args, col, options.boards, options.exclude)
 
     if options.print_prefix:
         err = show_toolchain_prefix(brds, toolchains)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 11/60] bulidman: Move more code to determine_series()

[Simon Glass]

Move some more series-related code here, to reduce the size of the main
function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Correct operation of -A

 tools/buildman/control.py | 82 ++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 40 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b94e066d1bdf..5bc2ab9063f2 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -148,14 +148,17 @@ def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
     return allow_missing
 
 
-def determine_series(count, has_range, branch, git_dir):
+def determine_series(selected, col, git_dir, count, branch, work_in_output):
     """Determine the series which is to be built, if any
 
     Args:
+        selected (list of Board(: List of Board objects that are marked
+            selected
+        col (Terminal.Color): Color object to use
+        git_dir (str): Git directory to use, e.g. './.git'
         count (int): Number of commits in branch
-        has_range (bool): True if a range of commits ('xx..yy') is being built
         branch (str): Name of branch to build, or None if none
-        git_dir (str): Git directory to use, e.g. './.git'
+        work_in_output (bool): True to work in the output directory
 
     Returns:
         Series: Series to build, or None for none
@@ -171,6 +174,40 @@ def determine_series(count, has_range, branch, git_dir):
     other hand conflicting tags will cause an error. So allow later tags
     to overwrite earlier ones by setting allow_overwrite=True
     """
+
+    # Work out how many commits to build. We want to build everything on the
+    # branch. We also build the upstream commit as a control so we can see
+    # problems introduced by the first commit on the branch.
+    has_range = branch and '..' in branch
+    if count == -1:
+        if not branch:
+            count = 1
+        else:
+            if has_range:
+                count, msg = gitutil.count_commits_in_range(git_dir, branch)
+            else:
+                count, msg = gitutil.count_commits_in_branch(git_dir, branch)
+            if count is None:
+                sys.exit(col.build(col.RED, msg))
+            elif count == 0:
+                sys.exit(col.build(col.RED,
+                                   f"Range '{branch}' has no commits"))
+            if msg:
+                print(col.build(col.YELLOW, msg))
+            count += 1   # Build upstream commit also
+
+    if not count:
+        msg = (f"No commits found to process in branch '{branch}': "
+               "set branch's upstream or use -c flag")
+        sys.exit(col.build(col.RED, msg))
+    if work_in_output:
+        if len(selected) != 1:
+            sys.exit(col.build(col.RED,
+                               '-w can only be used with a single board'))
+        if count != 1:
+            sys.exit(col.build(col.RED,
+                               '-w can only be used with a single commit'))
+
     if branch:
         if count == -1:
             if has_range:
@@ -339,43 +376,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             sys.exit(col.build(col.RED, err))
         return 0
 
-    # Work out how many commits to build. We want to build everything on the
-    # branch. We also build the upstream commit as a control so we can see
-    # problems introduced by the first commit on the branch.
-    count = options.count
-    has_range = options.branch and '..' in options.branch
-    if count == -1:
-        if not options.branch:
-            count = 1
-        else:
-            if has_range:
-                count, msg = gitutil.count_commits_in_range(git_dir,
-                                                            options.branch)
-            else:
-                count, msg = gitutil.count_commits_in_branch(git_dir,
-                                                             options.branch)
-            if count is None:
-                sys.exit(col.build(col.RED, msg))
-            elif count == 0:
-                sys.exit(col.build(col.RED,
-                                   f"Range '{options.branch}' has no commits"))
-            if msg:
-                print(col.build(col.YELLOW, msg))
-            count += 1   # Build upstream commit also
-
-    if not count:
-        msg = (f"No commits found to process in branch '{options.branch}': "
-               "set branch's upstream or use -c flag")
-        sys.exit(col.build(col.RED, msg))
-    if options.work_in_output:
-        if len(selected) != 1:
-            sys.exit(col.build(col.RED,
-                               '-w can only be used with a single board'))
-        if count != 1:
-            sys.exit(col.build(col.RED,
-                               '-w can only be used with a single commit'))
-
-    series = determine_series(count, has_range, options.branch, git_dir)
+    series = determine_series(selected, col, git_dir, options.count,
+                              options.branch, options.work_in_output)
     if not series and not options.dry_run:
         options.verbose = True
         if not options.summary:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 12/60] buildman: Move Boards-object code into a function

[Simon Glass]

Move the code which obtains a Boards object into its own function, to
reduce the size of the main function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 54 ++++++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 15 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 5bc2ab9063f2..5c01ea8e1743 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -259,6 +259,41 @@ def do_fetch_arch(toolchains, col, fetch_arch):
     return 0
 
 
+def get_boards_obj(output_dir, regen_board_list, threads, verbose):
+    """Object the Boards object to use
+
+    Creates the output directory and ensures there is a boards.cfg file, then
+    read it in.
+
+    Args:
+        output_dir (str): Output directory to use
+        regen_board_list (bool): True to just regenerate the board list
+        threads (int or None): Number of threads to use to create boards file
+        verbose (bool): False to suppress output from boards-file generation
+
+    Returns:
+        Either:
+            int: Operation completed and buildman should exit with exit code
+            Boards: Boards object to use
+    """
+    if not os.path.exists(output_dir):
+        os.makedirs(output_dir)
+    board_file = os.path.join(output_dir, 'boards.cfg')
+    if regen_board_list and regen_board_list != '-':
+        board_file = regen_board_list
+
+    brds = boards.Boards()
+    okay = brds.ensure_board_list(
+        board_file,
+        threads or multiprocessing.cpu_count(),
+        force=regen_board_list,
+        quiet=not verbose)
+    if regen_board_list:
+        return 0 if okay else 2
+    brds.read_boards(board_file)
+    return brds
+
+
 def determine_boards(brds, args, col, opt_boards, exclude):
     """Determine which boards to build
 
@@ -351,21 +386,10 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # Work out what subset of the boards we are building
     if not brds:
-        if not os.path.exists(options.output_dir):
-            os.makedirs(options.output_dir)
-        board_file = os.path.join(options.output_dir, 'boards.cfg')
-        if options.regen_board_list and options.regen_board_list != '-':
-            board_file = options.regen_board_list
-
-        brds = boards.Boards()
-        okay = brds.ensure_board_list(
-            board_file,
-            options.threads or multiprocessing.cpu_count(),
-            force=options.regen_board_list,
-            quiet=not options.verbose)
-        if options.regen_board_list:
-            return 0 if okay else 2
-        brds.read_boards(board_file)
+        brds = get_boards_obj(options.output_dir, options.regen_board_list,
+                              options.threads, options.verbose)
+        if isinstance(brds, int):
+            return brds
 
     selected, why_selected, board_warnings = determine_boards(
         brds, args, col, options.boards, options.exclude)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 13/60] bulidman: Move toolchain handling to a function

[Simon Glass]

Move the code for dealing with toolchains out into its own function, to
reduce the size of the main function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 53 ++++++++++++++++++++++++++++-----------
 1 file changed, 38 insertions(+), 15 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 5c01ea8e1743..f319dc8724e2 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -259,6 +259,41 @@ def do_fetch_arch(toolchains, col, fetch_arch):
     return 0
 
 
+def get_toolchains(toolchains, col, override_toolchain, fetch_arch,
+                   list_tool_chains, verbose):
+    """Get toolchains object to use
+
+    Args:
+        toolchains (Toolchains or None): Toolchains to use. If None, then a
+            Toolchains object will be created and scanned
+        col (Terminal.Color): Color object
+        override_toolchain (str or None): Override value for toolchain, or None
+        fetch_arch (bool): True to fetch the toolchain for the architectures
+        list_tool_chains (bool): True to list all tool chains
+        verbose (bool): True for verbose output when listing toolchains
+
+Returns:
+        Either:
+            int: Operation completed and buildman should exit with exit code
+            Toolchains: Toolchains object to use
+    """
+    no_toolchains = toolchains is None
+    if no_toolchains:
+        toolchains = toolchain.Toolchains(override_toolchain)
+
+    if fetch_arch:
+        return do_fetch_arch(toolchains, col, fetch_arch)
+
+    if no_toolchains:
+        toolchains.GetSettings()
+        toolchains.Scan(list_tool_chains and verbose)
+    if list_tool_chains:
+        toolchains.List()
+        print()
+        return 0
+    return toolchains
+
+
 def get_boards_obj(output_dir, regen_board_list, threads, verbose):
     """Object the Boards object to use
 
@@ -364,21 +399,9 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     git_dir = os.path.join(options.git, '.git')
 
-    no_toolchains = toolchains is None
-    if no_toolchains:
-        toolchains = toolchain.Toolchains(options.override_toolchain)
-
-    if options.fetch_arch:
-        return do_fetch_arch(toolchains, col, options.fetch_arch)
-
-    if no_toolchains:
-        toolchains.GetSettings()
-        toolchains.Scan(options.list_tool_chains and options.verbose)
-    if options.list_tool_chains:
-        toolchains.List()
-        print()
-        return 0
-
+    toolchains = get_toolchains(toolchains, col, options.override_toolchain,
+                                options.fetch_arch, options.list_tool_chains,
+                                options.verbose)
     if not options.output_dir:
         if options.work_in_output:
             sys.exit(col.build(col.RED, '-w requires that you specify -o'))
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 14/60] bulldman: Set up output_dir earlier

[Simon Glass]

Set up output_dir at the start of the main function, instead of updating
the options.output_dir option.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index f319dc8724e2..7c356ec5020e 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -402,14 +402,15 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     toolchains = get_toolchains(toolchains, col, options.override_toolchain,
                                 options.fetch_arch, options.list_tool_chains,
                                 options.verbose)
-    if not options.output_dir:
+    output_dir = options.output_dir
+    if not output_dir:
         if options.work_in_output:
             sys.exit(col.build(col.RED, '-w requires that you specify -o'))
-        options.output_dir = '..'
+        output_dir = '..'
 
     # Work out what subset of the boards we are building
     if not brds:
-        brds = get_boards_obj(options.output_dir, options.regen_board_list,
+        brds = get_boards_obj(output_dir, options.regen_board_list,
                               options.threads, options.verbose)
         if isinstance(brds, int):
             return brds
@@ -451,13 +452,12 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                                       options.branch)
 
     # Create a new builder with the selected options.
-    output_dir = options.output_dir
     if options.branch:
         dirname = options.branch.replace('/', '_')
         # As a special case allow the board directory to be placed in the
         # output directory itself rather than any subdirectory.
         if not options.no_subdirs:
-            output_dir = os.path.join(options.output_dir, dirname)
+            output_dir = os.path.join(output_dir, dirname)
         if clean_dir and os.path.exists(output_dir):
             shutil.rmtree(output_dir)
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 15/60] buildman: Move output-file setup into one place

[Simon Glass]

Collect the two parts of the output-file handling into single place.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7c356ec5020e..7759784b22a9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -407,6 +407,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         if options.work_in_output:
             sys.exit(col.build(col.RED, '-w requires that you specify -o'))
         output_dir = '..'
+    if options.branch and not options.no_subdirs:
+        # As a special case allow the board directory to be placed in the
+        # output directory itself rather than any subdirectory.
+        dirname = options.branch.replace('/', '_')
+        output_dir = os.path.join(output_dir, dirname)
+        if clean_dir and os.path.exists(output_dir):
+            shutil.rmtree(output_dir)
 
     # Work out what subset of the boards we are building
     if not brds:
@@ -452,14 +459,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                                       options.branch)
 
     # Create a new builder with the selected options.
-    if options.branch:
-        dirname = options.branch.replace('/', '_')
-        # As a special case allow the board directory to be placed in the
-        # output directory itself rather than any subdirectory.
-        if not options.no_subdirs:
-            output_dir = os.path.join(output_dir, dirname)
-        if clean_dir and os.path.exists(output_dir):
-            shutil.rmtree(output_dir)
 
     # For a dry run, just show our actions as a sanity check
     if options.dry_run:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 16/60] buildman: Pass option values to get_action_summary()

[Simon Glass]

Pass in the individual values rather than the whole options object, so we
can see what is needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 7759784b22a9..e5e807b87390 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -29,22 +29,30 @@ def get_plural(count):
     """Returns a plural 's' if count is not 1"""
     return 's' if count != 1 else ''
 
-def get_action_summary(is_summary, commits, selected, options):
+def get_action_summary(is_summary, commits, selected, step, threads, jobs):
     """Return a string summarising the intended action.
 
+    Args:
+        is_summary (bool): True if this is a summary (otherwise it is building)
+        commits (list): List of commits being built
+        selected (list of Board): List of Board objects that are marked
+        step (int): Step increment through commits
+        threads (int): Number of processor threads being used
+        jobs (int): Number of jobs to build at once
+
     Returns:
         Summary string.
     """
     if commits:
         count = len(commits)
-        count = (count + options.step - 1) // options.step
+        count = (count + step - 1) // step
         commit_str = f'{count} commit{get_plural(count)}'
     else:
         commit_str = 'current source'
     msg = (f"{'Summary of' if is_summary else 'Building'} "
            f'{commit_str} for {len(selected)} boards')
-    msg += (f' ({options.threads} thread{get_plural(options.threads)}, '
-            f'{options.jobs} job{get_plural(options.jobs)} per thread)')
+    msg += (f' ({threads} thread{get_plural(threads)}, '
+            f'{jobs} job{get_plural(jobs)} per thread)')
     return msg
 
 # pylint: disable=R0913
@@ -73,7 +81,7 @@ def show_actions(series, why_selected, boards_selected, output_dir, options,
     else:
         commits = None
     print(get_action_summary(False, commits, boards_selected,
-            options))
+                             options.step, options.threads, options.jobs))
     print(f'Build directory: {output_dir}')
     if commits:
         for upto in range(0, len(series.commits), options.step):
@@ -152,7 +160,7 @@ def determine_series(selected, col, git_dir, count, branch, work_in_output):
     """Determine the series which is to be built, if any
 
     Args:
-        selected (list of Board(: List of Board objects that are marked
+        selected (list of Board): List of Board objects that are marked
             selected
         col (Terminal.Color): Color object to use
         git_dir (str): Git directory to use, e.g. './.git'
@@ -514,7 +522,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     if not options.ide:
         tprint(get_action_summary(options.summary, commits, board_selected,
-                                options))
+                                  options.step, options.threads, options.jobs))
 
     # We can't show function sizes without board details at present
     if options.show_bloat:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 17/60] buildman: Pass option values to show_actions()

[Simon Glass]

Pass in the individual values rather than the whole options object, so we
can see what is needed.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 22 +++++++++++++---------
 1 file changed, 13 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index e5e807b87390..af3f2a6a0c42 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -56,8 +56,8 @@ def get_action_summary(is_summary, commits, selected, step, threads, jobs):
     return msg
 
 # pylint: disable=R0913
-def show_actions(series, why_selected, boards_selected, output_dir, options,
-                 board_warnings):
+def show_actions(series, why_selected, boards_selected, output_dir,
+                 board_warnings, step, threads, jobs, verbose):
     """Display a list of actions that we would take, if not a dry run.
 
     Args:
@@ -70,8 +70,11 @@ def show_actions(series, why_selected, boards_selected, output_dir, options,
         boards_selected: Dict of selected boards, key is target name,
                 value is Board object
         output_dir (str): Output directory for builder
-        options: Command line options object
         board_warnings: List of warnings obtained from board selected
+        step (int): Step increment through commits
+        threads (int): Number of processor threads being used
+        jobs (int): Number of jobs to build at once
+        verbose (bool): True to indicate why each board was selected
     """
     col = terminal.Color()
     print('Dry run, so not doing much. But I would do this:')
@@ -80,11 +83,11 @@ def show_actions(series, why_selected, boards_selected, output_dir, options,
         commits = series.commits
     else:
         commits = None
-    print(get_action_summary(False, commits, boards_selected,
-                             options.step, options.threads, options.jobs))
+    print(get_action_summary(False, commits, boards_selected, step, threads,
+                             jobs))
     print(f'Build directory: {output_dir}')
     if commits:
-        for upto in range(0, len(series.commits), options.step):
+        for upto in range(0, len(series.commits), step):
             commit = series.commits[upto]
             print('   ', col.build(col.YELLOW, commit.hash[:8], bright=False), end=' ')
             print(commit.subject)
@@ -92,7 +95,7 @@ def show_actions(series, why_selected, boards_selected, output_dir, options,
     for arg in why_selected:
         if arg != 'all':
             print(arg, f': {len(why_selected[arg])} boards')
-            if options.verbose:
+            if verbose:
                 print(f"   {' '.join(why_selected[arg])}")
     print('Total boards to build for each '
           f"commit: {len(why_selected['all'])}\n")
@@ -470,8 +473,9 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # For a dry run, just show our actions as a sanity check
     if options.dry_run:
-        show_actions(series, why_selected, selected, output_dir, options,
-                    board_warnings)
+        show_actions(series, why_selected, selected, output_dir, board_warnings,
+                     options.step, options.threads, options.jobs,
+                     options.verbose)
         return 0
 
     adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 18/60] buildman: Build option-adjusting into a function

[Simon Glass]

Create a separate function to adjust options. Also move show_actions() up
as far as we can in the function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 53 ++++++++++++++++++++++++---------------
 1 file changed, 33 insertions(+), 20 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index af3f2a6a0c42..0479dfd17d0c 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -381,6 +381,32 @@ def determine_boards(brds, args, col, opt_boards, exclude):
     return selected, why_selected, board_warnings
 
 
+def adjust_options(options, series, selected):
+    """Adjust options according to various constraints
+
+    Updates verbose, show_errors, threads, jobs and step
+
+    Args:
+        options (Options): Options object to adjust
+        series (Series): Series being built / summarised
+        selected (list of Board): List of Board objects that are marked
+    """
+    if not series and not options.dry_run:
+        options.verbose = True
+        if not options.summary:
+            options.show_errors = True
+
+    # By default we have one thread per CPU. But if there are not enough jobs
+    # we can have fewer threads and use a high '-j' value for make.
+    if options.threads is None:
+        options.threads = min(multiprocessing.cpu_count(), len(selected))
+    if not options.jobs:
+        options.jobs = max(1, (multiprocessing.cpu_count() +
+                len(selected) - 1) // len(selected))
+
+    if not options.step:
+        options.step = len(series.commits) - 1
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -444,21 +470,15 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     series = determine_series(selected, col, git_dir, options.count,
                               options.branch, options.work_in_output)
-    if not series and not options.dry_run:
-        options.verbose = True
-        if not options.summary:
-            options.show_errors = True
 
-    # By default we have one thread per CPU. But if there are not enough jobs
-    # we can have fewer threads and use a high '-j' value for make.
-    if options.threads is None:
-        options.threads = min(multiprocessing.cpu_count(), len(selected))
-    if not options.jobs:
-        options.jobs = max(1, (multiprocessing.cpu_count() +
-                len(selected) - 1) // len(selected))
+    adjust_options(options, series, selected)
 
-    if not options.step:
-        options.step = len(series.commits) - 1
+    # For a dry run, just show our actions as a sanity check
+    if options.dry_run:
+        show_actions(series, why_selected, selected, output_dir, board_warnings,
+                     options.step, options.threads, options.jobs,
+                     options.verbose)
+        return 0
 
     gnu_make = command.output(os.path.join(options.git,
             'scripts/show-gnu-make'), raise_on_error=False).rstrip()
@@ -471,13 +491,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # Create a new builder with the selected options.
 
-    # For a dry run, just show our actions as a sanity check
-    if options.dry_run:
-        show_actions(series, why_selected, selected, output_dir, board_warnings,
-                     options.step, options.threads, options.jobs,
-                     options.verbose)
-        return 0
-
     adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
 
     # Drop LOCALVERSION_AUTO since it changes the version string on every commit
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 19/60] buildman: Move counting of commits into a function

[Simon Glass]

Move this code into a separate function to avoid a pylint warning in
determine_series().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 63 +++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 22 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 0479dfd17d0c..25142a46066b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -159,6 +159,46 @@ def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
     return allow_missing
 
 
+def count_commits(branch, count, col, git_dir):
+    """Could the number of commits in the branch/ranch being built
+
+    Args:
+        branch (str): Name of branch to build, or None if none
+        count (int): Number of commits to build, or -1 for all
+        col (Terminal.Color): Color object to use
+        git_dir (str): Git directory to use, e.g. './.git'
+
+    Returns:
+        tuple:
+            Number of commits being built
+            True if the 'branch' string contains a range rather than a simple
+                name
+    """
+    has_range = branch and '..' in branch
+    if count == -1:
+        if not branch:
+            count = 1
+        else:
+            if has_range:
+                count, msg = gitutil.count_commits_in_range(git_dir, branch)
+            else:
+                count, msg = gitutil.count_commits_in_branch(git_dir, branch)
+            if count is None:
+                sys.exit(col.build(col.RED, msg))
+            elif count == 0:
+                sys.exit(col.build(col.RED,
+                                   f"Range '{branch}' has no commits"))
+            if msg:
+                print(col.build(col.YELLOW, msg))
+            count += 1   # Build upstream commit also
+
+    if not count:
+        msg = (f"No commits found to process in branch '{branch}': "
+               "set branch's upstream or use -c flag")
+        sys.exit(col.build(col.RED, msg))
+    return count, has_range
+
+
 def determine_series(selected, col, git_dir, count, branch, work_in_output):
     """Determine the series which is to be built, if any
 
@@ -189,28 +229,7 @@ def determine_series(selected, col, git_dir, count, branch, work_in_output):
     # Work out how many commits to build. We want to build everything on the
     # branch. We also build the upstream commit as a control so we can see
     # problems introduced by the first commit on the branch.
-    has_range = branch and '..' in branch
-    if count == -1:
-        if not branch:
-            count = 1
-        else:
-            if has_range:
-                count, msg = gitutil.count_commits_in_range(git_dir, branch)
-            else:
-                count, msg = gitutil.count_commits_in_branch(git_dir, branch)
-            if count is None:
-                sys.exit(col.build(col.RED, msg))
-            elif count == 0:
-                sys.exit(col.build(col.RED,
-                                   f"Range '{branch}' has no commits"))
-            if msg:
-                print(col.build(col.YELLOW, msg))
-            count += 1   # Build upstream commit also
-
-    if not count:
-        msg = (f"No commits found to process in branch '{branch}': "
-               "set branch's upstream or use -c flag")
-        sys.exit(col.build(col.RED, msg))
+    count, has_range = count_commits(branch, count, col, git_dir)
     if work_in_output:
         if len(selected) != 1:
             sys.exit(col.build(col.RED,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 20/60] buildman: Move setting up the output dir into a function

[Simon Glass]

Move this code into a separate function to reduce the size of the main
do_buildman() directory.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 45 ++++++++++++++++++++++++++++-----------
 1 file changed, 33 insertions(+), 12 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 25142a46066b..9ff9f20aa128 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -426,6 +426,36 @@ def adjust_options(options, series, selected):
     if not options.step:
         options.step = len(series.commits) - 1
 
+
+def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
+                     clean_dir):
+    """Set up the output directory
+
+    Args:
+        output_dir (str): Output directory provided by the user, or None if none
+        work_in_output (bool): True to work in the output directory
+        branch (str): Name of branch to build, or None if none
+        no_subdirs (bool): True to put the output in the top-level output dir
+        clean_dir: Used for tests only, indicates that the existing output_dir
+            should be removed before starting the build
+
+    Returns:
+        str: Updated output directory pathname
+    """
+    if not output_dir:
+        if work_in_output:
+            sys.exit(col.build(col.RED, '-w requires that you specify -o'))
+        output_dir = '..'
+    if branch and not no_subdirs:
+        # As a special case allow the board directory to be placed in the
+        # output directory itself rather than any subdirectory.
+        dirname = branch.replace('/', '_')
+        output_dir = os.path.join(output_dir, dirname)
+        if clean_dir and os.path.exists(output_dir):
+            shutil.rmtree(output_dir)
+    return output_dir
+
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -458,18 +488,9 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     toolchains = get_toolchains(toolchains, col, options.override_toolchain,
                                 options.fetch_arch, options.list_tool_chains,
                                 options.verbose)
-    output_dir = options.output_dir
-    if not output_dir:
-        if options.work_in_output:
-            sys.exit(col.build(col.RED, '-w requires that you specify -o'))
-        output_dir = '..'
-    if options.branch and not options.no_subdirs:
-        # As a special case allow the board directory to be placed in the
-        # output directory itself rather than any subdirectory.
-        dirname = options.branch.replace('/', '_')
-        output_dir = os.path.join(output_dir, dirname)
-        if clean_dir and os.path.exists(output_dir):
-            shutil.rmtree(output_dir)
+    output_dir = setup_output_dir(
+        options.output_dir, options.work_in_output, options.branch,
+        options.no_subdirs, col, clean_dir)
 
     # Work out what subset of the boards we are building
     if not brds:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 21/60] buildman: Move commit numbering into determine_series()

[Simon Glass]

Commits are numbered for use in tests. Do this in determine_series() since
it is already dealing with the series.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 9ff9f20aa128..a38798af2138 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -202,6 +202,9 @@ def count_commits(branch, count, col, git_dir):
 def determine_series(selected, col, git_dir, count, branch, work_in_output):
     """Determine the series which is to be built, if any
 
+    If there is a series, the commits in that series are numbered by setting
+    their sequence value (starting from 0). This is used by tests.
+
     Args:
         selected (list of Board): List of Board objects that are marked
             selected
@@ -254,6 +257,10 @@ def determine_series(selected, col, git_dir, count, branch, work_in_output):
             # Honour the count
             series = patchstream.get_metadata_for_list(branch,
                     git_dir, count, series=None, allow_overwrite=True)
+
+        # Number the commits for test purposes
+        for i, commit in enumerate(series.commits):
+            commit.sequence = i
     else:
         series = None
     return series
@@ -571,9 +578,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     if series:
         commits = series.commits
-        # Number the commits for test purposes
-        for i, commit in enumerate(commits):
-            commit.sequence = i
     else:
         commits = None
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 22/60] buildman: Avoid too many returns in do_buildman()

[Simon Glass]

Fix the pylint warning by using a variable instead of lots of 'return'
statements.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index a38798af2138..b0d3cf85c52c 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -593,15 +593,16 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         options.show_bloat, options.list_error_boards, options.show_config,
         options.show_environment, options.filter_dtb_warnings,
         options.filter_migration_warnings, options.ide)
+    retval = 0
     if options.summary:
         builder.ShowSummary(commits, board_selected)
     else:
         fail, warned, excs = builder.BuildBoards(
             commits, board_selected, options.keep_outputs, options.verbose)
         if excs:
-            return 102
+            retval = 102
         if fail:
-            return 100
+            retval = 100
         if warned and not options.ignore_warnings:
-            return 101
-    return 0
+            retval = 101
+    return retval
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 23/60] buildman: Move remaining builder properties to constructor

[Simon Glass]

Do these all in the constructor, so it is consistent.

Move the stray builder comment into the correct place.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builder.py | 25 ++++++++++++++++++-------
 tools/buildman/control.py | 18 +++++++-----------
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index d81752e99438..cb3628a8a085 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -255,7 +255,10 @@ class Builder:
                  config_only=False, squash_config_y=False,
                  warnings_as_errors=False, work_in_output=False,
                  test_thread_exceptions=False, adjust_cfg=None,
-                 allow_missing=False, no_lto=False, reproducible_builds=False):
+                 allow_missing=False, no_lto=False, reproducible_builds=False,
+                 force_build=False, force_build_failures=False,
+                 force_reconfig=False, in_tree=False,
+                 force_config_on_failure=False, make_func=None):
         """Create a new Builder object
 
         Args:
@@ -295,7 +298,14 @@ class Builder:
                         a string Kconfig
             allow_missing: Run build with BINMAN_ALLOW_MISSING=1
             no_lto (bool): True to set the NO_LTO flag when building
-
+            force_build (bool): Rebuild even commits that are already built
+            force_build_failures (bool): Rebuild commits that have not been
+                built, or failed to build
+            force_reconfig (bool): Reconfigure on each commit
+            in_tree (bool): Bulid in tree instead of out-of-tree
+            force_config_on_failure (bool): Reconfigure the build before
+                retrying a failed build
+            make_func (function): Function to call to run 'make'
         """
         self.toolchains = toolchains
         self.base_dir = base_dir
@@ -304,7 +314,7 @@ class Builder:
         else:
             self._working_dir = os.path.join(base_dir, '.bm-work')
         self.threads = []
-        self.do_make = self.Make
+        self.do_make = make_func or self.Make
         self.gnu_make = gnu_make
         self.checkout = checkout
         self.num_threads = num_threads
@@ -318,11 +328,7 @@ class Builder:
         self._complete_delay = None
         self._next_delay_update = datetime.now()
         self._start_time = datetime.now()
-        self.force_config_on_failure = True
-        self.force_build_failures = False
-        self.force_reconfig = False
         self._step = step
-        self.in_tree = False
         self._error_lines = 0
         self.no_subdirs = no_subdirs
         self.full_path = full_path
@@ -336,6 +342,11 @@ class Builder:
         self._ide = False
         self.no_lto = no_lto
         self.reproducible_builds = reproducible_builds
+        self.force_build = force_build
+        self.force_build_failures = force_build_failures
+        self.force_reconfig = force_reconfig
+        self.in_tree = in_tree
+        self.force_config_on_failure = force_config_on_failure
 
         if not self.squash_config_y:
             self.config_filenames += EXTRA_CONFIG_FILENAMES
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b0d3cf85c52c..83ce8141eae4 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -536,8 +536,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                                       options.no_allow_missing, len(selected),
                                       options.branch)
 
-    # Create a new builder with the selected options.
-
     adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
 
     # Drop LOCALVERSION_AUTO since it changes the version string on every commit
@@ -548,6 +546,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         else:
             adjust_cfg['LOCALVERSION_AUTO'] = '~'
 
+    # Create a new builder with the selected options
     builder = Builder(toolchains, output_dir, git_dir,
             options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
             show_unknown=options.show_unknown, step=options.step,
@@ -562,16 +561,13 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             test_thread_exceptions=test_thread_exceptions,
             adjust_cfg=adjust_cfg,
             allow_missing=allow_missing, no_lto=options.no_lto,
-            reproducible_builds=options.reproducible_builds)
+            reproducible_builds=options.reproducible_builds,
+            force_build = options.force_build,
+            force_build_failures = options.force_build_failures,
+            force_reconfig = options.force_reconfig, in_tree = options.in_tree,
+            force_config_on_failure=not options.quick, make_func=make_func)
+
     TEST_BUILDER = builder
-    builder.force_config_on_failure = not options.quick
-    if make_func:
-        builder.do_make = make_func
-
-    builder.force_build = options.force_build
-    builder.force_build_failures = options.force_build_failures
-    builder.force_reconfig = options.force_reconfig
-    builder.in_tree = options.in_tree
 
     # Work out which boards to build
     board_selected = brds.get_selected_dict()
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 24/60] buildman: Tweak commits and show_bloat

[Simon Glass]

Move setting of show_bloat to adjust_options() and adjust how the commits
variable is set. Together these remove the pylint warning about too many
statements.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 83ce8141eae4..4f493006474e 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -433,6 +433,10 @@ def adjust_options(options, series, selected):
     if not options.step:
         options.step = len(series.commits) - 1
 
+    # We can't show function sizes without board details at present
+    if options.show_bloat:
+        options.show_detail = True
+
 
 def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
                      clean_dir):
@@ -572,18 +576,11 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     # Work out which boards to build
     board_selected = brds.get_selected_dict()
 
-    if series:
-        commits = series.commits
-    else:
-        commits = None
-
+    commits = series.commits if series else None
     if not options.ide:
         tprint(get_action_summary(options.summary, commits, board_selected,
                                   options.step, options.threads, options.jobs))
 
-    # We can't show function sizes without board details at present
-    if options.show_bloat:
-        options.show_detail = True
     builder.SetDisplayOptions(
         options.show_errors, options.show_sizes, options.show_detail,
         options.show_bloat, options.list_error_boards, options.show_config,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 25/60] buildman: Moving running of the builder into a function

[Simon Glass]

Move this code into a new function. This removes the pylint warning about
too many branches.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 56 ++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 21 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 4f493006474e..84d672619d80 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -466,6 +466,40 @@ def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
             shutil.rmtree(output_dir)
     return output_dir
 
+def run_builder(builder, commits, board_selected, options):
+    """Run the builder or show the summary
+
+    Args:
+        commits (list of Commit): List of commits being built, None if no branch
+        boards_selected (dict): Dict of selected boards:
+            key: target name
+            value: Board object
+        options (Options): Options to use
+
+    Returns:
+        int: Return code for buildman
+    """
+    if not options.ide:
+        tprint(get_action_summary(options.summary, commits, board_selected,
+                                  options.step, options.threads, options.jobs))
+
+    builder.SetDisplayOptions(
+        options.show_errors, options.show_sizes, options.show_detail,
+        options.show_bloat, options.list_error_boards, options.show_config,
+        options.show_environment, options.filter_dtb_warnings,
+        options.filter_migration_warnings, options.ide)
+    if options.summary:
+        builder.ShowSummary(commits, board_selected)
+    else:
+        fail, warned, excs = builder.BuildBoards(
+            commits, board_selected, options.keep_outputs, options.verbose)
+        if excs:
+            return 102
+        if fail:
+            return 100
+        if warned and not options.ignore_warnings:
+            return 101
+    return 0
 
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
@@ -577,25 +611,5 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     board_selected = brds.get_selected_dict()
 
     commits = series.commits if series else None
-    if not options.ide:
-        tprint(get_action_summary(options.summary, commits, board_selected,
-                                  options.step, options.threads, options.jobs))
-
-    builder.SetDisplayOptions(
-        options.show_errors, options.show_sizes, options.show_detail,
-        options.show_bloat, options.list_error_boards, options.show_config,
-        options.show_environment, options.filter_dtb_warnings,
-        options.filter_migration_warnings, options.ide)
-    retval = 0
-    if options.summary:
-        builder.ShowSummary(commits, board_selected)
-    else:
-        fail, warned, excs = builder.BuildBoards(
-            commits, board_selected, options.keep_outputs, options.verbose)
-        if excs:
-            retval = 102
-        if fail:
-            retval = 100
-        if warned and not options.ignore_warnings:
-            retval = 101
+    retval = run_builder(builder, commits, board_selected, options)
     return retval
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 26/60] buildman: Drop some unnecessary variables

[Simon Glass]

Drop some variables at the end of the do_bulidman() function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 84d672619d80..b5181304d8f7 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -607,9 +607,5 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     TEST_BUILDER = builder
 
-    # Work out which boards to build
-    board_selected = brds.get_selected_dict()
-
-    commits = series.commits if series else None
-    retval = run_builder(builder, commits, board_selected, options)
-    return retval
+    return run_builder(builder, series.commits if series else None,
+                       brds.get_selected_dict(), options)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 27/60] buildman: Adjust show_toolchain_prefix() to not return

[Simon Glass]

This function does not need to return. Simplify the code by exiting
immediately.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index b5181304d8f7..66fd04d90e44 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -121,10 +121,9 @@ def show_toolchain_prefix(brds, toolchains):
     for brd in board_selected.values():
         tc_set.add(toolchains.Select(brd.arch))
     if len(tc_set) != 1:
-        return 'Supplied boards must share one toolchain'
+        sys.exit('Supplied boards must share one toolchain')
     tchain = tc_set.pop()
     print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
-    return None
 
 def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
     """Figure out whether to allow external blobs
@@ -548,9 +547,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
         brds, args, col, options.boards, options.exclude)
 
     if options.print_prefix:
-        err = show_toolchain_prefix(brds, toolchains)
-        if err:
-            sys.exit(col.build(col.RED, err))
+        show_toolchain_prefix(brds, toolchains)
         return 0
 
     series = determine_series(selected, col, git_dir, options.count,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 28/60] buildman: Move checking for make into run_builder()

[Simon Glass]

This is not needed until the builder is run. Move it there to reduce the
size of the do_buildman() function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 66fd04d90e44..06d8e7b9527b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -465,6 +465,7 @@ def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
             shutil.rmtree(output_dir)
     return output_dir
 
+
 def run_builder(builder, commits, board_selected, options):
     """Run the builder or show the summary
 
@@ -478,6 +479,12 @@ def run_builder(builder, commits, board_selected, options):
     Returns:
         int: Return code for buildman
     """
+    gnu_make = command.output(os.path.join(options.git,
+            'scripts/show-gnu-make'), raise_on_error=False).rstrip()
+    if not gnu_make:
+        sys.exit('GNU Make not found')
+    builder.gnu_make = gnu_make
+
     if not options.ide:
         tprint(get_action_summary(options.summary, commits, board_selected,
                                   options.step, options.threads, options.jobs))
@@ -562,11 +569,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                      options.verbose)
         return 0
 
-    gnu_make = command.output(os.path.join(options.git,
-            'scripts/show-gnu-make'), raise_on_error=False).rstrip()
-    if not gnu_make:
-        sys.exit('GNU Make not found')
-
     allow_missing = get_allow_missing(options.allow_missing,
                                       options.no_allow_missing, len(selected),
                                       options.branch)
@@ -583,7 +585,7 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
 
     # Create a new builder with the selected options
     builder = Builder(toolchains, output_dir, git_dir,
-            options.threads, options.jobs, gnu_make=gnu_make, checkout=True,
+            options.threads, options.jobs, checkout=True,
             show_unknown=options.show_unknown, step=options.step,
             no_subdirs=options.no_subdirs, full_path=options.full_path,
             verbose_build=options.verbose_build,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 29/60] buildman: Move getting the adjust_cfg into run_builder()

[Simon Glass]

Move this into its own function to reduce the size of do_buildman().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 38 +++++++++++++++++++++++++++-----------
 1 file changed, 27 insertions(+), 11 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 06d8e7b9527b..873b5aadfca8 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -507,6 +507,31 @@ def run_builder(builder, commits, board_selected, options):
             return 101
     return 0
 
+
+def calc_adjust_cfg(adjust_cfg, reproducible_builds):
+    """Calculate the value to use for adjust_cfg
+
+    Args:
+        adjust_cfg (list of str): List of configuration changes. See cfgutil for
+            details
+        reproducible_builds (bool): True to adjust the configuration to get
+            reproduceable builds
+
+    Returns:
+        adjust_cfg (list of str): List of configuration changes
+    """
+    adjust_cfg = cfgutil.convert_list_to_dict(adjust_cfg)
+
+    # Drop LOCALVERSION_AUTO since it changes the version string on every commit
+    if reproducible_builds:
+        # If these are mentioned, leave the local version alone
+        if 'LOCALVERSION' in adjust_cfg or 'LOCALVERSION_AUTO' in adjust_cfg:
+            print('Not dropping LOCALVERSION_AUTO for reproducible build')
+        else:
+            adjust_cfg['LOCALVERSION_AUTO'] = '~'
+    return adjust_cfg
+
+
 def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
@@ -573,16 +598,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                                       options.no_allow_missing, len(selected),
                                       options.branch)
 
-    adjust_cfg = cfgutil.convert_list_to_dict(options.adjust_cfg)
-
-    # Drop LOCALVERSION_AUTO since it changes the version string on every commit
-    if options.reproducible_builds:
-        # If these are mentioned, leave the local version alone
-        if 'LOCALVERSION' in adjust_cfg or 'LOCALVERSION_AUTO' in adjust_cfg:
-            print('Not dropping LOCALVERSION_AUTO for reproducible build')
-        else:
-            adjust_cfg['LOCALVERSION_AUTO'] = '~'
-
     # Create a new builder with the selected options
     builder = Builder(toolchains, output_dir, git_dir,
             options.threads, options.jobs, checkout=True,
@@ -596,7 +611,8 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             warnings_as_errors=options.warnings_as_errors,
             work_in_output=options.work_in_output,
             test_thread_exceptions=test_thread_exceptions,
-            adjust_cfg=adjust_cfg,
+            adjust_cfg=calc_adjust_cfg(options.adjust_cfg,
+                                       options.reproducible_builds),
             allow_missing=allow_missing, no_lto=options.no_lto,
             reproducible_builds=options.reproducible_builds,
             force_build = options.force_build,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 30/60] buildman: Use get_alow_missing() directly to avoid var

[Simon Glass]

Avoid an unnecessary local variable by moving this code to a function.
This fixes the pylint warning about too many local variables.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 873b5aadfca8..c2ac56e43cc9 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -594,10 +594,6 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
                      options.verbose)
         return 0
 
-    allow_missing = get_allow_missing(options.allow_missing,
-                                      options.no_allow_missing, len(selected),
-                                      options.branch)
-
     # Create a new builder with the selected options
     builder = Builder(toolchains, output_dir, git_dir,
             options.threads, options.jobs, checkout=True,
@@ -613,7 +609,10 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
             test_thread_exceptions=test_thread_exceptions,
             adjust_cfg=calc_adjust_cfg(options.adjust_cfg,
                                        options.reproducible_builds),
-            allow_missing=allow_missing, no_lto=options.no_lto,
+            allow_missing=get_allow_missing(options.allow_missing,
+                                            options.no_allow_missing,
+                                            len(selected), options.branch),
+            no_lto=options.no_lto,
             reproducible_builds=options.reproducible_builds,
             force_build = options.force_build,
             force_build_failures = options.force_build_failures,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 31/60] buildman: Create a function to get number of built commits

[Simon Glass]

Move this code into a function. This removes the last pylint error in
the control module.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/control.py | 34 +++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index c2ac56e43cc9..bf4843c4170d 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -29,7 +29,24 @@ def get_plural(count):
     """Returns a plural 's' if count is not 1"""
     return 's' if count != 1 else ''
 
-def get_action_summary(is_summary, commits, selected, step, threads, jobs):
+
+def count_build_commits(commits, step):
+    """Calculate the number of commits to be built
+
+    Args:
+        commits (list of Commit): Commits to build or None
+        step (int): Step value for commits, typically 1
+
+    Returns:
+        Number of commits that will be built
+    """
+    if commits:
+        count = len(commits)
+        return (count + step - 1) // step
+    return 0
+
+
+def get_action_summary(is_summary, commit_count, selected, threads, jobs):
     """Return a string summarising the intended action.
 
     Args:
@@ -43,10 +60,8 @@ def get_action_summary(is_summary, commits, selected, step, threads, jobs):
     Returns:
         Summary string.
     """
-    if commits:
-        count = len(commits)
-        count = (count + step - 1) // step
-        commit_str = f'{count} commit{get_plural(count)}'
+    if commit_count:
+        commit_str = f'{commit_count} commit{get_plural(commit_count)}'
     else:
         commit_str = 'current source'
     msg = (f"{'Summary of' if is_summary else 'Building'} "
@@ -83,8 +98,8 @@ def show_actions(series, why_selected, boards_selected, output_dir,
         commits = series.commits
     else:
         commits = None
-    print(get_action_summary(False, commits, boards_selected, step, threads,
-                             jobs))
+    print(get_action_summary(False, count_build_commits(commits, step),
+                             boards_selected, threads, jobs))
     print(f'Build directory: {output_dir}')
     if commits:
         for upto in range(0, len(series.commits), step):
@@ -486,8 +501,9 @@ def run_builder(builder, commits, board_selected, options):
     builder.gnu_make = gnu_make
 
     if not options.ide:
-        tprint(get_action_summary(options.summary, commits, board_selected,
-                                  options.step, options.threads, options.jobs))
+        commit_count = count_build_commits(commits, options.step)
+        tprint(get_action_summary(options.summary, commit_count, board_selected,
+                                  options.threads, options.jobs))
 
     builder.SetDisplayOptions(
         options.show_errors, options.show_sizes, options.show_detail,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 32/60] buildman: Convert camel case in cmdline.py

[Simon Glass]

Convert this file to snake case and update all files which use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/cmdline.py   | 2 +-
 tools/buildman/func_test.py | 2 +-
 tools/buildman/main.py      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index ea43685e394d..3066a0b38bbf 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -9,7 +9,7 @@ import pathlib
 BUILDMAN_DIR = pathlib.Path(__file__).parent
 HAS_TESTS = os.path.exists(BUILDMAN_DIR / "test.py")
 
-def ParseArgs():
+def parse_args():
     """Parse command line arguments from sys.argv[]
 
     Returns:
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 5dbbb53b6572..412718cc3db9 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -244,7 +244,7 @@ class TestFunctional(unittest.TestCase):
             result code from buildman
         """
         sys.argv = [sys.argv[0]] + list(args)
-        options, args = cmdline.ParseArgs()
+        options, args = cmdline.parse_args()
         if brds == False:
             brds = self._boards
         result = control.do_buildman(
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 2253401709a8..0f711c8653b3 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -59,7 +59,7 @@ def run_buildman():
     This is the main program. It collects arguments and runs either the tests or
     the control module.
     """
-    options, args = cmdline.ParseArgs()
+    options, args = cmdline.parse_args()
 
     if not options.debug:
         sys.tracebacklimit = 0
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 33/60] buildman: Correct most pylint warnings in cmdline

[Simon Glass]

Tidu up warnings in this file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/cmdline.py | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 3066a0b38bbf..91571a5adb1d 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -2,6 +2,11 @@
 # Copyright (c) 2014 Google, Inc
 #
 
+"""Handles parsing of buildman arguments
+
+This creates the argument parser and uses it to parse the arguments passed in
+"""
+
 from optparse import OptionParser
 import os
 import pathlib
@@ -71,7 +76,8 @@ def parse_args():
     parser.add_option('-k', '--keep-outputs', action='store_true',
           default=False, help='Keep all build output files (e.g. binaries)')
     parser.add_option('-K', '--show-config', action='store_true',
-          default=False, help='Show configuration changes in summary (both board config files and Kconfig)')
+          default=False,
+          help='Show configuration changes in summary (both board config files and Kconfig)')
     parser.add_option('--preserve-config-y', action='store_true',
           default=False, help="Don't convert y to 1 in configs")
     parser.add_option('-l', '--list-error-boards', action='store_true',
@@ -84,14 +90,15 @@ def parse_args():
           default=False, help="Run 'make mrproper before reconfiguring")
     parser.add_option(
           '-M', '--allow-missing', action='store_true', default=False,
-          help='Tell binman to allow missing blobs and generate fake ones as needed'),
+          help='Tell binman to allow missing blobs and generate fake ones as needed')
     parser.add_option(
           '--no-allow-missing', action='store_true', default=False,
-          help='Disable telling binman to allow missing blobs'),
+          help='Disable telling binman to allow missing blobs')
     parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
           default=False, help="Do a dry run (describe actions, but do nothing)")
     parser.add_option('-N', '--no-subdirs', action='store_true', dest='no_subdirs',
-          default=False, help="Don't create subdirectories when building current source for a single board")
+          default=False,
+          help="Don't create subdirectories when building current source for a single board")
     parser.add_option('-o', '--output-dir', type='string', dest='output_dir',
           help='Directory where all builds happen and buildman has its workspace (default is ../)')
     parser.add_option('-O', '--override-toolchain', type='string',
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 34/60] buildman: Add a test for --boards

[Simon Glass]

Add a simple functional test for the --boards option. Fix the example in
the docs while we are here. Also improve the docs for Builder.count so it
is clearer what it contains.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Use snake case for tests

 tools/buildman/builder.py   |  3 ++-
 tools/buildman/buildman.rst |  2 +-
 tools/buildman/func_test.py | 19 +++++++++++++++++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index cb3628a8a085..620b7b8c31a8 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -163,7 +163,8 @@ class Builder:
         checkout: True to check out source, False to skip that step.
             This is used for testing.
         col: terminal.Color() object
-        count: Number of commits to build
+        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
         fail: Number of builds that failed due to error
         force_build: Force building even if a build already exists
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index b28aea477dfe..d8c3957097e8 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -159,7 +159,7 @@ on the command line:
 
 .. code-block:: bash
 
-  buildman --boards sandbox,snow --boards
+  buildman --boards sandbox,snow --boards firefly-rk3399
 
 It is convenient to use the -n option to see what will be built based on
 the subset given. Use -v as well to get an actual list of boards.
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 412718cc3db9..c0c35408c839 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -791,3 +791,22 @@ CONFIG_LOCALVERSION=y
             result = self._RunControl('-A', 'board0')
         self.assertEqual('arm-\n', stdout.getvalue())
         self.assertEqual('', stderr.getvalue())
+
+    def test_regen_boards(self):
+        """Test that we can regenerate the boards.cfg file"""
+        outfile = os.path.join(self._output_dir, 'test-boards.cfg')
+        if os.path.exists(outfile):
+            os.remove(outfile)
+        result = self._RunControl('-R', outfile, brds=None, get_builder=False)
+        self.assertTrue(os.path.exists(outfile))
+
+    def test_single_boards(self):
+        """Test building single boards"""
+        self._RunControl('--boards', 'board1')
+        self.assertEqual(1, self._builder.count)
+
+        self._RunControl('--boards', 'board1', '--boards', 'board2')
+        self.assertEqual(2, self._builder.count)
+
+        self._RunControl('--boards', 'board1,board2', '--boards', 'board4')
+        self.assertEqual(3, self._builder.count)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 35/60] buildman: Convert to argparse

[Simon Glass]

Use argparse to parse the arguments, since OptionParser is deprecated now.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/cmdline.py   | 125 ++++++++++++++++----------------
 tools/buildman/control.py   | 140 ++++++++++++++++++------------------
 tools/buildman/func_test.py |   6 +-
 tools/buildman/main.py      |  16 ++---
 4 files changed, 145 insertions(+), 142 deletions(-)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 91571a5adb1d..6b9c1db14af7 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -7,7 +7,7 @@
 This creates the argument parser and uses it to parse the arguments passed in
 """
 
-from optparse import OptionParser
+import argparse
 import os
 import pathlib
 
@@ -22,135 +22,138 @@ def parse_args():
             options: command line options
             args: command lin arguments
     """
-    parser = OptionParser()
-    parser.add_option('-a', '--adjust-cfg', type=str, action='append',
+    epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+
+    Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+
+    parser = argparse.ArgumentParser(epilog=epilog)
+    parser.add_argument('-a', '--adjust-cfg', type=str, action='append',
           help='Adjust the Kconfig settings in .config before building')
-    parser.add_option('-A', '--print-prefix', action='store_true',
+    parser.add_argument('-A', '--print-prefix', action='store_true',
           help='Print the tool-chain prefix for a board (CROSS_COMPILE=)')
-    parser.add_option('-b', '--branch', type='string',
+    parser.add_argument('-b', '--branch', type=str,
           help='Branch name to build, or range of commits to build')
-    parser.add_option('-B', '--bloat', dest='show_bloat',
+    parser.add_argument('-B', '--bloat', dest='show_bloat',
           action='store_true', default=False,
           help='Show changes in function code size for each board')
-    parser.add_option('--boards', type='string', action='append',
+    parser.add_argument('--boards', type=str, action='append',
           help='List of board names to build separated by comma')
-    parser.add_option('-c', '--count', dest='count', type='int',
+    parser.add_argument('-c', '--count', dest='count', type=int,
           default=-1, help='Run build on the top n commits')
-    parser.add_option('-C', '--force-reconfig', dest='force_reconfig',
+    parser.add_argument('-C', '--force-reconfig', dest='force_reconfig',
           action='store_true', default=False,
           help='Reconfigure for every commit (disable incremental build)')
-    parser.add_option('-d', '--detail', dest='show_detail',
+    parser.add_argument('-d', '--detail', dest='show_detail',
           action='store_true', default=False,
           help='Show detailed size delta for each board in the -S summary')
-    parser.add_option('-D', '--config-only', action='store_true', default=False,
+    parser.add_argument('-D', '--config-only', action='store_true',
+                        default=False,
           help="Don't build, just configure each commit")
-    parser.add_option('--debug', action='store_true',
+    parser.add_argument('--debug', action='store_true',
         help='Enabling debugging (provides a full traceback on error)')
-    parser.add_option('-e', '--show_errors', action='store_true',
+    parser.add_argument('-e', '--show_errors', action='store_true',
           default=False, help='Show errors and warnings')
-    parser.add_option('-E', '--warnings-as-errors', action='store_true',
+    parser.add_argument('-E', '--warnings-as-errors', action='store_true',
           default=False, help='Treat all compiler warnings as errors')
-    parser.add_option('-f', '--force-build', dest='force_build',
+    parser.add_argument('-f', '--force-build', dest='force_build',
           action='store_true', default=False,
           help='Force build of boards even if already built')
-    parser.add_option('-F', '--force-build-failures', dest='force_build_failures',
+    parser.add_argument('-F', '--force-build-failures', dest='force_build_failures',
           action='store_true', default=False,
           help='Force build of previously-failed build')
-    parser.add_option('--fetch-arch', type='string',
+    parser.add_argument('--fetch-arch', type=str,
           help="Fetch a toolchain for architecture FETCH_ARCH ('list' to list)."
               ' You can also fetch several toolchains separate by comma, or'
               " 'all' to download all")
-    parser.add_option('-g', '--git', type='string',
+    parser.add_argument('-g', '--git', type=str,
           help='Git repo containing branch to build', default='.')
-    parser.add_option('-G', '--config-file', type='string',
+    parser.add_argument('-G', '--config-file', type=str,
           help='Path to buildman config file', default='')
-    parser.add_option('-H', '--full-help', action='store_true', dest='full_help',
+    parser.add_argument('-H', '--full-help', action='store_true', dest='full_help',
           default=False, help='Display the README file')
-    parser.add_option('-i', '--in-tree', dest='in_tree',
+    parser.add_argument('-i', '--in-tree', dest='in_tree',
           action='store_true', default=False,
           help='Build in the source tree instead of a separate directory')
-    parser.add_option('-I', '--ide', action='store_true', default=False,
+    parser.add_argument('-I', '--ide', action='store_true', default=False,
           help='Create build output that can be parsed by an IDE')
-    parser.add_option('-j', '--jobs', dest='jobs', type='int',
+    parser.add_argument('-j', '--jobs', dest='jobs', type=int,
           default=None, help='Number of jobs to run at once (passed to make)')
-    parser.add_option('-k', '--keep-outputs', action='store_true',
+    parser.add_argument('-k', '--keep-outputs', action='store_true',
           default=False, help='Keep all build output files (e.g. binaries)')
-    parser.add_option('-K', '--show-config', action='store_true',
+    parser.add_argument('-K', '--show-config', action='store_true',
           default=False,
           help='Show configuration changes in summary (both board config files and Kconfig)')
-    parser.add_option('--preserve-config-y', action='store_true',
+    parser.add_argument('--preserve-config-y', action='store_true',
           default=False, help="Don't convert y to 1 in configs")
-    parser.add_option('-l', '--list-error-boards', action='store_true',
+    parser.add_argument('-l', '--list-error-boards', action='store_true',
           default=False, help='Show a list of boards next to each error/warning')
-    parser.add_option('-L', '--no-lto', action='store_true',
+    parser.add_argument('-L', '--no-lto', action='store_true',
           default=False, help='Disable Link-time Optimisation (LTO) for builds')
-    parser.add_option('--list-tool-chains', action='store_true', default=False,
+    parser.add_argument('--list-tool-chains', action='store_true', default=False,
           help='List available tool chains (use -v to see probing detail)')
-    parser.add_option('-m', '--mrproper', action='store_true',
+    parser.add_argument('-m', '--mrproper', action='store_true',
           default=False, help="Run 'make mrproper before reconfiguring")
-    parser.add_option(
+    parser.add_argument(
           '-M', '--allow-missing', action='store_true', default=False,
           help='Tell binman to allow missing blobs and generate fake ones as needed')
-    parser.add_option(
+    parser.add_argument(
           '--no-allow-missing', action='store_true', default=False,
           help='Disable telling binman to allow missing blobs')
-    parser.add_option('-n', '--dry-run', action='store_true', dest='dry_run',
+    parser.add_argument('-n', '--dry-run', action='store_true', dest='dry_run',
           default=False, help="Do a dry run (describe actions, but do nothing)")
-    parser.add_option('-N', '--no-subdirs', action='store_true', dest='no_subdirs',
+    parser.add_argument('-N', '--no-subdirs', action='store_true', dest='no_subdirs',
           default=False,
           help="Don't create subdirectories when building current source for a single board")
-    parser.add_option('-o', '--output-dir', type='string', dest='output_dir',
+    parser.add_argument('-o', '--output-dir', type=str, dest='output_dir',
           help='Directory where all builds happen and buildman has its workspace (default is ../)')
-    parser.add_option('-O', '--override-toolchain', type='string',
+    parser.add_argument('-O', '--override-toolchain', type=str,
           help="Override host toochain to use for sandbox (e.g. 'clang-7')")
-    parser.add_option('-Q', '--quick', action='store_true',
+    parser.add_argument('-Q', '--quick', action='store_true',
           default=False, help='Do a rough build, with limited warning resolution')
-    parser.add_option('-p', '--full-path', action='store_true',
+    parser.add_argument('-p', '--full-path', action='store_true',
           default=False, help="Use full toolchain path in CROSS_COMPILE")
-    parser.add_option('-P', '--per-board-out-dir', action='store_true',
+    parser.add_argument('-P', '--per-board-out-dir', action='store_true',
           default=False, help="Use an O= (output) directory per board rather than per thread")
-    parser.add_option('-r', '--reproducible-builds', action='store_true',
+    parser.add_argument('-r', '--reproducible-builds', action='store_true',
           help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build')
-    parser.add_option('-R', '--regen-board-list', type='string',
+    parser.add_argument('-R', '--regen-board-list', type=str,
           help='Force regeneration of the list of boards, like the old boards.cfg file')
-    parser.add_option('-s', '--summary', action='store_true',
+    parser.add_argument('-s', '--summary', action='store_true',
           default=False, help='Show a build summary')
-    parser.add_option('-S', '--show-sizes', action='store_true',
+    parser.add_argument('-S', '--show-sizes', action='store_true',
           default=False, help='Show image size variation in summary')
-    parser.add_option('--step', type='int',
+    parser.add_argument('--step', type=int,
           default=1, help='Only build every n commits (0=just first and last)')
     if HAS_TESTS:
-        parser.add_option('--skip-net-tests', action='store_true', default=False,
+        parser.add_argument('--skip-net-tests', action='store_true', default=False,
                           help='Skip tests which need the network')
-        parser.add_option('-t', '--test', action='store_true', dest='test',
+        parser.add_argument('-t', '--test', action='store_true', dest='test',
                           default=False, help='run tests')
-    parser.add_option('-T', '--threads', type='int',
+    parser.add_argument('-T', '--threads', type=int,
           default=None,
           help='Number of builder threads to use (0=single-thread)')
-    parser.add_option('-u', '--show_unknown', action='store_true',
+    parser.add_argument('-u', '--show_unknown', action='store_true',
           default=False, help='Show boards with unknown build result')
-    parser.add_option('-U', '--show-environment', action='store_true',
+    parser.add_argument('-U', '--show-environment', action='store_true',
           default=False, help='Show environment changes in summary')
-    parser.add_option('-v', '--verbose', action='store_true',
+    parser.add_argument('-v', '--verbose', action='store_true',
           default=False, help='Show build results while the build progresses')
-    parser.add_option('-V', '--verbose-build', action='store_true',
+    parser.add_argument('-V', '--verbose-build', action='store_true',
           default=False, help='Run make with V=1, logging all output')
-    parser.add_option('-w', '--work-in-output', action='store_true',
+    parser.add_argument('-w', '--work-in-output', action='store_true',
           default=False, help='Use the output directory as the work directory')
-    parser.add_option('-W', '--ignore-warnings', action='store_true',
+    parser.add_argument('-W', '--ignore-warnings', action='store_true',
           default=False, help='Return success even if there are warnings')
-    parser.add_option('-x', '--exclude', dest='exclude',
-          type='string', action='append',
+    parser.add_argument('-x', '--exclude', dest='exclude',
+          type=str, action='append',
           help='Specify a list of boards to exclude, separated by comma')
-    parser.add_option('-y', '--filter-dtb-warnings', action='store_true',
+    parser.add_argument('-y', '--filter-dtb-warnings', action='store_true',
           default=False,
           help='Filter out device-tree-compiler warnings from output')
-    parser.add_option('-Y', '--filter-migration-warnings', action='store_true',
+    parser.add_argument('-Y', '--filter-migration-warnings', action='store_true',
           default=False,
           help='Filter out migration warnings from output')
-
-    parser.usage += """ [list of target/arch/cpu/board/vendor/soc to build]
-
-    Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+    parser.add_argument('terms', type=str, nargs='*',
+                        help='Board / SoC names to build')
 
     return parser.parse_args()
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index bf4843c4170d..e67d410b6295 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -421,35 +421,35 @@ def determine_boards(brds, args, col, opt_boards, exclude):
     return selected, why_selected, board_warnings
 
 
-def adjust_options(options, series, selected):
-    """Adjust options according to various constraints
+def adjust_args(args, series, selected):
+    """Adjust arguments according to various constraints
 
     Updates verbose, show_errors, threads, jobs and step
 
     Args:
-        options (Options): Options object to adjust
+        args (Namespace): Namespace object to adjust
         series (Series): Series being built / summarised
         selected (list of Board): List of Board objects that are marked
     """
-    if not series and not options.dry_run:
-        options.verbose = True
-        if not options.summary:
-            options.show_errors = True
+    if not series and not args.dry_run:
+        args.verbose = True
+        if not args.summary:
+            args.show_errors = True
 
     # By default we have one thread per CPU. But if there are not enough jobs
     # we can have fewer threads and use a high '-j' value for make.
-    if options.threads is None:
-        options.threads = min(multiprocessing.cpu_count(), len(selected))
-    if not options.jobs:
-        options.jobs = max(1, (multiprocessing.cpu_count() +
+    if args.threads is None:
+        args.threads = min(multiprocessing.cpu_count(), len(selected))
+    if not args.jobs:
+        args.jobs = max(1, (multiprocessing.cpu_count() +
                 len(selected) - 1) // len(selected))
 
-    if not options.step:
-        options.step = len(series.commits) - 1
+    if not args.step:
+        args.step = len(series.commits) - 1
 
     # We can't show function sizes without board details at present
-    if options.show_bloat:
-        options.show_detail = True
+    if args.show_bloat:
+        args.show_detail = True
 
 
 def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
@@ -481,7 +481,7 @@ def setup_output_dir(output_dir, work_in_output, branch, no_subdirs, col,
     return output_dir
 
 
-def run_builder(builder, commits, board_selected, options):
+def run_builder(builder, commits, board_selected, args):
     """Run the builder or show the summary
 
     Args:
@@ -489,37 +489,37 @@ def run_builder(builder, commits, board_selected, options):
         boards_selected (dict): Dict of selected boards:
             key: target name
             value: Board object
-        options (Options): Options to use
+        args (Namespace): Namespace to use
 
     Returns:
         int: Return code for buildman
     """
-    gnu_make = command.output(os.path.join(options.git,
+    gnu_make = command.output(os.path.join(args.git,
             'scripts/show-gnu-make'), raise_on_error=False).rstrip()
     if not gnu_make:
         sys.exit('GNU Make not found')
     builder.gnu_make = gnu_make
 
-    if not options.ide:
-        commit_count = count_build_commits(commits, options.step)
-        tprint(get_action_summary(options.summary, commit_count, board_selected,
-                                  options.threads, options.jobs))
+    if not args.ide:
+        commit_count = count_build_commits(commits, args.step)
+        tprint(get_action_summary(args.summary, commit_count, board_selected,
+                                  args.threads, args.jobs))
 
     builder.SetDisplayOptions(
-        options.show_errors, options.show_sizes, options.show_detail,
-        options.show_bloat, options.list_error_boards, options.show_config,
-        options.show_environment, options.filter_dtb_warnings,
-        options.filter_migration_warnings, options.ide)
-    if options.summary:
+        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)
+    if args.summary:
         builder.ShowSummary(commits, board_selected)
     else:
         fail, warned, excs = builder.BuildBoards(
-            commits, board_selected, options.keep_outputs, options.verbose)
+            commits, board_selected, args.keep_outputs, args.verbose)
         if excs:
             return 102
         if fail:
             return 100
-        if warned and not options.ignore_warnings:
+        if warned and not args.ignore_warnings:
             return 101
     return 0
 
@@ -548,12 +548,12 @@ def calc_adjust_cfg(adjust_cfg, reproducible_builds):
     return adjust_cfg
 
 
-def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
+def do_buildman(args, toolchains=None, make_func=None, brds=None,
                 clean_dir=False, test_thread_exceptions=False):
     """The main control code for buildman
 
     Args:
-        options: Command line options object
+        args: ArgumentParser object
         args: Command line arguments (list of strings)
         toolchains: Toolchains to use - this should be a Toolchains()
                 object. If None, then it will be created and scanned
@@ -575,67 +575,67 @@ def do_buildman(options, args, toolchains=None, make_func=None, brds=None,
     gitutil.setup()
     col = terminal.Color()
 
-    git_dir = os.path.join(options.git, '.git')
+    git_dir = os.path.join(args.git, '.git')
 
-    toolchains = get_toolchains(toolchains, col, options.override_toolchain,
-                                options.fetch_arch, options.list_tool_chains,
-                                options.verbose)
+    toolchains = get_toolchains(toolchains, col, args.override_toolchain,
+                                args.fetch_arch, args.list_tool_chains,
+                                args.verbose)
     output_dir = setup_output_dir(
-        options.output_dir, options.work_in_output, options.branch,
-        options.no_subdirs, col, clean_dir)
+        args.output_dir, args.work_in_output, args.branch,
+        args.no_subdirs, col, clean_dir)
 
     # Work out what subset of the boards we are building
     if not brds:
-        brds = get_boards_obj(output_dir, options.regen_board_list,
-                              options.threads, options.verbose)
+        brds = get_boards_obj(output_dir, args.regen_board_list,
+                              args.threads, args.verbose)
         if isinstance(brds, int):
             return brds
 
     selected, why_selected, board_warnings = determine_boards(
-        brds, args, col, options.boards, options.exclude)
+        brds, args.terms, col, args.boards, args.exclude)
 
-    if options.print_prefix:
+    if args.print_prefix:
         show_toolchain_prefix(brds, toolchains)
         return 0
 
-    series = determine_series(selected, col, git_dir, options.count,
-                              options.branch, options.work_in_output)
+    series = determine_series(selected, col, git_dir, args.count,
+                              args.branch, args.work_in_output)
 
-    adjust_options(options, series, selected)
+    adjust_args(args, series, selected)
 
     # For a dry run, just show our actions as a sanity check
-    if options.dry_run:
+    if args.dry_run:
         show_actions(series, why_selected, selected, output_dir, board_warnings,
-                     options.step, options.threads, options.jobs,
-                     options.verbose)
+                     args.step, args.threads, args.jobs,
+                     args.verbose)
         return 0
 
-    # Create a new builder with the selected options
+    # Create a new builder with the selected args
     builder = Builder(toolchains, output_dir, git_dir,
-            options.threads, options.jobs, checkout=True,
-            show_unknown=options.show_unknown, step=options.step,
-            no_subdirs=options.no_subdirs, full_path=options.full_path,
-            verbose_build=options.verbose_build,
-            mrproper=options.mrproper,
-            per_board_out_dir=options.per_board_out_dir,
-            config_only=options.config_only,
-            squash_config_y=not options.preserve_config_y,
-            warnings_as_errors=options.warnings_as_errors,
-            work_in_output=options.work_in_output,
+            args.threads, args.jobs, checkout=True,
+            show_unknown=args.show_unknown, step=args.step,
+            no_subdirs=args.no_subdirs, full_path=args.full_path,
+            verbose_build=args.verbose_build,
+            mrproper=args.mrproper,
+            per_board_out_dir=args.per_board_out_dir,
+            config_only=args.config_only,
+            squash_config_y=not args.preserve_config_y,
+            warnings_as_errors=args.warnings_as_errors,
+            work_in_output=args.work_in_output,
             test_thread_exceptions=test_thread_exceptions,
-            adjust_cfg=calc_adjust_cfg(options.adjust_cfg,
-                                       options.reproducible_builds),
-            allow_missing=get_allow_missing(options.allow_missing,
-                                            options.no_allow_missing,
-                                            len(selected), options.branch),
-            no_lto=options.no_lto,
-            reproducible_builds=options.reproducible_builds,
-            force_build = options.force_build,
-            force_build_failures = options.force_build_failures,
-            force_reconfig = options.force_reconfig, in_tree = options.in_tree,
-            force_config_on_failure=not options.quick, make_func=make_func)
+            adjust_cfg=calc_adjust_cfg(args.adjust_cfg,
+                                       args.reproducible_builds),
+            allow_missing=get_allow_missing(args.allow_missing,
+                                            args.no_allow_missing,
+                                            len(selected), args.branch),
+            no_lto=args.no_lto,
+            reproducible_builds=args.reproducible_builds,
+            force_build = args.force_build,
+            force_build_failures = args.force_build_failures,
+            force_reconfig = args.force_reconfig, in_tree = args.in_tree,
+            force_config_on_failure=not args.quick, make_func=make_func)
 
     TEST_BUILDER = builder
 
     return run_builder(builder, series.commits if series else None,
-                       brds.get_selected_dict(), options)
+                       brds.get_selected_dict(), args)
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index c0c35408c839..8d15e3a9cce2 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -244,12 +244,12 @@ class TestFunctional(unittest.TestCase):
             result code from buildman
         """
         sys.argv = [sys.argv[0]] + list(args)
-        options, args = cmdline.parse_args()
+        args = cmdline.parse_args()
         if brds == False:
             brds = self._boards
         result = control.do_buildman(
-            options, args, toolchains=self._toolchains,
-            make_func=self._HandleMake, brds=brds, clean_dir=clean_dir,
+            args, toolchains=self._toolchains, make_func=self._HandleMake,
+            brds=brds, clean_dir=clean_dir,
             test_thread_exceptions=test_thread_exceptions)
         if get_builder:
             self._builder = control.TEST_BUILDER
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 0f711c8653b3..8ce4d8678abc 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -40,7 +40,7 @@ def run_tests(skip_net_tests, verbose, args):
     from buildman import func_test
     from buildman import test
 
-    test_name = args and args[0] or None
+    test_name = args.terms and args.terms[0] or None
     if skip_net_tests:
         test.use_network = False
 
@@ -59,22 +59,22 @@ def run_buildman():
     This is the main program. It collects arguments and runs either the tests or
     the control module.
     """
-    options, args = cmdline.parse_args()
+    args = cmdline.parse_args()
 
-    if not options.debug:
+    if not args.debug:
         sys.tracebacklimit = 0
 
     # Run our meagre tests
-    if cmdline.HAS_TESTS and options.test:
-        run_tests(options.skip_net_tests, options.verbose, args)
+    if cmdline.HAS_TESTS and args.test:
+        run_tests(args.skip_net_tests, args.verbose, args)
 
-    elif options.full_help:
+    elif args.full_help:
         tools.print_full_help(str(files('buildman').joinpath('README.rst')))
 
     # Build selected commits for selected boards
     else:
-        bsettings.Setup(options.config_file)
-        ret_code = control.do_buildman(options, args)
+        bsettings.Setup(args.config_file)
+        ret_code = control.do_buildman(args)
         sys.exit(ret_code)
 
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 36/60] buildman: Convert camel case in bsettings.py

[Simon Glass]

Convert this file to snake case and update all files which use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/bsettings.py | 14 +++++++-------
 tools/buildman/control.py   |  2 +-
 tools/buildman/func_test.py | 12 ++++++------
 tools/buildman/main.py      |  2 +-
 tools/buildman/test.py      |  4 ++--
 tools/buildman/toolchain.py | 14 +++++++-------
 tools/moveconfig.py         |  2 +-
 7 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index 0eb894a558c1..612ec0c28464 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -7,7 +7,7 @@ import io
 
 config_fname = None
 
-def Setup(fname=''):
+def setup(fname=''):
     """Set up the buildman settings module by reading config files
 
     Args:
@@ -23,15 +23,15 @@ def Setup(fname=''):
             config_fname = '%s/.buildman' % os.getenv('HOME')
         if not os.path.exists(config_fname):
             print('No config file found ~/.buildman\nCreating one...\n')
-            CreateBuildmanConfigFile(config_fname)
+            create_buildman_config_file(config_fname)
             print('To install tool chains, please use the --fetch-arch option')
         if config_fname:
             settings.read(config_fname)
 
-def AddFile(data):
+def add_file(data):
     settings.readfp(io.StringIO(data))
 
-def GetItems(section):
+def get_items(section):
     """Get the items from a section of the config.
 
     Args:
@@ -47,7 +47,7 @@ def GetItems(section):
     except:
         raise
 
-def GetGlobalItemValue(name):
+def get_global_item_value(name):
     """Get an item from the 'global' section of the config.
 
     Args:
@@ -58,7 +58,7 @@ def GetGlobalItemValue(name):
     """
     return settings.get('global', name, fallback=None)
 
-def SetItem(section, tag, value):
+def set_item(section, tag, value):
     """Set an item and write it back to the settings file"""
     global settings
     global config_fname
@@ -68,7 +68,7 @@ def SetItem(section, tag, value):
         with open(config_fname, 'w') as fd:
             settings.write(fd)
 
-def CreateBuildmanConfigFile(config_fname):
+def create_buildman_config_file(config_fname):
     """Creates a new config file with no tool chain information.
 
     Args:
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index e67d410b6295..2b25fb2abf8b 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -157,7 +157,7 @@ def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
             external blobs are used
     """
     allow_missing = False
-    am_setting = bsettings.GetGlobalItemValue('allow-missing')
+    am_setting = bsettings.get_global_item_value('allow-missing')
     if am_setting:
         if am_setting == 'always':
             allow_missing = True
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 8d15e3a9cce2..7162d8c2fcc0 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -184,8 +184,8 @@ class TestFunctional(unittest.TestCase):
         self._buildman_pathname = sys.argv[0]
         self._buildman_dir = os.path.dirname(os.path.realpath(sys.argv[0]))
         command.test_result = self._HandleCommand
-        bsettings.Setup(None)
-        bsettings.AddFile(settings_data)
+        bsettings.setup(None)
+        bsettings.add_file(settings_data)
         self.setupToolchains()
         self._toolchains.Add('arm-gcc', test=False)
         self._toolchains.Add('powerpc-gcc', test=False)
@@ -691,7 +691,7 @@ Some images are invalid'''
 
     def testBlobSettingsAlways(self):
         """Test the 'always' policy"""
-        bsettings.SetItem('global', 'allow-missing', 'always')
+        bsettings.set_item('global', 'allow-missing', 'always')
         self.assertEqual(True,
                          control.get_allow_missing(False, False, 1, False))
         self.assertEqual(False,
@@ -699,7 +699,7 @@ Some images are invalid'''
 
     def testBlobSettingsBranch(self):
         """Test the 'branch' policy"""
-        bsettings.SetItem('global', 'allow-missing', 'branch')
+        bsettings.set_item('global', 'allow-missing', 'branch')
         self.assertEqual(False,
                          control.get_allow_missing(False, False, 1, False))
         self.assertEqual(True,
@@ -709,7 +709,7 @@ Some images are invalid'''
 
     def testBlobSettingsMultiple(self):
         """Test the 'multiple' policy"""
-        bsettings.SetItem('global', 'allow-missing', 'multiple')
+        bsettings.set_item('global', 'allow-missing', 'multiple')
         self.assertEqual(False,
                          control.get_allow_missing(False, False, 1, False))
         self.assertEqual(True,
@@ -719,7 +719,7 @@ Some images are invalid'''
 
     def testBlobSettingsBranchMultiple(self):
         """Test the 'branch multiple' policy"""
-        bsettings.SetItem('global', 'allow-missing', 'branch multiple')
+        bsettings.set_item('global', 'allow-missing', 'branch multiple')
         self.assertEqual(False,
                          control.get_allow_missing(False, False, 1, False))
         self.assertEqual(True,
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 8ce4d8678abc..593911353101 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -73,7 +73,7 @@ def run_buildman():
 
     # Build selected commits for selected boards
     else:
-        bsettings.Setup(args.config_file)
+        bsettings.setup(args.config_file)
         ret_code = control.do_buildman(args)
         sys.exit(ret_code)
 
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 7eb25aa80eba..3f55035ea2e4 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -138,8 +138,8 @@ class TestBuild(unittest.TestCase):
         self.brds.select_boards([])
 
         # Add some test settings
-        bsettings.Setup(None)
-        bsettings.AddFile(settings_data)
+        bsettings.setup(None)
+        bsettings.add_file(settings_data)
 
         # Set up the toolchains
         self.toolchains = toolchain.Toolchains()
diff --git a/tools/buildman/toolchain.py b/tools/buildman/toolchain.py
index 0ecd8458b912..57bf6149ca61 100644
--- a/tools/buildman/toolchain.py
+++ b/tools/buildman/toolchain.py
@@ -139,7 +139,7 @@ class Toolchain:
         """Get toolchain wrapper from the setting file.
         """
         value = ''
-        for name, value in bsettings.GetItems('toolchain-wrapper'):
+        for name, value in bsettings.get_items('toolchain-wrapper'):
             if not value:
                 print("Warning: Wrapper not found")
         if value:
@@ -249,7 +249,7 @@ class Toolchains:
         self.prefixes = {}
         self.paths = []
         self.override_toolchain = override_toolchain
-        self._make_flags = dict(bsettings.GetItems('make-flags'))
+        self._make_flags = dict(bsettings.get_items('make-flags'))
 
     def GetPathList(self, show_warning=True):
         """Get a list of available toolchain paths
@@ -261,7 +261,7 @@ class Toolchains:
             List of strings, each a path to a toolchain mentioned in the
             [toolchain] section of the settings file.
         """
-        toolchains = bsettings.GetItems('toolchain')
+        toolchains = bsettings.get_items('toolchain')
         if show_warning and not toolchains:
             print(("Warning: No tool chains. Please run 'buildman "
                    "--fetch-arch all' to download all available toolchains, or "
@@ -283,7 +283,7 @@ class Toolchains:
         Args:
             show_warning: True to show a warning if there are no tool chains.
         """
-        self.prefixes = bsettings.GetItems('toolchain-prefix')
+        self.prefixes = bsettings.get_items('toolchain-prefix')
         self.paths += self.GetPathList(show_warning)
 
     def Add(self, fname, test=True, verbose=False, priority=PRIORITY_CALC,
@@ -399,7 +399,7 @@ class Toolchains:
         returns:
             toolchain object, or None if none found
         """
-        for tag, value in bsettings.GetItems('toolchain-alias'):
+        for tag, value in bsettings.get_items('toolchain-alias'):
             if arch == tag:
                 for alias in value.split():
                     if alias in self.toolchains:
@@ -421,7 +421,7 @@ class Toolchains:
         Returns:
             Resolved string
 
-        >>> bsettings.Setup(None)
+        >>> bsettings.setup(None)
         >>> tcs = Toolchains()
         >>> tcs.Add('fred', False)
         >>> var_dict = {'oblique' : 'OBLIQUE', 'first' : 'fi${second}rst', \
@@ -598,5 +598,5 @@ class Toolchains:
         if not self.TestSettingsHasPath(dirpath):
             print(("Adding 'download' to config file '%s'" %
                    bsettings.config_fname))
-            bsettings.SetItem('toolchain', 'download', '%s/*/*' % dest)
+            bsettings.set_item('toolchain', 'download', '%s/*/*' % dest)
         return 0
diff --git a/tools/moveconfig.py b/tools/moveconfig.py
index c4d72ede3684..6cbecc3d5c80 100755
--- a/tools/moveconfig.py
+++ b/tools/moveconfig.py
@@ -2037,7 +2037,7 @@ doc/develop/moveconfig.rst for documentation.'''
 
     if not args.cleanup_headers_only:
         check_clean_directory()
-        bsettings.Setup('')
+        bsettings.setup('')
         toolchains = toolchain.Toolchains()
         toolchains.GetSettings()
         toolchains.Scan(verbose=False)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 37/60] buildman: Convert camel case in builder.py

[Simon Glass]

Convert this file to snake case and update all files which use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builder.py       | 228 ++++++++++++++++----------------
 tools/buildman/builderthread.py |  26 ++--
 tools/buildman/control.py       |  13 +-
 tools/buildman/func_test.py     |   2 +-
 tools/buildman/test.py          |  22 +--
 5 files changed, 145 insertions(+), 146 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 620b7b8c31a8..80c05fbd1e73 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -134,7 +134,7 @@ class Config:
         for fname in config_filename:
             self.config[fname] = {}
 
-    def Add(self, fname, key, value):
+    def add(self, fname, key, value):
         self.config[fname][key] = value
 
     def __hash__(self):
@@ -151,7 +151,7 @@ class Environment:
         self.target = target
         self.environment = {}
 
-    def Add(self, key, value):
+    def add(self, key, value):
         self.environment[key] = value
 
 class Builder:
@@ -315,7 +315,7 @@ class Builder:
         else:
             self._working_dir = os.path.join(base_dir, '.bm-work')
         self.threads = []
-        self.do_make = make_func or self.Make
+        self.do_make = make_func or self.make
         self.gnu_make = gnu_make
         self.checkout = checkout
         self.num_threads = num_threads
@@ -401,7 +401,7 @@ class Builder:
     def signal_handler(self, signal, frame):
         sys.exit(1)
 
-    def SetDisplayOptions(self, show_errors=False, show_sizes=False,
+    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,
@@ -434,7 +434,7 @@ class Builder:
         self._filter_migration_warnings = filter_migration_warnings
         self._ide = ide
 
-    def _AddTimestamp(self):
+    def _add_timestamp(self):
         """Add a new timestamp to the list and record the build period.
 
         The build period is the length of time taken to perform a single
@@ -463,14 +463,14 @@ class Builder:
             self._timestamps.popleft()
             count -= 1
 
-    def SelectCommit(self, commit, checkout=True):
+    def select_commit(self, commit, checkout=True):
         """Checkout the selected commit for this build
         """
         self.commit = commit
         if checkout and self.checkout:
             gitutil.checkout(commit.hash)
 
-    def Make(self, commit, brd, stage, cwd, *args, **kwargs):
+    def make(self, commit, brd, stage, cwd, *args, **kwargs):
         """Run make
 
         Args:
@@ -515,7 +515,7 @@ class Builder:
             result.combined = '%s\n' % (' '.join(cmd)) + result.combined
         return result
 
-    def ProcessResult(self, result):
+    def process_result(self, result):
         """Process the result of a build, showing progress information
 
         Args:
@@ -536,8 +536,8 @@ class Builder:
             if self._verbose:
                 terminal.print_clear()
                 boards_selected = {target : result.brd}
-                self.ResetResultSummary(boards_selected)
-                self.ProduceResultSummary(result.commit_upto, self.commits,
+                self.reset_result_summary(boards_selected)
+                self.produce_result_summary(result.commit_upto, self.commits,
                                           boards_selected)
         else:
             target = '(starting)'
@@ -556,7 +556,7 @@ class Builder:
             line += ' ' * 8
 
         # Add our current completion time estimate
-        self._AddTimestamp()
+        self._add_timestamp()
         if self._complete_delay:
             line += '%s  : ' % self._complete_delay
 
@@ -565,7 +565,7 @@ class Builder:
             terminal.print_clear()
             tprint(line, newline=False, limit_to_line=True)
 
-    def _GetOutputDir(self, commit_upto):
+    def _get_output_dir(self, commit_upto):
         """Get the name of the output directory for a commit number
 
         The output directory is typically .../<branch>/<commit>.
@@ -580,7 +580,7 @@ class Builder:
         if self.commits:
             commit = self.commits[commit_upto]
             subject = commit.subject.translate(trans_valid_chars)
-            # See _GetOutputSpaceRemovals() which parses this name
+            # See _get_output_space_removals() which parses this name
             commit_dir = ('%02d_g%s_%s' % (commit_upto + 1,
                     commit.hash, subject[:20]))
         elif not self.no_subdirs:
@@ -589,7 +589,7 @@ class Builder:
             return self.base_dir
         return os.path.join(self.base_dir, commit_dir)
 
-    def GetBuildDir(self, commit_upto, target):
+    def get_build_dir(self, commit_upto, target):
         """Get the name of the build directory for a commit number
 
         The build directory is typically .../<branch>/<commit>/<target>.
@@ -598,30 +598,30 @@ class Builder:
             commit_upto: Commit number to use (0..self.count-1)
             target: Target name
         """
-        output_dir = self._GetOutputDir(commit_upto)
+        output_dir = self._get_output_dir(commit_upto)
         if self.work_in_output:
             return output_dir
         return os.path.join(output_dir, target)
 
-    def GetDoneFile(self, commit_upto, target):
+    def get_done_file(self, commit_upto, target):
         """Get the name of the done file for a commit number
 
         Args:
             commit_upto: Commit number to use (0..self.count-1)
             target: Target name
         """
-        return os.path.join(self.GetBuildDir(commit_upto, target), 'done')
+        return os.path.join(self.get_build_dir(commit_upto, target), 'done')
 
-    def GetSizesFile(self, commit_upto, target):
+    def get_sizes_file(self, commit_upto, target):
         """Get the name of the sizes file for a commit number
 
         Args:
             commit_upto: Commit number to use (0..self.count-1)
             target: Target name
         """
-        return os.path.join(self.GetBuildDir(commit_upto, target), 'sizes')
+        return os.path.join(self.get_build_dir(commit_upto, target), 'sizes')
 
-    def GetFuncSizesFile(self, commit_upto, target, elf_fname):
+    def get_func_sizes_file(self, commit_upto, target, elf_fname):
         """Get the name of the funcsizes file for a commit number and ELF file
 
         Args:
@@ -629,10 +629,10 @@ class Builder:
             target: Target name
             elf_fname: Filename of elf image
         """
-        return os.path.join(self.GetBuildDir(commit_upto, target),
+        return os.path.join(self.get_build_dir(commit_upto, target),
                             '%s.sizes' % elf_fname.replace('/', '-'))
 
-    def GetObjdumpFile(self, commit_upto, target, elf_fname):
+    def get_objdump_file(self, commit_upto, target, elf_fname):
         """Get the name of the objdump file for a commit number and ELF file
 
         Args:
@@ -640,20 +640,20 @@ class Builder:
             target: Target name
             elf_fname: Filename of elf image
         """
-        return os.path.join(self.GetBuildDir(commit_upto, target),
+        return os.path.join(self.get_build_dir(commit_upto, target),
                             '%s.objdump' % elf_fname.replace('/', '-'))
 
-    def GetErrFile(self, commit_upto, target):
+    def get_err_file(self, commit_upto, target):
         """Get the name of the err file for a commit number
 
         Args:
             commit_upto: Commit number to use (0..self.count-1)
             target: Target name
         """
-        output_dir = self.GetBuildDir(commit_upto, target)
+        output_dir = self.get_build_dir(commit_upto, target)
         return os.path.join(output_dir, 'err')
 
-    def FilterErrors(self, lines):
+    def filter_errors(self, lines):
         """Filter out errors in which we have no interest
 
         We should probably use map().
@@ -676,7 +676,7 @@ class Builder:
             out_lines.append(line)
         return out_lines
 
-    def ReadFuncSizes(self, fname, fd):
+    def read_func_sizes(self, fname, fd):
         """Read function sizes from the output of 'nm'
 
         Args:
@@ -700,7 +700,7 @@ class Builder:
                         sym[name] = sym.get(name, 0) + int(size, 16)
         return sym
 
-    def _ProcessConfig(self, fname):
+    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
@@ -737,7 +737,7 @@ class Builder:
                     config[key] = value
         return config
 
-    def _ProcessEnvironment(self, fname):
+    def _process_environment(self, fname):
         """Read in a uboot.env file
 
         This function reads in environment variables from a file.
@@ -762,7 +762,7 @@ class Builder:
                         pass
         return environment
 
-    def GetBuildOutcome(self, commit_upto, target, read_func_sizes,
+    def get_build_outcome(self, commit_upto, target, read_func_sizes,
                         read_config, read_environment):
         """Work out the outcome of a build.
 
@@ -776,8 +776,8 @@ class Builder:
         Returns:
             Outcome object
         """
-        done_file = self.GetDoneFile(commit_upto, target)
-        sizes_file = self.GetSizesFile(commit_upto, target)
+        done_file = self.get_done_file(commit_upto, target)
+        sizes_file = self.get_sizes_file(commit_upto, target)
         sizes = {}
         func_sizes = {}
         config = {}
@@ -791,10 +791,10 @@ class Builder:
                     # Try a rebuild
                     return_code = 1
                 err_lines = []
-                err_file = self.GetErrFile(commit_upto, target)
+                err_file = self.get_err_file(commit_upto, target)
                 if os.path.exists(err_file):
                     with open(err_file, 'r') as fd:
-                        err_lines = self.FilterErrors(fd.readlines())
+                        err_lines = self.filter_errors(fd.readlines())
 
                 # Decide whether the build was ok, failed or created warnings
                 if return_code:
@@ -823,30 +823,30 @@ class Builder:
                             sizes[values[5]] = size_dict
 
             if read_func_sizes:
-                pattern = self.GetFuncSizesFile(commit_upto, target, '*')
+                pattern = self.get_func_sizes_file(commit_upto, target, '*')
                 for fname in glob.glob(pattern):
                     with open(fname, 'r') as fd:
                         dict_name = os.path.basename(fname).replace('.sizes',
                                                                     '')
-                        func_sizes[dict_name] = self.ReadFuncSizes(fname, fd)
+                        func_sizes[dict_name] = self.read_func_sizes(fname, fd)
 
             if read_config:
-                output_dir = self.GetBuildDir(commit_upto, target)
+                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._ProcessConfig(fname)
+                    config[name] = self._process_config(fname)
 
             if read_environment:
-                output_dir = self.GetBuildDir(commit_upto, target)
+                output_dir = self.get_build_dir(commit_upto, target)
                 fname = os.path.join(output_dir, 'uboot.env')
-                environment = self._ProcessEnvironment(fname)
+                environment = self._process_environment(fname)
 
             return Builder.Outcome(rc, err_lines, sizes, func_sizes, config,
                                    environment)
 
         return Builder.Outcome(OUTCOME_UNKNOWN, [], {}, {}, {}, {})
 
-    def GetResultSummary(self, boards_selected, commit_upto, read_func_sizes,
+    def get_result_summary(self, boards_selected, commit_upto, read_func_sizes,
                          read_config, read_environment):
         """Calculate a summary of the results of building a commit.
 
@@ -877,7 +877,7 @@ class Builder:
                     key: environment variable
                     value: value of environment variable
         """
-        def AddLine(lines_summary, lines_boards, line, board):
+        def add_line(lines_summary, lines_boards, line, board):
             line = line.rstrip()
             if line in lines_boards:
                 lines_boards[line].append(board)
@@ -894,7 +894,7 @@ class Builder:
         environment = {}
 
         for brd in boards_selected.values():
-            outcome = self.GetBuildOutcome(commit_upto, brd.target,
+            outcome = self.get_build_outcome(commit_upto, brd.target,
                                            read_func_sizes, read_config,
                                            read_environment)
             board_dict[brd.target] = outcome
@@ -911,15 +911,15 @@ class Builder:
                         is_note = self._re_note.match(line)
                         if is_warning or (last_was_warning and is_note):
                             if last_func:
-                                AddLine(warn_lines_summary, warn_lines_boards,
+                                add_line(warn_lines_summary, warn_lines_boards,
                                         last_func, brd)
-                            AddLine(warn_lines_summary, warn_lines_boards,
+                            add_line(warn_lines_summary, warn_lines_boards,
                                     line, brd)
                         else:
                             if last_func:
-                                AddLine(err_lines_summary, err_lines_boards,
+                                add_line(err_lines_summary, err_lines_boards,
                                         last_func, brd)
-                            AddLine(err_lines_summary, err_lines_boards,
+                            add_line(err_lines_summary, err_lines_boards,
                                     line, brd)
                         last_was_warning = is_warning
                         last_func = None
@@ -927,19 +927,19 @@ class Builder:
             for fname in self.config_filenames:
                 if outcome.config:
                     for key, value in outcome.config[fname].items():
-                        tconfig.Add(fname, key, value)
+                        tconfig.add(fname, key, value)
             config[brd.target] = tconfig
 
             tenvironment = Environment(brd.target)
             if outcome.environment:
                 for key, value in outcome.environment.items():
-                    tenvironment.Add(key, value)
+                    tenvironment.add(key, value)
             environment[brd.target] = tenvironment
 
         return (board_dict, err_lines_summary, err_lines_boards,
                 warn_lines_summary, warn_lines_boards, config, environment)
 
-    def AddOutcome(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
@@ -969,19 +969,19 @@ class Builder:
                 arch_list[arch] += str
 
 
-    def ColourNum(self, num):
+    def colour_num(self, num):
         color = self.col.RED if num > 0 else self.col.GREEN
         if num == 0:
             return '0'
         return self.col.build(color, str(num))
 
-    def ResetResultSummary(self, board_selected):
+    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 PrintResultSummary() will use this
+        Following this, calls to print_result_summary() will use this
         information to work out what has changed.
 
         Args:
@@ -998,7 +998,7 @@ class Builder:
         self._base_config = None
         self._base_environment = None
 
-    def PrintFuncSizeDetail(self, fname, old, new):
+    def print_func_size_detail(self, fname, old, new):
         grow, shrink, add, remove, up, down = 0, 0, 0, 0, 0, 0
         delta, common = [], {}
 
@@ -1032,7 +1032,7 @@ class Builder:
         args = [add, -remove, grow, -shrink, up, -down, up - down]
         if max(args) == 0 and min(args) == 0:
             return
-        args = [self.ColourNum(x) for x in args]
+        args = [self.colour_num(x) for x in args]
         indent = ' ' * 15
         tprint('%s%s: add: %s/%s, grow: %s/%s bytes: %s/%s (%s)' %
               tuple([indent, self.col.build(self.col.YELLOW, fname)] + args))
@@ -1046,7 +1046,7 @@ class Builder:
                 tprint(msg, colour=color)
 
 
-    def PrintSizeDetail(self, target_list, show_bloat):
+    def print_size_detail(self, target_list, show_bloat):
         """Show details size information for each board
 
         Args:
@@ -1079,12 +1079,12 @@ class Builder:
                     outcome = result['_outcome']
                     base_outcome = self._base_board_dict[target]
                     for fname in outcome.func_sizes:
-                        self.PrintFuncSizeDetail(fname,
+                        self.print_func_size_detail(fname,
                                                  base_outcome.func_sizes[fname],
                                                  outcome.func_sizes[fname])
 
 
-    def PrintSizeSummary(self, board_selected, board_dict, show_detail,
+    def print_size_summary(self, board_selected, board_dict, show_detail,
                          show_bloat):
         """Print a summary of image sizes broken down by section.
 
@@ -1185,10 +1185,10 @@ class Builder:
             if printed_arch:
                 tprint()
                 if show_detail:
-                    self.PrintSizeDetail(target_list, show_bloat)
+                    self.print_size_detail(target_list, show_bloat)
 
 
-    def PrintResultSummary(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, show_sizes, show_detail,
                            show_bloat, show_config, show_environment):
@@ -1224,7 +1224,7 @@ class Builder:
             show_config: Show config changes
             show_environment: Show environment changes
         """
-        def _BoardList(line, line_boards):
+        def _board_list(line, line_boards):
             """Helper function to get a line of boards containing a line
 
             Args:
@@ -1243,7 +1243,7 @@ class Builder:
                         board_set.add(brd)
             return brds
 
-        def _CalcErrorDelta(base_lines, base_line_boards, lines, line_boards,
+        def _calc_error_delta(base_lines, base_line_boards, lines, line_boards,
                             char):
             """Calculate the required output based on changes in errors
 
@@ -1267,17 +1267,17 @@ class Builder:
             worse_lines = []
             for line in lines:
                 if line not in base_lines:
-                    errline = ErrLine(char + '+', _BoardList(line, line_boards),
+                    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 + '-',
-                                      _BoardList(line, base_line_boards), line)
+                                      _board_list(line, base_line_boards), line)
                     better_lines.append(errline)
             return better_lines, worse_lines
 
-        def _CalcConfig(delta, name, config):
+        def _calc_config(delta, name, config):
             """Calculate configuration changes
 
             Args:
@@ -1295,7 +1295,7 @@ class Builder:
                 out += '%s=%s ' % (key, config[key])
             return '%s %s: %s' % (delta, name, out)
 
-        def _AddConfig(lines, name, config_plus, config_minus, config_change):
+        def _add_config(lines, name, config_plus, config_minus, config_change):
             """Add changes in configuration to a list
 
             Args:
@@ -1312,13 +1312,13 @@ class Builder:
                     value: config value
             """
             if config_plus:
-                lines.append(_CalcConfig('+', name, config_plus))
+                lines.append(_calc_config('+', name, config_plus))
             if config_minus:
-                lines.append(_CalcConfig('-', name, config_minus))
+                lines.append(_calc_config('-', name, config_minus))
             if config_change:
-                lines.append(_CalcConfig('c', name, config_change))
+                lines.append(_calc_config('c', name, config_change))
 
-        def _OutputConfigInfo(lines):
+        def _output_config_info(lines):
             for line in lines:
                 if not line:
                     continue
@@ -1330,7 +1330,7 @@ class Builder:
                     col = self.col.YELLOW
                 tprint('   ' + line, newline=True, colour=col)
 
-        def _OutputErrLines(err_lines, colour):
+        def _output_err_lines(err_lines, colour):
             """Output the line of error/warning lines, if not empty
 
             Also increments self._error_lines if err_lines not empty
@@ -1388,9 +1388,9 @@ class Builder:
                 new_boards.append(target)
 
         # Get a list of errors and warnings that have appeared, and disappeared
-        better_err, worse_err = _CalcErrorDelta(self._base_err_lines,
+        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 = _CalcErrorDelta(self._base_warn_lines,
+        better_warn, worse_warn = _calc_error_delta(self._base_warn_lines,
                 self._base_warn_line_boards, warn_lines, warn_line_boards, 'w')
 
         # For the IDE mode, print out all the output
@@ -1403,26 +1403,26 @@ class Builder:
         elif any((ok_boards, warn_boards, err_boards, unknown_boards, new_boards,
                 worse_err, better_err, worse_warn, better_warn)):
             arch_list = {}
-            self.AddOutcome(board_selected, arch_list, ok_boards, '',
+            self.add_outcome(board_selected, arch_list, ok_boards, '',
                     self.col.GREEN)
-            self.AddOutcome(board_selected, arch_list, warn_boards, 'w+',
+            self.add_outcome(board_selected, arch_list, warn_boards, 'w+',
                     self.col.YELLOW)
-            self.AddOutcome(board_selected, arch_list, err_boards, '+',
+            self.add_outcome(board_selected, arch_list, err_boards, '+',
                     self.col.RED)
-            self.AddOutcome(board_selected, arch_list, new_boards, '*', self.col.BLUE)
+            self.add_outcome(board_selected, arch_list, new_boards, '*', self.col.BLUE)
             if self._show_unknown:
-                self.AddOutcome(board_selected, arch_list, unknown_boards, '?',
+                self.add_outcome(board_selected, arch_list, unknown_boards, '?',
                         self.col.MAGENTA)
             for arch, target_list in arch_list.items():
                 tprint('%10s: %s' % (arch, target_list))
                 self._error_lines += 1
-            _OutputErrLines(better_err, colour=self.col.GREEN)
-            _OutputErrLines(worse_err, colour=self.col.RED)
-            _OutputErrLines(better_warn, colour=self.col.CYAN)
-            _OutputErrLines(worse_warn, colour=self.col.YELLOW)
+            _output_err_lines(better_err, colour=self.col.GREEN)
+            _output_err_lines(worse_err, colour=self.col.RED)
+            _output_err_lines(better_warn, colour=self.col.CYAN)
+            _output_err_lines(worse_warn, colour=self.col.YELLOW)
 
         if show_sizes:
-            self.PrintSizeSummary(board_selected, board_dict, show_detail,
+            self.print_size_summary(board_selected, board_dict, show_detail,
                                   show_bloat)
 
         if show_environment and self._base_environment:
@@ -1450,10 +1450,10 @@ class Builder:
                         desc = '%s -> %s' % (value, new_value)
                         environment_change[key] = desc
 
-                _AddConfig(lines, target, environment_plus, environment_minus,
+                _add_config(lines, target, environment_plus, environment_minus,
                            environment_change)
 
-            _OutputConfigInfo(lines)
+            _output_config_info(lines)
 
         if show_config and self._base_config:
             summary = {}
@@ -1516,9 +1516,9 @@ class Builder:
                     arch_config_minus[arch][name].update(config_minus)
                     arch_config_change[arch][name].update(config_change)
 
-                    _AddConfig(lines, name, config_plus, config_minus,
+                    _add_config(lines, name, config_plus, config_minus,
                                config_change)
-                _AddConfig(lines, 'all', all_config_plus, all_config_minus,
+                _add_config(lines, 'all', all_config_plus, all_config_minus,
                            all_config_change)
                 summary[target] = '\n'.join(lines)
 
@@ -1538,20 +1538,20 @@ class Builder:
                     all_plus.update(arch_config_plus[arch][name])
                     all_minus.update(arch_config_minus[arch][name])
                     all_change.update(arch_config_change[arch][name])
-                    _AddConfig(lines, name, arch_config_plus[arch][name],
+                    _add_config(lines, name, arch_config_plus[arch][name],
                                arch_config_minus[arch][name],
                                arch_config_change[arch][name])
-                _AddConfig(lines, 'all', all_plus, all_minus, all_change)
+                _add_config(lines, 'all', all_plus, all_minus, all_change)
                 #arch_summary[target] = '\n'.join(lines)
                 if lines:
                     tprint('%s:' % arch)
-                    _OutputConfigInfo(lines)
+                    _output_config_info(lines)
 
             for lines, targets in lines_by_target.items():
                 if not lines:
                     continue
                 tprint('%s :' % ' '.join(sorted(targets)))
-                _OutputConfigInfo(lines.split('\n'))
+                _output_config_info(lines.split('\n'))
 
 
         # Save our updated information for the next call to this function
@@ -1572,9 +1572,9 @@ class Builder:
             tprint("Boards not built (%d): %s" % (len(not_built),
                   ', '.join(not_built)))
 
-    def ProduceResultSummary(self, commit_upto, commits, board_selected):
+    def produce_result_summary(self, commit_upto, commits, board_selected):
             (board_dict, err_lines, err_line_boards, warn_lines,
-             warn_line_boards, config, environment) = self.GetResultSummary(
+             warn_line_boards, config, environment) = self.get_result_summary(
                     board_selected, commit_upto,
                     read_func_sizes=self._show_bloat,
                     read_config=self._show_config,
@@ -1583,13 +1583,13 @@ class Builder:
                 msg = '%02d: %s' % (commit_upto + 1,
                         commits[commit_upto].subject)
                 tprint(msg, colour=self.col.BLUE)
-            self.PrintResultSummary(board_selected, board_dict,
+            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)
 
-    def ShowSummary(self, commits, board_selected):
+    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
@@ -1601,16 +1601,16 @@ class Builder:
         """
         self.commit_count = len(commits) if commits else 1
         self.commits = commits
-        self.ResetResultSummary(board_selected)
+        self.reset_result_summary(board_selected)
         self._error_lines = 0
 
         for commit_upto in range(0, self.commit_count, self._step):
-            self.ProduceResultSummary(commit_upto, commits, board_selected)
+            self.produce_result_summary(commit_upto, commits, board_selected)
         if not self._error_lines:
             tprint('(no errors to report)', colour=self.col.GREEN)
 
 
-    def SetupBuild(self, board_selected, commits):
+    def setup_build(self, board_selected, commits):
         """Set up ready to start a build.
 
         Args:
@@ -1623,7 +1623,7 @@ class Builder:
         self.upto = self.warned = self.fail = 0
         self._timestamps = collections.deque()
 
-    def GetThreadDir(self, thread_num):
+    def get_thread_dir(self, thread_num):
         """Get the directory path to the working dir for a thread.
 
         Args:
@@ -1634,7 +1634,7 @@ class Builder:
             return self._working_dir
         return os.path.join(self._working_dir, '%02d' % max(thread_num, 0))
 
-    def _PrepareThread(self, thread_num, setup_git):
+    def _prepare_thread(self, thread_num, setup_git):
         """Prepare the working directory for a thread.
 
         This clones or fetches the repo into the thread's work directory.
@@ -1647,7 +1647,7 @@ class Builder:
                'clone' to set up a git clone
                'worktree' to set up a git worktree
         """
-        thread_dir = self.GetThreadDir(thread_num)
+        thread_dir = self.get_thread_dir(thread_num)
         builderthread.Mkdir(thread_dir)
         git_dir = os.path.join(thread_dir, '.git')
 
@@ -1684,7 +1684,7 @@ class Builder:
             else:
                 raise ValueError("Can't setup git repo with %s." % setup_git)
 
-    def _PrepareWorkingSpace(self, max_threads, setup_git):
+    def _prepare_working_space(self, max_threads, setup_git):
         """Prepare the working directory for use.
 
         Set up the git repo for each thread. Creates a linked working tree
@@ -1710,14 +1710,14 @@ class Builder:
 
         # Always do at least one thread
         for thread in range(max(max_threads, 1)):
-            self._PrepareThread(thread, setup_git)
+            self._prepare_thread(thread, setup_git)
 
-    def _GetOutputSpaceRemovals(self):
+    def _get_output_space_removals(self):
         """Get the output directories ready to receive files.
 
         Figure out what needs to be deleted in the output directory before it
         can be used. We only delete old buildman directories which have the
-        expected name pattern. See _GetOutputDir().
+        expected name pattern. See _get_output_dir().
 
         Returns:
             List of full paths of directories to remove
@@ -1726,7 +1726,7 @@ class Builder:
             return
         dir_list = []
         for commit_upto in range(self.commit_count):
-            dir_list.append(self._GetOutputDir(commit_upto))
+            dir_list.append(self._get_output_dir(commit_upto))
 
         to_remove = []
         for dirname in glob.glob(os.path.join(self.base_dir, '*')):
@@ -1737,14 +1737,14 @@ class Builder:
                     to_remove.append(dirname)
         return to_remove
 
-    def _PrepareOutputSpace(self):
+    def _prepare_output_space(self):
         """Get the output directories ready to receive files.
 
         We delete any output directories which look like ones we need to
         create. Having left over directories is confusing when the user wants
         to check the output manually.
         """
-        to_remove = self._GetOutputSpaceRemovals()
+        to_remove = self._get_output_space_removals()
         if to_remove:
             tprint('Removing %d old build directories...' % len(to_remove),
                   newline=False)
@@ -1752,7 +1752,7 @@ class Builder:
                 shutil.rmtree(dirname)
             terminal.print_clear()
 
-    def BuildBoards(self, commits, board_selected, keep_outputs, verbose):
+    def build_boards(self, commits, board_selected, keep_outputs, verbose):
         """Build all commits for a list of boards
 
         Args:
@@ -1771,15 +1771,15 @@ class Builder:
         self.commits = commits
         self._verbose = verbose
 
-        self.ResetResultSummary(board_selected)
+        self.reset_result_summary(board_selected)
         builderthread.Mkdir(self.base_dir, parents = True)
-        self._PrepareWorkingSpace(min(self.num_threads, len(board_selected)),
+        self._prepare_working_space(min(self.num_threads, len(board_selected)),
                 commits is not None)
-        self._PrepareOutputSpace()
+        self._prepare_output_space()
         if not self._ide:
             tprint('\rStarting build...', newline=False)
-        self.SetupBuild(board_selected, commits)
-        self.ProcessResult(None)
+        self.setup_build(board_selected, commits)
+        self.process_result(None)
         self.thread_exceptions = []
         # Create jobs to build all commits for each board
         for brd in board_selected.values():
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 635865c21c85..5f1200ae890f 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -77,7 +77,7 @@ class ResultThread(threading.Thread):
         """
         while True:
             result = self.builder.out_queue.get()
-            self.builder.ProcessResult(result)
+            self.builder.process_result(result)
             self.builder.out_queue.task_done()
 
 
@@ -176,7 +176,7 @@ class BuilderThread(threading.Thread):
             out_dir = os.path.join(work_dir, out_rel_dir)
 
         # Check if the job was already completed last time
-        done_file = self.builder.GetDoneFile(commit_upto, brd.target)
+        done_file = self.builder.get_done_file(commit_upto, brd.target)
         result.already_done = os.path.exists(done_file)
         will_build = (force_build or force_build_failures or
             not result.already_done)
@@ -194,7 +194,7 @@ class BuilderThread(threading.Thread):
             if result.return_code == RETURN_CODE_RETRY:
                 will_build = True
             elif will_build:
-                err_file = self.builder.GetErrFile(commit_upto, brd.target)
+                err_file = self.builder.get_err_file(commit_upto, brd.target)
                 if os.path.exists(err_file) and os.stat(err_file).st_size:
                     result.stderr = 'bad'
                 elif not force_build:
@@ -270,7 +270,7 @@ class BuilderThread(threading.Thread):
                 # SPL image. If we don't remove it (i.e. see do_config and
                 # self.mrproper below) then it will appear to be the output of
                 # this build, even if it does not produce SPL images.
-                build_dir = self.builder.GetBuildDir(commit_upto, brd.target)
+                build_dir = self.builder.get_build_dir(commit_upto, brd.target)
                 for elf in BASE_ELF_FILENAMES:
                     fname = os.path.join(out_dir, elf)
                     if os.path.exists(fname):
@@ -345,9 +345,9 @@ class BuilderThread(threading.Thread):
             return
 
         # Write the output and stderr
-        output_dir = self.builder._GetOutputDir(result.commit_upto)
+        output_dir = self.builder._get_output_dir(result.commit_upto)
         Mkdir(output_dir)
-        build_dir = self.builder.GetBuildDir(result.commit_upto,
+        build_dir = self.builder.get_build_dir(result.commit_upto,
                 result.brd.target)
         Mkdir(build_dir)
 
@@ -356,7 +356,7 @@ class BuilderThread(threading.Thread):
             if result.stdout:
                 fd.write(result.stdout)
 
-        errfile = self.builder.GetErrFile(result.commit_upto,
+        errfile = self.builder.get_err_file(result.commit_upto,
                 result.brd.target)
         if result.stderr:
             with open(errfile, 'w') as fd:
@@ -370,7 +370,7 @@ class BuilderThread(threading.Thread):
 
         if result.toolchain:
             # Write the build result and toolchain information.
-            done_file = self.builder.GetDoneFile(result.commit_upto,
+            done_file = self.builder.get_done_file(result.commit_upto,
                     result.brd.target)
             with open(done_file, 'w') as fd:
                 if maybe_aborted:
@@ -403,7 +403,7 @@ class BuilderThread(threading.Thread):
                         capture_stderr=True, cwd=result.out_dir,
                         raise_on_error=False, env=env)
                 if nm_result.stdout:
-                    nm = self.builder.GetFuncSizesFile(result.commit_upto,
+                    nm = self.builder.get_func_sizes_file(result.commit_upto,
                                     result.brd.target, fname)
                     with open(nm, 'w') as fd:
                         print(nm_result.stdout, end=' ', file=fd)
@@ -414,7 +414,7 @@ class BuilderThread(threading.Thread):
                         raise_on_error=False, env=env)
                 rodata_size = ''
                 if dump_result.stdout:
-                    objdump = self.builder.GetObjdumpFile(result.commit_upto,
+                    objdump = self.builder.get_objdump_file(result.commit_upto,
                                     result.brd.target, fname)
                     with open(objdump, 'w') as fd:
                         print(dump_result.stdout, end=' ', file=fd)
@@ -447,7 +447,7 @@ class BuilderThread(threading.Thread):
             # adds an additional hex value at the end of each line for the
             # rodata size
             if len(lines):
-                sizes = self.builder.GetSizesFile(result.commit_upto,
+                sizes = self.builder.get_sizes_file(result.commit_upto,
                                 result.brd.target)
                 with open(sizes, 'w') as fd:
                     print('\n'.join(lines), file=fd)
@@ -502,7 +502,7 @@ class BuilderThread(threading.Thread):
         if self.thread_num != -1:
             self.builder.out_queue.put(result)
         else:
-            self.builder.ProcessResult(result)
+            self.builder.process_result(result)
 
     def RunJob(self, job):
         """Run a single job
@@ -516,7 +516,7 @@ class BuilderThread(threading.Thread):
             List of Result objects
         """
         brd = job.brd
-        work_dir = self.builder.GetThreadDir(self.thread_num)
+        work_dir = self.builder.get_thread_dir(self.thread_num)
         self.toolchain = None
         if job.commits:
             # Run 'make board_defconfig' on the first commit
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 2b25fb2abf8b..4306d4eb2a7c 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -505,15 +505,14 @@ def run_builder(builder, commits, board_selected, args):
         tprint(get_action_summary(args.summary, commit_count, board_selected,
                                   args.threads, args.jobs))
 
-    builder.SetDisplayOptions(
-        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)
+    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)
     if args.summary:
-        builder.ShowSummary(commits, board_selected)
+        builder.show_summary(commits, board_selected)
     else:
-        fail, warned, excs = builder.BuildBoards(
+        fail, warned, excs = builder.build_boards(
             commits, board_selected, args.keep_outputs, args.verbose)
         if excs:
             return 102
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 7162d8c2fcc0..f5058559b4fa 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -501,7 +501,7 @@ Some images are invalid'''
         for commit in range(self._commits):
             for brd in self._boards.get_list():
                 if brd.arch != 'sandbox':
-                  errfile = self._builder.GetErrFile(commit, brd.target)
+                  errfile = self._builder.get_err_file(commit, brd.target)
                   fd = open(errfile)
                   self.assertEqual(fd.readlines(),
                           ['No tool chain for %s\n' % brd.arch])
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 3f55035ea2e4..7ac67260c337 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -208,8 +208,8 @@ class TestBuild(unittest.TestCase):
 
         # Build the boards for the pre-defined commits and warnings/errors
         # associated with each. This calls our Make() to inject the fake output.
-        build.BuildBoards(self.commits, board_selected, keep_outputs=False,
-                          verbose=False)
+        build.build_boards(self.commits, board_selected, keep_outputs=False,
+                           verbose=False)
         lines = terminal.get_print_test_lines()
         count = 0
         for line in lines:
@@ -219,8 +219,8 @@ class TestBuild(unittest.TestCase):
         # 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.SetDisplayOptions(**kwdisplay_args);
-        build.ShowSummary(self.commits, board_selected)
+        build.set_display_options(**kwdisplay_args);
+        build.show_summary(self.commits, board_selected)
         if echo_lines:
             terminal.echo_print_test_lines()
         return iter(terminal.get_print_test_lines())
@@ -528,17 +528,17 @@ class TestBuild(unittest.TestCase):
                                                    'sandbox']),
                          ({'all': ['board4'], 'sandbox': ['board4']}, []))
     def CheckDirs(self, build, dirname):
-        self.assertEqual('base%s' % dirname, build._GetOutputDir(1))
+        self.assertEqual('base%s' % dirname, build._get_output_dir(1))
         self.assertEqual('base%s/fred' % dirname,
-                         build.GetBuildDir(1, 'fred'))
+                         build.get_build_dir(1, 'fred'))
         self.assertEqual('base%s/fred/done' % dirname,
-                         build.GetDoneFile(1, 'fred'))
+                         build.get_done_file(1, 'fred'))
         self.assertEqual('base%s/fred/u-boot.sizes' % dirname,
-                         build.GetFuncSizesFile(1, 'fred', 'u-boot'))
+                         build.get_func_sizes_file(1, 'fred', 'u-boot'))
         self.assertEqual('base%s/fred/u-boot.objdump' % dirname,
-                         build.GetObjdumpFile(1, 'fred', 'u-boot'))
+                         build.get_objdump_file(1, 'fred', 'u-boot'))
         self.assertEqual('base%s/fred/err' % dirname,
-                         build.GetErrFile(1, 'fred'))
+                         build.get_err_file(1, 'fred'))
 
     def testOutputDir(self):
         build = builder.Builder(self.toolchains, BASE_DIR, None, 1, 2,
@@ -622,7 +622,7 @@ class TestBuild(unittest.TestCase):
         build = builder.Builder(self.toolchains, base_dir, None, 1, 2)
         build.commits = self.commits
         build.commit_count = len(commits)
-        result = set(build._GetOutputSpaceRemovals())
+        result = set(build._get_output_space_removals())
         expected = set([os.path.join(base_dir, f) for f in to_remove])
         self.assertEqual(expected, result)
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 38/60] buildman: Split parser creation in two

[Simon Glass]

Split this into two functions to avoid a warning about too many
statements.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/cmdline.py | 44 +++++++++++++++++++++++++++++----------
 1 file changed, 33 insertions(+), 11 deletions(-)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 6b9c1db14af7..e295c7aef1a0 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -14,19 +14,14 @@ import pathlib
 BUILDMAN_DIR = pathlib.Path(__file__).parent
 HAS_TESTS = os.path.exists(BUILDMAN_DIR / "test.py")
 
-def parse_args():
-    """Parse command line arguments from sys.argv[]
-
-    Returns:
-        tuple containing:
-            options: command line options
-            args: command lin arguments
-    """
-    epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+def add_upto_m(parser):
+    """Add arguments up to 'M'
 
-    Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+    Args:
+        parser (ArgumentParser): Parse to add to
 
-    parser = argparse.ArgumentParser(epilog=epilog)
+    This is split out to avoid having too many statements in one function
+    """
     parser.add_argument('-a', '--adjust-cfg', type=str, action='append',
           help='Adjust the Kconfig settings in .config before building')
     parser.add_argument('-A', '--print-prefix', action='store_true',
@@ -104,6 +99,16 @@ def parse_args():
     parser.add_argument('-N', '--no-subdirs', action='store_true', dest='no_subdirs',
           default=False,
           help="Don't create subdirectories when building current source for a single board")
+
+
+def add_after_m(parser):
+    """Add arguments after 'M'
+
+    Args:
+        parser (ArgumentParser): Parse to add to
+
+    This is split out to avoid having too many statements in one function
+    """
     parser.add_argument('-o', '--output-dir', type=str, dest='output_dir',
           help='Directory where all builds happen and buildman has its workspace (default is ../)')
     parser.add_argument('-O', '--override-toolchain', type=str,
@@ -153,6 +158,23 @@ def parse_args():
     parser.add_argument('-Y', '--filter-migration-warnings', action='store_true',
           default=False,
           help='Filter out migration warnings from output')
+
+
+def parse_args():
+    """Parse command line arguments from sys.argv[]
+
+    Returns:
+        tuple containing:
+            options: command line options
+            args: command lin arguments
+    """
+    epilog = """ [list of target/arch/cpu/board/vendor/soc to build]
+
+    Build U-Boot for all commits in a branch. Use -n to do a dry run"""
+
+    parser = argparse.ArgumentParser(epilog=epilog)
+    add_upto_m(parser)
+    add_after_m(parser)
     parser.add_argument('terms', type=str, nargs='*',
                         help='Board / SoC names to build')
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 39/60] buildman: Convert camel case in builderthread.py

[Simon Glass]

Convert this file to snake case and update all files which use it.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builder.py       |  8 +++---
 tools/buildman/builderthread.py | 50 ++++++++++++++++-----------------
 2 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 80c05fbd1e73..4cc266ff3049 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -1648,7 +1648,7 @@ class Builder:
                'worktree' to set up a git worktree
         """
         thread_dir = self.get_thread_dir(thread_num)
-        builderthread.Mkdir(thread_dir)
+        builderthread.mkdir(thread_dir)
         git_dir = os.path.join(thread_dir, '.git')
 
         # Create a worktree or a git repo clone for this thread if it
@@ -1696,7 +1696,7 @@ class Builder:
                 work
             setup_git: True to set up a git worktree or a git clone
         """
-        builderthread.Mkdir(self._working_dir)
+        builderthread.mkdir(self._working_dir)
         if setup_git and self.git_dir:
             src_dir = os.path.abspath(self.git_dir)
             if gitutil.check_worktree_is_available(src_dir):
@@ -1772,7 +1772,7 @@ class Builder:
         self._verbose = verbose
 
         self.reset_result_summary(board_selected)
-        builderthread.Mkdir(self.base_dir, parents = True)
+        builderthread.mkdir(self.base_dir, parents = True)
         self._prepare_working_space(min(self.num_threads, len(board_selected)),
                 commits is not None)
         self._prepare_output_space()
@@ -1793,7 +1793,7 @@ class Builder:
             if self.num_threads:
                 self.queue.put(job)
             else:
-                self._single_builder.RunJob(job)
+                self._single_builder.run_job(job)
 
         if self.num_threads:
             term = threading.Thread(target=self.queue.join)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 5f1200ae890f..3f52d95fed9f 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -16,7 +16,7 @@ from u_boot_pylib import command
 RETURN_CODE_RETRY = -1
 BASE_ELF_FILENAMES = ['u-boot', 'spl/u-boot-spl', 'tpl/u-boot-tpl']
 
-def Mkdir(dirname, parents = False):
+def mkdir(dirname, parents = False):
     """Make a directory if it doesn't already exist.
 
     Args:
@@ -108,7 +108,7 @@ class BuilderThread(threading.Thread):
         self.per_board_out_dir = per_board_out_dir
         self.test_exception = test_exception
 
-    def Make(self, commit, brd, stage, cwd, *args, **kwargs):
+    def make(self, commit, brd, stage, cwd, *args, **kwargs):
         """Run 'make' on a particular commit and board.
 
         The source code will already be checked out, so the 'commit'
@@ -130,7 +130,7 @@ class BuilderThread(threading.Thread):
         return self.builder.do_make(commit, brd, stage, cwd, *args,
                 **kwargs)
 
-    def RunCommit(self, commit_upto, brd, work_dir, do_config, config_only,
+    def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
         """Build a particular commit.
@@ -163,7 +163,7 @@ class BuilderThread(threading.Thread):
                 - boolean indicating whether 'make config' is still needed
         """
         # Create a default result - it will be overwritte by the call to
-        # self.Make() below, in the event that we do a build.
+        # self.make() below, in the event that we do a build.
         result = command.CommandResult()
         result.return_code = 0
         if work_in_output or self.builder.in_tree:
@@ -226,7 +226,7 @@ class BuilderThread(threading.Thread):
 
                 # Set up the environment and command line
                 env = self.toolchain.MakeEnvironment(self.builder.full_path)
-                Mkdir(out_dir)
+                mkdir(out_dir)
                 args = []
                 cwd = work_dir
                 src_dir = os.path.realpath(work_dir)
@@ -282,12 +282,12 @@ class BuilderThread(threading.Thread):
                 if do_config or adjust_cfg:
                     config_out = ''
                     if self.mrproper:
-                        result = self.Make(commit, brd, 'mrproper', cwd,
+                        result = self.make(commit, brd, 'mrproper', cwd,
                                 'mrproper', *args, env=env)
                         config_out += result.combined
                         cmd_list.append([self.builder.gnu_make, 'mrproper',
                                          *args])
-                    result = self.Make(commit, brd, 'config', cwd,
+                    result = self.make(commit, brd, 'config', cwd,
                             *(args + config_args), env=env)
                     cmd_list.append([self.builder.gnu_make] + args +
                                     config_args)
@@ -298,7 +298,7 @@ class BuilderThread(threading.Thread):
                 if result.return_code == 0:
                     if config_only:
                         args.append('cfg')
-                    result = self.Make(commit, brd, 'build', cwd, *args,
+                    result = self.make(commit, brd, 'build', cwd, *args,
                             env=env)
                     cmd_list.append([self.builder.gnu_make] + args)
                     if (result.return_code == 2 and
@@ -326,7 +326,7 @@ class BuilderThread(threading.Thread):
         result.out_dir = out_dir
         return result, do_config
 
-    def _WriteResult(self, result, keep_outputs, work_in_output):
+    def _write_result(self, result, keep_outputs, work_in_output):
         """Write a built result to the output directory.
 
         Args:
@@ -346,10 +346,10 @@ class BuilderThread(threading.Thread):
 
         # Write the output and stderr
         output_dir = self.builder._get_output_dir(result.commit_upto)
-        Mkdir(output_dir)
+        mkdir(output_dir)
         build_dir = self.builder.get_build_dir(result.commit_upto,
                 result.brd.target)
-        Mkdir(build_dir)
+        mkdir(build_dir)
 
         outfile = os.path.join(build_dir, 'log')
         with open(outfile, 'w') as fd:
@@ -440,7 +440,7 @@ class BuilderThread(threading.Thread):
                             raise_on_error=False, env=env)
             ubootenv = os.path.join(result.out_dir, 'uboot.env')
             if not work_in_output:
-                self.CopyFiles(result.out_dir, build_dir, '', ['uboot.env'])
+                self.copy_files(result.out_dir, build_dir, '', ['uboot.env'])
 
             # Write out the image sizes file. This is similar to the output
             # of binutil's 'size' utility, but it omits the header line and
@@ -455,7 +455,7 @@ class BuilderThread(threading.Thread):
         if not work_in_output:
             # Write out the configuration files, with a special case for SPL
             for dirname in ['', 'spl', 'tpl']:
-                self.CopyFiles(
+                self.copy_files(
                     result.out_dir, build_dir, dirname,
                     ['u-boot.cfg', 'spl/u-boot-spl.cfg', 'tpl/u-boot-tpl.cfg',
                      '.config', 'include/autoconf.mk',
@@ -463,12 +463,12 @@ class BuilderThread(threading.Thread):
 
             # Now write the actual build output
             if keep_outputs:
-                self.CopyFiles(
+                self.copy_files(
                     result.out_dir, build_dir, '',
                     ['u-boot*', '*.bin', '*.map', '*.img', 'MLO', 'SPL',
                      'include/autoconf.mk', 'spl/u-boot-spl*'])
 
-    def CopyFiles(self, out_dir, build_dir, dirname, patterns):
+    def copy_files(self, out_dir, build_dir, dirname, patterns):
         """Copy files from the build directory to the output.
 
         Args:
@@ -488,7 +488,7 @@ class BuilderThread(threading.Thread):
                         target = '%s-%s%s' % (base, dirname, ext)
                 shutil.copy(fname, os.path.join(build_dir, target))
 
-    def _SendResult(self, result):
+    def _send_result(self, result):
         """Send a result to the builder for processing
 
         Args:
@@ -504,7 +504,7 @@ class BuilderThread(threading.Thread):
         else:
             self.builder.process_result(result)
 
-    def RunJob(self, job):
+    def run_job(self, job):
         """Run a single job
 
         A job consists of a building a list of commits for a particular board.
@@ -524,7 +524,7 @@ class BuilderThread(threading.Thread):
             commit_upto  = 0
             force_build = False
             for commit_upto in range(0, len(job.commits), job.step):
-                result, request_config = self.RunCommit(commit_upto, brd,
+                result, request_config = self.run_commit(commit_upto, brd,
                         work_dir, do_config, self.builder.config_only,
                         force_build or self.builder.force_build,
                         self.builder.force_build_failures,
@@ -535,7 +535,7 @@ class BuilderThread(threading.Thread):
                     # If our incremental build failed, try building again
                     # with a reconfig.
                     if self.builder.force_config_on_failure:
-                        result, request_config = self.RunCommit(commit_upto,
+                        result, request_config = self.run_commit(commit_upto,
                             brd, work_dir, True, False, True, False,
                             job.work_in_output, job.adjust_cfg)
                         did_config = True
@@ -576,17 +576,17 @@ class BuilderThread(threading.Thread):
                         raise ValueError('Interrupt')
 
                 # We have the build results, so output the result
-                self._WriteResult(result, job.keep_outputs, job.work_in_output)
-                self._SendResult(result)
+                self._write_result(result, job.keep_outputs, job.work_in_output)
+                self._send_result(result)
         else:
             # Just build the currently checked-out build
-            result, request_config = self.RunCommit(None, brd, work_dir, True,
+            result, request_config = self.run_commit(None, brd, work_dir, True,
                         self.builder.config_only, True,
                         self.builder.force_build_failures, job.work_in_output,
                         job.adjust_cfg)
             result.commit_upto = 0
-            self._WriteResult(result, job.keep_outputs, job.work_in_output)
-            self._SendResult(result)
+            self._write_result(result, job.keep_outputs, job.work_in_output)
+            self._send_result(result)
 
     def run(self):
         """Our thread's run function
@@ -597,7 +597,7 @@ class BuilderThread(threading.Thread):
         while True:
             job = self.builder.queue.get()
             try:
-                self.RunJob(job)
+                self.run_job(job)
             except Exception as e:
                 print('Thread exception (use -T0 to run without threads):', e)
                 self.builder.thread_exceptions.append(e)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 40/60] buildman: Correct most pylint warnings in builderthread

[Simon Glass]

Fix the easy warnings in this file.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 95 ++++++++++++++++++---------------
 1 file changed, 51 insertions(+), 44 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 3f52d95fed9f..179fc477c008 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -2,6 +2,12 @@
 # Copyright (c) 2014 Google, Inc
 #
 
+"""Implementation the bulider threads
+
+This module provides the BuilderThread class, which handles calling the builder
+based on the jobs provided.
+"""
+
 import errno
 import glob
 import os
@@ -30,12 +36,12 @@ def mkdir(dirname, parents = False):
     except OSError as err:
         if err.errno == errno.EEXIST:
             if os.path.realpath('.') == os.path.realpath(dirname):
-                print("Cannot create the current working directory '%s'!" % dirname)
+                print(f"Cannot create the current working directory '{dirname}'!")
                 sys.exit(1)
-            pass
         else:
             raise
 
+# pylint: disable=R0903
 class BuilderJob:
     """Holds information about a job to be performed by a thread
 
@@ -107,6 +113,7 @@ class BuilderThread(threading.Thread):
         self.mrproper = mrproper
         self.per_board_out_dir = per_board_out_dir
         self.test_exception = test_exception
+        self.toolchain = None
 
     def make(self, commit, brd, stage, cwd, *args, **kwargs):
         """Run 'make' on a particular commit and board.
@@ -182,9 +189,9 @@ class BuilderThread(threading.Thread):
             not result.already_done)
         if result.already_done:
             # Get the return code from that build and use it
-            with open(done_file, 'r') as fd:
+            with open(done_file, 'r', encoding='utf-8') as outf:
                 try:
-                    result.return_code = int(fd.readline())
+                    result.return_code = int(outf.readline())
                 except ValueError:
                     # The file may be empty due to running out of disk space.
                     # Try a rebuild
@@ -240,11 +247,11 @@ class BuilderThread(threading.Thread):
                         # Symlinks can confuse U-Boot's Makefile since
                         # we may use '..' in our path, so remove them.
                         out_dir = os.path.realpath(out_dir)
-                        args.append('O=%s' % out_dir)
+                        args.append(f'O={out_dir}')
                         cwd = None
                         src_dir = os.getcwd()
                     else:
-                        args.append('O=%s' % out_rel_dir)
+                        args.append(f'O={out_rel_dir}')
                 if self.builder.verbose_build:
                     args.append('V=1')
                 else:
@@ -260,7 +267,7 @@ class BuilderThread(threading.Thread):
                     args.append('NO_LTO=1')
                 if self.builder.reproducible_builds:
                     args.append('SOURCE_DATE_EPOCH=0')
-                config_args = ['%s_defconfig' % brd.target]
+                config_args = [f'{brd.target}_defconfig']
                 config_out = ''
                 args.extend(self.builder.toolchains.GetMakeArguments(brd))
                 args.extend(self.toolchain.MakeArgs())
@@ -270,7 +277,6 @@ class BuilderThread(threading.Thread):
                 # SPL image. If we don't remove it (i.e. see do_config and
                 # self.mrproper below) then it will appear to be the output of
                 # this build, even if it does not produce SPL images.
-                build_dir = self.builder.get_build_dir(commit_upto, brd.target)
                 for elf in BASE_ELF_FILENAMES:
                     fname = os.path.join(out_dir, elf)
                     if os.path.exists(fname):
@@ -317,7 +323,7 @@ class BuilderThread(threading.Thread):
                 result.cmd_list = cmd_list
             else:
                 result.return_code = 1
-                result.stderr = 'No tool chain for %s\n' % brd.arch
+                result.stderr = f'No tool chain for {brd.arch}\n'
             result.already_done = False
 
         result.toolchain = self.toolchain
@@ -352,15 +358,15 @@ class BuilderThread(threading.Thread):
         mkdir(build_dir)
 
         outfile = os.path.join(build_dir, 'log')
-        with open(outfile, 'w') as fd:
+        with open(outfile, 'w', encoding='utf-8') as outf:
             if result.stdout:
-                fd.write(result.stdout)
+                outf.write(result.stdout)
 
         errfile = self.builder.get_err_file(result.commit_upto,
                 result.brd.target)
         if result.stderr:
-            with open(errfile, 'w') as fd:
-                fd.write(result.stderr)
+            with open(errfile, 'w', encoding='utf-8') as outf:
+                outf.write(result.stderr)
         elif os.path.exists(errfile):
             os.remove(errfile)
 
@@ -372,43 +378,44 @@ class BuilderThread(threading.Thread):
             # Write the build result and toolchain information.
             done_file = self.builder.get_done_file(result.commit_upto,
                     result.brd.target)
-            with open(done_file, 'w') as fd:
+            with open(done_file, 'w', encoding='utf-8') as outf:
                 if maybe_aborted:
                     # Special code to indicate we need to retry
-                    fd.write('%s' % RETURN_CODE_RETRY)
+                    outf.write(f'{RETURN_CODE_RETRY}')
                 else:
-                    fd.write('%s' % result.return_code)
-            with open(os.path.join(build_dir, 'toolchain'), 'w') as fd:
-                print('gcc', result.toolchain.gcc, file=fd)
-                print('path', result.toolchain.path, file=fd)
-                print('cross', result.toolchain.cross, file=fd)
-                print('arch', result.toolchain.arch, file=fd)
-                fd.write('%s' % result.return_code)
+                    outf.write(f'{result.return_code}')
+            with open(os.path.join(build_dir, 'toolchain'), 'w',
+                      encoding='utf-8') as outf:
+                print('gcc', result.toolchain.gcc, file=outf)
+                print('path', result.toolchain.path, file=outf)
+                print('cross', result.toolchain.cross, file=outf)
+                print('arch', result.toolchain.arch, file=outf)
+                outf.write(f'{result.return_code}')
 
             # Write out the image and function size information and an objdump
             env = result.toolchain.MakeEnvironment(self.builder.full_path)
-            with open(os.path.join(build_dir, 'out-env'), 'wb') as fd:
+            with open(os.path.join(build_dir, 'out-env'), 'wb') as outf:
                 for var in sorted(env.keys()):
-                    fd.write(b'%s="%s"' % (var, env[var]))
+                    outf.write(b'%s="%s"' % (var, env[var]))
 
             with open(os.path.join(build_dir, 'out-cmd'), 'w',
-                      encoding='utf-8') as fd:
+                      encoding='utf-8') as outf:
                 for cmd in result.cmd_list:
-                    print(' '.join(cmd), file=fd)
+                    print(' '.join(cmd), file=outf)
 
             lines = []
             for fname in BASE_ELF_FILENAMES:
-                cmd = ['%snm' % self.toolchain.cross, '--size-sort', fname]
+                cmd = [f'{self.toolchain.cross}nm', '--size-sort', fname]
                 nm_result = command.run_pipe([cmd], capture=True,
                         capture_stderr=True, cwd=result.out_dir,
                         raise_on_error=False, env=env)
                 if nm_result.stdout:
-                    nm = self.builder.get_func_sizes_file(result.commit_upto,
-                                    result.brd.target, fname)
-                    with open(nm, 'w') as fd:
-                        print(nm_result.stdout, end=' ', file=fd)
+                    nm_fname = self.builder.get_func_sizes_file(
+                        result.commit_upto, result.brd.target, fname)
+                    with open(nm_fname, 'w', encoding='utf-8') as outf:
+                        print(nm_result.stdout, end=' ', file=outf)
 
-                cmd = ['%sobjdump' % self.toolchain.cross, '-h', fname]
+                cmd = [f'{self.toolchain.cross}objdump', '-h', fname]
                 dump_result = command.run_pipe([cmd], capture=True,
                         capture_stderr=True, cwd=result.out_dir,
                         raise_on_error=False, env=env)
@@ -416,14 +423,14 @@ class BuilderThread(threading.Thread):
                 if dump_result.stdout:
                     objdump = self.builder.get_objdump_file(result.commit_upto,
                                     result.brd.target, fname)
-                    with open(objdump, 'w') as fd:
-                        print(dump_result.stdout, end=' ', file=fd)
+                    with open(objdump, 'w', encoding='utf-8') as outf:
+                        print(dump_result.stdout, end=' ', file=outf)
                     for line in dump_result.stdout.splitlines():
                         fields = line.split()
                         if len(fields) > 5 and fields[1] == '.rodata':
                             rodata_size = fields[2]
 
-                cmd = ['%ssize' % self.toolchain.cross, fname]
+                cmd = [f'{self.toolchain.cross}size', fname]
                 size_result = command.run_pipe([cmd], capture=True,
                         capture_stderr=True, cwd=result.out_dir,
                         raise_on_error=False, env=env)
@@ -432,13 +439,12 @@ class BuilderThread(threading.Thread):
                                  rodata_size)
 
             # Extract the environment from U-Boot and dump it out
-            cmd = ['%sobjcopy' % self.toolchain.cross, '-O', 'binary',
+            cmd = [f'{self.toolchain.cross}objcopy', '-O', 'binary',
                    '-j', '.rodata.default_environment',
                    'env/built-in.o', 'uboot.env']
             command.run_pipe([cmd], capture=True,
                             capture_stderr=True, cwd=result.out_dir,
                             raise_on_error=False, env=env)
-            ubootenv = os.path.join(result.out_dir, 'uboot.env')
             if not work_in_output:
                 self.copy_files(result.out_dir, build_dir, '', ['uboot.env'])
 
@@ -446,11 +452,11 @@ class BuilderThread(threading.Thread):
             # of binutil's 'size' utility, but it omits the header line and
             # adds an additional hex value at the end of each line for the
             # rodata size
-            if len(lines):
+            if lines:
                 sizes = self.builder.get_sizes_file(result.commit_upto,
                                 result.brd.target)
-                with open(sizes, 'w') as fd:
-                    print('\n'.join(lines), file=fd)
+                with open(sizes, 'w', encoding='utf-8') as outf:
+                    print('\n'.join(lines), file=outf)
 
         if not work_in_output:
             # Write out the configuration files, with a special case for SPL
@@ -485,7 +491,7 @@ class BuilderThread(threading.Thread):
                 if dirname:
                     base, ext = os.path.splitext(target)
                     if ext:
-                        target = '%s-%s%s' % (base, dirname, ext)
+                        target = f'{base}-{dirname}{ext}'
                 shutil.copy(fname, os.path.join(build_dir, target))
 
     def _send_result(self, result):
@@ -598,7 +604,8 @@ class BuilderThread(threading.Thread):
             job = self.builder.queue.get()
             try:
                 self.run_job(job)
-            except Exception as e:
-                print('Thread exception (use -T0 to run without threads):', e)
-                self.builder.thread_exceptions.append(e)
+            except Exception as exc:
+                print('Thread exception (use -T0 to run without threads):',
+                      exc)
+                self.builder.thread_exceptions.append(exc)
             self.builder.queue.task_done()
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 41/60] buildman: Export _get_output_dir() to avoid warnings

[Simon Glass]

Make this a public memory since it is used outside the class.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builder.py       | 8 ++++----
 tools/buildman/builderthread.py | 2 +-
 tools/buildman/test.py          | 2 +-
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 4cc266ff3049..ecbd368c47a5 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -565,7 +565,7 @@ class Builder:
             terminal.print_clear()
             tprint(line, newline=False, limit_to_line=True)
 
-    def _get_output_dir(self, commit_upto):
+    def get_output_dir(self, commit_upto):
         """Get the name of the output directory for a commit number
 
         The output directory is typically .../<branch>/<commit>.
@@ -598,7 +598,7 @@ class Builder:
             commit_upto: Commit number to use (0..self.count-1)
             target: Target name
         """
-        output_dir = self._get_output_dir(commit_upto)
+        output_dir = self.get_output_dir(commit_upto)
         if self.work_in_output:
             return output_dir
         return os.path.join(output_dir, target)
@@ -1717,7 +1717,7 @@ class Builder:
 
         Figure out what needs to be deleted in the output directory before it
         can be used. We only delete old buildman directories which have the
-        expected name pattern. See _get_output_dir().
+        expected name pattern. See get_output_dir().
 
         Returns:
             List of full paths of directories to remove
@@ -1726,7 +1726,7 @@ class Builder:
             return
         dir_list = []
         for commit_upto in range(self.commit_count):
-            dir_list.append(self._get_output_dir(commit_upto))
+            dir_list.append(self.get_output_dir(commit_upto))
 
         to_remove = []
         for dirname in glob.glob(os.path.join(self.base_dir, '*')):
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 179fc477c008..45ae6edf9f48 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -351,7 +351,7 @@ class BuilderThread(threading.Thread):
             return
 
         # Write the output and stderr
-        output_dir = self.builder._get_output_dir(result.commit_upto)
+        output_dir = self.builder.get_output_dir(result.commit_upto)
         mkdir(output_dir)
         build_dir = self.builder.get_build_dir(result.commit_upto,
                 result.brd.target)
diff --git a/tools/buildman/test.py b/tools/buildman/test.py
index 7ac67260c337..bdd3d84158a6 100644
--- a/tools/buildman/test.py
+++ b/tools/buildman/test.py
@@ -528,7 +528,7 @@ class TestBuild(unittest.TestCase):
                                                    'sandbox']),
                          ({'all': ['board4'], 'sandbox': ['board4']}, []))
     def CheckDirs(self, build, dirname):
-        self.assertEqual('base%s' % dirname, build._get_output_dir(1))
+        self.assertEqual('base%s' % dirname, build.get_output_dir(1))
         self.assertEqual('base%s/fred' % dirname,
                          build.get_build_dir(1, 'fred'))
         self.assertEqual('base%s/fred/done' % dirname,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 42/60] buildman: Correct invalid use of out_dir variable

[Simon Glass]

This variable has a different meaning in the outer scrop. Use a different
name to avoid confusion, or bugs.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 45ae6edf9f48..f110137e8f6e 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -246,8 +246,8 @@ class BuilderThread(threading.Thread):
                         #
                         # Symlinks can confuse U-Boot's Makefile since
                         # we may use '..' in our path, so remove them.
-                        out_dir = os.path.realpath(out_dir)
-                        args.append(f'O={out_dir}')
+                        real_dir = os.path.realpath(out_dir)
+                        args.append(f'O={real_dir}')
                         cwd = None
                         src_dir = os.getcwd()
                     else:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 43/60] buildman: Drop unnecessary assignment of config_out

[Simon Glass]

This is already set up earlier in the function, so drop the extra
assignment.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index f110137e8f6e..ad12e9ede7f4 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -286,7 +286,6 @@ class BuilderThread(threading.Thread):
                 cfg_file = os.path.join(out_dir, '.config')
                 cmd_list = []
                 if do_config or adjust_cfg:
-                    config_out = ''
                     if self.mrproper:
                         result = self.make(commit, brd, 'mrproper', cwd,
                                 'mrproper', *args, env=env)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 44/60] buildman: Start a function to set up the make arguments

[Simon Glass]

Move some of this code into a new funciion, to help reduce the size of the
run_commits() function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 38 ++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index ad12e9ede7f4..47ebf4dcdd90 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,6 +137,28 @@ class BuilderThread(threading.Thread):
         return self.builder.do_make(commit, brd, stage, cwd, *args,
                 **kwargs)
 
+    def _build_args(self, args):
+        """Set up arguments to the args list based on the settings
+
+        Args:
+            args (list of str): List of string arguments to add things to
+        """
+        if self.builder.verbose_build:
+            args.append('V=1')
+        else:
+            args.append('-s')
+        if self.builder.num_jobs is not None:
+            args.extend(['-j', str(self.builder.num_jobs)])
+        if self.builder.warnings_as_errors:
+            args.append('KCFLAGS=-Werror')
+            args.append('HOSTCFLAGS=-Werror')
+        if self.builder.allow_missing:
+            args.append('BINMAN_ALLOW_MISSING=1')
+        if self.builder.no_lto:
+            args.append('NO_LTO=1')
+        if self.builder.reproducible_builds:
+            args.append('SOURCE_DATE_EPOCH=0')
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -252,21 +274,7 @@ class BuilderThread(threading.Thread):
                         src_dir = os.getcwd()
                     else:
                         args.append(f'O={out_rel_dir}')
-                if self.builder.verbose_build:
-                    args.append('V=1')
-                else:
-                    args.append('-s')
-                if self.builder.num_jobs is not None:
-                    args.extend(['-j', str(self.builder.num_jobs)])
-                if self.builder.warnings_as_errors:
-                    args.append('KCFLAGS=-Werror')
-                    args.append('HOSTCFLAGS=-Werror')
-                if self.builder.allow_missing:
-                    args.append('BINMAN_ALLOW_MISSING=1')
-                if self.builder.no_lto:
-                    args.append('NO_LTO=1')
-                if self.builder.reproducible_builds:
-                    args.append('SOURCE_DATE_EPOCH=0')
+                self._build_args(args)
                 config_args = [f'{brd.target}_defconfig']
                 config_out = ''
                 args.extend(self.builder.toolchains.GetMakeArguments(brd))
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 45/60] buildman: Move setting of toolchain arguments to _build_args()

[Simon Glass]

Move a few more pieces to this new function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 47ebf4dcdd90..e7d9a29d03ef 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,11 +137,12 @@ class BuilderThread(threading.Thread):
         return self.builder.do_make(commit, brd, stage, cwd, *args,
                 **kwargs)
 
-    def _build_args(self, args):
+    def _build_args(self, args, brd):
         """Set up arguments to the args list based on the settings
 
         Args:
             args (list of str): List of string arguments to add things to
+            brd (Board): Board to create arguments for
         """
         if self.builder.verbose_build:
             args.append('V=1')
@@ -158,6 +159,8 @@ class BuilderThread(threading.Thread):
             args.append('NO_LTO=1')
         if self.builder.reproducible_builds:
             args.append('SOURCE_DATE_EPOCH=0')
+        args.extend(self.builder.toolchains.GetMakeArguments(brd))
+        args.extend(self.toolchain.MakeArgs())
 
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
@@ -274,11 +277,9 @@ class BuilderThread(threading.Thread):
                         src_dir = os.getcwd()
                     else:
                         args.append(f'O={out_rel_dir}')
-                self._build_args(args)
+                self._build_args(args, brd)
                 config_args = [f'{brd.target}_defconfig']
                 config_out = ''
-                args.extend(self.builder.toolchains.GetMakeArguments(brd))
-                args.extend(self.toolchain.MakeArgs())
 
                 # Remove any output targets. Since we use a build directory that
                 # was previously used by another board, it may have produced an
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 46/60] buildman: Move more things into _build_args()

[Simon Glass]

Move more of the argument-building code into this function. Fix a missing
assignment for out_rel_dir too.

Rename the function since it now builds all the arguments.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 55 ++++++++++++++++++++-------------
 1 file changed, 34 insertions(+), 21 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index e7d9a29d03ef..42af5197dace 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -137,13 +137,40 @@ class BuilderThread(threading.Thread):
         return self.builder.do_make(commit, brd, stage, cwd, *args,
                 **kwargs)
 
-    def _build_args(self, args, brd):
+    def _build_args(self, brd, out_dir, out_rel_dir, work_dir, commit_upto):
         """Set up arguments to the args list based on the settings
 
         Args:
-            args (list of str): List of string arguments to add things to
             brd (Board): Board to create arguments for
+            out_dir (str): Path to output directory containing the files
+            out_rel_dir (str): Output directory relative to the current dir
+            work_dir (str): Directory to which the source will be checked out
+            commit_upto (int): Commit number to build (0...n-1)
+
+        Returns:
+            tuple:
+                list of str: Arguments to pass to make
+                str: Current working directory, or None if no commit
+                str: Source directory (typically the work directory)
         """
+        args = []
+        cwd = work_dir
+        src_dir = os.path.realpath(work_dir)
+        if not self.builder.in_tree:
+            if commit_upto is None:
+                # In this case we are building in the original source directory
+                # (i.e. the current directory where buildman is invoked. The
+                # output directory is set to this thread's selected work
+                # directory.
+                #
+                # Symlinks can confuse U-Boot's Makefile since we may use '..'
+                # in our path, so remove them.
+                real_dir = os.path.realpath(out_dir)
+                args.append(f'O={real_dir}')
+                cwd = None
+                src_dir = os.getcwd()
+            else:
+                args.append(f'O={out_rel_dir}')
         if self.builder.verbose_build:
             args.append('V=1')
         else:
@@ -161,6 +188,7 @@ class BuilderThread(threading.Thread):
             args.append('SOURCE_DATE_EPOCH=0')
         args.extend(self.builder.toolchains.GetMakeArguments(brd))
         args.extend(self.toolchain.MakeArgs())
+        return args, cwd, src_dir
 
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
@@ -199,6 +227,7 @@ class BuilderThread(threading.Thread):
         result = command.CommandResult()
         result.return_code = 0
         if work_in_output or self.builder.in_tree:
+            out_rel_dir = None
             out_dir = work_dir
         else:
             if self.per_board_out_dir:
@@ -259,25 +288,9 @@ class BuilderThread(threading.Thread):
                 # Set up the environment and command line
                 env = self.toolchain.MakeEnvironment(self.builder.full_path)
                 mkdir(out_dir)
-                args = []
-                cwd = work_dir
-                src_dir = os.path.realpath(work_dir)
-                if not self.builder.in_tree:
-                    if commit_upto is None:
-                        # In this case we are building in the original source
-                        # directory (i.e. the current directory where buildman
-                        # is invoked. The output directory is set to this
-                        # thread's selected work directory.
-                        #
-                        # Symlinks can confuse U-Boot's Makefile since
-                        # we may use '..' in our path, so remove them.
-                        real_dir = os.path.realpath(out_dir)
-                        args.append(f'O={real_dir}')
-                        cwd = None
-                        src_dir = os.getcwd()
-                    else:
-                        args.append(f'O={out_rel_dir}')
-                self._build_args(args, brd)
+
+                args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
+                                                      work_dir, commit_upto)
                 config_args = [f'{brd.target}_defconfig']
                 config_out = ''
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 47/60] buildman: Convert config_out to string IO

[Simon Glass]

This is probably a little more efficient and it allows passing the object
to another function to write data. Convert config_out to use a string I/O
device.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 42af5197dace..9c2938d1b071 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -10,6 +10,7 @@ based on the jobs provided.
 
 import errno
 import glob
+import io
 import os
 import shutil
 import sys
@@ -292,7 +293,7 @@ class BuilderThread(threading.Thread):
                 args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
                                                       work_dir, commit_upto)
                 config_args = [f'{brd.target}_defconfig']
-                config_out = ''
+                config_out = io.StringIO()
 
                 # Remove any output targets. Since we use a build directory that
                 # was previously used by another board, it may have produced an
@@ -311,14 +312,14 @@ class BuilderThread(threading.Thread):
                     if self.mrproper:
                         result = self.make(commit, brd, 'mrproper', cwd,
                                 'mrproper', *args, env=env)
-                        config_out += result.combined
+                        config_out.write(result.combined)
                         cmd_list.append([self.builder.gnu_make, 'mrproper',
                                          *args])
                     result = self.make(commit, brd, 'config', cwd,
                             *(args + config_args), env=env)
                     cmd_list.append([self.builder.gnu_make] + args +
                                     config_args)
-                    config_out += result.combined
+                    config_out.write(result.combined)
                     do_config = False   # No need to configure next time
                     if adjust_cfg:
                         cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
@@ -340,7 +341,7 @@ class BuilderThread(threading.Thread):
                             result.return_code = 1
                 result.stderr = result.stderr.replace(src_dir + '/', '')
                 if self.builder.verbose_build:
-                    result.stdout = config_out + result.stdout
+                    result.stdout = config_out.getvalue() + result.stdout
                 result.cmd_list = cmd_list
             else:
                 result.return_code = 1
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 48/60] buildman: Move reconfigure code into its own function

[Simon Glass]

Split this into its own function so reduce the size of run_commit().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 41 ++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 9c2938d1b071..80fca2a61bb3 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -191,6 +191,33 @@ class BuilderThread(threading.Thread):
         args.extend(self.toolchain.MakeArgs())
         return args, cwd, src_dir
 
+    def _reconfigure(self, commit, brd, cwd, args, env, config_args, config_out,
+                     cmd_list):
+        """Reconfigure the build
+
+        Args:
+            commit (Commit): Commit only being built
+            brd (Board): Board being built
+            cwd (str): Current working directory
+            args (list of str): Arguments to pass to make
+            env (dict): Environment strings
+            config_args (list of str): defconfig arg for this board
+            cmd_list (list of str): List to add the commands to, for logging
+
+        Returns:
+            CommandResult object
+        """
+        if self.mrproper:
+            result = self.make(commit, brd, 'mrproper', cwd, 'mrproper', *args,
+                               env=env)
+            config_out.write(result.combined)
+            cmd_list.append([self.builder.gnu_make, 'mrproper', *args])
+        result = self.make(commit, brd, 'config', cwd, *(args + config_args),
+                           env=env)
+        cmd_list.append([self.builder.gnu_make] + args + config_args)
+        config_out.write(result.combined)
+        return result
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -309,17 +336,9 @@ class BuilderThread(threading.Thread):
                 cfg_file = os.path.join(out_dir, '.config')
                 cmd_list = []
                 if do_config or adjust_cfg:
-                    if self.mrproper:
-                        result = self.make(commit, brd, 'mrproper', cwd,
-                                'mrproper', *args, env=env)
-                        config_out.write(result.combined)
-                        cmd_list.append([self.builder.gnu_make, 'mrproper',
-                                         *args])
-                    result = self.make(commit, brd, 'config', cwd,
-                            *(args + config_args), env=env)
-                    cmd_list.append([self.builder.gnu_make] + args +
-                                    config_args)
-                    config_out.write(result.combined)
+                    result = self._reconfigure(
+                        commit, brd, cwd, args, env, config_args, config_out,
+                        cmd_list)
                     do_config = False   # No need to configure next time
                     if adjust_cfg:
                         cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 49/60] buildman: Move bulid code into its own function

[Simon Glass]

Split this into its own function so reduce the size of run_commit().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 40 ++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 10 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 80fca2a61bb3..2d54e6283023 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -218,6 +218,32 @@ class BuilderThread(threading.Thread):
         config_out.write(result.combined)
         return result
 
+    def _build(self, commit, brd, cwd, args, env, cmd_list, config_only):
+        """Perform the build
+
+        Args:
+            commit (Commit): Commit only being built
+            brd (Board): Board being built
+            cwd (str): Current working directory
+            args (list of str): Arguments to pass to make
+            env (dict): Environment strings
+            cmd_list (list of str): List to add the commands to, for logging
+            config_only (bool): True if this is a config-only build (using the
+                'make cfg' target)
+
+        Returns:
+            CommandResult object
+        """
+        if config_only:
+            args.append('cfg')
+        result = self.make(commit, brd, 'build', cwd, *args, env=env)
+        cmd_list.append([self.builder.gnu_make] + args)
+        if (result.return_code == 2 and
+            ('Some images are invalid' in result.stderr)):
+            # This is handled later by the check for output in stderr
+            result.return_code = 0
+        return result
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -342,17 +368,11 @@ class BuilderThread(threading.Thread):
                     do_config = False   # No need to configure next time
                     if adjust_cfg:
                         cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
+
+                # Now do the build, if everything looks OK
                 if result.return_code == 0:
-                    if config_only:
-                        args.append('cfg')
-                    result = self.make(commit, brd, 'build', cwd, *args,
-                            env=env)
-                    cmd_list.append([self.builder.gnu_make] + args)
-                    if (result.return_code == 2 and
-                        ('Some images are invalid' in result.stderr)):
-                        # This is handled later by the check for output in
-                        # stderr
-                        result.return_code = 0
+                    result = self._build(commit, brd, cwd, args, env, cmd_list,
+                                         config_only)
                     if adjust_cfg:
                         errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
                         if errs:
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 50/60] buildman: Move reading of the done file into a function

[Simon Glass]

Move this logic into its own function to reduce the size of the
run_commt() function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 66 +++++++++++++++++++++------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 2d54e6283023..b4891059b6db 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -244,6 +244,46 @@ class BuilderThread(threading.Thread):
             result.return_code = 0
         return result
 
+    def _read_done_file(self, commit_upto, brd, result, force_build,
+                        force_build_failures):
+        """Check the 'done' file and see if this commit should be built
+
+        Args:
+            commit (Commit): Commit only being built
+            brd (Board): Board being built
+            result (CommandResult): result object to update
+            force_build (bool): Force a build even if one was previously done
+            force_build_failures (bool): Force a bulid if the previous result
+                showed failure
+
+        Returns:
+            bool: True if build should be built
+        """
+        done_file = self.builder.get_done_file(commit_upto, brd.target)
+        result.already_done = os.path.exists(done_file)
+        will_build = (force_build or force_build_failures or
+            not result.already_done)
+        if result.already_done:
+            with open(done_file, 'r', encoding='utf-8') as outf:
+                try:
+                    result.return_code = int(outf.readline())
+                except ValueError:
+                    # The file may be empty due to running out of disk space.
+                    # Try a rebuild
+                    result.return_code = RETURN_CODE_RETRY
+
+            # Check the signal that the build needs to be retried
+            if result.return_code == RETURN_CODE_RETRY:
+                will_build = True
+            elif will_build:
+                err_file = self.builder.get_err_file(commit_upto, brd.target)
+                if os.path.exists(err_file) and os.stat(err_file).st_size:
+                    result.stderr = 'bad'
+                elif not force_build:
+                    # The build passed, so no need to build it again
+                    will_build = False
+        return will_build
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -291,30 +331,8 @@ class BuilderThread(threading.Thread):
             out_dir = os.path.join(work_dir, out_rel_dir)
 
         # Check if the job was already completed last time
-        done_file = self.builder.get_done_file(commit_upto, brd.target)
-        result.already_done = os.path.exists(done_file)
-        will_build = (force_build or force_build_failures or
-            not result.already_done)
-        if result.already_done:
-            # Get the return code from that build and use it
-            with open(done_file, 'r', encoding='utf-8') as outf:
-                try:
-                    result.return_code = int(outf.readline())
-                except ValueError:
-                    # The file may be empty due to running out of disk space.
-                    # Try a rebuild
-                    result.return_code = RETURN_CODE_RETRY
-
-            # Check the signal that the build needs to be retried
-            if result.return_code == RETURN_CODE_RETRY:
-                will_build = True
-            elif will_build:
-                err_file = self.builder.get_err_file(commit_upto, brd.target)
-                if os.path.exists(err_file) and os.stat(err_file).st_size:
-                    result.stderr = 'bad'
-                elif not force_build:
-                    # The build passed, so no need to build it again
-                    will_build = False
+        will_build = self._read_done_file(commit_upto, brd, result, force_build,
+                                          force_build_failures)
 
         if will_build:
             # We are going to have to build it. First, get a toolchain
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 51/60] buildman: Move code to remove old outputs

[Simon Glass]

Put this in its own function to reduce the size of the run_commit()
function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index b4891059b6db..4abad86ebc7b 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -42,6 +42,24 @@ def mkdir(dirname, parents = False):
         else:
             raise
 
+
+def _remove_old_outputs(out_dir):
+    """Remove any old output-target files
+
+    Args:
+        out_dir (str): Output directory for the build
+
+    Since we use a build directory that was previously used by another
+    board, it may have produced an SPL image. If we don't remove it (i.e.
+    see do_config and self.mrproper below) then it will appear to be the
+    output of this build, even if it does not produce SPL images.
+    """
+    for elf in BASE_ELF_FILENAMES:
+        fname = os.path.join(out_dir, elf)
+        if os.path.exists(fname):
+            os.remove(fname)
+
+
 # pylint: disable=R0903
 class BuilderJob:
     """Holds information about a job to be performed by a thread
@@ -366,15 +384,7 @@ class BuilderThread(threading.Thread):
                 config_args = [f'{brd.target}_defconfig']
                 config_out = io.StringIO()
 
-                # Remove any output targets. Since we use a build directory that
-                # was previously used by another board, it may have produced an
-                # SPL image. If we don't remove it (i.e. see do_config and
-                # self.mrproper below) then it will appear to be the output of
-                # this build, even if it does not produce SPL images.
-                for elf in BASE_ELF_FILENAMES:
-                    fname = os.path.join(out_dir, elf)
-                    if os.path.exists(fname):
-                        os.remove(fname)
+                _remove_old_outputs(out_dir)
 
                 # If we need to reconfigure, do that now
                 cfg_file = os.path.join(out_dir, '.config')
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 52/60] buildman: Move code to decide output dirs

[Simon Glass]

Put this in its own function to reduce the size of the run_commit()
function.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 34 ++++++++++++++++++++++++---------
 1 file changed, 25 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 4abad86ebc7b..78405956ef51 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -302,6 +302,30 @@ class BuilderThread(threading.Thread):
                     will_build = False
         return will_build
 
+    def _decide_dirs(self, brd, work_dir, work_in_output):
+        """Decide the output directory to use
+
+        Args:
+            work_dir (str): Directory to which the source will be checked out
+            work_in_output (bool): Use the output directory as the work
+                directory and don't write to a separate output directory.
+
+        Returns:
+            tuple:
+                out_dir (str): Output directory for the build
+                out_rel_dir (str): Output directory relatie to the current dir
+        """
+        if work_in_output or self.builder.in_tree:
+            out_rel_dir = None
+            out_dir = work_dir
+        else:
+            if self.per_board_out_dir:
+                out_rel_dir = os.path.join('..', brd.target)
+            else:
+                out_rel_dir = 'build'
+            out_dir = os.path.join(work_dir, out_rel_dir)
+        return out_dir, out_rel_dir
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -338,15 +362,7 @@ class BuilderThread(threading.Thread):
         # self.make() below, in the event that we do a build.
         result = command.CommandResult()
         result.return_code = 0
-        if work_in_output or self.builder.in_tree:
-            out_rel_dir = None
-            out_dir = work_dir
-        else:
-            if self.per_board_out_dir:
-                out_rel_dir = os.path.join('..', brd.target)
-            else:
-                out_rel_dir = 'build'
-            out_dir = os.path.join(work_dir, out_rel_dir)
+        out_dir, out_rel_dir = self._decide_dirs(brd, work_dir, work_in_output)
 
         # Check if the job was already completed last time
         will_build = self._read_done_file(commit_upto, brd, result, force_build,
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 53/60] buildman: Move checkout code to a separate function

[Simon Glass]

Put this in its own function to reduce the size of the run_commit()
function

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 78405956ef51..0c73b8646b1c 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -326,6 +326,26 @@ class BuilderThread(threading.Thread):
             out_dir = os.path.join(work_dir, out_rel_dir)
         return out_dir, out_rel_dir
 
+    def _checkout(self, commit_upto, work_dir):
+        """Checkout the right commit
+
+        Args:
+            commit_upto (int): Commit number to build (0...n-1)
+            work_dir (str): Directory to which the source will be checked out
+
+        Returns:
+            Commit: Commit being built, or 'current' for current source
+        """
+        if self.builder.commits:
+            commit = self.builder.commits[commit_upto]
+            if self.builder.checkout:
+                git_dir = os.path.join(work_dir, '.git')
+                gitutil.checkout(commit.hash, git_dir, work_dir, force=True)
+        else:
+            commit = 'current'
+        return commit
+
+
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
                   adjust_cfg):
@@ -381,15 +401,7 @@ class BuilderThread(threading.Thread):
                     # to be reported.
 
             if self.toolchain:
-                # Checkout the right commit
-                if self.builder.commits:
-                    commit = self.builder.commits[commit_upto]
-                    if self.builder.checkout:
-                        git_dir = os.path.join(work_dir, '.git')
-                        gitutil.checkout(commit.hash, git_dir, work_dir,
-                                         force=True)
-                else:
-                    commit = 'current'
+                commit = self._checkout(commit_upto, work_dir)
 
                 # Set up the environment and command line
                 env = self.toolchain.MakeEnvironment(self.builder.full_path)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 54/60] buildman: Create a function to handle config and build

[Simon Glass]

Move this code into a _config_and_build() function, so reduce the size of
run_commit().

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 97 +++++++++++++++++++++------------
 1 file changed, 61 insertions(+), 36 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 0c73b8646b1c..aa4a9d9919c6 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -345,6 +345,64 @@ class BuilderThread(threading.Thread):
             commit = 'current'
         return commit
 
+    def _config_and_build(self, commit_upto, brd, work_dir, do_config,
+                          config_only, adjust_cfg, commit, out_dir, out_rel_dir,
+                          result):
+        """Do the build, configuring first if necessary
+
+        Args:
+            commit_upto (int): Commit number to build (0...n-1)
+            brd (Board): Board to create arguments for
+            work_dir (str): Directory to which the source will be checked out
+            do_config (bool): True to run a make <board>_defconfig on the source
+            config_only (bool): Only configure the source, do not build it
+            adjust_cfg (list of str): See the cfgutil module and run_commit()
+            commit (Commit): Commit only being built
+            out_dir (str): Output directory for the build
+            out_rel_dir (str): Output directory relatie to the current dir
+            result (CommandResult): Previous result
+
+        Returns:
+            tuple:
+                result (CommandResult): Result of the build
+                do_config (bool): indicates whether 'make config' is needed on
+                    the next incremental build
+        """
+        # Set up the environment and command line
+        env = self.toolchain.MakeEnvironment(self.builder.full_path)
+        mkdir(out_dir)
+
+        args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
+                                              work_dir, commit_upto)
+        config_args = [f'{brd.target}_defconfig']
+        config_out = io.StringIO()
+
+        _remove_old_outputs(out_dir)
+
+        # If we need to reconfigure, do that now
+        cfg_file = os.path.join(out_dir, '.config')
+        cmd_list = []
+        if do_config or adjust_cfg:
+            result = self._reconfigure(
+                commit, brd, cwd, args, env, config_args, config_out, cmd_list)
+            do_config = False   # No need to configure next time
+            if adjust_cfg:
+                cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
+
+        # Now do the build, if everything looks OK
+        if result.return_code == 0:
+            result = self._build(commit, brd, cwd, args, env, cmd_list,
+                                 config_only)
+            if adjust_cfg:
+                errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
+                if errs:
+                    result.stderr += errs
+                    result.return_code = 1
+        result.stderr = result.stderr.replace(src_dir + '/', '')
+        if self.builder.verbose_build:
+            result.stdout = config_out.getvalue() + result.stdout
+        result.cmd_list = cmd_list
+        return result, do_config
 
     def run_commit(self, commit_upto, brd, work_dir, do_config, config_only,
                   force_build, force_build_failures, work_in_output,
@@ -402,42 +460,9 @@ class BuilderThread(threading.Thread):
 
             if self.toolchain:
                 commit = self._checkout(commit_upto, work_dir)
-
-                # Set up the environment and command line
-                env = self.toolchain.MakeEnvironment(self.builder.full_path)
-                mkdir(out_dir)
-
-                args, cwd, src_dir = self._build_args(brd, out_dir, out_rel_dir,
-                                                      work_dir, commit_upto)
-                config_args = [f'{brd.target}_defconfig']
-                config_out = io.StringIO()
-
-                _remove_old_outputs(out_dir)
-
-                # If we need to reconfigure, do that now
-                cfg_file = os.path.join(out_dir, '.config')
-                cmd_list = []
-                if do_config or adjust_cfg:
-                    result = self._reconfigure(
-                        commit, brd, cwd, args, env, config_args, config_out,
-                        cmd_list)
-                    do_config = False   # No need to configure next time
-                    if adjust_cfg:
-                        cfgutil.adjust_cfg_file(cfg_file, adjust_cfg)
-
-                # Now do the build, if everything looks OK
-                if result.return_code == 0:
-                    result = self._build(commit, brd, cwd, args, env, cmd_list,
-                                         config_only)
-                    if adjust_cfg:
-                        errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
-                        if errs:
-                            result.stderr += errs
-                            result.return_code = 1
-                result.stderr = result.stderr.replace(src_dir + '/', '')
-                if self.builder.verbose_build:
-                    result.stdout = config_out.getvalue() + result.stdout
-                result.cmd_list = cmd_list
+                result, do_config = self._config_and_build(
+                    commit_upto, brd, work_dir, do_config, config_only,
+                    adjust_cfg, commit, out_dir, out_rel_dir, result)
             else:
                 result.return_code = 1
                 result.stderr = f'No tool chain for {brd.arch}\n'
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 55/60] buildman: Avoid passing result into _read_done_file()

[Simon Glass]

Move the creating of the result object into the function which sets it
up, to simplify the code.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index aa4a9d9919c6..d3912390bc4f 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -262,21 +262,26 @@ class BuilderThread(threading.Thread):
             result.return_code = 0
         return result
 
-    def _read_done_file(self, commit_upto, brd, result, force_build,
+    def _read_done_file(self, commit_upto, brd, force_build,
                         force_build_failures):
         """Check the 'done' file and see if this commit should be built
 
         Args:
             commit (Commit): Commit only being built
             brd (Board): Board being built
-            result (CommandResult): result object to update
             force_build (bool): Force a build even if one was previously done
             force_build_failures (bool): Force a bulid if the previous result
                 showed failure
 
         Returns:
-            bool: True if build should be built
+            tuple:
+                bool: True if build should be built
+                CommandResult: if there was a previous run:
+                    - already_done set to True
+                    - return_code set to return code
+                    - result.stderr set to 'bad' if stderr output was recorded
         """
+        result = command.CommandResult()
         done_file = self.builder.get_done_file(commit_upto, brd.target)
         result.already_done = os.path.exists(done_file)
         will_build = (force_build or force_build_failures or
@@ -300,7 +305,7 @@ class BuilderThread(threading.Thread):
                 elif not force_build:
                     # The build passed, so no need to build it again
                     will_build = False
-        return will_build
+        return will_build, result
 
     def _decide_dirs(self, brd, work_dir, work_in_output):
         """Decide the output directory to use
@@ -438,13 +443,11 @@ class BuilderThread(threading.Thread):
         """
         # Create a default result - it will be overwritte by the call to
         # self.make() below, in the event that we do a build.
-        result = command.CommandResult()
-        result.return_code = 0
         out_dir, out_rel_dir = self._decide_dirs(brd, work_dir, work_in_output)
 
         # Check if the job was already completed last time
-        will_build = self._read_done_file(commit_upto, brd, result, force_build,
-                                          force_build_failures)
+        will_build, result = self._read_done_file(commit_upto, brd, force_build,
+                                                  force_build_failures)
 
         if will_build:
             # We are going to have to build it. First, get a toolchain
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 56/60] buildman: Tidy up reporting of a toolchain error

[Simon Glass]

Provide the text of the exception when something goes wrong.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 7 +------
 tools/buildman/func_test.py     | 6 ++++--
 2 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index d3912390bc4f..043e92b6d92d 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -457,18 +457,13 @@ class BuilderThread(threading.Thread):
                 except ValueError as err:
                     result.return_code = 10
                     result.stdout = ''
-                    result.stderr = str(err)
-                    # TODO(sjg@chromium.org): This gets swallowed, but needs
-                    # to be reported.
+                    result.stderr = f'Tool chain error for {brd.arch}: {str(err)}'
 
             if self.toolchain:
                 commit = self._checkout(commit_upto, work_dir)
                 result, do_config = self._config_and_build(
                     commit_upto, brd, work_dir, do_config, config_only,
                     adjust_cfg, commit, out_dir, out_rel_dir, result)
-            else:
-                result.return_code = 1
-                result.stderr = f'No tool chain for {brd.arch}\n'
             result.already_done = False
 
         result.toolchain = self.toolchain
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index f5058559b4fa..aa0838538d23 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -503,8 +503,10 @@ Some images are invalid'''
                 if brd.arch != 'sandbox':
                   errfile = self._builder.get_err_file(commit, brd.target)
                   fd = open(errfile)
-                  self.assertEqual(fd.readlines(),
-                          ['No tool chain for %s\n' % brd.arch])
+                  self.assertEqual(
+                      fd.readlines(),
+                      [f'Tool chain error for {brd.arch}: '
+                       f"No tool chain found for arch '{brd.arch}'"])
                   fd.close()
 
     def testBranch(self):
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 57/60] buildman: Tidy up some comments in builderthread

[Simon Glass]

Make sure all functions have full argument documentation.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 66 ++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 30 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 043e92b6d92d..ed84a0f6baa3 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -23,11 +23,15 @@ from u_boot_pylib import command
 RETURN_CODE_RETRY = -1
 BASE_ELF_FILENAMES = ['u-boot', 'spl/u-boot-spl', 'tpl/u-boot-tpl']
 
-def mkdir(dirname, parents = False):
+def mkdir(dirname, parents=False):
     """Make a directory if it doesn't already exist.
 
     Args:
-        dirname: Directory to create
+        dirname (str): Directory to create
+        parents (bool): True to also make parent directories
+
+    Raises:
+        OSError: File already exists
     """
     try:
         if parents:
@@ -141,14 +145,16 @@ class BuilderThread(threading.Thread):
         argument is only for information.
 
         Args:
-            commit: Commit object that is being built
-            brd: Board object that is being built
-            stage: Stage of the build. Valid stages are:
+            commit (Commit): Commit that is being built
+            brd (Board): Board that is being built
+            stage (str): Stage of the build. Valid stages are:
                         mrproper - can be called to clean source
                         config - called to configure for a board
                         build - the main make invocation - it does the build
-            args: A list of arguments to pass to 'make'
-            kwargs: A list of keyword arguments to pass to command.run_pipe()
+            cwd (str): Working directory to set, or None to leave it alone
+            *args (list of str): Arguments to pass to 'make'
+            **kwargs (dict): A list of keyword arguments to pass to
+                command.run_pipe()
 
         Returns:
             CommandResult object
@@ -418,16 +424,16 @@ class BuilderThread(threading.Thread):
         the build and just return the previously-saved results.
 
         Args:
-            commit_upto: Commit number to build (0...n-1)
-            brd: Board object to build
-            work_dir: Directory to which the source will be checked out
-            do_config: True to run a make <board>_defconfig on the source
-            config_only: Only configure the source, do not build it
-            force_build: Force a build even if one was previously done
-            force_build_failures: Force a bulid if the previous result showed
-                failure
-            work_in_output: Use the output directory as the work directory and
-                don't write to a separate output directory.
+            commit_upto (int): Commit number to build (0...n-1)
+            brd (Board): Board to build
+            work_dir (str): Directory to which the source will be checked out
+            do_config (bool): True to run a make <board>_defconfig on the source
+            config_only (bool): Only configure the source, do not build it
+            force_build (bool): Force a build even if one was previously done
+            force_build_failures (bool): Force a bulid if the previous result
+                showed failure
+            work_in_output (bool) : Use the output directory as the work
+                directory and don't write to a separate output directory.
             adjust_cfg (list of str): List of changes to make to .config file
                 before building. Each is one of (where C is either CONFIG_xxx
                 or just xxx):
@@ -476,11 +482,11 @@ class BuilderThread(threading.Thread):
         """Write a built result to the output directory.
 
         Args:
-            result: CommandResult object containing result to write
-            keep_outputs: True to store the output binaries, False
+            result (CommandResult): result to write
+            keep_outputs (bool): True to store the output binaries, False
                 to delete them
-            work_in_output: Use the output directory as the work directory and
-                don't write to a separate output directory.
+            work_in_output (bool): Use the output directory as the work
+                directory and don't write to a separate output directory.
         """
         # If we think this might have been aborted with Ctrl-C, record the
         # failure but not that we are 'done' with this board. A retry may fix
@@ -618,10 +624,10 @@ class BuilderThread(threading.Thread):
         """Copy files from the build directory to the output.
 
         Args:
-            out_dir: Path to output directory containing the files
-            build_dir: Place to copy the files
-            dirname: Source directory, '' for normal U-Boot, 'spl' for SPL
-            patterns: A list of filenames (strings) to copy, each relative
+            out_dir (str): Path to output directory containing the files
+            build_dir (str): Place to copy the files
+            dirname (str): Source directory, '' for normal U-Boot, 'spl' for SPL
+            patterns (list of str): A list of filenames to copy, each relative
                to the build directory
         """
         for pattern in patterns:
@@ -638,10 +644,10 @@ class BuilderThread(threading.Thread):
         """Send a result to the builder for processing
 
         Args:
-            result: CommandResult object containing the results of the build
+            result (CommandResult): results of the build
 
         Raises:
-            ValueError if self.test_exception is true (for testing)
+            ValueError: self.test_exception is true (for testing)
         """
         if self.test_exception:
             raise ValueError('test exception')
@@ -656,10 +662,10 @@ class BuilderThread(threading.Thread):
         A job consists of a building a list of commits for a particular board.
 
         Args:
-            job: Job to build
+            job (Job): Job to build
 
-        Returns:
-            List of Result objects
+        Raises:
+            ValueError: Thread was interrupted
         """
         brd = job.brd
         work_dir = self.builder.get_thread_dir(self.thread_num)
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 58/60] buildman: Move copy_files() out ot BuilderThread class

[Simon Glass]

This does not need to be in the class. Move it out to avoid a pylint
warning.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

(no changes since v1)

 tools/buildman/builderthread.py | 47 +++++++++++++++++----------------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index ed84a0f6baa3..25f460c207db 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -64,6 +64,27 @@ def _remove_old_outputs(out_dir):
             os.remove(fname)
 
 
+def copy_files(out_dir, build_dir, dirname, patterns):
+    """Copy files from the build directory to the output.
+
+    Args:
+        out_dir (str): Path to output directory containing the files
+        build_dir (str): Place to copy the files
+        dirname (str): Source directory, '' for normal U-Boot, 'spl' for SPL
+        patterns (list of str): A list of filenames to copy, each relative
+           to the build directory
+    """
+    for pattern in patterns:
+        file_list = glob.glob(os.path.join(out_dir, dirname, pattern))
+        for fname in file_list:
+            target = os.path.basename(fname)
+            if dirname:
+                base, ext = os.path.splitext(target)
+                if ext:
+                    target = f'{base}-{dirname}{ext}'
+            shutil.copy(fname, os.path.join(build_dir, target))
+
+
 # pylint: disable=R0903
 class BuilderJob:
     """Holds information about a job to be performed by a thread
@@ -592,7 +613,7 @@ class BuilderThread(threading.Thread):
                             capture_stderr=True, cwd=result.out_dir,
                             raise_on_error=False, env=env)
             if not work_in_output:
-                self.copy_files(result.out_dir, build_dir, '', ['uboot.env'])
+                copy_files(result.out_dir, build_dir, '', ['uboot.env'])
 
             # Write out the image sizes file. This is similar to the output
             # of binutil's 'size' utility, but it omits the header line and
@@ -607,7 +628,7 @@ class BuilderThread(threading.Thread):
         if not work_in_output:
             # Write out the configuration files, with a special case for SPL
             for dirname in ['', 'spl', 'tpl']:
-                self.copy_files(
+                copy_files(
                     result.out_dir, build_dir, dirname,
                     ['u-boot.cfg', 'spl/u-boot-spl.cfg', 'tpl/u-boot-tpl.cfg',
                      '.config', 'include/autoconf.mk',
@@ -615,31 +636,11 @@ class BuilderThread(threading.Thread):
 
             # Now write the actual build output
             if keep_outputs:
-                self.copy_files(
+                copy_files(
                     result.out_dir, build_dir, '',
                     ['u-boot*', '*.bin', '*.map', '*.img', 'MLO', 'SPL',
                      'include/autoconf.mk', 'spl/u-boot-spl*'])
 
-    def copy_files(self, out_dir, build_dir, dirname, patterns):
-        """Copy files from the build directory to the output.
-
-        Args:
-            out_dir (str): Path to output directory containing the files
-            build_dir (str): Place to copy the files
-            dirname (str): Source directory, '' for normal U-Boot, 'spl' for SPL
-            patterns (list of str): A list of filenames to copy, each relative
-               to the build directory
-        """
-        for pattern in patterns:
-            file_list = glob.glob(os.path.join(out_dir, dirname, pattern))
-            for fname in file_list:
-                target = os.path.basename(fname)
-                if dirname:
-                    base, ext = os.path.splitext(target)
-                    if ext:
-                        target = f'{base}-{dirname}{ext}'
-                shutil.copy(fname, os.path.join(build_dir, target))
-
     def _send_result(self, result):
         """Send a result to the builder for processing
 
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 59/60] buildman: Add a way to print the architecture for a board

[Simon Glass]

This is useful for some tools and is easily available for buildman. Add
a new --print-arch option.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Add new patch to print the architecture for a board

 tools/buildman/cmdline.py   |  2 ++
 tools/buildman/control.py   | 24 ++++++++++++++++++++++++
 tools/buildman/func_test.py |  7 +++++++
 3 files changed, 33 insertions(+)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index e295c7aef1a0..41384840427a 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -119,6 +119,8 @@ def add_after_m(parser):
           default=False, help="Use full toolchain path in CROSS_COMPILE")
     parser.add_argument('-P', '--per-board-out-dir', action='store_true',
           default=False, help="Use an O= (output) directory per board rather than per thread")
+    parser.add_argument('--print-arch', action='store_true',
+          default=False, help="Print the architecture for a board (ARCH=)")
     parser.add_argument('-r', '--reproducible-builds', action='store_true',
           help='Set SOURCE_DATE_EPOCH=0 to suuport a reproducible build')
     parser.add_argument('-R', '--regen-board-list', type=str,
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 4306d4eb2a7c..8a29534d602f 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -140,6 +140,26 @@ def show_toolchain_prefix(brds, toolchains):
     tchain = tc_set.pop()
     print(tchain.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
 
+def show_arch(brds):
+    """Show information about a the architecture used by one or more boards
+
+    The function checks that all boards use the same architecture, then prints
+    the correct value for ARCH.
+
+    Args:
+        boards: Boards object containing selected boards
+
+    Return:
+        None on success, string error message otherwise
+    """
+    board_selected = brds.get_selected_dict()
+    arch_set = set()
+    for brd in board_selected.values():
+        arch_set.add(brd.arch)
+    if len(arch_set) != 1:
+        sys.exit('Supplied boards must share one arch')
+    print(arch_set.pop())
+
 def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
     """Figure out whether to allow external blobs
 
@@ -597,6 +617,10 @@ def do_buildman(args, toolchains=None, make_func=None, brds=None,
         show_toolchain_prefix(brds, toolchains)
         return 0
 
+    if args.print_arch:
+        show_arch(brds)
+        return 0
+
     series = determine_series(selected, col, git_dir, args.count,
                               args.branch, args.work_in_output)
 
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index aa0838538d23..d89bde1d95e2 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -812,3 +812,10 @@ CONFIG_LOCALVERSION=y
 
         self._RunControl('--boards', 'board1,board2', '--boards', 'board4')
         self.assertEqual(3, self._builder.count)
+
+    def test_print_arch(self):
+        """Test that we can print the board architecture"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            result = self._RunControl('--print-arch', 'board0')
+        self.assertEqual('arm\n', stdout.getvalue())
+        self.assertEqual('', stderr.getvalue())
-- 
2.41.0.255.g8b1d071c50-goog

[PATCH v2 60/60] buildman: Enable test coverage

[Simon Glass]

Enable measuring test coverage for buildman so we can see the gaps. It is
currently at 68%.

Signed-off-by: Simon Glass <sjg@chromium.org>
---

Changes in v2:
- Drop patch to move -A logic up a little (since it breaks it)

 tools/buildman/cmdline.py       |  2 ++
 tools/buildman/main.py          | 15 ++++++++++++++-
 tools/u_boot_pylib/test_util.py | 10 +++++++---
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index 41384840427a..983165546ea2 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -136,6 +136,8 @@ def add_after_m(parser):
                           help='Skip tests which need the network')
         parser.add_argument('-t', '--test', action='store_true', dest='test',
                           default=False, help='run tests')
+        parser.add_argument('--coverage', action='store_true',
+                            help='Calculated test coverage')
     parser.add_argument('-T', '--threads', type=int,
           default=None,
           help='Number of builder threads to use (0=single-thread)')
diff --git a/tools/buildman/main.py b/tools/buildman/main.py
index 593911353101..cb3d0b4fc663 100755
--- a/tools/buildman/main.py
+++ b/tools/buildman/main.py
@@ -47,12 +47,22 @@ def run_tests(skip_net_tests, verbose, args):
     # Run the entry tests first ,since these need to be the first to import the
     # 'entry' module.
     result = test_util.run_test_suites(
-        'buildman', False, verbose, False, None, test_name, [],
+        'buildman', False, verbose, False, args.threads, test_name, [],
         [test.TestBuild, func_test.TestFunctional,
          'buildman.toolchain', 'patman.gitutil'])
 
     return (0 if result.wasSuccessful() else 1)
 
+def run_test_coverage():
+    """Run the tests and check that we get 100% coverage"""
+    test_util.run_test_coverage(
+        'tools/buildman/buildman', None,
+        ['tools/patman/*.py', 'tools/u_boot_pylib/*', '*test_fdt.py',
+         'tools/buildman/kconfiglib.py', 'tools/buildman/*test*.py',
+         'tools/buildman/main.py'],
+        '/tmp/b', single_thread='-T1')
+
+
 def run_buildman():
     """Run bulidman
 
@@ -68,6 +78,9 @@ def run_buildman():
     if cmdline.HAS_TESTS and args.test:
         run_tests(args.skip_net_tests, args.verbose, args)
 
+    elif cmdline.HAS_TESTS and args.coverage:
+        run_test_coverage()
+
     elif args.full_help:
         tools.print_full_help(str(files('buildman').joinpath('README.rst')))
 
diff --git a/tools/u_boot_pylib/test_util.py b/tools/u_boot_pylib/test_util.py
index e7564e10c997..f18d385d9951 100644
--- a/tools/u_boot_pylib/test_util.py
+++ b/tools/u_boot_pylib/test_util.py
@@ -24,7 +24,7 @@ except:
 
 
 def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None,
-                    extra_args=None):
+                    extra_args=None, single_thread='-P1'):
     """Run tests and check that we get 100% coverage
 
     Args:
@@ -39,6 +39,9 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
         required: List of modules which must be in the coverage report
         extra_args (str): Extra arguments to pass to the tool before the -t/test
             arg
+        single_thread (str): Argument string to make the tests run
+            single-threaded. This is necessary to get proper coverage results.
+            The default is '-P0'
 
     Raises:
         ValueError if the code coverage is not 100%
@@ -58,8 +61,9 @@ def run_test_coverage(prog, filter_fname, exclude_list, build_dir, required=None
     if build_dir:
         prefix = 'PYTHONPATH=$PYTHONPATH:%s/sandbox_spl/tools ' % build_dir
     cmd = ('%spython3-coverage run '
-           '--omit "%s" %s %s %s -P1' % (prefix, ','.join(glob_list),
-                                         prog, extra_args or '', test_cmd))
+           '--omit "%s" %s %s %s %s' % (prefix, ','.join(glob_list),
+                                        prog, extra_args or '', test_cmd,
+                                        single_thread or '-P1'))
     os.system(cmd)
     stdout = command.output('python3-coverage', 'report')
     lines = stdout.splitlines()
-- 
2.41.0.255.g8b1d071c50-goog