[OpenWrt-Devel] image: ignore usign build errors

Message ID 20181002205039.9298-1-hauke@hauke-m.de
State Accepted
Delegated to: Hauke Mehrtens
Headers show
Series
  • [OpenWrt-Devel] image: ignore usign build errors
Related show

Commit Message

Hauke Mehrtens Oct. 2, 2018, 8:50 p.m.
The tl-wa850re-v2 images from the ar71xx/tiny target are getting too big
with the default packages. The size check is done before the meta data
is added so there is no file to add meta data to or to sign. Originally
errors in Build/append-metadata were getting ignored, but if the signing
fails the error is not ignored.
This patch makes make ignore errors in the signing which is the case for
too big images because then the needed file is not created in the
previous process.

Fixes: 848b455d2e94 ("image: use ucert to append signature")
Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
---
 include/image-commands.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Henrique de Moraes Holschuh Oct. 3, 2018, 10:57 a.m. | #1
Em 02/10/18 17:50, Hauke Mehrtens escreveu:
> The tl-wa850re-v2 images from the ar71xx/tiny target are getting too big
> with the default packages. The size check is done before the meta data
> is added so there is no file to add meta data to or to sign. Originally
> errors in Build/append-metadata were getting ignored, but if the signing
> fails the error is not ignored.

I would very much like to be able to NOT ignore any errors that result 
in a missing or unsigned image.

IMHO, it is quite annoying to have a build that resulted on missing 
images at the end finish successfully, when you are actually interested 
in *every* target image you requested in Kconfig to be built.

It is fine if this it is not made the default behavior, the build system 
can ignore such failures by default as far as I am concerned.  As long 
as one could set a Kconfig option or environment variable to have the 
build system abort when it does an incomplete job, it would be enough.

And by an incomplete build job, I do mean failing to create any of the 
requested images, or creating images that are too big for their target, 
or failing to add metadata to the images, or failing to sign the final 
images, etc.
Felix Fietkau Oct. 5, 2018, 10:09 a.m. | #2
On 2018-10-02 22:50, Hauke Mehrtens wrote:
> The tl-wa850re-v2 images from the ar71xx/tiny target are getting too big
> with the default packages. The size check is done before the meta data
> is added so there is no file to add meta data to or to sign. Originally
> errors in Build/append-metadata were getting ignored, but if the signing
> fails the error is not ignored.
> This patch makes make ignore errors in the signing which is the case for
> too big images because then the needed file is not created in the
> previous process.
> 
> Fixes: 848b455d2e94 ("image: use ucert to append signature")
> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
> ---
>  include/image-commands.mk | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/image-commands.mk b/include/image-commands.mk
> index ae01706b14..9f5876bccd 100644
> --- a/include/image-commands.mk
> +++ b/include/image-commands.mk
> @@ -332,7 +332,7 @@ metadata_json = \
>  
>  define Build/append-metadata
>  	$(if $(SUPPORTED_DEVICES),-echo $(call metadata_json,$(SUPPORTED_DEVICES)) | fwtool -I - $@)
> -	[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \
> +	-[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \

How about this instead?

[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" -o ! -s "$@" ] || ...

- Felix
Hauke Mehrtens Oct. 7, 2018, 8:22 p.m. | #3
On 10/05/2018 12:09 PM, Felix Fietkau wrote:
> On 2018-10-02 22:50, Hauke Mehrtens wrote:
>> The tl-wa850re-v2 images from the ar71xx/tiny target are getting too big
>> with the default packages. The size check is done before the meta data
>> is added so there is no file to add meta data to or to sign. Originally
>> errors in Build/append-metadata were getting ignored, but if the signing
>> fails the error is not ignored.
>> This patch makes make ignore errors in the signing which is the case for
>> too big images because then the needed file is not created in the
>> previous process.
>>
>> Fixes: 848b455d2e94 ("image: use ucert to append signature")
>> Signed-off-by: Hauke Mehrtens <hauke@hauke-m.de>
>> ---
>>  include/image-commands.mk | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/image-commands.mk b/include/image-commands.mk
>> index ae01706b14..9f5876bccd 100644
>> --- a/include/image-commands.mk
>> +++ b/include/image-commands.mk
>> @@ -332,7 +332,7 @@ metadata_json = \
>>  
>>  define Build/append-metadata
>>  	$(if $(SUPPORTED_DEVICES),-echo $(call metadata_json,$(SUPPORTED_DEVICES)) | fwtool -I - $@)
>> -	[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \
>> +	-[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \
> 
> How about this instead?
> 
> [ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" -o ! -s "$@" ] || ...
> 
> - Felix
> 
Hi Felix,

Thanks, I changed to patch to do it like you suggested and just check if
the file exists.

Hauke

Patch

diff --git a/include/image-commands.mk b/include/image-commands.mk
index ae01706b14..9f5876bccd 100644
--- a/include/image-commands.mk
+++ b/include/image-commands.mk
@@ -332,7 +332,7 @@  metadata_json = \
 
 define Build/append-metadata
 	$(if $(SUPPORTED_DEVICES),-echo $(call metadata_json,$(SUPPORTED_DEVICES)) | fwtool -I - $@)
-	[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \
+	-[ ! -s "$(BUILD_KEY)" -o ! -s "$(BUILD_KEY).ucert" ] || { \
 		cp "$(BUILD_KEY).ucert" "$@.ucert" ;\
 		usign -S -m "$@" -s "$(BUILD_KEY)" -x "$@.sig" ;\
 		ucert -A -c "$@.ucert" -x "$@.sig" ;\