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

[Simon Glass]

Hi Tom,

On Mon, 12 Dec 2022 at 16:43, Tom Rini <trini at konsulko.com> wrote:
>
> 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?

Yes I have your email printed out but travel and family things have
made it hard to get to it. I do think you need the flag, but the extra
case you mention need another look and probably a test, as you say.

Regards,
Simon