buildman: Add --allow-missing flag to allow missing blobs

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>

.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;

.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;

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

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:

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))

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
 --------------------------
 

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',

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

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))