[RFC] binman: add support for creating dummy files for external blobs

[Simon Glass]

Hi Heiko,

On Tue, 16 Nov 2021 at 03:47, Heiko Thiery <heiko.thiery at gmail.com> wrote:
>
> While converting to binman for an imx8mq board, it has been found that
> building in the u-boot CI fails. This is because an imx8mq requires an
> external binary (signed_hdmi_imx8m.bin). If this file cannot be found
> mkimage fails.
> To be able to build this board in the u-boot CI a binman option
> (--fake-ext-blobs) is introduced that can be switched on via the u-boot
> makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are
> created.
>
> Signed-off-by: Heiko Thiery <heiko.thiery at gmail.com>
> ---
>  Makefile                       |  1 +
>  tools/binman/cmdline.py        |  2 ++
>  tools/binman/control.py        | 16 ++++++++++++++++
>  tools/binman/entry.py          |  1 +
>  tools/binman/etype/blob_ext.py |  1 +
>  5 files changed, 21 insertions(+)

This seems like a reasonable solution to me.

>
> diff --git a/Makefile b/Makefile
> index f911f70344..1a833a1637 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1307,6 +1307,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
>                 -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \
>                 -a spl-dtb=$(CONFIG_SPL_OF_REAL) \
>                 -a tpl-dtb=$(CONFIG_SPL_OF_REAL) \
> +               $(if $(BINMAN_FAKE_EXT_BLOBS),--fake-ext-blobs) \
>                 $(BINMAN_$(@F))
>
>  OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex
> diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
> index d6156df408..2b29981cb4 100644
> --- a/tools/binman/cmdline.py
> +++ b/tools/binman/cmdline.py
> @@ -52,6 +52,8 @@ controlled by a description in the board device tree.'''
>              help='Configuration file (.dtb) to use')
>      build_parser.add_argument('--fake-dtb', action='store_true',
>              help='Use fake device tree contents (for testing only)')
> +    build_parser.add_argument('--fake-ext-blobs', action='store_true',
> +            help='Create fake ext blobs with dummy content (for testing only)')
>      build_parser.add_argument('-i', '--image', type=str, action='append',
>              help='Image filename to build (if not specified, build all)')
>      build_parser.add_argument('-I', '--indir', action='append',
> diff --git a/tools/binman/control.py b/tools/binman/control.py
> index 0dbcbc28e9..f95aadd1f3 100644
> --- a/tools/binman/control.py
> +++ b/tools/binman/control.py
> @@ -8,6 +8,7 @@
>  from collections import OrderedDict
>  import glob
>  import os
> +import pathlib
>  import pkg_resources
>  import re
>
> @@ -401,6 +402,17 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths,
>      return image
>
>
> +def PrepareFakeExtBlobs(images):

Please add a function comment

> +    for (_, image) in images.items():
> +        for entry in image._entries.values():
> +            if entry.allow_dummy:
> +                if not pathlib.Path(entry._filename).is_file():
> +                    tout.Warning("Missing external blob '%s', fake it" %
> +                                 entry._filename)
> +                    with open(entry._filename, "wb") as out:
> +                        out.truncate(1024)

Won't this create files for all external blobs that are missing? Is
that the intent? If so, we can do this in Entry_blob.ObtainContents().

Should we use a size of 0? Or will that make some tools complain?

> +
> +
>  def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded):
>      """Prepare the images to be processed and select the device tree
>
> @@ -629,6 +641,10 @@ def Binman(args):
>
>              images = PrepareImagesAndDtbs(dtb_fname, args.image,
>                                            args.update_fdt, use_expanded)
> +
> +            if args.fake_ext_blobs:
> +                PrepareFakeExtBlobs(images)

This should go in ProcessImage(). See the SetAllowMissing()  and see
if you can follow the same pattern. Basically we tell each section
that it should allow missing blobs, then check that in
Entry_blot.ObtainContents()

> +
>              if args.test_section_timeout:
>                  # Set the first image to timeout, used in testThreadTimeout()
>                  images[list(images.keys())[0]].test_section_timeout = True
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 70222718ea..2f5c38d15a 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -100,6 +100,7 @@ class Entry(object):
>          self.missing = False
>          self.external = False
>          self.allow_missing = False
> +        self.allow_dummy = False

Please add a comment about this above.

>
>      @staticmethod
>      def Lookup(node_path, etype, expanded):
> diff --git a/tools/binman/etype/blob_ext.py b/tools/binman/etype/blob_ext.py
> index d6b0ca17c3..85bc9fe6b8 100644
> --- a/tools/binman/etype/blob_ext.py
> +++ b/tools/binman/etype/blob_ext.py
> @@ -26,3 +26,4 @@ class Entry_blob_ext(Entry_blob):
>      def __init__(self, section, etype, node):
>          Entry_blob.__init__(self, section, etype, node)
>          self.external = True
> +        self.allow_dummy = True
> --
> 2.30.2
>

Also please run 'binman test -T' as you need a test for this.

Regards,
Simon