[PATCH v4 2/3] binman: Support help messages for missing blobs

Alper Nebi Yasak alpernebiyasak at gmail.com
Tue Sep 8 18:37:06 CEST 2020


On 06/09/2020 19:39, Simon Glass wrote:
> When an external blob is missing it can be quite confusing for the user.
> Add a way to provide a help message that is shown.
> 
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
> 
> (no changes since v3)
> 
> Changes in v3:
> - Add a way to show help messages for missing blobs
> 
>  tools/binman/README                        |  6 ++
>  tools/binman/control.py                    | 69 +++++++++++++++++++++-
>  tools/binman/entry.py                      |  9 +++
>  tools/binman/ftest.py                      | 18 +++++-
>  tools/binman/missing-blob-help             | 11 ++++
>  tools/binman/test/168_fit_missing_blob.dts |  9 ++-
>  6 files changed, 119 insertions(+), 3 deletions(-)
>  create mode 100644 tools/binman/missing-blob-help
> 
> diff --git a/tools/binman/README b/tools/binman/README
> index 37ee3fc2d3b..f7bf285a915 100644
> --- a/tools/binman/README
> +++ b/tools/binman/README
> @@ -343,6 +343,12 @@ compress:
>  	Sets the compression algortihm to use (for blobs only). See the entry
>  	documentation for details.
>  
> +missing-msg:
> +	Sets the tag of the message to show if this entry is missing. This is
> +	used for external blobs. When they are missing it is helpful to show
> +	information about what needs to be fixed. See missing-blob-help for the
> +	message for each tag.
> +
>  The attributes supported for images and sections are described below. Several
>  are similar to those for entries.
>  
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index 15bfac582a7..ee5771e7292 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -9,6 +9,7 @@ from collections import OrderedDict
>  import glob
>  import os
>  import pkg_resources
> +import re
>  
>  import sys
>  from patman import tools
> @@ -22,6 +23,11 @@ from patman import tout
>  # Make this global so that it can be referenced from tests
>  images = OrderedDict()
>  
> +# Help text for each type of missing blob, dict:
> +#    key: Value of the entry's 'missing-msg' or entry name
> +#    value: Text for the help
> +missing_blob_help = {}
> +
>  def _ReadImageDesc(binman_node):
>      """Read the image descriptions from the /binman node
>  
> @@ -54,6 +60,66 @@ def _FindBinmanNode(dtb):
>              return node
>      return None
>  
> +def _ReadMissingBlobHelp():
> +    """Read the missing-blob-help file
> +
> +    This file containins help messages explaining what to do when external blobs
> +    are missing.
> +
> +    Returns:
> +        Dict:
> +            key: Message tag (str)
> +            value: Message text (str)
> +    """
> +
> +    def _FinishTag(tag, msg, result):
> +        if tag:
> +            result[tag] = msg.rstrip()
> +            tag = None
> +            msg = ''
> +        return tag, msg
> +
> +    my_data = pkg_resources.resource_string(__name__, 'missing-blob-help')
> +    re_tag = re.compile('^([-a-z0-9]+):$')
> +    result = {}
> +    tag = None
> +    msg = ''
> +    for line in my_data.decode('utf-8').splitlines():
> +        if not line.startswith('#'):
> +            m_tag = re_tag.match(line)
> +            if m_tag:
> +                _, msg = _FinishTag(tag, msg, result)
> +                tag = m_tag.group(1)
> +            elif tag:
> +                msg += line + '\n'
> +    _FinishTag(tag, msg, result)
> +    return result

This looks a bit complex, I think something like this would be clearer:

    result = {}
    tag = None
    for line in my_data.decode('utf-8').splitlines():
        m_tag = re_tag.match(line)
        if line.startswith('#'):
            continue
        elif m_tag:
            tag = m_tag.group(1)
            result[tag] = []
        elif tag:
            result[tag].append(line)
    for tag, lines in result.items():
        result[tag] = "".join(lines).rstrip()
    return result

> +
> +def _ShowBlobHelp(path, text):
> +    tout.Warning('\n%s:' % path)
> +    for line in text.splitlines():
> +        tout.Warning('   %s' % line)
> +
> +def _ShowHelpForMissingBlobs(missing_list):
> +    """Show help for each missing blob to help the user take action
> +
> +    Args:
> +        missing_list: List of Entry objects to show help for
> +    """
> +    global missing_blob_help
> +
> +    if not missing_blob_help:
> +        missing_blob_help = _ReadMissingBlobHelp()
> +
> +    for entry in missing_list:
> +        tags = entry.GetHelpTags()
> +
> +        # Show the first match help message
> +        for tag in tags:
> +            if tag in missing_blob_help:
> +                _ShowBlobHelp(entry._node.path, missing_blob_help[tag])
> +                break
> +
>  def GetEntryModules(include_testing=True):
>      """Get a set of entry class implementations
>  
> @@ -478,6 +544,7 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True,
>      if missing_list:
>          tout.Warning("Image '%s' is missing external blobs and is non-functional: %s" %
>                       (image.name, ' '.join([e.name for e in missing_list])))
> +        _ShowHelpForMissingBlobs(missing_list)
>      return bool(missing_list)
>  
>  
> @@ -563,7 +630,7 @@ def Binman(args):
>                  tools.WriteFile(dtb_item._fname, dtb_item.GetContents())
>  
>              if missing:
> -                tout.Warning("Some images are invalid")
> +                tout.Warning("\nSome images are invalid")
>          finally:
>              tools.FinaliseOutputDir()
>      finally:
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 0f128c4cf5e..f7adc3b1abb 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -178,6 +178,7 @@ class Entry(object):
>          self.align_end = fdt_util.GetInt(self._node, 'align-end')
>          self.offset_unset = fdt_util.GetBool(self._node, 'offset-unset')
>          self.expand_size = fdt_util.GetBool(self._node, 'expand-size')
> +        self.missing_msg = fdt_util.GetString(self._node, 'missing-msg')
>  
>      def GetDefaultFilename(self):
>          return None
> @@ -827,3 +828,11 @@ features to produce new behaviours.
>              True if allowed, False if not allowed
>          """
>          return self.allow_missing
> +
> +    def GetHelpTags(self):
> +        """Get the tags use for missing-blob help
> +
> +        Returns:
> +            list of possible tags, most desirable first
> +        """
> +        return list(filter(None, [self.missing_msg, self.name, self.etype]))
> diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> index a269a7497cb..95b17d0b749 100644
> --- a/tools/binman/ftest.py
> +++ b/tools/binman/ftest.py
> @@ -3561,7 +3561,7 @@ class TestFunctional(unittest.TestCase):
>              self._DoTestFile('168_fit_missing_blob.dts',
>                               allow_missing=True)
>          err = stderr.getvalue()
> -        self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
> +        self.assertRegex(err, "Image 'main-section'.*missing.*: atf-bl31")
>  
>      def testBlobNamedByArgMissing(self):
>          """Test handling of a missing entry arg"""
> @@ -3692,5 +3692,21 @@ class TestFunctional(unittest.TestCase):
>          self.assertIn("default-dt entry argument 'test-fdt3' not found in fdt list: test-fdt1, test-fdt2",
>                        str(e.exception))
>  
> +    def testFitExtblobMissingHelp(self):
> +        """Test display of help messages when an external blob is missing"""
> +        control.missing_blob_help = control._ReadMissingBlobHelp()
> +        control.missing_blob_help['wibble'] = 'Wibble test'
> +        control.missing_blob_help['another'] = 'Another test'
> +        with test_util.capture_sys_output() as (stdout, stderr):
> +            self._DoTestFile('168_fit_missing_blob.dts',
> +                             allow_missing=True)
> +        err = stderr.getvalue()
> +
> +        # We can get the tag from the name, the type or the missing-msg
> +        # property. Check all three.
> +        self.assertIn('You may need to build ARM Trusted', err)
> +        self.assertIn('Wibble test', err)
> +        self.assertIn('Another test', err)
> +
>  if __name__ == "__main__":
>      unittest.main()
> diff --git a/tools/binman/missing-blob-help b/tools/binman/missing-blob-help
> new file mode 100644
> index 00000000000..3de195c23c8
> --- /dev/null
> +++ b/tools/binman/missing-blob-help
> @@ -0,0 +1,11 @@
> +# This file contains help messages for missing external blobs. Each message has
> +# a tag (MUST be just lower-case text, digits and hyphens) starting in column 1,
> +# followed by a colon (:) to indicate its start. The message can include any
> +# number of lines, including blank lines.
> +#
> +# When looking for a tag, Binman uses the value of 'missing-msg' for the entry,
> +# the entry name or the entry type, in that order
> +
> +atf-bl31:
> +See the documentation for your board. You may need to build ARM Trusted
> +Firmware and build with BL31=/path/to/bl31.bin
> diff --git a/tools/binman/test/168_fit_missing_blob.dts b/tools/binman/test/168_fit_missing_blob.dts
> index e007aa41d8a..15f6cc07e5d 100644
> --- a/tools/binman/test/168_fit_missing_blob.dts
> +++ b/tools/binman/test/168_fit_missing_blob.dts
> @@ -29,9 +29,16 @@
>  					hash-2 {
>  						algo = "sha1";
>  					};
> -					blob-ext {
> +					atf-bl31 {
>  						filename = "missing";
>  					};
> +					cros-ec-rw {
> +						type = "atf-bl31";
> +						missing-msg = "wibble";
> +					};
> +					another {
> +						type = "atf-bl31";
> +					};
>  				};
>  			};
>  		};
> 


More information about the U-Boot mailing list