Hi Jonas,
On Wed, 28 Jun 2023 at 23:35, Jonas Karlman <jo...@kwiboo.se> wrote:
>
> Hi Simon,
> On 2023-06-28 13:41, Simon Glass wrote:
> > From: Marek Vasut <ma...@denx.de>
> >
> > This is needed to handle mkimage with inner section located itself in a
> > section.
> >
> > Signed-off-by: Marek Vasut <ma...@denx.de>
> > Use BuildSectionData() instead of ObtainContents(), add tests and a few
> > other minor fixes:
> > Signed-off-by: Simon Glass <s...@chromium.org>
> > ---
> >
> > tools/binman/entry.py | 6 +-
> > tools/binman/etype/mkimage.py | 76 ++++++++++++++---------
> > tools/binman/ftest.py | 45 +++++++++++++-
> > tools/binman/test/283_mkimage_special.dts | 24 +++++++
> > 4 files changed, 117 insertions(+), 34 deletions(-)
> > create mode 100644 tools/binman/test/283_mkimage_special.dts
> >
> > diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> > index 328b5bc568a9..8f06fea51ad4 100644
> > --- a/tools/binman/entry.py
> > +++ b/tools/binman/entry.py
> > @@ -1311,10 +1311,8 @@ features to produce new behaviours.
> > """
> > data = b''
> > for entry in entries:
> > - # First get the input data and put it in a file. If not
> > available,
> > - # try later.
> > - if not entry.ObtainContents(fake_size=fake_size):
> > - return None, None, None
> > + # First get the input data and put it in a file
> > + entry.ObtainContents(fake_size=fake_size)
> > data += entry.GetData()
> > uniq = self.GetUniqueName()
> > fname = tools.get_output_filename(f'{prefix}.{uniq}')
> > diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> > index cb3e10672ad7..8311fed59762 100644
> > --- a/tools/binman/etype/mkimage.py
> > +++ b/tools/binman/etype/mkimage.py
> > @@ -8,10 +8,11 @@
> > from collections import OrderedDict
> >
> > from binman.entry import Entry
> > +from binman.etype.section import Entry_section
> > from dtoc import fdt_util
> > from u_boot_pylib import tools
> >
> > -class Entry_mkimage(Entry):
> > +class Entry_mkimage(Entry_section):
> > """Binary produced by mkimage
> >
> > Properties / Entry arguments:
> > @@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
> > """
> > def __init__(self, section, etype, node):
> > super().__init__(section, etype, node)
> > - self._mkimage_entries = OrderedDict()
> > self._imagename = None
> > - self._filename = fdt_util.GetString(self._node, 'filename')
> > - self.align_default = None
> > + self._multiple_data_files = False
> >
> > def ReadNode(self):
> > super().ReadNode()
> > @@ -135,41 +134,60 @@ class Entry_mkimage(Entry):
> > 'data-to-imagename')
> > if self._data_to_imagename and self._node.FindNode('imagename'):
> > self.Raise('Cannot use both imagename node and
> > data-to-imagename')
> > - self.ReadEntries()
> >
> > def ReadEntries(self):
> > """Read the subnodes to find out what should go in this image"""
> > for node in self._node.subnodes:
> > - entry = Entry.Create(self, node)
> > + if self.IsSpecialSubnode(node):
> > + continue
> > + entry = Entry.Create(self, node,
> > + expanded=self.GetImage().use_expanded,
> > +
> > missing_etype=self.GetImage().missing_etype)
> > entry.ReadNode()
> > + entry.SetPrefix(self._name_prefix)
> > if entry.name == 'imagename':
> > self._imagename = entry
> > else:
> > - self._mkimage_entries[entry.name] = entry
> > + self._entries[entry.name] = entry
> >
> > - def ObtainContents(self):
> > + def BuildSectionData(self, required):
> > + """Build mkimage entry contents
> > +
> > + Runs mkimage to build the entry contents
> > +
> > + Args:
> > + required (bool): True if the data must be present, False if it
> > is OK
> > + to return None
> > +
> > + Returns:
> > + bytes: Contents of the section
> > + """
> > # Use a non-zero size for any fake files to keep mkimage happy
> > # Note that testMkimageImagename() relies on this 'mkimage'
> > parameter
> > fake_size = 1024
> > if self._multiple_data_files:
> > fnames = []
> > uniq = self.GetUniqueName()
> > - for entry in self._mkimage_entries.values():
> > - if not entry.ObtainContents(fake_size=fake_size):
> > - return False
> > - if entry._pathname:
> > - fnames.append(entry._pathname)
> > + for entry in self._entries.values():
> > + entry.ObtainContents(fake_size=fake_size)
> > +
> > + # If this is a section, put the contents in a temporary
> > file.
> > + # Otherwise, assume it is a blob and use the pathname
> > + if isinstance(entry, Entry_section):
> > + ename = f'mkimage-in-{uniq}-{entry.name}'
> > + fname = tools.get_output_filename(ename)
> > + tools.write_file(fname, entry.data)
> > + elif entry._pathname:
> > + fname = entry._pathname
> > + fnames.append(fname)
> > input_fname = ":".join(fnames)
> > + data = b''
> > else:
> > data, input_fname, uniq = self.collect_contents_to_file(
> > - self._mkimage_entries.values(), 'mkimage', fake_size)
> > - if data is None:
> > - return False
> > + self._entries.values(), 'mkimage', fake_size)
> > if self._imagename:
> > image_data, imagename_fname, _ = self.collect_contents_to_file(
> > [self._imagename], 'mkimage-n', 1024)
> > - if image_data is None:
> > - return False
> > outfile = self._filename if self._filename else 'mkimage-out.%s' %
> > uniq
> > output_fname = tools.get_output_filename(outfile)
> >
> > @@ -177,8 +195,7 @@ class Entry_mkimage(Entry):
> > self.CheckMissing(missing_list)
> > self.missing = bool(missing_list)
> > if self.missing:
> > - self.SetContents(b'')
> > - return self.allow_missing
> > + return b''
> >
> > args = ['-d', input_fname]
> > if self._data_to_imagename:
> > @@ -187,17 +204,15 @@ class Entry_mkimage(Entry):
> > args += ['-n', imagename_fname]
> > args += self._args + [output_fname]
> > if self.mkimage.run_cmd(*args) is not None:
> > - self.SetContents(tools.read_file(output_fname))
> > + return tools.read_file(output_fname)
> > else:
> > # Bintool is missing; just use the input data as the output
> > self.record_missing_bintool(self.mkimage)
> > - self.SetContents(data)
> > -
> > - return True
> > + return data
> >
> > def GetEntries(self):
> > # Make a copy so we don't change the original
> > - entries = OrderedDict(self._mkimage_entries)
> > + entries = OrderedDict(self._entries)
> > if self._imagename:
> > entries['imagename'] = self._imagename
> > return entries
> > @@ -209,7 +224,7 @@ class Entry_mkimage(Entry):
> > allow_missing: True if allowed, False if not allowed
> > """
> > self.allow_missing = allow_missing
> > - for entry in self._mkimage_entries.values():
> > + for entry in self._entries.values():
>
> With the changes to use GetEntries() in Entry_section for this and
> following Check and Set functions, I suspect these can be removed and
> use the base implementation from Entry_section.
>
> GetEntries() returns self._entries + self._imagename
Yes, good idea, will do in v2. Tht special code has annoyed me.
Regards,
Simon
