All of lore.kernel.org
 help / color / mirror / Atom feed
From: Simon Glass <sjg@chromium.org>
To: u-boot@lists.denx.de
Subject: [PATCH v3 01/12] binman: Allow entry args to be required
Date: Tue,  1 Sep 2020 05:13:54 -0600	[thread overview]
Message-ID: <20200901051400.v3.1.I658eaf634ae1882a849d51ca76de38cdd2f7ff40@changeid> (raw)
In-Reply-To: <20200901111405.358391-1-sjg@chromium.org>

If an entry argument is needed by an entry but the entry argument is not
present, then a strange error can occur when trying to read the file.

Fix this by allowing arguments to be required. Select this option for the
cros-ec-rw entry. If a filename is provided in the node, allow that to be
used.

Also tidy up a few related tests to make the error string easier to find,
and fully ignore unused return values.

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

(no changes since v1)

 tools/binman/README.entries             |  7 ++++++-
 tools/binman/etype/blob_named_by_arg.py | 10 ++++++----
 tools/binman/etype/cros_ec_rw.py        |  3 +--
 tools/binman/ftest.py                   | 15 +++++++++++----
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/tools/binman/README.entries b/tools/binman/README.entries
index bf8edce02b4..97bfae16116 100644
--- a/tools/binman/README.entries
+++ b/tools/binman/README.entries
@@ -60,7 +60,7 @@ Entry: blob-named-by-arg: A blob entry which gets its filename property from its
 
 Properties / Entry arguments:
     - <xxx>-path: Filename containing the contents of this entry (optional,
-        defaults to 0)
+        defaults to None)
 
 where <xxx> is the blob_fname argument to the constructor.
 
@@ -691,6 +691,11 @@ Properties / Entry arguments: (see binman README for more information)
     name-prefix: Adds a prefix to the name of every entry in the section
         when writing out the map
 
+Properties:
+    _allow_missing: True if this section permits external blobs to be
+        missing their contents. The second will produce an image but of
+        course it will not work.
+
 Since a section is also an entry, it inherits all the properies of entries
 too.
 
diff --git a/tools/binman/etype/blob_named_by_arg.py b/tools/binman/etype/blob_named_by_arg.py
index e95dabe4d07..7c486b2dc91 100644
--- a/tools/binman/etype/blob_named_by_arg.py
+++ b/tools/binman/etype/blob_named_by_arg.py
@@ -17,7 +17,7 @@ class Entry_blob_named_by_arg(Entry_blob):
 
     Properties / Entry arguments:
         - <xxx>-path: Filename containing the contents of this entry (optional,
-            defaults to 0)
+            defaults to None)
 
     where <xxx> is the blob_fname argument to the constructor.
 
@@ -28,7 +28,9 @@ class Entry_blob_named_by_arg(Entry_blob):
 
     See cros_ec_rw for an example of this.
     """
-    def __init__(self, section, etype, node, blob_fname):
+    def __init__(self, section, etype, node, blob_fname, required=False):
         super().__init__(section, etype, node)
-        self._filename, = self.GetEntryArgsOrProps(
-            [EntryArg('%s-path' % blob_fname, str)])
+        filename, = self.GetEntryArgsOrProps(
+            [EntryArg('%s-path' % blob_fname, str)], required=required)
+        if filename:
+            self._filename = filename
diff --git a/tools/binman/etype/cros_ec_rw.py b/tools/binman/etype/cros_ec_rw.py
index 741372e1af4..bf676b2d1a7 100644
--- a/tools/binman/etype/cros_ec_rw.py
+++ b/tools/binman/etype/cros_ec_rw.py
@@ -7,7 +7,6 @@
 
 from binman.etype.blob_named_by_arg import Entry_blob_named_by_arg
 
-
 class Entry_cros_ec_rw(Entry_blob_named_by_arg):
     """A blob entry which contains a Chromium OS read-write EC image
 
