Re: [PATCH v2 16/49] binman: Detect when valid images are not produced

Hi Simon,

On Sun, Jun 14, 2020 at 10:57 AM Simon Glass sjg@chromium.org wrote:

When external blobs are missing, show a message indicating that the images are not functional.

Signed-off-by: Simon Glass sjg@chromium.org

(no changes since v1)

tools/binman/control.py | 16 +++++++++++-- tools/binman/entry.py | 21 +++++++++++++++++ tools/binman/etype/blob_ext.py | 1 + tools/binman/etype/section.py | 16 ++++++++++++- tools/binman/ftest.py | 14 ++++++++++- .../binman/test/159_blob_ext_missing_sect.dts | 23 +++++++++++++++++++ tools/patman/tout.py | 1 + 7 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/159_blob_ext_missing_sect.dts

diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c6eae83f1..343b0a0c35 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -403,6 +403,9 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, allow_resize: True to allow entries to change size (this does a re-pack of the entries), False to raise an exception allow_missing: Allow blob_ext objects to be missing

• Returns:

•    True if one or more external blobs are missing, False if all are present
  

  """ if get_contents: image.SetAllowMissing(allow_missing)

@@ -450,6 +453,12 @@ def ProcessImage(image, update_fdt, write_map, get_contents=True, image.BuildImage() if write_map: image.WriteMap()

• missing_list = []
• image.CheckMissing(missing_list)
• 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])))
  

• return bool(missing_list)

def Binman(args): @@ -524,14 +533,17 @@ def Binman(args):

         images = PrepareImagesAndDtbs(dtb_fname, args.image,
                                       args.update_fdt)

•        missing = False
         for image in images.values():
  

•            ProcessImage(image, args.update_fdt, args.map,
  

•                         allow_missing=args.allow_missing)
  

•            missing |= ProcessImage(image, args.update_fdt, args.map,
  

•                                    allow_missing=args.allow_missing)
  
         # Write the updated FDTs to our output files
         for dtb_item in state.GetAllFdts():
             tools.WriteFile(dtb_item._fname, dtb_item.GetContents())
  
  

•        if missing:
  

•            tout.Warning("Some images are invalid")
     finally:
         tools.FinaliseOutputDir()
  

  finally:

diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 90ffd27617..4a42e0bf34 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -84,6 +84,7 @@ class Entry(object): self.image_pos = None self._expand_size = False self.compress = 'none'

•    self.missing = False
  
  

  @staticmethod def Lookup(node_path, etype):

@@ -794,3 +795,23 @@ features to produce new behaviours. elif self == entries[-1]: return 'end' return 'middle'

• def SetAllowMissing(self, allow_missing):

•    """Set whether a section allows missing external blobs
  

•    Args:
  

•        allow_missing: True if allowed, False if not allowed
  

•    """
  

•    # This is meaningless for
  

for what?

•    self._allow_missing = allow_missing
  

Should the above changes be in patch "[v2,14/49] binman: Allow external binaries to be missing" ?

• def CheckMissing(self, missing_list):

•    """Check if any entries in this section have missing external blobs
  

•    If there are missing blobs, the entries are added to the list
  

•    Args:
  

•        missing_list: List of Entry objects to be added to
  

•    """
  

•    if self.missing:
  

•        missing_list.append(self)
  

diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index 51779c88c9..8d641001a9 100644 --- a/tools/binman/etype/blob_ext.py +++ b/tools/binman/etype/blob_ext.py @@ -34,5 +34,6 @@ class Entry_blob_ext(Entry_blob): # Allow the file to be missing if not self._pathname: self.SetContents(b'')

•        self.missing = True
         return True
     return super().ObtainContents()
  

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 44b13b2575..3b42a5890f 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -50,6 +50,7 @@ class Entry_section(Entry): self._skip_at_start = None self._end_4gb = False self._allow_missing = False

•    self.missing = False
  
  

  def ReadNode(self): """Read properties from the image node"""

@@ -543,7 +544,9 @@ class Entry_section(Entry): Args: allow_missing: True if allowed, False if not allowed """

•    self._allow_missing = allow_missing
  

•    super().SetAllowMissing(allow_missing)
  

•    for entry in self._entries.values():
  

•        entry.SetAllowMissing(allow_missing)
  
  

  def GetAllowMissing(self): """Get whether a section allows missing external blobs

@@ -552,3 +555,14 @@ class Entry_section(Entry): True if allowed, False if not allowed """ return self._allow_missing

• def CheckMissing(self, missing_list):

•    """Check if any entries in this section have missing external blobs
  

•    If there are missing blobs, the entries are added to the list
  

•    Args:
  

•        missing_list: List of Entry objects to be added to
  

•    """
  

•    for entry in self._entries.values():
  

•        entry.CheckMissing(missing_list)
  

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 928d3608a3..cc551c9f17 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -3380,7 +3380,19 @@ class TestFunctional(unittest.TestCase):

 def testExtblobMissingOk(self):
     """Test an image with an missing external blob that is allowed"""

•    self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True)
  

•    with test_util.capture_sys_output() as (stdout, stderr):
  

•        self._DoTestFile('158_blob_ext_missing.dts', allow_missing=True)
  

•    err = stderr.getvalue()
  

•    self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
  

• def testExtblobMissingOkSect(self):

•    """Test an image with an missing external blob that is allowed"""
  

•    with test_util.capture_sys_output() as (stdout, stderr):
  

•        self._DoTestFile('159_blob_ext_missing_sect.dts',
  

•                         allow_missing=True)
  

•    err = stderr.getvalue()
  

•    self.assertRegex(err, "Image 'main-section'.*missing.*: "
  

•                     "blob-ext blob-ext2")
  
  
  

if __name__ == "__main__": diff --git a/tools/binman/test/159_blob_ext_missing_sect.dts b/tools/binman/test/159_blob_ext_missing_sect.dts new file mode 100644 index 0000000000..5f14c54138 --- /dev/null +++ b/tools/binman/test/159_blob_ext_missing_sect.dts @@ -0,0 +1,23 @@ +// SPDX-License-Identifier: GPL-2.0+

+/dts-v1/;

+/ {

•   #address-cells = <1>;
  

•   #size-cells = <1>;
  

•   binman {
  

•           size = <0x80>;
  

•           section {
  

•                   blob-ext {
  

•                           filename = "missing-file";
  

•                   };
  

•           };
  

•           blob-ext2 {
  

•                   type = "blob-ext";
  

•                   filename = "missing-file2";
  

•           };
  

•   };
  

+}; diff --git a/tools/patman/tout.py b/tools/patman/tout.py index 91a53f4073..33305263d8 100644 --- a/tools/patman/tout.py +++ b/tools/patman/tout.py @@ -171,6 +171,7 @@ def Init(_verbose=WARNING, stdout=sys.stdout):

 # TODO(sjg): Move this into Chromite libraries when we have them
 stdout_is_tty = hasattr(sys.stdout, 'isatty') and sys.stdout.isatty()

• stderr_is_tty = hasattr(sys.stderr, 'isatty') and sys.stderr.isatty()

def Uninit(): ClearProgress() --

Regards, Bin