diff mbox series

[1/3] ovmf: simplify PACKAGECONFIG's default value

Message ID 20260511092935.2631121-2-joaomarcos.costa@bootlin.com
State Under Review
Headers show
Series ovmf: add some rework to the recipe | expand

Commit Message

Joao Marcos Costa May 11, 2026, 9:29 a.m. UTC
The two append operations call bb.utils.contains, while a single
contains_any() does the trick in a cleaner way.

Regarding the default value, if tpm is not enabled, PACKAGECONFIG ends
up with a couple empty spaces, and this can be avoided by redefining the
default value to the result of contains_any().

Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
---
 meta/recipes-core/ovmf/ovmf_git.bb | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

Comments

Quentin Schulz May 11, 2026, 1:19 p.m. UTC | #1
Hi João,

On 5/11/26 11:29 AM, João Marcos Costa wrote:
> The two append operations call bb.utils.contains, while a single
> contains_any() does the trick in a cleaner way.
> 
> Regarding the default value, if tpm is not enabled, PACKAGECONFIG ends
> up with a couple empty spaces, and this can be avoided by redefining the
> default value to the result of contains_any().
> 
> Signed-off-by: João Marcos Costa <joaomarcos.costa@bootlin.com>
> ---
>   meta/recipes-core/ovmf/ovmf_git.bb | 4 +---
>   1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
> index 958f42fc10..779e9f8196 100644
> --- a/meta/recipes-core/ovmf/ovmf_git.bb
> +++ b/meta/recipes-core/ovmf/ovmf_git.bb
> @@ -9,9 +9,7 @@ LIC_FILES_CHKSUM = "file://OvmfPkg/License.txt;md5=06357ddc23f46577c2aeaeaf7b776
>   # Enabling Secure Boot adds a dependency on OpenSSL and implies
>   # compiling OVMF twice, so it is disabled by default. Distros
>   # may change that default.
> -PACKAGECONFIG ??= ""
> -PACKAGECONFIG += "${@bb.utils.filter('MACHINE_FEATURES', 'tpm', d)}"
> -PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm2', 'tpm', '', d)}"
> +PACKAGECONFIG ?= "${@bb.utils.contains_any('MACHINE_FEATURES', 'tpm tpm2', 'tpm', '', d)}"

Just a heads up that this is *not* equivalent.

The use of ??= meant that any ?= (and ??=) parsed anywhere before that 
line is meant that it would set the default values, and then we would 
append to those default values tpm.

Now by merging those into a single ?=, a previous ?= will override this 
new ?= operator, meaning tpm won't make  it to the PACKAGECONFIG even if 
the MACHINE_FEATURES is set to tpm/tpm2. Also, an earlier ??= won't do 
anything anymore.

Whether that's an actual issue, I don't know, but you may break users.

Merging the two += into a single one would truly be equivalent.

Cheers,
Quentin
diff mbox series

Patch

diff --git a/meta/recipes-core/ovmf/ovmf_git.bb b/meta/recipes-core/ovmf/ovmf_git.bb
index 958f42fc10..779e9f8196 100644
--- a/meta/recipes-core/ovmf/ovmf_git.bb
+++ b/meta/recipes-core/ovmf/ovmf_git.bb
@@ -9,9 +9,7 @@  LIC_FILES_CHKSUM = "file://OvmfPkg/License.txt;md5=06357ddc23f46577c2aeaeaf7b776
 # Enabling Secure Boot adds a dependency on OpenSSL and implies
 # compiling OVMF twice, so it is disabled by default. Distros
 # may change that default.
-PACKAGECONFIG ??= ""
-PACKAGECONFIG += "${@bb.utils.filter('MACHINE_FEATURES', 'tpm', d)}"
-PACKAGECONFIG += "${@bb.utils.contains('MACHINE_FEATURES', 'tpm2', 'tpm', '', d)}"
+PACKAGECONFIG ?= "${@bb.utils.contains_any('MACHINE_FEATURES', 'tpm tpm2', 'tpm', '', d)}"
 PACKAGECONFIG[debug] = ",,,"
 PACKAGECONFIG[secureboot] = ",,,"
 PACKAGECONFIG[tpm] = "-D TPM_ENABLE=TRUE,-D TPM_ENABLE=FALSE,,"