Re: [OE-core] [master][PATCH] image_types: add 7-Zip support in conversion types and commands

[Luca Ceresoli <luca.ceresoli@bootlin.com>]

Hello Livius,

thank you for your patch!

I have a few comments, see below.

On Thu, 25 Aug 2022 12:59:34 -0700
"Livius" <egyszeregy@freemail.hu> wrote:

> I added to support 7-Zip in conversion types/commands. It is fully configurable in compression level, method and file extension.
> 
> From: "Benjamin Szőke" <egyszeregy@freemail.hu>
> Date: Thu, 25 Aug 2022 21:45:55 +0200
> Subject: [PATCH] image_types: add 7-Zip support in conversion types and commands

These 3 lines should be in the mail header, not in the body. All the
lines that appear in the body before the '---' line will be part of the
commit message, and of course we don't want these 3 lines in the commit
message.

I recommend you to use 'git send-email' to send patches.

See this link  (from README.md in the meta-yocto sources) for more info:
https://www.openembedded.org/wiki/How_to_submit_a_patch_to_OpenEmbedded

> ---
> meta/classes-recipe/image_types.bbclass | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/meta/classes-recipe/image_types.bbclass b/meta/classes-recipe/image_types.bbclass
> index a731e585b2..94aa1d9510 100644
> --- a/meta/classes-recipe/image_types.bbclass
> +++ b/meta/classes-recipe/image_types.bbclass
> @@ -59,6 +59,10 @@ XZ_INTEGRITY_CHECK ?= "crc32"
> 
> ZIP_COMPRESSION_LEVEL ?= "-9"
> 
> +7ZIP_COMPRESSION_LEVEL ?= "9"
> +7ZIP_COMPRESSION_METHOD ?= "BZip2"

Did you run some performance test to check that this is actually a good
balance between compression ratio and compression time?

> +7ZIP_EXTENSION ?= "zip"

I think the 7zip default extension is '7z', why defaulting to 'zip'?
Seems like generating confusion for no reason.

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com