Hi Tom,
On Wed, 22 Feb 2023 at 14:45, Tom Rini trini@konsulko.com wrote:
On Wed, Feb 22, 2023 at 02:20:08PM -0700, Simon Glass wrote:
Hi Tom,
On Tue, 21 Feb 2023 at 16:09, Tom Rini trini@konsulko.com wrote:
On Tue, Feb 21, 2023 at 12:41:52PM -0700, Simon Glass wrote:
Hi Jonas
+Tom Rini
On Sun, 19 Feb 2023 at 15:02, Jonas Karlman jonas@kwiboo.se wrote:
When binman is invoked during a build of U-Boot and an external blob is missing, the user is usually presented with a generic file not found in input path message.
Invoke binman with --allow-missing so that binman can show relevant missing blob help messages. Build continue to fail with missing blobs unless BINMAN_ALLOW_MISSING=1 is used.
This changes the following error message:
binman: Filename 'atf-bl31' not found in input path (...)
to the following:
Image 'itb' is missing external blobs and is non-functional: atf-blob
/binman/itb/fit/images/atf/atf-blob (bl31.bin): See the documentation for your board. You may need to build ARM Trusted Firmware and build with BL31=/path/to/bl31.bin
Signed-off-by: Jonas Karlman jonas@kwiboo.se
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index 58f8c7a35335..c2860824f6f2 100644 --- a/Makefile +++ b/Makefile @@ -1326,7 +1326,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 --ignore-missing) \
--allow-missing $(if $(BINMAN_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.39.2
I agree this is better, but we should see what Tom thinks.
Reviewed-by: Simon Glass sjg@chromium.org
This sounds like a binman bug. We shouldn't need to say --allow-missing to then make use of the missing-msg node.
Without --allow-missing binman just dies when it cannot find something. This is useful because it allows debugging of problems.
I'm not sure how easy it would be to change this. The error is reported by the tools module which has no knowledge of binman. We could catch the exception, but again that makes things non-debuggable.
Why wouldn't it be debuggable? Wouldn't it be more debuggable now that we're saying "You're missing foo.blob, this is typically provided by the foo project", which we can just say because the message has been filled out already in the etype.
Perhaps just take my word for it? The feature you are asking for here is exactly what the -M flag does, but you are trying to make it so that a 'file not found' error is not reported immediately, but appears later. That makes it hard to debug, since the 'exception' is supressed and only later do you see there is a problem...so hard to debug.
At the moment we have two cases:
- normal: missing files cause an immediate abort
- --allow-missing: missing files are collected with a report at the end
Worse, we can have missing tools, not just missing blobs. The complexity of this gets completely out of hand if we try to meld the normal and --allow-missing cases together.
I actually don't see much of a downside to passing allow-missing always. But perhaps the BINMAN_ALLOW_MISSING should be renamed to BINMAN_IGNORE_MISSING (with a similar change in buildman). We could actually keep BINMAN_ALLOW_MISSING to mean to pass --allow-missing, pointing out in the docs that this is a bad idea as you won't see any blob help?
Maybe the problem is the flag is badly named? I still think this speaks to bugs in the tooling if we can't make use of the helpful error message that we have available without some special flags to be set.
Without -M binman is just opening the files you tell it, failing if it cannot, exactly as things used to be before binman.
Yes --allow-missing might not be the best name, since we don't really allow it, just collect the information and report it. Binman still fails the build in that case. Perhaps we need:
(nothing) - die as soon as any missing tool or file is --collect-missing - collect all the missing blobs and tools, rather than dying right away, returning 103 if there are any problems --allow-missing - report success (with warnings output) even if there are missing blobs / tools
Regards, Simon