[meta-arm][PATCH 1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE

[meta-arm][PATCH 1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE

[Jose Quaresma]

When append to variable a space is required at the beginning.

Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---
 meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
index acea7502..40423615 100644
--- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
+++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
@@ -7,7 +7,8 @@ DEPENDS:append = "\
                  '' , \
                  d)}"
 
-EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', \
+EXTRA_OEMAKE:append = "\
+                       ${@bb.utils.contains('MACHINE_FEATURES', \
                       'optee-ftpm', \
                       'CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', \
                       '', \
-- 
2.34.1

[meta-arm][PATCH 2/3] optee-ftpm/optee-os: rewite in a way to be less error prone

[Jose Quaresma]

So it becomes clearer the need to use the space in the append

Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---
 .../optee-ftpm/optee-os_%.bbappend               | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
index 40423615..332c7c1c 100644
--- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
+++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
@@ -1,15 +1,7 @@
 FTPM_UUID="bc50d971-d4c9-42c4-82cb-343fb7f37896"
 
-DEPENDS:append = "\
-                  ${@bb.utils.contains('MACHINE_FEATURES', \
-                 'optee-ftpm', \
-                 'optee-ftpm', \
-                 '' , \
-                 d)}"
+DEPENDS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \
+                 ' optee-ftpm', '' , d)}"
 
-EXTRA_OEMAKE:append = "\
-                       ${@bb.utils.contains('MACHINE_FEATURES', \
-                      'optee-ftpm', \
-                      'CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', \
-                      '', \
-                      d)} "
+EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \
+                      ' CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', '', d)} "
-- 
2.34.1

Re: [meta-arm][PATCH 2/3] optee-ftpm/optee-os: rewite in a way to be less error prone

[Mikko Rapeli]

Hi,

On Wed, Oct 26, 2022 at 02:18:53PM +0000, Jose Quaresma wrote:
> So it becomes clearer the need to use the space in the append
> 
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> ---
>  .../optee-ftpm/optee-os_%.bbappend               | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> index 40423615..332c7c1c 100644
> --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> @@ -1,15 +1,7 @@
>  FTPM_UUID="bc50d971-d4c9-42c4-82cb-343fb7f37896"
>  
> -DEPENDS:append = "\
> -                  ${@bb.utils.contains('MACHINE_FEATURES', \
> -                 'optee-ftpm', \
> -                 'optee-ftpm', \
> -                 '' , \
> -                 d)}"
> +DEPENDS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \

Same here, don't use :append, use += which will add the space too.

> +                 ' optee-ftpm', '' , d)}"
>  
> -EXTRA_OEMAKE:append = "\
> -                       ${@bb.utils.contains('MACHINE_FEATURES', \
> -                      'optee-ftpm', \
> -                      'CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', \
> -                      '', \
> -                      d)} "
> +EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \

Ditto.

Cheers,

-Mikko

Re: [meta-arm][PATCH 2/3] optee-ftpm/optee-os: rewite in a way to be less error prone

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 2143 bytes --]

Hi

Mikko Rapeli <mikko.rapeli@linaro.org> escreveu no dia quinta, 27/10/2022
à(s) 07:19:

> Hi,
>
> On Wed, Oct 26, 2022 at 02:18:53PM +0000, Jose Quaresma wrote:
> > So it becomes clearer the need to use the space in the append
> >
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> > ---
> >  .../optee-ftpm/optee-os_%.bbappend               | 16 ++++------------
> >  1 file changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > index 40423615..332c7c1c 100644
> > --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > @@ -1,15 +1,7 @@
> >  FTPM_UUID="bc50d971-d4c9-42c4-82cb-343fb7f37896"
> >
> > -DEPENDS:append = "\
> > -                  ${@bb.utils.contains('MACHINE_FEATURES', \
> > -                 'optee-ftpm', \
> > -                 'optee-ftpm', \
> > -                 '' , \
> > -                 d)}"
> > +DEPENDS:append = "${@bb.utils.contains('MACHINE_FEATURES',
> 'optee-ftpm', \
>
> Same here, don't use :append, use += which will add the space too.
>
> > +                 ' optee-ftpm', '' , d)}"
>

