[PATCH] Makefile: add -W for BINMAN_ALLOW_MISSING
From: Nikita Shubin n.shubin@yadro.com
Otherwise make will produce an error even with --allow-missing and --fake-ext-blobs set.
Fixes: b38da15a054 ("binman: Use an exit code when blobs are missing") Signed-off-by: Nikita Shubin n.shubin@yadro.com --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index de5746399a6..3fec08e7081 100644 --- a/Makefile +++ b/Makefile @@ -1334,7 +1334,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 --fake-ext-blobs) \ + $(if $(BINMAN_ALLOW_MISSING),--allow-missing --fake-ext-blobs -W) \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ $(foreach f,$(BINMAN_INDIRS),-I $(f)) \
Hi,
+Tom Rini
On Fri, 16 Dec 2022 at 02:27, Nikita Shubin nikita.shubin@maquefel.me wrote:
From: Nikita Shubin n.shubin@yadro.com
Otherwise make will produce an error even with --allow-missing and --fake-ext-blobs set.
Fixes: b38da15a054 ("binman: Use an exit code when blobs are missing") Signed-off-by: Nikita Shubin n.shubin@yadro.com
Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/Makefile b/Makefile index de5746399a6..3fec08e7081 100644 --- a/Makefile +++ b/Makefile @@ -1334,7 +1334,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 --fake-ext-blobs) \
$(if $(BINMAN_ALLOW_MISSING),--allow-missing --fake-ext-blobs -W) \ -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \ -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \ $(foreach f,$(BINMAN_INDIRS),-I $(f)) \-- 2.37.4
+Tom Rini
We do actually want to report the failure, since it means that the image will not function. This was a recent change requested by a few people.
Note that buildman looks for the message 'Some images are invalid' and either returning 103, or 0 if -W is given.
There is no attempt to produce a special exit code from the Makefile. It generally returns 2 (as per 'man make'), which is why buildman has this extra processing.
Regards, Simon
Hello Tom and Simon!
On Sat, 17 Dec 2022 14:38:30 -0700 Simon Glass sjg@chromium.org wrote:
+Tom Rini
We do actually want to report the failure, since it means that the image will not function. This was a recent change requested by a few people.
It doesn't make sense to me - if i am passing "--allow-missing" than binman shouldn't fail drastically, cause i literally told him "It's okay if files are missing". What purpose does it have now, it we are failing regardless we are providing this flag or not ?
This breaks old behaviour by the way, when passing "--allow-missing" for missing blobs produced a warning instead of error.
Note that buildman looks for the message 'Some images are invalid' and either returning 103, or 0 if -W is given.
There is no attempt to produce a special exit code from the Makefile. It generally returns 2 (as per 'man make'), which is why buildman has this extra processing.
Well, there are only 3 codes for make and 2 indicates any failure:
"A status of two will be returned if any errors were encountered." (c)
This new behaviour looks the same with or without BINMAN_ALLOW_MISSING flag from top point of view:
With BINMAN_ALLOW_MISSING=1: Some images are invalid make[1]: *** [Makefile:1114: .binman_stamp] Error 103 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u
$ echo $? 2-boot-nodtb.bin] Error 2
Without BINMAN_ALLOW_MISSING:
binman: Filename 'fw_dynamic.bin' not found in input path (.,.,./board/syntacore/scr7_elct,arch/riscv/dts) (cwd='/home/maquefel/workshop/overlord/u-boot') make[1]: *** [Makefile:1114: .binman_stamp] Error 1 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u-boot-nodtb.bin] Error 2
$ echo $? 2
So that's the difference if build is failing either way ?
Yours, Nikita Shubin.
Regards, Simon
On Mon, Dec 19, 2022 at 11:21:45AM +0300, Nikita Shubin wrote:
Hello Tom and Simon!
On Sat, 17 Dec 2022 14:38:30 -0700 Simon Glass sjg@chromium.org wrote:
+Tom Rini
We do actually want to report the failure, since it means that the image will not function. This was a recent change requested by a few people.
It doesn't make sense to me - if i am passing "--allow-missing" than binman shouldn't fail drastically, cause i literally told him "It's okay if files are missing". What purpose does it have now, it we are failing regardless we are providing this flag or not ?
This breaks old behaviour by the way, when passing "--allow-missing" for missing blobs produced a warning instead of error.
Note that buildman looks for the message 'Some images are invalid' and either returning 103, or 0 if -W is given.
There is no attempt to produce a special exit code from the Makefile. It generally returns 2 (as per 'man make'), which is why buildman has this extra processing.
Well, there are only 3 codes for make and 2 indicates any failure:
"A status of two will be returned if any errors were encountered." (c)
This new behaviour looks the same with or without BINMAN_ALLOW_MISSING flag from top point of view:
With BINMAN_ALLOW_MISSING=1: Some images are invalid make[1]: *** [Makefile:1114: .binman_stamp] Error 103 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u
$ echo $? 2-boot-nodtb.bin] Error 2
Without BINMAN_ALLOW_MISSING:
binman: Filename 'fw_dynamic.bin' not found in input path (.,.,./board/syntacore/scr7_elct,arch/riscv/dts) (cwd='/home/maquefel/workshop/overlord/u-boot') make[1]: *** [Makefile:1114: .binman_stamp] Error 1 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u-boot-nodtb.bin] Error 2
$ echo $? 2
So that's the difference if build is failing either way ?
So, with what is in master right now, BINMAN_ALLOW_MISSING=1 should work as intended, while it did not at its introduction. Please confirm if your use cases work now, or not. The problem we needed to solve was the one that by default previously, you could run "make fooboard_config all", not have BL31/etc available, get a warning printed and a zero exit code, leading to non-obvious failures if you build indirectly (buildroot, yocto/OE, etc).
Hello Tom!
On Mon, 19 Dec 2022 08:36:30 -0500 Tom Rini trini@konsulko.com wrote:
On Mon, Dec 19, 2022 at 11:21:45AM +0300, Nikita Shubin wrote:
Hello Tom and Simon!
On Sat, 17 Dec 2022 14:38:30 -0700 Simon Glass sjg@chromium.org wrote:
+Tom Rini
We do actually want to report the failure, since it means that the image will not function. This was a recent change requested by a few people.
It doesn't make sense to me - if i am passing "--allow-missing" than binman shouldn't fail drastically, cause i literally told him "It's okay if files are missing". What purpose does it have now, it we are failing regardless we are providing this flag or not ?
This breaks old behaviour by the way, when passing "--allow-missing" for missing blobs produced a warning instead of error.
Note that buildman looks for the message 'Some images are invalid' and either returning 103, or 0 if -W is given.
There is no attempt to produce a special exit code from the Makefile. It generally returns 2 (as per 'man make'), which is why buildman has this extra processing.
Well, there are only 3 codes for make and 2 indicates any failure:
"A status of two will be returned if any errors were encountered." (c)
This new behaviour looks the same with or without BINMAN_ALLOW_MISSING flag from top point of view:
With BINMAN_ALLOW_MISSING=1: Some images are invalid make[1]: *** [Makefile:1114: .binman_stamp] Error 103 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u
$ echo $? 2-boot-nodtb.bin] Error 2
Without BINMAN_ALLOW_MISSING:
binman: Filename 'fw_dynamic.bin' not found in input path (.,.,./board/syntacore/scr7_elct,arch/riscv/dts) (cwd='/home/maquefel/workshop/overlord/u-boot') make[1]: *** [Makefile:1114: .binman_stamp] Error 1 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u-boot-nodtb.bin] Error 2
$ echo $? 2
So that's the difference if build is failing either way ?
So, with what is in master right now, BINMAN_ALLOW_MISSING=1 should work as intended, while it did not at its introduction. Please confirm if your use cases work now, or not.
They don't actually, a few iterations ago i didn't even needed BINMAN_ALLOW_MISSING (now it's clear for me that i never needed it), as make produced only a warning and not a error.
I have kernel and ramdisk sections in my binman file and FIT image is fully functional even if they are missing.
And now there is no way to tell u-boot not to fail if some blobs are missing.
The problem we needed to solve was the one that by default previously, you could run "make fooboard_config all", not have BL31/etc available, get a warning printed and a zero exit code, leading to non-obvious failures if you build indirectly (buildroot, yocto/OE, etc).
May be they shouldn't use BINMAN_ALLOW_MISSING when building u-boot at all then ?
Or can we, at least, have some BINMAN_REALLY_ALLOW_MISSING option or simply passing some flags with BINMAN_OPTS for example ?
Through: -W, --ignore-missing Return success even if there are missing blobs/bintools (requires -M)
seems to have really good synergy with:
-M, --allow-missing Allow external blobs and bintools to be missing
For BINMAN_ALLOW_MISSING.
Yours, Nikita Shubin.
On Thu, Dec 22, 2022 at 10:01:16AM +0300, Nikita Shubin wrote:
Hello Tom!
On Mon, 19 Dec 2022 08:36:30 -0500 Tom Rini trini@konsulko.com wrote:
On Mon, Dec 19, 2022 at 11:21:45AM +0300, Nikita Shubin wrote:
Hello Tom and Simon!
On Sat, 17 Dec 2022 14:38:30 -0700 Simon Glass sjg@chromium.org wrote:
+Tom Rini
We do actually want to report the failure, since it means that the image will not function. This was a recent change requested by a few people.
It doesn't make sense to me - if i am passing "--allow-missing" than binman shouldn't fail drastically, cause i literally told him "It's okay if files are missing". What purpose does it have now, it we are failing regardless we are providing this flag or not ?
This breaks old behaviour by the way, when passing "--allow-missing" for missing blobs produced a warning instead of error.
Note that buildman looks for the message 'Some images are invalid' and either returning 103, or 0 if -W is given.
There is no attempt to produce a special exit code from the Makefile. It generally returns 2 (as per 'man make'), which is why buildman has this extra processing.
Well, there are only 3 codes for make and 2 indicates any failure:
"A status of two will be returned if any errors were encountered." (c)
This new behaviour looks the same with or without BINMAN_ALLOW_MISSING flag from top point of view:
With BINMAN_ALLOW_MISSING=1: Some images are invalid make[1]: *** [Makefile:1114: .binman_stamp] Error 103 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u
$ echo $? 2-boot-nodtb.bin] Error 2
Without BINMAN_ALLOW_MISSING:
binman: Filename 'fw_dynamic.bin' not found in input path (.,.,./board/syntacore/scr7_elct,arch/riscv/dts) (cwd='/home/maquefel/workshop/overlord/u-boot') make[1]: *** [Makefile:1114: .binman_stamp] Error 1 make[1]: Leaving directory '/home/maquefel/workshop/overlord/u-boot' make: *** [Makefile:271: u-boot/u-boot-nodtb.bin] Error 2
$ echo $? 2
So that's the difference if build is failing either way ?
So, with what is in master right now, BINMAN_ALLOW_MISSING=1 should work as intended, while it did not at its introduction. Please confirm if your use cases work now, or not.
They don't actually, a few iterations ago i didn't even needed BINMAN_ALLOW_MISSING (now it's clear for me that i never needed it), as make produced only a warning and not a error.
Yes, it would only ever produce a warning before.
I have kernel and ramdisk sections in my binman file and FIT image is fully functional even if they are missing.
And now there is no way to tell u-boot not to fail if some blobs are missing.
What do you mean? That's what BINMAN_ALLOW_MISSING=1 does: $ make CROSS_COMPILE=~/.buildman-toolchains/gcc-12.2.0-nolibc/aarch64-linux/bin/aarch64-linux- BINMAN_ALLOW_MISSING=1 -sj pine64_plus_defconfig all -sj;echo $? ... Image 'main-section' is missing external blobs and is non-functional: atf-bl31 scp
/binman/u-boot-sunxi-with-spl/fit/images/atf/atf-bl31: Please read the section on ARM Trusted Firmware (ATF) in board/sunxi/README.sunxi64
/binman/u-boot-sunxi-with-spl/fit/images/scp/scp: SCP firmware is required for system suspend, but is otherwise optional. Please read the section on SCP firmware in board/sunxi/README.sunxi64
Some images are invalid 0 $
Is what I get on top of master right now.
The problem we needed to solve was the one that by default previously, you could run "make fooboard_config all", not have BL31/etc available, get a warning printed and a zero exit code, leading to non-obvious failures if you build indirectly (buildroot, yocto/OE, etc).
May be they shouldn't use BINMAN_ALLOW_MISSING when building u-boot at all then ?
Or can we, at least, have some BINMAN_REALLY_ALLOW_MISSING option or simply passing some flags with BINMAN_OPTS for example ?
Through: -W, --ignore-missing Return success even if there are missing blobs/bintools (requires -M)
seems to have really good synergy with:
-M, --allow-missing Allow external blobs and bintools to be missing
For BINMAN_ALLOW_MISSING.
Yes, that's what BINMAN_ALLOW_MISSING=1 does. If top of tree master doesn't work for you, can you please post a patch so that the rest of us can replicate the failure and see what to do next? Thanks.
Hello Tom!
On Thu, 22 Dec 2022 09:14:59 -0500 Tom Rini trini@konsulko.com wrote:
Yes, that's what BINMAN_ALLOW_MISSING=1 does. If top of tree master doesn't work for you, can you please post a patch so that the rest of us can replicate the failure and see what to do next? Thanks.
You are right - i accidentally got stuck right between "global: Do not default to faking missing binaries for buildman" (which broke build) and "Makefile: With BINMAN_ALLOW_MISSING=1 don't error on missing" (which i didn't have on my branch).
Everything is just perfect on the top of latest master!
Thank you for your patience and sorry for the noise.
Yours, Nikita Shubin
participants (3)
-
Nikita Shubin -
Simon Glass -
Tom Rini