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

[Simon Glass]

Hi Tom,

On Wed, 12 Oct 2022 at 08:29, Tom Rini <trini at konsulko.com> wrote:
>
> 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.

Yes I suppose we could do that, so long as buildman shows the build as
a failure as [1]. We want to make sure that people don't think that
the build actually succeeded, even with missing blobs. It can be a
warning (suppressed with -W or forced to an error with -E) but it must
start off as a warning.

>
> 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).

Should we start a whitelist here and require new boards to compile cleanly?

>
> 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.

Yes, the Makefile ones. Of course there are quite a few we don't have,
like DM_ETH.

Also DM_SPI should be dropped I think, since we finished that some years ago.

I'll start a new thread on DM_SPL

Regards,
Simon

[1] https://patchwork.ozlabs.org/project/uboot/patch/20221011141541.538175-6-sjg@chromium.org/