binman: Collect bintools for images when replacing entries

Binman entries can use other executables to compute their data, usually
in their ObtainContents() methods. Subclasses of Entry_section would use
bintools in their BuildSectionData() method instead, which is called
from several places including their Pack().

These binary tools are resolved correctly while building an image from a
device-tree description so that they can be used from these methods.
However, this is not being done when replacing entries in an image,
which can result in an error as the Pack() methods attempt to use them.

Collect and resolve entries' bintools also when replacing entries to fix
Pack() errors. Add a way to mock bintool usage in the testing entry type
and tests that check bintools are being resolved for such an entry.

Signed-off-by: Alper Nebi Yasak <alpernebiyasak@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>

diff --git a/tools/binman/control.py b/tools/binman/control.py
index d4c8dc89201bd6eb3313f34fe06093aae94549dc..e170aeae4fab07b10169a8966b4eae5134ce7d41 100644 (file)
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -299,6 +299,7 @@ def BeforeReplace(image, allow_resize):
     """
     state.PrepareFromLoadedData(image)
     image.LoadData()
+    image.CollectBintools()
 
     # If repacking, drop the old offset/size values except for the original
     # ones, so we are only left with the constraints.

diff --git a/tools/binman/etype/_testing.py b/tools/binman/etype/_testing.py
index 5089de36429459da65acf561691f72e361fba3fc..69600487814779dddfda12397cb2255535b33a80 100644 (file)
--- a/tools/binman/etype/_testing.py
+++ b/tools/binman/etype/_testing.py
@@ -39,6 +39,10 @@ class Entry__testing(Entry):
             error if not)
         force-bad-datatype: Force a call to GetEntryArgsOrProps() with a bad
             data type (generating an error)
+        require-bintool-for-contents: Raise an error if the specified
+            bintool isn't usable in ObtainContents()
+        require-bintool-for-pack: Raise an error if the specified
+            bintool isn't usable in Pack()
     """
     def __init__(self, section, etype, node):
         super().__init__(section, etype, node)
@@ -82,6 +86,26 @@ class Entry__testing(Entry):
         self.return_contents = True
         self.contents = b'aa'
 
+        # Set to the required bintool when collecting bintools.
+        self.bintool_for_contents = None
+        self.require_bintool_for_contents = fdt_util.GetString(self._node,
+                                              'require-bintool-for-contents')
+        if self.require_bintool_for_contents == '':
+            self.require_bintool_for_contents = '_testing'
+
+        self.bintool_for_pack = None
+        self.require_bintool_for_pack = fdt_util.GetString(self._node,
+                                                  'require-bintool-for-pack')
+        if self.require_bintool_for_pack == '':
+            self.require_bintool_for_pack = '_testing'
+
+    def Pack(self, offset):
+        """Figure out how to pack the entry into the section"""
+        if self.require_bintool_for_pack:
+            if self.bintool_for_pack is None:
+                self.Raise("Required bintool unusable in Pack()")
+        return super().Pack(offset)
+
     def ObtainContents(self, fake_size=0):
         if self.return_unknown_contents or not self.return_contents:
             return False
@@ -92,6 +116,9 @@ class Entry__testing(Entry):
         self.contents_size = len(self.data)
         if self.return_contents_once:
             self.return_contents = False
+        if self.require_bintool_for_contents:
+            if self.bintool_for_contents is None:
+                self.Raise("Required bintool unusable in ObtainContents()")
         return True
 
     def GetOffsets(self):
@@ -127,3 +154,12 @@ class Entry__testing(Entry):
         if not self.never_complete_process_fdt:
             self.process_fdt_ready = True
         return ready
+
+    def AddBintools(self, btools):
+        """Add the bintools used by this entry type"""
+        if self.require_bintool_for_contents is not None:
+            self.bintool_for_contents = self.AddBintool(btools,
+                    self.require_bintool_for_contents)
+        if self.require_bintool_for_pack is not None:
+            self.bintool_for_pack = self.AddBintool(btools,
+                    self.require_bintool_for_pack)

diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 94c4389b1b52fb419f34d28db53ab5b28dcf3351..03e6d9233b2af0f954f30584c3236791cfc3c5f3 100644 (file)
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -5554,5 +5554,44 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
 
         self._CheckSafeUniqueNames(orig_image, image)
 
+    def testReplaceCmdWithBintool(self):
+        """Test replacing an entry that needs a bintool to pack"""
+        data = self._DoReadFileRealDtb('232_replace_with_bintool.dts')
+        expected = U_BOOT_DATA + b'aa'
+        self.assertEqual(expected, data[:len(expected)])
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            fname = os.path.join(tmpdir, 'update-testing.bin')
+            tools.write_file(fname, b'zz')
+            self._DoBinman('replace', '-i', updated_fname,
+                           '_testing', '-f', fname)
+
+            data = tools.read_file(updated_fname)
+            expected = U_BOOT_DATA + b'zz'
+            self.assertEqual(expected, data[:len(expected)])
+        finally:
+            shutil.rmtree(tmpdir)
+
+    def testReplaceCmdOtherWithBintool(self):
+        """Test replacing an entry when another needs a bintool to pack"""
+        data = self._DoReadFileRealDtb('232_replace_with_bintool.dts')
+        expected = U_BOOT_DATA + b'aa'
+        self.assertEqual(expected, data[:len(expected)])
+
+        try:
+            tmpdir, updated_fname = self._SetupImageInTmpdir()
+            fname = os.path.join(tmpdir, 'update-u-boot.bin')
+            tools.write_file(fname, b'x' * len(U_BOOT_DATA))
+            self._DoBinman('replace', '-i', updated_fname,
+                           'u-boot', '-f', fname)
+
+            data = tools.read_file(updated_fname)
+            expected = b'x' * len(U_BOOT_DATA) + b'aa'
+            self.assertEqual(expected, data[:len(expected)])
+        finally:
+            shutil.rmtree(tmpdir)
+
+
 if __name__ == "__main__":
     unittest.main()

diff --git a/tools/binman/test/232_replace_with_bintool.dts b/tools/binman/test/232_replace_with_bintool.dts
new file mode 100644 (file)
index 0000000..d7fabd2
--- /dev/null
+++ b/tools/binman/test/232_replace_with_bintool.dts
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+       #address-cells = <1>;
+       #size-cells = <1>;
+
+       binman {
+               size = <0xc00>;
+               allow-repack;
+
+               u-boot {
+               };
+
+               _testing {
+                       require-bintool-for-contents;
+                       require-bintool-for-pack;
+               };
+
+               fdtmap {
+               };
+
+               u-boot2 {
+                       type = "u-boot";
+               };
+
+               text {
+                       text = "some text";
+               };
+
+               u-boot-dtb {
+               };
+
+               image-header {
+                       location = "end";
+               };
+       };
+};