[PATCH 9/9] Makefile: Show binman missing blob message

[Simon Glass]

Hi Tom,

On Tue, 21 Feb 2023 at 16:09, Tom Rini <trini at konsulko.com> wrote:
>
> On Tue, Feb 21, 2023 at 12:41:52PM -0700, Simon Glass wrote:
> > Hi Jonas
> >
> > +Tom Rini
> >
> > On Sun, 19 Feb 2023 at 15:02, Jonas Karlman <jonas at kwiboo.se> wrote:
> > >
> > > When binman is invoked during a build of U-Boot and an external blob is
> > > missing, the user is usually presented with a generic file not found in
> > > input path message.
> > >
> > > Invoke binman with --allow-missing so that binman can show relevant
> > > missing blob help messages. Build continue to fail with missing blobs
> > > unless BINMAN_ALLOW_MISSING=1 is used.
> > >
> > > This changes the following error message:
> > >
> > >   binman: Filename 'atf-bl31' not found in input path (...)
> > >
> > > to the following:
> > >
> > >   Image 'itb' is missing external blobs and is non-functional: atf-blob
> > >
> > >   /binman/itb/fit/images/atf/atf-blob (bl31.bin):
> > >      See the documentation for your board. You may need to build ARM Trusted
> > >      Firmware and build with BL31=/path/to/bl31.bin
> > >
> > > Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> > > ---
> > >  Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 58f8c7a35335..c2860824f6f2 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -1326,7 +1326,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 --ignore-missing) \
> > > +               --allow-missing $(if $(BINMAN_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.39.2
> > >
> >
> > I agree this is better, but we should see what Tom thinks.
> >
> > Reviewed-by: Simon Glass <sjg at chromium.org>
>
> This sounds like a binman bug. We shouldn't need to say --allow-missing
> to then make use of the missing-msg node.

Without --allow-missing binman just dies when it cannot find
something. This is useful because it allows debugging of problems.

I'm not sure how easy it would be to change this. The error is
reported by the tools module which has no knowledge of binman. We
could catch the exception, but again that makes things non-debuggable.

At the moment we have two cases:
- normal: missing files cause an immediate abort
- --allow-missing: missing files are collected with a report at the end

Worse, we can have missing tools, not just missing blobs. The
complexity of this gets completely out of hand if we try to meld the
normal and --allow-missing cases together.

I actually don't see much of a downside to passing allow-missing
always. But perhaps the BINMAN_ALLOW_MISSING should be renamed to
BINMAN_IGNORE_MISSING (with a similar change in buildman). We could
actually keep BINMAN_ALLOW_MISSING to mean to pass --allow-missing,
pointing out in the docs that this is a bad idea as you won't see any
blob help?

Regards,
Simon