Message ID | 20221107232847.753769-12-sjg@chromium.org |
---|---|
State | Superseded |
Delegated to: | Simon Glass |
Headers | show |
Series | buildman: Correct various issues with missing blobs | expand |
Hi Simon, On 11/8/22 00:28, Simon Glass wrote: > From: Tom Rini <trini@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@prevas.dk> > Cc: Simon Glass <sjg@chromium.org> > Signed-off-by: Tom Rini <trini@konsulko.com> > Signed-off-by: Simon Glass <sjg@chromium.org> > --- > > (no changes since v3) > There were actually substantial changes between v3 and v4. > Changes in v3: > - Add tests docs and a settings-file option > > .azure-pipelines.yml | 2 +- > .gitlab-ci.yml | 6 +-- > tools/buildman/bsettings.py | 16 ++++++ > tools/buildman/builder.py | 5 +- > tools/buildman/builderthread.py | 2 + > tools/buildman/buildman.rst | 43 +++++++++++++++ > tools/buildman/cmdline.py | 6 +++ > tools/buildman/control.py | 29 +++++++++- > tools/buildman/func_test.py | 96 +++++++++++++++++++++++++++++++-- > 9 files changed, 196 insertions(+), 9 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 dcc200ea79d..9df87f53e49 100644 > --- a/tools/buildman/bsettings.py > +++ b/tools/buildman/bsettings.py > @@ -5,6 +5,7 @@ import configparser > import os > import io > > +config_fname = None > This seems unrelated? > def Setup(fname=''): > """Set up the buildman settings module by reading config files > @@ -46,6 +47,21 @@ def GetItems(section): > except: > raise > > +def GetGlobalItem(name): I would rename this to GetGlobalItemValue or something more explicit, it's not clear that you're returning the value of a key from its name. > + """Get an items from the 'global' section of the config. > + s/items/item/ > + Args: > + name: name of item to retrieve > + > + Returns: > + str: Value of item, or None if not present > + """ > + items = GetItems('global') > + for item in items: > + if item[0] == name: > + return item[1] I had to run an example myself to see why this was done this way. configparser returns tuples with (key, value). You could simply do: return settings.get('global', name, fallback=None) > + return 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 33ad6d9e2c9..dd1a2be27b1 100644 > --- a/tools/buildman/buildman.rst > +++ b/tools/buildman/buildman.rst > @@ -913,6 +913,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 '[global]' section to match how the other sections are shown? I don't think it is specific? > + 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 Double-tick quote the arguments to highlight them (I also remember (maybe incorrectly) that two dashes are modified into something else than just two dashes when not highlighted?). > + 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 > @@ -1140,6 +1159,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..b1a22cdfc14 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.GetGlobalItem('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,15 @@ 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) > + > + if options.allow_missing: > + allow_missing = True > + if options.no_allow_missing: > + allow_missing = False > + This is already done in get_allow_missing so no need to be rundundant I think? > # Create a new builder with the selected options. > output_dir = options.output_dir > if options.branch: > @@ -329,7 +355,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 b4f3b46fcb1..32e305b8c59 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,13 +206,16 @@ 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() > > def tearDown(self): > shutil.rmtree(self._base_dir) > - #shutil.rmtree(self._output_dir) > + shutil.rmtree(self._output_dir) > Is this actually related? > def setupToolchains(self): > self._toolchains = toolchain.Toolchains() > @@ -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,78 @@ 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)) > + > + # Allow missing blobs - still failure but a different exit code > + result = self._RunControl('board0', '-o', self._output_dir, '-M', > + clean_dir=True) > + self.assertEqual(101, result) > + self.assertTrue(os.path.exists(errfile)) > + self.assertIn(b'Some images are invalid', tools.read_file(errfile)) > + I would have a separate test for this one. > + # Allow missing blobs and ignore warnings > + result = self._RunControl('board0', '-o', self._output_dir, '-MW') > + self.assertEqual(0, result) > + self.assertIn(b'Some images are invalid', tools.read_file(errfile)) > + Ditto. Otherwise if there's a regression you don't know which one exactly easily. > + 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)) > + Same here and the tests below. Cheers, Quentin
Hi Quentin & Heinrich, On Wed, 9 Nov 2022 at 09:03, Quentin Schulz <quentin.schulz@theobroma-systems.com> wrote: > > Hi Simon, > > On 11/8/22 00:28, Simon Glass wrote: > > From: Tom Rini <trini@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@prevas.dk> > > Cc: Simon Glass <sjg@chromium.org> > > Signed-off-by: Tom Rini <trini@konsulko.com> > > Signed-off-by: Simon Glass <sjg@chromium.org> > > --- > > > > (no changes since v3) > > > > There were actually substantial changes between v3 and v4. > > > Changes in v3: > > - Add tests docs and a settings-file option > > > > .azure-pipelines.yml | 2 +- > > .gitlab-ci.yml | 6 +-- > > tools/buildman/bsettings.py | 16 ++++++ > > tools/buildman/builder.py | 5 +- > > tools/buildman/builderthread.py | 2 + > > tools/buildman/buildman.rst | 43 +++++++++++++++ > > tools/buildman/cmdline.py | 6 +++ > > tools/buildman/control.py | 29 +++++++++- > > tools/buildman/func_test.py | 96 +++++++++++++++++++++++++++++++-- > > 9 files changed, 196 insertions(+), 9 deletions(-) Thanks for the reviews. I'll send v5 soon. Regards, Simon
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 dcc200ea79d..9df87f53e49 100644 --- a/tools/buildman/bsettings.py +++ b/tools/buildman/bsettings.py @@ -5,6 +5,7 @@ import configparser import os import io +config_fname = None def Setup(fname=''): """Set up the buildman settings module by reading config files @@ -46,6 +47,21 @@ def GetItems(section): except: raise +def GetGlobalItem(name): + """Get an items from the 'global' section of the config. + + Args: + name: name of item to retrieve + + Returns: + str: Value of item, or None if not present + """ + items = GetItems('global') + for item in items: + if item[0] == name: + return item[1] + return 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 33ad6d9e2c9..dd1a2be27b1 100644 --- a/tools/buildman/buildman.rst +++ b/tools/buildman/buildman.rst @@ -913,6 +913,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 @@ -1140,6 +1159,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..b1a22cdfc14 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.GetGlobalItem('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,15 @@ 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) + + if options.allow_missing: + allow_missing = True + if options.no_allow_missing: + allow_missing = False + # Create a new builder with the selected options. output_dir = options.output_dir if options.branch: @@ -329,7 +355,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 b4f3b46fcb1..32e305b8c59 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,13 +206,16 @@ 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() def tearDown(self): shutil.rmtree(self._base_dir) - #shutil.rmtree(self._output_dir) + shutil.rmtree(self._output_dir) def setupToolchains(self): self._toolchains = toolchain.Toolchains() @@ -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,78 @@ 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)) + + # Allow missing blobs - still failure but a different exit code + result = self._RunControl('board0', '-o', self._output_dir, '-M', + clean_dir=True) + self.assertEqual(101, result) + self.assertTrue(os.path.exists(errfile)) + self.assertIn(b'Some images are invalid', tools.read_file(errfile)) + + # Allow missing blobs and ignore warnings + result = self._RunControl('board0', '-o', self._output_dir, '-MW') + self.assertEqual(0, result) + 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)) + + # Check 'always' + 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)) + + # Check 'branch' + 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)) + + # Check 'multiple' + 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)) + + # Check 'branch multiple' + 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))