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

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@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";
  

• 			};
  	};
  };
  

  };