[PATCH] Makefile: With BINMAN_ALLOW_MISSING=1 don't error on missing

[Tom Rini]

On Tue, Dec 06, 2022 at 10:03:37AM -0500, Tom Rini wrote:
> On Tue, Dec 06, 2022 at 03:36:49PM +1300, Simon Glass wrote:
> > Hi Tom,
> > 
> > On Tue, 6 Dec 2022 at 15:03, Tom Rini <trini at konsulko.com> wrote:
> > >
> > > When the user builds with BINMAN_ALLOW_MISSING=1 they're explicitly
> > > setting the flag to allow for additional binaries to be missing and so
> > > have acknowledged the output might not work. In this case we want to
> > > default to not passing a non-zero exit code.
> > >
> > > Cc: Simon Glass <sjg at chromium.org>
> > > Reported-by: Peter Robinson <pbrobinson at gmail.com>
> > > Signed-off-by: Tom Rini <trini at konsulko.com>
> > > ---
> > > This passes CI as-is:
> > > https://source.denx.de/u-boot/u-boot/-/pipelines/14340
> > > ---
> > >  Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index de5746399a63..03de1da1bfd0 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1334,7 +1334,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \
> > >                  --toolpath $(objtree)/tools \
> > >                 $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
> > >                 build -u -d u-boot.dtb -O . -m \
> > > -               $(if $(BINMAN_ALLOW_MISSING),--allow-missing --fake-ext-blobs) \
> > > +               $(if $(BINMAN_ALLOW_MISSING),--allow-missing --ignore-missing) \
> > >                 -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
> > >                 -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
> > >                 $(foreach f,$(BINMAN_INDIRS),-I $(f)) \
> > > --
> > > 2.25.1
> > >
> > 
> > I believe we need the --fake-ext-blobs flag as well, since otherwise
> > boards which use tools (like mkimage) on things that don't exist will
> > not work.
> 
> So, we need to list out all the use cases perhaps, as we had missed one
> before. In my mind we have:
> - Board requires 1 or more blobs, developer will be running this on
>   hardware, expects output from U-Boot 'make' (or buildman) to boot.
>   Blobs must be present or non-zero exist status by default.
>   We didn't have this before, and we do now.
> - Board requires 1 or more blobs AND has optional blobs, will be running
>   this on hardware, expects output from U-Boot 'make' (or buildman) to
>   boot. This is the case we had missed before as allwinner requires bl31
>   and has optional PMIC firmware. This is the case Peter reported and we
>   had missed before, which this patch allows for, with the caveat that
>   if you forget BL31 you're not going to boot and make won't complain
>   exit-code wise. Another way to resolve this would be a property in the
>   binman node to mark a blob as optional and warn if missing, rather
>   than error if missing.
> - Board requires 1 or more blobs, output will NOT be run. This is the CI
>   case and the compile testing lots of board cases. This is what CI
>   passes still, with the above.
> - Board requires 1 or more blobs, developer will be running this on
>   hardware, BUT will be injecting the blobs later on. I think this is
>   the use case you're talking about?

Simon, can you test this last case by chance? I think that's the one
that's unclear about if this patch breaks or not and I want to know if I
need to respin to still include the fake blobs flag still too or not.
Or does anyone else use this case and can test?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20221212/29987ffd/attachment.sig>