Hi Tom,
On Tue, 11 Oct 2022 at 10:22, Tom Rini trini@konsulko.com wrote:
On Mon, Oct 10, 2022 at 05:48:55PM -0600, Simon Glass wrote:
Hi Tom,
On Mon, 10 Oct 2022 at 09:18, Tom Rini trini@konsulko.com wrote:
Add a new flag to buildman so that we will in turn pass BINMAN_ALLOW_MISSING=1 to 'make'. Make use of this flag in CI.
Cc: Rasmus Villemoes rasmus.villemoes@prevas.dk Cc: Simon Glass sjg@chromium.org Signed-off-by: Tom Rini trini@konsulko.com
.azure-pipelines.yml | 2 +- .gitlab-ci.yml | 6 +++--- tools/buildman/builder.py | 5 ++++- tools/buildman/builderthread.py | 2 ++ tools/buildman/cmdline.py | 3 +++ tools/buildman/control.py | 3 ++- 6 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index f200b40dbb24..c932c2b3c619 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -553,7 +553,7 @@ stages: cat << "EOF" >> build.sh if [[ "${BUILDMAN}" != "" ]]; then ret=0;
tools/buildman/buildman -o /tmp -P -E -W ${BUILDMAN} ${OVERRIDE} || ret=$?;
tools/buildman/buildman -o /tmp -P -E -W --allow-missing-binaries ${BUILDMAN} ${OVERRIDE} || ret=$?;This is fine for CI.
I think one of the issues here is we need to agree on the common use cases. I don't think most people use buildman to build, they use make.
OK. With the new -I/--ide flag I use buildman directly for single builds on some machines at least.
Of the people that do use buildman, how many aren't already using a wrapper script? I know I always do.
I don't know, but I always use buildman directly for build-testing multiple boards.
Can we also not just deal with setting this flag in ~/.buildman ? Would it Just Work as: [builder] allow-missing-binaries = True
Or is more code needed?
Yes I think that would work, although I think we would need two flags, one for building current source (where people might want it off) and another for building multiple boards (when presumably you want it on).
...although I don't see the advantage of this approach over the series I sent. Basically, the build should fail if there are missing blobs. The only question is whether we want to handle missing blobs by:
1. failing immediately when we see the first missing blob (your approach) 2. failing at the end after all binman processing is done (my approach)
The nice thing about 2 is that the build does complete and the exit code lets buildman decide what to do afterwards (i.e. we can make missing blobs be an error or a warning).
Option 1 has the benefit that we don't do any of the blob handling, so it just dies right away. Perhaps this is a philosophical question, but it is a little subtle and I'm not actually sure people would notice the difference so long as they get the errors as expected.
BTW we need a one-character flag if we do this as it will be a very common requirement.
But having to provide a flag to do build testing seems like the tail is wagging the dog. Boards should be discouraged to use blobs and we don't want to make it hard for everyone else (who doesn't have the blobs) to test whether their patch breaks a build.
I'm not sure this ends up being a common case. If someone is build testing for something they can't run test, the error is going to be after whatever they compile-tested was compiled/linked, and if they're testing everything it's via CI.
I don't think people will be able to tell that. The 'make' fails with an error so it looks like a failure. It happens to be at the end in the binman step, but it is still a build failure.
My overall concern is constantly having to rerun a build because I forgot to add a flag, when I don't have the blobs for the boards anyway. Like the LTO thing that cost 4 seconds on every build, that could get really annoying. Blobs should not get in the way of development.
Regards, Simon