Re: [PATCH v2 12/25] binman: Change how faked blobs are created

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 sjg@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 alpernebiyasak@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 sjg@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
  
  

[...]