binman: Detect when valid images are not produced

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

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

diff --git a/tools/binman/control.py b/tools/binman/control.py
index 8c6eae83f158367f6f626bf2f3443d479c250390..343b0a0c35b4f90d64efaf5b654c259bad53dd90 100644 (file)
--- 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 9388586e7cbf9b4976cf329ea780f642908aae10..3434a3f8048cfdce271da9022ee47dbffc2eca27 100644 (file)
--- 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):
@@ -803,3 +804,14 @@ features to produce new behaviours.
         """
         # This is meaningless for anything other than sections
         pass
+
+    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 51779c88c900c5dacf2f248b6ed6c5036ae93414..8d641001a9e4080a51765a247b561f2f6a1de275 100644 (file)
--- 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 9b718f1fa77a6e424a3721f578d5a803f99dfae6..dd7f1ccd098f92e333969ccf712c21f27391908d 100644 (file)
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -55,6 +55,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"""
@@ -559,3 +560,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 928d3608a31a72af739f38d069514295568a7a02..cc551c9f176f39404cdf526b525795c9be08e6b8 100644 (file)
--- 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 (file)
index 0000000..5f14c54
--- /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 91a53f4073d2e4d3736f17f422a069f954d67aae..33305263d8ee38268fccfd93c2554fc1aa1f1781 100644 (file)
--- 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()