[PATCH 4/5] binman: Use an exit code when blobs are missing

[Tom Rini]

On Wed, Oct 12, 2022 at 06:59:21AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Tue, 11 Oct 2022 at 15:05, Tom Rini <trini at konsulko.com> wrote:
> >
> > On Mon, Oct 10, 2022 at 04:25:36PM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 10 Oct 2022 at 14:41, Tom Rini <trini at konsulko.com> wrote:
> > > >
> > > > On Mon, Oct 10, 2022 at 02:00:31PM -0600, Simon Glass wrote:
> > > >
> > > > > At present binman returns success when told to handle missing blobs.
> > > > > This is confusing this in fact the resulting image cannot work.
> > > > >
> > > > > Use exit code 103 to signal this problem, with a -W option to convert
> > > > > it to a warning.
> > > > >
> > > > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > >
> > > > I'm still not sure I like this, rather than changing the default "make"
> > > > behavior.
> > >
> > > I did change that, in the sense that 'make' now fails if there are
> > > missing blobs.
> > >
> > > > And then it gets me a bit worried that we have CI not doing
> > > > some other things "right" as we don't want to ignore warnings in CI, we
> > > > want warnings to become errors so that new warnings don't make it
> > > > in-tree.
> > >
> > > That would be quite a big effort, and unrelated to this series. Here
> > > are some warning types I'm aware of:
> > >
> > > - migration
> > > - device tree
> > > - compiler
> > > - missing blobs
> > >
> > > Do we need a per-board whitelist for warnings? It seems pretty tricky to me.
> > >
> > > It is true that warnings are ignored in CI and this does create
> > > problems...I'd love to make them into errors if we can.
> >
> > This is I guess also a related concern. When I say warnings, I mean
> > C compiler warnings. That we have flags to suppress "warnings" that are
> > other kinds of valid warnings is at least a little confusing.
> 
> Yes. But of course binman doesn't have any other kind of warning, so
> so long as we don't propagate that flag further, it makes sense I
> think.
> 
> The warnings in U-Boot are at least somewhat out of control IMO, as
> per my list above.

Well, lets dig down in to those for a minute?

For "missing blobs", if we're passing the "fake these blobs" flag AND we
don't ever default to passing that flag in regular build rules, I can
see making it so that we only see those messages either when we don't
pass the fake these blobs flag or we have a verbose option set.

For device tree warnings? I think we're mirroring the kernel still in
terms of ignoring problems so it's likely a matter of poking board
maintainers to resync their trees and/or address the warnings upstream
too (which upstream would appreciate).

For migration warnings, well, which? Are there some that can be easily
done and compile tested? Looking at the Makefile atm, I think the
CONFIG_DM migration warning logic can go away as there's no boards
missing that. There's nothing in the "this passed so long ago we should
delete boards" list, but it would be a good idea (so I'll fire off some
builds in a moment) to see who trips up on each and email maintainers.

-- 
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/20221012/dbd0204a/attachment.sig>