Re: [PATCH v5 6/6] binman: Mark mkimage entry missing when its subnodes is missing

2023-02-27 Thread Kever Yang 



On 2023/2/26 03:01, Jonas Karlman wrote:

Using the mkimage entry with the multiple-data-files prop and having a
missing external blob result in an unexpected ValueError exception using
the --allow-missing flag.

   ValueError: Filename 'missing.bin' not found in input path (...)

Fix this by using _pathname that is resolved by ObtainContents for blob
entries, ObtainContents also handles allow missing for external blobs.

Mark mkimage entry as missing and return without running mkimage when
missing entries is reported by CheckMissing.

Signed-off-by: Jonas Karlman 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
v5:
- New patch based on [1]

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20230219220158.4160763-7-jo...@kwiboo.se/

  tools/binman/etype/mkimage.py | 24 ++-
  tools/binman/ftest.py | 11 +
  .../test/278_mkimage_missing_multiple.dts | 19 +++
  3 files changed, 53 insertions(+), 1 deletion(-)
  create mode 100644 tools/binman/test/278_mkimage_missing_multiple.dts

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb264c3cad0b..8a13d5ea8d77 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -156,7 +156,8 @@ class Entry_mkimage(Entry):
  for entry in self._mkimage_entries.values():
  if not entry.ObtainContents(fake_size=fake_size):
  return False
