[PATCH] Makefile: With BINMAN_ALLOW_MISSING=1 don't error on missing
Simon Glass
sjg at chromium.org
Sat Dec 17 22:40:36 CET 2022
Hi Tom,
On Tue, 13 Dec 2022 at 09:39, Tom Rini <trini at konsulko.com> wrote:
>
> On Mon, Dec 12, 2022 at 10:59:01PM -0700, Simon Glass wrote:
> > 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.
>
> Yeah, understandable. My plan is to take this patch before the next -rc,
> so Fedora won't be broken and if we need the other flag (and I'll check
> in before release) we can follow up there and have a fixes tag on top of
> this one here.
I just sent a patch that allows entries (with external blobs) to be
marked as optional. I believe this covers the remaining case.
You still need to keep --fake-ext-blobs (in this case) since otherwise
boards that rely on faked blobs will not build - e.g. will get an
error in the mkimage step if an input file is missing.
Regards,
Simon
More information about the U-Boot
mailing list