[PATCH v5 15/16] buildman: Add --allow-missing flag to allow missing blobs
Simon Glass
sjg at chromium.org
Thu Nov 10 03:14:53 CET 2022
From: Tom Rini <trini at konsulko.com>
Add a new flag to buildman so that we will in turn pass
BINMAN_ALLOW_MISSING=1 to 'make'. Make use of this flag in CI.
Allow the settings file to control this.
Cc: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
Cc: Simon Glass <sjg at chromium.org>
Signed-off-by: Tom Rini <trini at konsulko.com>
Signed-off-by: Simon Glass <sjg at chromium.org>
---
Changes in v5:
- Split out config_fname change into a separate patch
- Rename GetGlobalValue to GetGlobalItemValue
- Fix 'items' typo
- Simplify implementation of GetGlobalItemValue()
- Use '[global]' for the doc heading
- Use double tick for arguments
- Drop redundant allow_missing code
- Split out the tests more
Changes in v4:
- Various changes that were unfortunately not recorded
Changes in v3:
- Add tests docs and a settings-file option
.azure-pipelines.yml | 2 +-
.gitlab-ci.yml | 6 +-
tools/buildman/bsettings.py | 11 ++++
tools/buildman/builder.py | 5 +-
tools/buildman/builderthread.py | 2 +
tools/buildman/buildman.rst | 43 +++++++++++++
tools/buildman/cmdline.py | 6 ++
tools/buildman/control.py | 24 +++++++-
tools/buildman/func_test.py | 106 +++++++++++++++++++++++++++++++-
9 files changed, 197 insertions(+), 8 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index bda762451fd..665b5d2026f 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -553,7 +553,7 @@ stages:
cat << "EOF" >> build.sh
if [[ "${BUILDMAN}" != "" ]]; then
ret=0;
- tools/buildman/buildman -o /tmp -P -E -W ${BUILDMAN} ${OVERRIDE} || ret=$?;
+ tools/buildman/buildman -o /tmp -PEWM ${BUILDMAN} ${OVERRIDE} || ret=$?;
if [[ $ret -ne 0 ]]; then
tools/buildman/buildman -o /tmp -seP ${BUILDMAN};
exit $ret;
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 6f4c34fc4a3..3deaeca1cdd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -81,7 +81,7 @@ build all 32bit ARM platforms:
stage: world build
script:
- ret=0;
- ./tools/buildman/buildman -o /tmp -P -E -W arm -x aarch64 || ret=$?;
+ ./tools/buildman/buildman -o /tmp -PEWM arm -x aarch64 || ret=$?;
if [[ $ret -ne 0 ]]; then
./tools/buildman/buildman -o /tmp -seP;
exit $ret;
@@ -93,7 +93,7 @@ build all 64bit ARM platforms:
- virtualenv -p /usr/bin/python3 /tmp/venv
- . /tmp/venv/bin/activate
- ret=0;
- ./tools/buildman/buildman -o /tmp -P -E -W aarch64 || ret=$?;
+ ./tools/buildman/buildman -o /tmp -PEWM aarch64 || ret=$?;
if [[ $ret -ne 0 ]]; then
./tools/buildman/buildman -o /tmp -seP;
exit $ret;
@@ -113,7 +113,7 @@ build all other platforms:
stage: world build
script:
- ret=0;
- ./tools/buildman/buildman -o /tmp -P -E -W -x arm,powerpc || ret=$?;
+ ./tools/buildman/buildman -o /tmp -PEWM -x arm,powerpc || ret=$?;
if [[ $ret -ne 0 ]]; then
./tools/buildman/buildman -o /tmp -seP;
exit $ret;
diff --git a/tools/buildman/bsettings.py b/tools/buildman/bsettings.py
index 9b93b7a51e1..0eb894a558c 100644
--- a/tools/buildman/bsettings.py
+++ b/tools/buildman/bsettings.py
@@ -47,6 +47,17 @@ def GetItems(section):
except:
raise
+def GetGlobalItemValue(name):
+ """Get an item from the 'global' section of the config.
+
+ Args:
+ name: name of item to retrieve
+
+ Returns:
+ str: Value of item, or None if not present
+ """
+ return settings.get('global', name, fallback=None)
+
def SetItem(section, tag, value):
"""Set an item and write it back to the settings file"""
global settings
diff --git a/tools/buildman/builder.py b/tools/buildman/builder.py
index 76252b90792..c2a69027f88 100644
--- a/tools/buildman/builder.py
+++ b/tools/buildman/builder.py
@@ -252,7 +252,8 @@ class Builder:
mrproper=False, per_board_out_dir=False,
config_only=False, squash_config_y=False,
warnings_as_errors=False, work_in_output=False,
- test_thread_exceptions=False, adjust_cfg=None):
+ test_thread_exceptions=False, adjust_cfg=None,
+ allow_missing=False):
"""Create a new Builder object
Args:
@@ -290,6 +291,7 @@ class Builder:
~C to disable C
C=val to set the value of C (val must have quotes if C is
a string Kconfig
+ allow_missing: Run build with BINMAN_ALLOW_MISSING=1
"""
self.toolchains = toolchains
@@ -327,6 +329,7 @@ class Builder:
self.config_filenames = BASE_CONFIG_FILENAMES
self.work_in_output = work_in_output
self.adjust_cfg = adjust_cfg
+ self.allow_missing = allow_missing
self._ide = False
if not self.squash_config_y:
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 065d836d68c..680efae02d7 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -253,6 +253,8 @@ class BuilderThread(threading.Thread):
args.extend(['-j', str(self.builder.num_jobs)])
if self.builder.warnings_as_errors:
args.append('KCFLAGS=-Werror')
+ if self.builder.allow_missing:
+ args.append('BINMAN_ALLOW_MISSING=1')
config_args = ['%s_defconfig' % brd.target]
config_out = ''
args.extend(self.builder.toolchains.GetMakeArguments(brd))
diff --git a/tools/buildman/buildman.rst b/tools/buildman/buildman.rst
index 2a7593d08b9..2a83cb7e4f8 100644
--- a/tools/buildman/buildman.rst
+++ b/tools/buildman/buildman.rst
@@ -906,6 +906,25 @@ also allows build flags to be passed to 'make'. It consists of several
sections, with the section name in square brackets. Within each section are
a set of (tag, value) pairs.
+'[global]' section
+ allow-missing
+ Indicates the policy to use for missing blobs. Note that the flags
+ ``--allow-missing`` (``-M``) and ``--no-allow-missing`` (``--no-a``)
+ override these setting.
+
+ always
+ Run with ``-M`` by default.
+
+ multiple
+ Run with ``-M`` if more than one board is being built.
+
+ branch
+ Run with ``-M`` if a branch is being built.
+
+ Note that the last two can be given together::
+
+ allow-missing = multiple branch
+
'[toolchain]' section
This lists the available toolchains. The tag here doesn't matter, but
make sure it is unique. The value is the path to the toolchain. Buildman
@@ -1133,6 +1152,30 @@ not cause the build to fail:
buildman -o /tmp/build --board sandbox -wWI
+Support for binary blobs
+------------------------
+
+U-Boot is moving to using Binman (see :doc:`../develop/package/binman`) for
+dealing with the complexities of packaging U-Boot along with binary files from
+other projects. These are called 'external blobs' by Binman.
+
+Typically a missing external blob causes a build failure. For build testing of
+a lot of boards, or boards for which you do not have the blobs, you can use the
+-M flag to allow missing blobs. This marks the build as if it succeeded,
+although with warnings shown, including 'Some images are invalid'. If any boards
+fail in this way, buildman exits with status 101.
+
+To convert warnings to errors, use -E. To make buildman return success with
+these warnings, use -W.
+
+It is generally safe to default to enabling -M for all runs of buildman, so long
+as you check the exit code. To do this, add::
+
+ allow-missing = "always"
+
+to the top of the buildman_settings_ file.
+
+
Changing the configuration
--------------------------
diff --git a/tools/buildman/cmdline.py b/tools/buildman/cmdline.py
index b29c1eb5ee7..c485994e9fe 100644
--- a/tools/buildman/cmdline.py
+++ b/tools/buildman/cmdline.py
@@ -75,6 +75,12 @@ def ParseArgs():
help='List available tool chains (use -v to see probing detail)')
parser.add_option('-m', '--mrproper', action='store_true',
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'),
+ parser.add_option(
+ '--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',
default=False, help="Do a dry run (describe actions, but do nothing)")
parser.add_option('-N', '--no-subdirs', action='store_true', dest='no_subdirs',
diff --git a/tools/buildman/control.py b/tools/buildman/control.py
index 377b580b253..87e7d0e2012 100644
--- a/tools/buildman/control.py
+++ b/tools/buildman/control.py
@@ -111,6 +111,23 @@ def ShowToolchainPrefix(brds, toolchains):
print(tc.GetEnvArgs(toolchain.VAR_CROSS_COMPILE))
return None
+def get_allow_missing(opt_allow, opt_no_allow, num_selected, has_branch):
+ allow_missing = False
+ am_setting = bsettings.GetGlobalItemValue('allow-missing')
+ if am_setting:
+ if am_setting == 'always':
+ allow_missing = True
+ if 'multiple' in am_setting and num_selected > 1:
+ allow_missing = True
+ if 'branch' in am_setting and has_branch:
+ allow_missing = True
+
+ if opt_allow:
+ allow_missing = True
+ if opt_no_allow:
+ allow_missing = False
+ return allow_missing
+
def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
clean_dir=False, test_thread_exceptions=False):
"""The main control code for buildman
@@ -305,6 +322,10 @@ def DoBuildman(options, args, toolchains=None, make_func=None, brds=None,
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)
+
# Create a new builder with the selected options.
output_dir = options.output_dir
if options.branch:
@@ -329,7 +350,8 @@ def DoBuildman(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=adjust_cfg,
+ allow_missing=allow_missing)
builder.force_config_on_failure = not options.quick
if make_func:
builder.do_make = make_func
diff --git a/tools/buildman/func_test.py b/tools/buildman/func_test.py
index 37535518d86..559e4edf74b 100644
--- a/tools/buildman/func_test.py
+++ b/tools/buildman/func_test.py
@@ -22,6 +22,7 @@ from patman import tools
settings_data = '''
# Buildman settings file
+[global]
[toolchain]
@@ -205,6 +206,9 @@ class TestFunctional(unittest.TestCase):
self._test_branch = TEST_BRANCH
+ # Set to True to report missing blobs
+ self._missing = False
+
# Avoid sending any output and clear all terminal output
terminal.set_print_test_mode()
terminal.get_print_test_lines()
@@ -424,10 +428,21 @@ class TestFunctional(unittest.TestCase):
out_dir = arg[2:]
fname = os.path.join(cwd or '', out_dir, 'u-boot')
tools.write_file(fname, b'U-Boot')
- if type(commit) is not str:
+
+ # Handle missing blobs
+ if self._missing:
+ if 'BINMAN_ALLOW_MISSING=1' in args:
+ stderr = '''+Image 'main-section' is missing external blobs and is non-functional: intel-descriptor intel-ifwi intel-fsp-m intel-fsp-s intel-vbt
+Image 'main-section' has faked external blobs and is non-functional: descriptor.bin fsp_m.bin fsp_s.bin vbt.bin
+
+Some images are invalid'''
+ else:
+ stderr = "binman: Filename 'fsp.bin' not found in input path"
+ elif type(commit) is not str:
stderr = self._error.get((brd.target, commit.sequence))
+
if stderr:
- return command.CommandResult(return_code=1, stderr=stderr)
+ return command.CommandResult(return_code=2, stderr=stderr)
return command.CommandResult(return_code=0)
# Not handled, so abort
@@ -621,3 +636,90 @@ class TestFunctional(unittest.TestCase):
self.assertIn(
'Thread exception (use -T0 to run without threads): test exception',
stdout.getvalue())
+
+ def testBlobs(self):
+ """Test handling of missing blobs"""
+ self._missing = True
+
+ board0_dir = os.path.join(self._output_dir, 'current', 'board0')
+ errfile = os.path.join(board0_dir, 'err')
+ logfile = os.path.join(board0_dir, 'log')
+
+ # We expect failure when there are missing blobs
+ result = self._RunControl('board0', '-o', self._output_dir)
+ self.assertEqual(100, result)
+ self.assertTrue(os.path.exists(os.path.join(board0_dir, 'done')))
+ self.assertTrue(os.path.exists(errfile))
+ self.assertIn(b"Filename 'fsp.bin' not found in input path",
+ tools.read_file(errfile))
+
+ def testBlobsAllowMissing(self):
+ """Allow missing blobs - still failure but a different exit code"""
+ self._missing = True
+ result = self._RunControl('board0', '-o', self._output_dir, '-M',
+ clean_dir=True)
+ self.assertEqual(101, result)
+ board0_dir = os.path.join(self._output_dir, 'current', 'board0')
+ errfile = os.path.join(board0_dir, 'err')
+ self.assertTrue(os.path.exists(errfile))
+ self.assertIn(b'Some images are invalid', tools.read_file(errfile))
+
+ def testBlobsWarning(self):
+ """Allow missing blobs and ignore warnings"""
+ self._missing = True
+ result = self._RunControl('board0', '-o', self._output_dir, '-MW')
+ self.assertEqual(0, result)
+ board0_dir = os.path.join(self._output_dir, 'current', 'board0')
+ errfile = os.path.join(board0_dir, 'err')
+ self.assertIn(b'Some images are invalid', tools.read_file(errfile))
+
+ def testBlobSettings(self):
+ """Test with no settings"""
+ self.assertEqual(False,
+ control.get_allow_missing(False, False, 1, False))
+ self.assertEqual(True,
+ control.get_allow_missing(True, False, 1, False))
+ self.assertEqual(False,
+ control.get_allow_missing(True, True, 1, False))
+
+ def testBlobSettingsAlways(self):
+ """Test the 'always' policy"""
+ bsettings.SetItem('global', 'allow-missing', 'always')
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 1, False))
+ self.assertEqual(False,
+ control.get_allow_missing(False, True, 1, False))
+
+ def testBlobSettingsBranch(self):
+ """Test the 'branch' policy"""
+ bsettings.SetItem('global', 'allow-missing', 'branch')
+ self.assertEqual(False,
+ control.get_allow_missing(False, False, 1, False))
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 1, True))
+ self.assertEqual(False,
+ control.get_allow_missing(False, True, 1, True))
+
+ def testBlobSettingsMultiple(self):
+ """Test the 'multiple' policy"""
+ bsettings.SetItem('global', 'allow-missing', 'multiple')
+ self.assertEqual(False,
+ control.get_allow_missing(False, False, 1, False))
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 2, False))
+ self.assertEqual(False,
+ control.get_allow_missing(False, True, 2, False))
+
+ def testBlobSettingsBranchMultiple(self):
+ """Test the 'branch multiple' policy"""
+ bsettings.SetItem('global', 'allow-missing', 'branch multiple')
+ self.assertEqual(False,
+ control.get_allow_missing(False, False, 1, False))
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 1, True))
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 2, False))
+ self.assertEqual(True,
+ control.get_allow_missing(False, False, 2, True))
+ self.assertEqual(False,
+ control.get_allow_missing(False, True, 2, True))
--
2.38.1.431.g37b22c650d-goog
More information about the U-Boot
mailing list