-
fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
+if entry._pathname:
+fnames.append(entry._pathname)
  input_fname = ":".join(fnames)
  else:
  data, input_fname, uniq = self.collect_contents_to_file(
@@ -171,6 +172,13 @@ class Entry_mkimage(Entry):
  outfile = self._filename if self._filename else 'mkimage-out.%s' % 
uniq
  output_fname = tools.get_output_filename(outfile)
  
+missing_list = []

+self.CheckMissing(missing_list)
+self.missing = bool(missing_list)
+if self.missing:
+self.SetContents(b'')
+return self.allow_missing
+
  args = ['-d', input_fname]
  if self._data_to_imagename:
  args += ['-n', input_fname]
@@ -216,6 +224,20 @@ class Entry_mkimage(Entry):
  if self._imagename:
  self._imagename.SetAllowFakeBlob(allow_fake)
  
+def CheckMissing(self, missing_list):

+"""Check if any entries in this section have missing external blobs
+
+If there are missing (non-optional) blobs, the entries are added to the
+list
+
+Args:
+missing_list: List of Entry objects to be added to
+"""
+for entry in self._mkimage_entries.values():
+entry.CheckMissing(missing_list)
+if self._imagename:
+self._imagename.CheckMissing(missing_list)
+
  def CheckFakedBlobs(self, faked_blobs_list):
  """Check if any entries in this section have faked external blobs
  
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py

index 48ac1540bfd8..d74aa90a6207 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6393,6 +6393,17 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
  data = self._DoReadFile('277_rockchip_tpl.dts')
  self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
  
+def testMkimageMissingBlobMultiple(self):

+"""Test missing blob with mkimage entry and multiple-data-files"""
+with test_util.capture_sys_output() as (stdout, stderr):
+self._DoTestFile('278_mkimage_missing_multiple.dts', 
allow_missing=True)
+err = stderr.getvalue()
+self.assertIn("is missing external blobs and is non-functional", err)
+
+with self.assertRaises(ValueError) as e:
+self._DoTestFile('278_mkimage_missing_multiple.dts', 
allow_missing=False)
+self.assertIn("not found in input path", str(e.exception))
+
  
  if __name__ == "__main__":

  unittest.main()
diff --git a/tools/binman/test/278_mkimage_missing_multiple.dts 
b/tools/binman/test/278_mkimage_missing_multiple.dts
new file mode 100644
index ..f84aea49ead9
--- /dev/null
+++ b/tools/binman/test/278_mkimage_missing_multiple.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   mkimage {
+   args = "-n test -T script";
+   multiple-data-files;
+
+   blob-ext {
+   filename = "missing.bin";
+   };
+   };
+   };
+};

Re: [PATCH v5 6/6] binman: Mark mkimage entry missing when its subnodes is missing

2023-02-26 Thread Simon Glass 
On Sat, 25 Feb 2023 at 12:01, Jonas Karlman  wrote:
>
> Using the mkimage entry with the multiple-data-files prop and having a
> missing external blob result in an unexpected ValueError exception using
> the --allow-missing flag.
>
>   ValueError: Filename 'missing.bin' not found in input path (...)
>
> Fix this by using _pathname that is resolved by ObtainContents for blob
> entries, ObtainContents also handles allow missing for external blobs.
>
> Mark mkimage entry as missing and return without running mkimage when
> missing entries is reported by CheckMissing.
>
> Signed-off-by: Jonas Karlman 
> ---
> v5:
> - New patch based on [1]
>
> [1] 
> https://patchwork.ozlabs.org/project/uboot/patch/20230219220158.4160763-7-jo...@kwiboo.se/
>
>  tools/binman/etype/mkimage.py | 24 ++-
>  tools/binman/ftest.py | 11 +
>  .../test/278_mkimage_missing_multiple.dts | 19 +++
>  3 files changed, 53 insertions(+), 1 deletion(-)
>  create mode 100644 tools/binman/test/278_mkimage_missing_multiple.dts

Reviewed-by: Simon Glass

[PATCH v5 6/6] binman: Mark mkimage entry missing when its subnodes is missing

2023-02-25 Thread Jonas Karlman 
Using the mkimage entry with the multiple-data-files prop and having a
missing external blob result in an unexpected ValueError exception using
the --allow-missing flag.

  ValueError: Filename 'missing.bin' not found in input path (...)

Fix this by using _pathname that is resolved by ObtainContents for blob
entries, ObtainContents also handles allow missing for external blobs.

Mark mkimage entry as missing and return without running mkimage when
missing entries is reported by CheckMissing.

Signed-off-by: Jonas Karlman 
---
v5:
- New patch based on [1]

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20230219220158.4160763-7-jo...@kwiboo.se/

 tools/binman/etype/mkimage.py | 24 ++-
 tools/binman/ftest.py | 11 +
 .../test/278_mkimage_missing_multiple.dts | 19 +++
 3 files changed, 53 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/278_mkimage_missing_multiple.dts

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb264c3cad0b..8a13d5ea8d77 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -156,7 +156,8 @@ class Entry_mkimage(Entry):
 for entry in self._mkimage_entries.values():
 if not entry.ObtainContents(fake_size=fake_size):
 return False
-
fnames.append(tools.get_input_filename(entry.GetDefaultFilename()))
+if entry._pathname:
+fnames.append(entry._pathname)
 input_fname = ":".join(fnames)
 else:
 data, input_fname, uniq = self.collect_contents_to_file(
@@ -171,6 +172,13 @@ class Entry_mkimage(Entry):
 outfile = self._filename if self._filename else 'mkimage-out.%s' % uniq
 output_fname = tools.get_output_filename(outfile)
 
+missing_list = []
+self.CheckMissing(missing_list)
+self.missing = bool(missing_list)
+if self.missing:
+self.SetContents(b'')
+return self.allow_missing
+
 args = ['-d', input_fname]
 if self._data_to_imagename:
 args += ['-n', input_fname]
@@ -216,6 +224,20 @@ class Entry_mkimage(Entry):
 if self._imagename:
 self._imagename.SetAllowFakeBlob(allow_fake)
 
+def CheckMissing(self, missing_list):
+"""Check if any entries in this section have missing external blobs
+
+If there are missing (non-optional) blobs, the entries are added to the
+list
+
+Args:
+missing_list: List of Entry objects to be added to
+"""
+for entry in self._mkimage_entries.values():
+entry.CheckMissing(missing_list)
+if self._imagename:
+self._imagename.CheckMissing(missing_list)
+
 def CheckFakedBlobs(self, faked_blobs_list):
 """Check if any entries in this section have faked external blobs
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 48ac1540bfd8..d74aa90a6207 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6393,6 +6393,17 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 data = self._DoReadFile('277_rockchip_tpl.dts')
 self.assertEqual(ROCKCHIP_TPL_DATA, data[:len(ROCKCHIP_TPL_DATA)])
 
+def testMkimageMissingBlobMultiple(self):
+"""Test missing blob with mkimage entry and multiple-data-files"""
+with test_util.capture_sys_output() as (stdout, stderr):
+self._DoTestFile('278_mkimage_missing_multiple.dts', 
allow_missing=True)
+err = stderr.getvalue()
+self.assertIn("is missing external blobs and is non-functional", err)
+
+with self.assertRaises(ValueError) as e:
+self._DoTestFile('278_mkimage_missing_multiple.dts', 
allow_missing=False)
+self.assertIn("not found in input path", str(e.exception))
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/278_mkimage_missing_multiple.dts 
b/tools/binman/test/278_mkimage_missing_multiple.dts
new file mode 100644
index ..f84aea49ead9
--- /dev/null
+++ b/tools/binman/test/278_mkimage_missing_multiple.dts
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   mkimage {
+   args = "-n test -T script";
+   multiple-data-files;
+
+   blob-ext {
+   filename = "missing.bin";
+   };
+   };
+   };
+};
-- 
2.39.2