| Message ID | 20200613205717.v2.16.Ia87bd305fe68d527b8148b31d2b16ef40af7d84c@changeid |
|---|---|
| State | Accepted |
| Commit | 13262c93626502873786067fcbe2e2ab5894b90f |
| Headers | show |
| Series | rockchip: x86: Support building ROM files automatically with binman | expand |
Hi Simon, On Sun, Jun 14, 2020 at 10:57 AM Simon Glass <sjg at chromium.org> wrote: > > When external blobs are missing, show a message indicating that the images > are not functional. > > Signed-off-by: Simon Glass <sjg at chromium.org> > --- > > (no changes since v1) > > tools/binman/control.py | 16 +++++++++++-- > tools/binman/entry.py | 21 +++++++++++++++++ > tools/binman/etype/blob_ext.py | 1 + > tools/binman/etype/section.py | 16 ++++++++++++- > tools/binman/ftest.py | 14 ++++++++++- > .../binman/test/159_blob_ext_missing_sect.dts | 23 +++++++++++++++++++ > tools/patman/tout.py | 1 + > 7 files changed, 88 insertions(+), 4 deletions(-) > create mode 100644 tools/binman/test/159_blob_ext_missing_sect.dts > > diff --git a/tools/binman/control.py b/tools/binman/control.py > index 8c6eae83f1..343b0a0c35 100644 > --- 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 90ffd27617..4a42e0bf34 100644 > --- 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): > @@ -794,3 +795,23 @@ features to produce new behaviours. > elif self == entries[-1]: > return 'end' > return 'middle' > + > + def SetAllowMissing(self, allow_missing): > + """Set whether a section allows missing external blobs > + > + Args: > + allow_missing: True if allowed, False if not allowed > + """ > + # This is meaningless for for what? > + self._allow_missing = allow_missing > + Should the above changes be in patch "[v2,14/49] binman: Allow external binaries to be 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 > + """ > + if self.missing: > + missing_list.append(self) > diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py > index 51779c88c9..8d641001a9 100644 > --- 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 44b13b2575..3b42a5890f 100644 > --- a/tools/binman/etype/section.py > +++ b/tools/binman/etype/section.py > @@ -50,6 +50,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""" > @@ -543,7 +544,9 @@ class Entry_section(Entry): > Args: > allow_missing: True if allowed, False if not allowed > """ > - self._allow_missing = allow_missing > + super().SetAllowMissing(allow_missing) > + for entry in self._entries.values(): > + entry.SetAllowMissing(allow_missing) > > def GetAllowMissing(self): > """Get whether a section allows missing external blobs > @@ -552,3 +555,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 928d3608a3..cc551c9f17 100644 > --- 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 > index 0000000000..5f14c54138 > --- /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 91a53f4073..33305263d8 100644 > --- 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() > -- Regards, Bin
diff --git a/tools/binman/control.py b/tools/binman/control.py index 8c6eae83f1..343b0a0c35 100644 --- 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 90ffd27617..4a42e0bf34 100644 --- 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): @@ -794,3 +795,23 @@ features to produce new behaviours. elif self == entries[-1]: return 'end' return 'middle' + + def SetAllowMissing(self, allow_missing): + """Set whether a section allows missing external blobs + + Args: + allow_missing: True if allowed, False if not allowed + """ + # This is meaningless for + self._allow_missing = 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 + """ + if self.missing: + missing_list.append(self) diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py index 51779c88c9..8d641001a9 100644 --- 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 44b13b2575..3b42a5890f 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -50,6 +50,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""" @@ -543,7 +544,9 @@ class Entry_section(Entry): Args: allow_missing: True if allowed, False if not allowed """ - self._allow_missing = allow_missing + super().SetAllowMissing(allow_missing) + for entry in self._entries.values(): + entry.SetAllowMissing(allow_missing) def GetAllowMissing(self): """Get whether a section allows missing external blobs @@ -552,3 +555,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 928d3608a3..cc551c9f17 100644 --- 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 index 0000000000..5f14c54138 --- /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 91a53f4073..33305263d8 100644 --- 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()
When external blobs are missing, show a message indicating that the images are not functional. Signed-off-by: Simon Glass <sjg at chromium.org> --- (no changes since v1) tools/binman/control.py | 16 +++++++++++-- tools/binman/entry.py | 21 +++++++++++++++++ tools/binman/etype/blob_ext.py | 1 + tools/binman/etype/section.py | 16 ++++++++++++- tools/binman/ftest.py | 14 ++++++++++- .../binman/test/159_blob_ext_missing_sect.dts | 23 +++++++++++++++++++ tools/patman/tout.py | 1 + 7 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 tools/binman/test/159_blob_ext_missing_sect.dts