Hi Tom, On Tue, 11 Oct 2022 at 10:22, Tom Rini <tr...@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 <tr...@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.villem...@prevas.dk> > > > Cc: Simon Glass <s...@chromium.org> > > > Signed-off-by: Tom Rini <tr...@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