diff mbox series

[v2,05/14] binman: Support an assumed size for missing binaries

Message ID 20240623175515.1466908-6-sjg@chromium.org
State Accepted
Commit 404936e5731ee366a513b0452e2306e799de59cb
Delegated to: Simon Glass
Headers show
Series Tools updates for Labgrid | expand

Commit Message

Simon Glass June 23, 2024, 5:55 p.m. UTC
Binman has a the useful feature of handling missing external blobs
gracefully, including allowing them to be missing, deciding whether the
resulting image is functional or not and faking blobs when this is
necessary for particular tools (e.g. mkimage).

This feature is widely used in CI. One drawback is that if U-Boot grows
too large to fit along with the required blobs, then this is not
discovered until someone does a 'real' build which includes the blobs.

Add a 'assume-size' property to entries to allow Binman to reserve a
given size for missing external blobs.

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

(no changes since v1)

 tools/binman/binman.rst                  |  7 ++++++
 tools/binman/entry.py                    |  1 +
 tools/binman/etype/blob.py               |  7 +++++-
 tools/binman/ftest.py                    | 28 ++++++++++++++++++++++++
 tools/binman/test/326_assume_size.dts    | 16 ++++++++++++++
 tools/binman/test/327_assume_size_ok.dts | 16 ++++++++++++++
 6 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/326_assume_size.dts
 create mode 100644 tools/binman/test/327_assume_size_ok.dts

Comments

Simon Glass July 15, 2024, 1:31 p.m. UTC | #1
Binman has a the useful feature of handling missing external blobs
gracefully, including allowing them to be missing, deciding whether the
resulting image is functional or not and faking blobs when this is
necessary for particular tools (e.g. mkimage).

This feature is widely used in CI. One drawback is that if U-Boot grows
too large to fit along with the required blobs, then this is not
discovered until someone does a 'real' build which includes the blobs.

Add a 'assume-size' property to entries to allow Binman to reserve a
given size for missing external blobs.

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

(no changes since v1)

 tools/binman/binman.rst                  |  7 ++++++
 tools/binman/entry.py                    |  1 +
 tools/binman/etype/blob.py               |  7 +++++-
 tools/binman/ftest.py                    | 28 ++++++++++++++++++++++++
 tools/binman/test/326_assume_size.dts    | 16 ++++++++++++++
 tools/binman/test/327_assume_size_ok.dts | 16 ++++++++++++++
 6 files changed, 74 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/326_assume_size.dts
 create mode 100644 tools/binman/test/327_assume_size_ok.dts

Applied to u-boot-dm, thanks!
diff mbox series

Patch

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 230e055667f..872e9746c8c 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -711,6 +711,13 @@  missing-msg:
     information about what needs to be fixed. See missing-blob-help for the
     message for each tag.
 
+assume-size:
+    Sets the assumed size of a blob entry if it is missing. This allows for a
+    check that the rest of the image fits into the available space, even when
+    the contents are not available. If the entry is missing, Binman will use
+    this assumed size for the entry size, including creating a fake file of that
+    size if requested.
+
 no-expanded:
     By default binman substitutes entries with expanded versions if available,
     so that a `u-boot` entry type turns into `u-boot-expanded`, for example. The
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 2ed65800d22..219d5dcecab 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -315,6 +315,7 @@  class Entry(object):
         self.overlap = fdt_util.GetBool(self._node, 'overlap')
         if self.overlap:
             self.required_props += ['offset', 'size']
+        self.assume_size = fdt_util.GetInt(self._node, 'assume-size', 0)
 
         # This is only supported by blobs and sections at present
         self.compress = fdt_util.GetString(self._node, 'compress', 'none')
diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py
index 064fae50365..041e1122953 100644
--- a/tools/binman/etype/blob.py
+++ b/tools/binman/etype/blob.py
@@ -48,11 +48,16 @@  class Entry_blob(Entry):
             self.external and (self.optional or self.section.GetAllowMissing()))
         # Allow the file to be missing
         if not self._pathname:
+            if not fake_size and self.assume_size:
+                fake_size = self.assume_size
             self._pathname, faked = self.check_fake_fname(self._filename,
                                                           fake_size)
             self.missing = True
             if not faked:
-                self.SetContents(b'')
+                content_size = 0
+                if self.assume_size: # Ensure we get test coverage on next line
+                    content_size = self.assume_size
+                self.SetContents(tools.get_bytes(0, content_size))
                 return True
 
         self.ReadBlobContents()
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 8a44bc051b3..bd0a10ff885 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -7460,5 +7460,33 @@  fdt         fdtmap                Extract the devicetree blob from the fdtmap
         with self.assertRaises(ValueError) as e:
             self._DoReadFile('323_capsule_accept_revert_missing.dts')
 
+    def test_assume_size(self):
+        """Test handling of the assume-size property for external blob"""
+        with self.assertRaises(ValueError) as e:
+            self._DoTestFile('326_assume_size.dts', allow_missing=True,
+                             allow_fake_blobs=True)
+        self.assertIn("contents size 0xa (10) exceeds section size 0x9 (9)",
+                      str(e.exception))
+
+    def test_assume_size_ok(self):
+        """Test handling of the assume-size where it fits OK"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoTestFile('327_assume_size_ok.dts', allow_missing=True,
+                             allow_fake_blobs=True)
+        err = stderr.getvalue()
+        self.assertRegex(
+            err,
+            "Image '.*' has faked external blobs and is non-functional: .*")
+
+    def test_assume_size_no_fake(self):
+        """Test handling of the assume-size where it fits OK"""
+        with test_util.capture_sys_output() as (stdout, stderr):
+            self._DoTestFile('327_assume_size_ok.dts', allow_missing=True)
+        err = stderr.getvalue()
+        self.assertRegex(
+            err,
+            "Image '.*' is missing external blobs and is non-functional: .*")
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/tools/binman/test/326_assume_size.dts b/tools/binman/test/326_assume_size.dts
new file mode 100644
index 00000000000..4c5f8b418d8
--- /dev/null
+++ b/tools/binman/test/326_assume_size.dts
@@ -0,0 +1,16 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <9>;
+		blob-ext {
+			filename = "assume_blob";
+			assume-size = <10>;
+		};
+	};
+};
diff --git a/tools/binman/test/327_assume_size_ok.dts b/tools/binman/test/327_assume_size_ok.dts
new file mode 100644
index 00000000000..00ed726f872
--- /dev/null
+++ b/tools/binman/test/327_assume_size_ok.dts
@@ -0,0 +1,16 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+	#address-cells = <1>;
+	#size-cells = <1>;
+
+	binman {
+		size = <10>;
+		blob-ext {
+			filename = "assume_blob";
+			assume-size = <10>;
+		};
+	};
+};