Here the space is only added if needed and together with the value.


> >
> > -EXTRA_OEMAKE:append = "\
> > -                       ${@bb.utils.contains('MACHINE_FEATURES', \
> > -                      'optee-ftpm', \
> > -                      'CFG_EARLY_TA=y
> EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"',
> \
> > -                      '', \
> > -                      d)} "
> > +EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES',
> 'optee-ftpm', \
>
> Ditto.
>

+EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES',
'optee-ftpm', \
+                      ' CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_
TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', '', d)} "

The same here

Jose


>
> Cheers,
>
> -Mikko
>


-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 3671 bytes --]

Re: [meta-arm][PATCH 2/3] optee-ftpm/optee-os: rewite in a way to be less error prone

[Jon Mason]

On Wed, Oct 26, 2022 at 02:18:53PM +0000, Jose Quaresma wrote:
> So it becomes clearer the need to use the space in the append
> 
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> ---
>  .../optee-ftpm/optee-os_%.bbappend               | 16 ++++------------
>  1 file changed, 4 insertions(+), 12 deletions(-)
> 
> diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> index 40423615..332c7c1c 100644
> --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> @@ -1,15 +1,7 @@
>  FTPM_UUID="bc50d971-d4c9-42c4-82cb-343fb7f37896"
>  
> -DEPENDS:append = "\
> -                  ${@bb.utils.contains('MACHINE_FEATURES', \
> -                 'optee-ftpm', \
> -                 'optee-ftpm', \
> -                 '' , \
> -                 d)}"
> +DEPENDS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \
> +                 ' optee-ftpm', '' , d)}"
>  
> -EXTRA_OEMAKE:append = "\
> -                       ${@bb.utils.contains('MACHINE_FEATURES', \
> -                      'optee-ftpm', \
> -                      'CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', \
> -                      '', \
> -                      d)} "
> +EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', 'optee-ftpm', \
> +                      ' CFG_EARLY_TA=y EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"', '', d)} "
> -- 
> 2.34.1
> 
> 

This is a "style" patch, and doesn't really fix an issue.  I believe
there are other instances of this in the tree.  If you were to do a
patch (or more likely a series of patches) that unifies all of the
recipes to have a unified style, I'd be more inclined to take it (but
honestly I don't think that is a good use of your time).

The other two of the series look to fix bugs and I'll pull those in.
Thank you for the patches :)

Thanks,
Jon

Re: [meta-arm][PATCH 2/3] optee-ftpm/optee-os: rewite in a way to be less error prone

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 2511 bytes --]

Hi Jon,

Jon Mason <jdmason@kudzu.us> escreveu no dia quinta, 27/10/2022 à(s) 15:03:

> On Wed, Oct 26, 2022 at 02:18:53PM +0000, Jose Quaresma wrote:
> > So it becomes clearer the need to use the space in the append
> >
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> > ---
> >  .../optee-ftpm/optee-os_%.bbappend               | 16 ++++------------
> >  1 file changed, 4 insertions(+), 12 deletions(-)
> >
> > diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > index 40423615..332c7c1c 100644
> > --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > @@ -1,15 +1,7 @@
> >  FTPM_UUID="bc50d971-d4c9-42c4-82cb-343fb7f37896"
> >
> > -DEPENDS:append = "\
> > -                  ${@bb.utils.contains('MACHINE_FEATURES', \
> > -                 'optee-ftpm', \
> > -                 'optee-ftpm', \
> > -                 '' , \
> > -                 d)}"
> > +DEPENDS:append = "${@bb.utils.contains('MACHINE_FEATURES',
> 'optee-ftpm', \
> > +                 ' optee-ftpm', '' , d)}"
> >
> > -EXTRA_OEMAKE:append = "\
> > -                       ${@bb.utils.contains('MACHINE_FEATURES', \
> > -                      'optee-ftpm', \
> > -                      'CFG_EARLY_TA=y
> EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"',
> \
> > -                      '', \
> > -                      d)} "
> > +EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES',
> 'optee-ftpm', \
> > +                      ' CFG_EARLY_TA=y
> EARLY_TA_PATHS="${STAGING_DIR_TARGET}/lib/optee_armtz/${FTPM_UUID}.stripped.elf"',
> '', d)} "
> > --
> > 2.34.1
> >
> >
>
> This is a "style" patch, and doesn't really fix an issue.  I believe
> there are other instances of this in the tree.  If you were to do a
> patch (or more likely a series of patches) that unifies all of the
> recipes to have a unified style, I'd be more inclined to take it (but
> honestly I don't think that is a good use of your time).
>