@@ -18,5 +17,5 @@ class Entry_cros_ec_rw(Entry_blob_named_by_arg):
     updating the EC on startup via software sync.
     """
     def __init__(self, section, etype, node):
-        super().__init__(section, etype, node, 'cros-ec-rw')
+        super().__init__(section, etype, node, 'cros-ec-rw', required=True)
         self.external = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index e672967dbaa..f000a794325 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1382,8 +1382,9 @@ class TestFunctional(unittest.TestCase):
         }
         with self.assertRaises(ValueError) as e:
             self._DoReadFileDtb('064_entry_args_required.dts')
-        self.assertIn("Node '/binman/_testing': Missing required "
-            'properties/entry args: test-str-arg, test-int-fdt, test-int-arg',
+        self.assertIn("Node '/binman/_testing': "
+            'Missing required properties/entry args: test-str-arg, '
+            'test-int-fdt, test-int-arg',
             str(e.exception))
 
     def testEntryArgsInvalidFormat(self):
@@ -1487,8 +1488,7 @@ class TestFunctional(unittest.TestCase):
         entry_args = {
             'cros-ec-rw-path': 'ecrw.bin',
         }
-        data, _, _, _ = self._DoReadFileDtb('068_blob_named_by_arg.dts',
-                                            entry_args=entry_args)
+        self._DoReadFileDtb('068_blob_named_by_arg.dts', entry_args=entry_args)
 
     def testFill(self):
         """Test for an fill entry type"""
@@ -3523,5 +3523,12 @@ class TestFunctional(unittest.TestCase):
         err = stderr.getvalue()
         self.assertRegex(err, "Image 'main-section'.*missing.*: blob-ext")
 
+    def testBlobNamedByArgMissing(self):
+        """Test handling of a missing entry arg"""
+        with self.assertRaises(ValueError) as e:
+            self._DoReadFile('068_blob_named_by_arg.dts')
+        self.assertIn("Missing required properties/entry args: cros-ec-rw-path",
+                      str(e.exception))
+
 if __name__ == "__main__":
     unittest.main()
-- 
2.28.0.402.g5ffc5be6b7-goog

  reply	other threads:[~2020-09-01 11:13 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-01 11:13 [PATCH v3 00/12] binman: Add support for generating more complex FITs Simon Glass
2020-09-01 11:13 ` Simon Glass [this message]
2020-09-01 11:13 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
2020-09-01 11:13 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
2020-09-01 11:13 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
2020-09-01 11:13 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
2020-09-05 22:57   ` Samuel Holland
2020-09-06  0:17     ` Simon Glass
2020-09-01 11:13 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
2020-09-05 22:41   ` Samuel Holland
2020-09-05 23:19     ` Samuel Holland
2020-09-06  0:17       ` Simon Glass
2020-09-06  0:18     ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
2020-09-01 11:14 ` [PATCH v3 08/12] sunxi: Convert 64-bit boards to use binman Simon Glass
2020-09-05 23:10   ` Samuel Holland
2020-09-05 23:42     ` Samuel Holland
2020-09-06  0:17       ` Simon Glass
2020-09-06  0:18     ` Simon Glass
2020-09-06  1:49       ` Samuel Holland
2020-09-06  2:22         ` Simon Glass
2020-09-07 13:01       ` Michal Simek
2020-09-07 13:57         ` Simon Glass
2020-09-01 11:14 ` [PATCH v3 09/12] sunxi: Drop the FIT-generator script Simon Glass
2020-09-01 11:14 ` [PATCH v3 10/12] binman: Allow selecting default FIT configuration Simon Glass
2020-09-01 11:14 ` [PATCH v3 11/12] binman: Support help messages for missing blobs Simon Glass
2020-09-01 11:14 ` [PATCH v3 12/12] binman: sunxi: Add help message for missing sunxi ATF BL31 Simon Glass
2020-09-02 10:26 ` [PATCH v3 00/12] binman: Add support for generating more complex FITs Michal Simek
2020-09-02 17:07   ` Simon Glass
2020-09-03 13:31     ` Michal Simek
2020-09-05 21:10 ` [PATCH v3 07/12] Makefile: Support missing external blobs always Simon Glass
2020-09-05 21:10 ` [PATCH v3 06/12] binman: Support generating FITs with multiple dtbs Simon Glass
2020-09-05 21:10 ` [PATCH v3 05/12] binman: Add support for ATF BL31 Simon Glass
2020-09-05 21:10 ` [PATCH v3 04/12] binman: Move 'external' support into base class Simon Glass
2020-09-05 21:10 ` [PATCH v3 03/12] libfdt: Detected out-of-space with fdt_finish() Simon Glass
2020-09-05 21:10 ` [PATCH v3 02/12] binman: Fix up a few missing comments Simon Glass
2020-09-05 21:10 ` [PATCH v3 01/12] binman: Allow entry args to be required Simon Glass
2020-09-07  6:31   ` Michal Simek
2020-09-07 13:57     ` Simon Glass

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200901051400.v3.1.I658eaf634ae1882a849d51ca76de38cdd2f7ff40@changeid \
    --to=sjg@chromium.org \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.