On 24/02/2022 02:00, Simon Glass wrote: > At present fake blobs are created but internally an empty blob is used. > Change it to use the contents of the faked file. Also return whether the > blob was faked, in case the caller needs to know that. > > Add a TODO to put fake blobs in their own directory. > > Signed-off-by: Simon Glass <s...@chromium.org> > --- > > Changes in v2: > - Add a patch to change how faked blobs are created > > tools/binman/binman.rst | 3 ++- > tools/binman/entry.py | 9 ++++++--- > tools/binman/etype/blob.py | 7 ++++--- > tools/binman/etype/blob_ext_list.py | 2 +- > 4 files changed, 13 insertions(+), 8 deletions(-)
I feel a bit uneasy about all this fake file stuff, but can't figure out exactly why. I guess the patch doesn't make it that worse. Reviewed-by: Alper Nebi Yasak <alpernebiya...@gmail.com> > diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst > index 85f8337a31..a90364f2fb 100644 > --- a/tools/binman/binman.rst > +++ b/tools/binman/binman.rst > @@ -1500,7 +1500,8 @@ Some ideas: > - Figure out how to make Fdt support changing the node order, so that > Node.AddSubnode() can support adding a node before another, existing node. > Perhaps it should completely regenerate the flat tree? > - > +- Put faked files into a separate subdir and remove them on start-up, to > avoid > + seeing them as 'real' files on a subsequent run Do we need to create actual files, or is it a convenience thing for blob entry types (because they already read their contents from files)? Also, maybe it's better to create them somewhere in the /tmp/binman.* directories so they disappear without effort. > > -- > Simon Glass <s...@chromium.org> > diff --git a/tools/binman/entry.py b/tools/binman/entry.py > index 00a13c5839..2fb0050da5 100644 > --- a/tools/binman/entry.py > +++ b/tools/binman/entry.py > @@ -997,15 +997,18 @@ features to produce new behaviours. > fname (str): Filename to check > > Returns: > - fname (str): Filename of faked file > + tuple: > + fname (str): Filename of faked file > + bool: True if the blob was faked, False if not > """ > if self.allow_fake and not pathlib.Path(fname).is_file(): > outfname = tools.get_output_filename(os.path.basename(fname)) > with open(outfname, "wb") as out: > out.truncate(1024) > self.faked = True > - return outfname > - return fname > + tout.info(f"Entry '{self._node.path}': Faked file '{outfname}'") > + return outfname, True > + return fname, False Can't callers use self.faked for this? I think I see an edge case when calling this multiple times for the same filename, only the first call would recognize it being a fake file and only the first-entry-to-call would consider itself faked. > > def CheckFakedBlobs(self, faked_blobs_list): > """Check if any entries in this section have faked external blobs > diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py > index 25ec5d26c9..89f089e740 100644 > --- a/tools/binman/etype/blob.py > +++ b/tools/binman/etype/blob.py > @@ -41,10 +41,11 @@ class Entry_blob(Entry): > self.external and self.section.GetAllowMissing()) > # Allow the file to be missing > if not self._pathname: > - self._pathname = self.check_fake_fname(self._filename) > - self.SetContents(b'') > + self._pathname, faked = self.check_fake_fname(self._filename) > self.missing = True > - return True > + if not faked: > + self.SetContents(b'') > + return True > > self.ReadBlobContents() > return True > > [...]