Right, I changed the style because I am fixing a bug in the same file
but yeah this patch doesn't fix any issue and can be dropped.


>
> The other two of the series look to fix bugs and I'll pull those in.
> Thank you for the patches :)
>

It's my pleasure.

Jose


>
> Thanks,
> Jon
>
>

-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 3832 bytes --]

[meta-arm][PATCH 3/3] optee-os-ts: avoid using escape chars in EXTRA_OEMAKE

[Jose Quaresma]

This currently can break the bitbake parsing

Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
---
 meta-arm/recipes-security/optee/optee-os-ts.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/meta-arm/recipes-security/optee/optee-os-ts.inc b/meta-arm/recipes-security/optee/optee-os-ts.inc
index 10a41755..73b8c14f 100644
--- a/meta-arm/recipes-security/optee/optee-os-ts.inc
+++ b/meta-arm/recipes-security/optee/optee-os-ts.inc
@@ -51,4 +51,4 @@ DEPENDS:append  = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
 SP_PATHS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
                                         ' ${TS_BIN}/${SMM_GATEWAY_UUID}.stripped.elf', '', d)}"
 
-EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS=\'${SP_PATHS}\' ', d)}"
+EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS="${SP_PATHS}" ', d)}"
-- 
2.34.1

Re: [meta-arm][PATCH 3/3] optee-os-ts: avoid using escape chars in EXTRA_OEMAKE

[Mikko Rapeli]

Hi,

On Wed, Oct 26, 2022 at 02:18:54PM +0000, Jose Quaresma wrote:
> This currently can break the bitbake parsing
> 
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> ---
>  meta-arm/recipes-security/optee/optee-os-ts.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/meta-arm/recipes-security/optee/optee-os-ts.inc b/meta-arm/recipes-security/optee/optee-os-ts.inc
> index 10a41755..73b8c14f 100644
> --- a/meta-arm/recipes-security/optee/optee-os-ts.inc
> +++ b/meta-arm/recipes-security/optee/optee-os-ts.inc
> @@ -51,4 +51,4 @@ DEPENDS:append  = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
>  SP_PATHS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
>                                          ' ${TS_BIN}/${SMM_GATEWAY_UUID}.stripped.elf', '', d)}"
>  
> -EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS=\'${SP_PATHS}\' ', d)}"
> +EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS="${SP_PATHS}" ', d)}"

Use plain += instead of :append.

Cheers,

-Mikko

Re: [meta-arm][PATCH 3/3] optee-os-ts: avoid using escape chars in EXTRA_OEMAKE

[Jon Mason]

On Thu, Oct 27, 2022 at 09:20:24AM +0300, Mikko Rapeli wrote:
> Hi,
> 
> On Wed, Oct 26, 2022 at 02:18:54PM +0000, Jose Quaresma wrote:
> > This currently can break the bitbake parsing
> > 
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> > ---
> >  meta-arm/recipes-security/optee/optee-os-ts.inc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/meta-arm/recipes-security/optee/optee-os-ts.inc b/meta-arm/recipes-security/optee/optee-os-ts.inc
> > index 10a41755..73b8c14f 100644
> > --- a/meta-arm/recipes-security/optee/optee-os-ts.inc
> > +++ b/meta-arm/recipes-security/optee/optee-os-ts.inc
> > @@ -51,4 +51,4 @@ DEPENDS:append  = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
> >  SP_PATHS:append = "${@bb.utils.contains('MACHINE_FEATURES', 'ts-smm-gateway', \
> >                                          ' ${TS_BIN}/${SMM_GATEWAY_UUID}.stripped.elf', '', d)}"
> >  
> > -EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS=\'${SP_PATHS}\' ', d)}"
> > +EXTRA_OEMAKE:append = "${@oe.utils.conditional('SP_PATHS', '', '', ' CFG_SECURE_PARTITION=y SP_PATHS="${SP_PATHS}" ', d)}"
> 
> Use plain += instead of :append.

