[RFC] base: implement support for the PACKAGECONFIGEXTENDS variable

From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>

Some PACKAGECONFIG switches enable building of additional artifacts that
end up in sub-packages for a recipe. For example in foo.bb we have:

  PACKAGES =+ "foo-bar"
  PACKAGECONFIG[bar] = "--enable-bar,--disable-bar,libdep"
  FILES:foo-bar = "${bindir}/bar"

Where ${bindir}/bar is only installed if PACKAGECONFIG contains "bar".

In order to add foo-bar to the image we need to do:

  IMAGE_INSTALL:append = " foo-bar"
  PACKAGECONFIG:append:pn-foo = " bar"

This is redundant so with this change we need to add:

  PACKAGECONFIGEXTENDS:foo-bar = "bar"

to foo.bb and now adding "foo-bar" to IMAGE_INSTALL will automatically
append "bar" to foo.bb's PACKAGECONFIG.

Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
 documentation/ref-manual/variables.rst        | 15 +++++++++++
 .../packageconfigextends.bb                   | 27 +++++++++++++++++++
 .../packageconfigextends/pkgext-bar.sh        |  3 +++
 .../packageconfigextends/pkgext.sh            |  3 +++
 meta/classes-global/base.bbclass              | 13 +++++++++
 .../oeqa/selftest/cases/packageconfigdeps.py  | 11 ++++++++
 6 files changed, 72 insertions(+)
 create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends.bb
 create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext-bar.sh
 create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext.sh
 create mode 100644 meta/lib/oeqa/selftest/cases/packageconfigdeps.py

On Wed, Apr 5, 2023 at 3:24 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
>
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Some PACKAGECONFIG switches enable building of additional artifacts that
> end up in sub-packages for a recipe. For example in foo.bb we have:
>
>   PACKAGES =+ "foo-bar"
>   PACKAGECONFIG[bar] = "--enable-bar,--disable-bar,libdep"
>   FILES:foo-bar = "${bindir}/bar"
>
> Where ${bindir}/bar is only installed if PACKAGECONFIG contains "bar".
>
> In order to add foo-bar to the image we need to do:
>
>   IMAGE_INSTALL:append = " foo-bar"
>   PACKAGECONFIG:append:pn-foo = " bar"
>
> This is redundant so with this change we need to add:
>
>   PACKAGECONFIGEXTENDS:foo-bar = "bar"

FWIW, I prefer the explicit IMAGE_INSTALL and PACKAGECONFIG, as it
keeps the manipulations visible.

The level of coordination that this is doing is more like a distro
feature, or somewhere around an image feature .. and image features
are manipulated in image recipes, as the contents aren't global.

So regardless of whether or not the behind the scenes work is desired,
from what I'm seeing, this breaks the namespace and separation between
a recipe variable and an image recipe variable. Depending on what the
target of a build is (the recipe versus an image) both won't even be
valid in a given run, so the variable tweaks won't work or will create
races/other issues.

Then again, I may be misreading the patch and not understanding the
reasoning behind the change.

Bruce