He's using the existing style.  I think his change was the least
disruptive (than changing append to +=).

Thanks,
Jon

> 
> Cheers,
> 
> -Mikko
> 

Re: [meta-arm][PATCH 1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE

[Mikko Rapeli]

Hi,

On Wed, Oct 26, 2022 at 02:18:52PM +0000, Jose Quaresma wrote:
> When append to variable a space is required at the beginning.
> 
> Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> ---
>  meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> index acea7502..40423615 100644
> --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> @@ -7,7 +7,8 @@ DEPENDS:append = "\
>                   '' , \
>                   d)}"
>  
> -EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', \
> +EXTRA_OEMAKE:append = "\

There is no need to use :append here. += will do and will add the
space.

Using :append everywhere without machine or distro qualifiers is
an annoying antipattern which intermediate layers should not use.

Higher level product specific layers can't easily change the variables
which have been amended with :append.

Cheers,

-Mikko

Re: [meta-arm][PATCH 1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE

[Jose Quaresma]

[-- Attachment #1: Type: text/plain, Size: 1987 bytes --]

Hi Mikko,

Mikko Rapeli <mikko.rapeli@linaro.org> escreveu no dia quinta, 27/10/2022
à(s) 07:18:

> Hi,
>
> On Wed, Oct 26, 2022 at 02:18:52PM +0000, Jose Quaresma wrote:
> > When append to variable a space is required at the beginning.
> >
> > Signed-off-by: Jose Quaresma <jose.quaresma@foundries.io>
> > ---
> >  meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > index acea7502..40423615 100644
> > --- a/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > +++ b/meta-arm/recipes-security/optee-ftpm/optee-os_%.bbappend
> > @@ -7,7 +7,8 @@ DEPENDS:append = "\
> >                   '' , \
> >                   d)}"
> >
> > -EXTRA_OEMAKE:append = "${@bb.utils.contains('MACHINE_FEATURES', \
> > +EXTRA_OEMAKE:append = "\
>
> There is no need to use :append here. += will do and will add the
> space.


Right, I can use the += but in this case I follow this pattern to make the
change consistent
with what already existed in the file. I can change it, let's see the
maintainer's opinion.


>
> Using :append everywhere without machine or distro qualifiers is
> an annoying antipattern which intermediate layers should not use.
>
> Higher level product specific layers can't easily change the variables
> which have been amended with :append.
>

This will always be tricky to do, a lot due to the freedom of bitbake
syntax.
With :append usage we need to use the :remove because this needs to be done
at the end of the parsing.
While with += you can assign a new value for the variable during parsing
but you will lose all the others that had already possible been added with
+=

Great to talk with you again about bitbake stuff ;)

Jose


> Cheers,
>
> -Mikko
>


-- 
Best regards,

José Quaresma

[-- Attachment #2: Type: text/html, Size: 2960 bytes --]

Re: [PATCH 1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE

[Jon Mason]

On Wed, 26 Oct 2022 14:18:52 +0000, Jose Quaresma wrote:
> When append to variable a space is required at the beginning.

Applied, thanks!

[1/3] optee-ftpm/optee-os: add missing space in EXTRA_OEMAKE
      commit: 518294d518968a9f8895a17c5d6d0f1a95246ef3
[2/3] optee-ftpm/optee-os: rewite in a way to be less error prone
      (no commit info)
[3/3] optee-os-ts: avoid using escape chars in EXTRA_OEMAKE
      commit: 3080a94bde5a2f316960bc6a6c649f798a76e239

Best regards,
-- 
Jon Mason <jon.mason@arm.com>