>
> to foo.bb and now adding "foo-bar" to IMAGE_INSTALL will automatically
> append "bar" to foo.bb's PACKAGECONFIG.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
>  documentation/ref-manual/variables.rst        | 15 +++++++++++
>  .../packageconfigextends.bb                   | 27 +++++++++++++++++++
>  .../packageconfigextends/pkgext-bar.sh        |  3 +++
>  .../packageconfigextends/pkgext.sh            |  3 +++
>  meta/classes-global/base.bbclass              | 13 +++++++++
>  .../oeqa/selftest/cases/packageconfigdeps.py  | 11 ++++++++
>  6 files changed, 72 insertions(+)
>  create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends.bb
>  create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext-bar.sh
>  create mode 100644 meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext.sh
>  create mode 100644 meta/lib/oeqa/selftest/cases/packageconfigdeps.py
>
> diff --git a/documentation/ref-manual/variables.rst b/documentation/ref-manual/variables.rst
> index c787a17937..dcb5cdccd9 100644
> --- a/documentation/ref-manual/variables.rst
> +++ b/documentation/ref-manual/variables.rst
> @@ -6016,6 +6016,21 @@ system and gives an overview of their function and contents.
>
>              PACKAGECONFIG:append:pn-recipename = " f4"
>
> +   :term:`PACKAGECONFIGEXTENDS`
> +      List of :term:`PACKAGECONFIG` flags automatically enabled for a package
> +      present in :term:`IMAGE_INSTALL`. The :term:`PACKAGES` variable lists
> +      the packages generated by a recipe.
> +
> +      Some :term:`PACKAGECONFIG` switches enable building additional artifacts
> +      that are part of sub-packages for a recipe. In order to avoid having to
> +      explictly add :term:`PACKAGECONFIG` flags AND include the additional
> +      packages in :term:`IMAGE_INSTALL`, the user can just do::
> +
> +         PACKAGECONFIGEXTENDS:packagename = " foo"
> +
> +      in which case pulling in `packagename` from the recipe will automatically
> +      add `foo` to this recipe's :term:`PACKAGECONFIG`.
> +
>     :term:`PACKAGECONFIG_CONFARGS`
>        A space-separated list of configuration options generated from the
>        :term:`PACKAGECONFIG` setting.
> diff --git a/meta-selftest/recipes-test/packageconfigextends/packageconfigextends.bb b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends.bb
> new file mode 100644
> index 0000000000..189b9779b3
> --- /dev/null
> +++ b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends.bb
> @@ -0,0 +1,27 @@
> +SUMMARY = "Test case making sure that sub-packages pull in correct PACKAGECONFIG switches."
> +LICENSE = "MIT"
> +LIC_FILES_CHKSUM = "file://${COREBASE}/meta/COPYING.MIT;md5=3da9cfbcb788c80a0384361b4de20420"
> +
> +SRC_URI = " \
> +    file://pkgext.sh \
> +    file://pkgext-bar.sh \
> +"
> +
> +PACKAGES =+ "${PN}-bar"
> +FILES:${PN}-bar += "${bindir}/pkgext-bar"
> +
> +PACKAGECONFIG = ""
> +PACKAGECONFIG[bar] = ""
> +
> +PACKAGECONFIGEXTENDS:${PN}-bar = "bar"
> +
> +do_compile[noexec] = "1"
> +
> +S = "${WORKDIR}"
> +
> +do_install() {
> +    install -D -m 0755 ${S}/pkgext.sh ${D}${bindir}/pkgext
> +    if [ "${@bb.utils.contains("PACKAGECONFIG", "bar", "1", "", d)}" = "1" ]; then
> +        install -D -m 0755 ${S}/pkgext-bar.sh ${D}${bindir}/pkgext-bar
> +    fi
> +}
> diff --git a/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext-bar.sh b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext-bar.sh
> new file mode 100644
> index 0000000000..1b208b154f
> --- /dev/null
> +++ b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext-bar.sh
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +
> +echo "BAR"
> diff --git a/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext.sh b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext.sh
> new file mode 100644
> index 0000000000..c5beb15283
> --- /dev/null
> +++ b/meta-selftest/recipes-test/packageconfigextends/packageconfigextends/pkgext.sh
> @@ -0,0 +1,3 @@
> +#!/bin/sh
> +
> +echo "FOO"
> diff --git a/meta/classes-global/base.bbclass b/meta/classes-global/base.bbclass
> index b6e339ed9c..7ee43d85e0 100644
> --- a/meta/classes-global/base.bbclass
> +++ b/meta/classes-global/base.bbclass
> @@ -429,6 +429,19 @@ python () {
>      # PACKAGECONFIG[foo] = "--enable-foo,--disable-foo,foo_depends,foo_runtime_depends,foo_runtime_recommends,foo_conflict_packageconfig"
>      pkgconfigflags = d.getVarFlags("PACKAGECONFIG") or {}
>      if pkgconfigflags:
> +        # Preprocess PACKAGECONFIG for recipe sub-packages that are in
> +        # IMAGE_INSTALL but for which the required PACKAGECONFIG options
> +        # were not selected.
> +        img_inst = (d.getVar("IMAGE_INSTALL") or "").split()
> +        recipe_pkgs = d.getVar("PACKAGES").split()
> +        pkgconfig = (d.getVar("PACKAGECONFIG") or "").split()
> +        for pkg in recipe_pkgs:
> +            if pkg in img_inst:
> +                deps = (d.getVar("PACKAGECONFIGEXTENDS:{}".format(pkg)) or "").split()
> +                for dep in deps:
> +                    if dep not in pkgconfig:
> +                        d.appendVar("PACKAGECONFIG", " " + dep)
> +
>          pkgconfig = (d.getVar('PACKAGECONFIG') or "").split()
>          pn = d.getVar("PN")
>
> diff --git a/meta/lib/oeqa/selftest/cases/packageconfigdeps.py b/meta/lib/oeqa/selftest/cases/packageconfigdeps.py
> new file mode 100644
> index 0000000000..69e23c1083
> --- /dev/null
> +++ b/meta/lib/oeqa/selftest/cases/packageconfigdeps.py
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: MIT
> +#
> +# Copyright (C) 2023: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> +
> +from oeqa.selftest.case import OESelftestTestCase
> +from oeqa.utils.commands import bitbake
> +
> +class PackageconfigDepsTest(OESelftestTestCase):
> +    def test_pkgcfgdeps(self):
> +        self.write_config('IMAGE_INSTALL:append = " packageconfigextends-bar"')
> +        bitbake('core-image-minimal')
> --
> 2.37.2

On Wed, 2023-04-05 at 16:20 -0400, Bruce Ashfield wrote:
> On Wed, Apr 5, 2023 at 3:24 PM Bartosz Golaszewski <brgl@bgdev.pl> wrote:
> > 
> > From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> > 
> > Some PACKAGECONFIG switches enable building of additional artifacts that
> > end up in sub-packages for a recipe. For example in foo.bb we have:
> > 
> >   PACKAGES =+ "foo-bar"
> >   PACKAGECONFIG[bar] = "--enable-bar,--disable-bar,libdep"
> >   FILES:foo-bar = "${bindir}/bar"
> > 
> > Where ${bindir}/bar is only installed if PACKAGECONFIG contains "bar".
> > 
> > In order to add foo-bar to the image we need to do:
> > 
> >   IMAGE_INSTALL:append = " foo-bar"
> >   PACKAGECONFIG:append:pn-foo = " bar"
> > 
> > This is redundant so with this change we need to add:
> > 
> >   PACKAGECONFIGEXTENDS:foo-bar = "bar"
> 
> FWIW, I prefer the explicit IMAGE_INSTALL and PACKAGECONFIG, as it
> keeps the manipulations visible.
> 
> The level of coordination that this is doing is more like a distro
> feature, or somewhere around an image feature .. and image features
> are manipulated in image recipes, as the contents aren't global.
> 
> So regardless of whether or not the behind the scenes work is desired,
> from what I'm seeing, this breaks the namespace and separation between
> a recipe variable and an image recipe variable. Depending on what the
> target of a build is (the recipe versus an image) both won't even be
> valid in a given run, so the variable tweaks won't work or will create
> races/other issues.

I'd agree with Bruce.

PACKAGECONFIG is controlling which packages get built and is a recipe
or distro configuration change.

Images are a separate thing which may or may not include a given
package. You may build multiple images in a single build which may or
may not include a given package.

Mixing up these two concepts is asking for trouble and will just end up
causing confusion.

Cheers,

Richard