diff mbox series

[v5,1/6] gobject-introspection: reduce dependencies

Message ID 20230403110646.2555294-1-kubiznak@2n.com
State New
Headers show
Series [v5,1/6] gobject-introspection: reduce dependencies | expand

Commit Message

Petr Kubizňák - 2N April 3, 2023, 11:06 a.m. UTC
When GI_DATA_ENABLED is 'False' (e.g. because
'gobject-introspection-data' is not in DISTRO_FEATURES),
gobject-introspection, gobject-introspection-native and qemu-native
should not be added to DEPENDS. This is to reduce dependency chain
when g-i is disabled.

Signed-off-by: Petr Kubizňák <kubiznak@2n.com>
---
 meta/classes-recipe/gobject-introspection.bbclass | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Martin Jansa May 15, 2023, 11:40 a.m. UTC | #1
On Mon, Apr 3, 2023 at 1:08 PM Petr Kubizňák <kubiznak@2n.com> wrote:

> When GI_DATA_ENABLED is 'False' (e.g. because
> 'gobject-introspection-data' is not in DISTRO_FEATURES),
> gobject-introspection, gobject-introspection-native and qemu-native
> should not be added to DEPENDS. This is to reduce dependency chain
> when g-i is disabled.
>
> Signed-off-by: Petr Kubizňák <kubiznak@2n.com>
> ---
>  meta/classes-recipe/gobject-introspection.bbclass | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/meta/classes-recipe/gobject-introspection.bbclass
> b/meta/classes-recipe/gobject-introspection.bbclass
> index 0c7b7d200a..98edb93761 100644
> --- a/meta/classes-recipe/gobject-introspection.bbclass
> +++ b/meta/classes-recipe/gobject-introspection.bbclass
> @@ -35,7 +35,7 @@ EXTRA_OEMESON:prepend:class-nativesdk = "${@['',
> '${GIRMESONBUILD}'][d.getVar('G
>
>  # Generating introspection data depends on a combination of native and
> target
>  # introspection tools, and qemu to run the target tools.
> -DEPENDS:append:class-target = " gobject-introspection
> gobject-introspection-native qemu-native"
> +DEPENDS:append:class-target = " ${@bb.utils.contains('GI_DATA_ENABLED',
> 'True', 'gobject-introspection gobject-introspection-native qemu-native',
> '', d)}"
>
>  # Even though introspection is disabled on -native, gobject-introspection
> package is still
>  # needed for m4 macros.
> @@ -46,10 +46,12 @@ DEPENDS:append:class-nativesdk = "
> gobject-introspection-native"
>  export XDG_DATA_DIRS = "${STAGING_DATADIR}:${STAGING_LIBDIR}"
>
>  do_configure:prepend:class-target () {
> -    # introspection.m4 pre-packaged with upstream tarballs does not yet
> -    # have our fixes
> -    mkdir -p ${S}/m4
> -    cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4 ${S}/m4
> +    if [ "${@bb.utils.contains('GI_DATA_ENABLED', 'True', '1', '0', d)}"
> = "1" ] ; then
> +        # introspection.m4 pre-packaged with upstream tarballs does not
> yet
> +        # have our fixes
> +        mkdir -p ${S}/m4
> +        cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4
> ${S}/m4
> +    fi
>

Was this extra .m4 file causing issues in builds without
gobject-introspection-data in DISTRO_FEATURES?

I've noticed some maybe unexpected side-effects from this change, see:
libblockdev:
https://lists.openembedded.org/g/openembedded-devel/message/102599
glade: https://lists.openembedded.org/g/openembedded-devel/message/102601
Ross Burton May 15, 2023, 1:58 p.m. UTC | #2
On 15 May 2023, at 12:40, Martin Jansa via lists.openembedded.org <Martin.Jansa=gmail.com@lists.openembedded.org> wrote:
>  do_configure:prepend:class-target () {
> -    # introspection.m4 pre-packaged with upstream tarballs does not yet
> -    # have our fixes
> -    mkdir -p ${S}/m4
> -    cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4 ${S}/m4
> +    if [ "${@bb.utils.contains('GI_DATA_ENABLED', 'True', '1', '0', d)}" = "1" ] ; then
> +        # introspection.m4 pre-packaged with upstream tarballs does not yet
> +        # have our fixes
> +        mkdir -p ${S}/m4
> +        cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4 ${S}/m4
> +    fi
> 
> Was this extra .m4 file causing issues in builds without gobject-introspection-data in DISTRO_FEATURES?
> 
> I've noticed some maybe unexpected side-effects from this change, see:
> libblockdev: https://lists.openembedded.org/g/openembedded-devel/message/102599
> glade: https://lists.openembedded.org/g/openembedded-devel/message/102601

That would be from:

-DEPENDS:append:class-target = " gobject-introspection gobject-introspection-native qemu-native"
+DEPENDS:append:class-target = " ${@bb.utils.contains('GI_DATA_ENABLED', 'True', 'gobject-introspection gobject-introspection-native qemu-native', '', d)}”

As the comment below explains:

# Even though introspection is disabled on -native, gobject-introspection package is still
# needed for m4 macros.

g-i-native will *always* be needed as a build dependency, because packages built from git may not have any m4 macros in.

Ross
Petr Kubizňák - 2N May 15, 2023, 2:49 p.m. UTC | #3
Hi Martin and Ross,

My original intention was to only condition the dependency on gobject-introspection but the reviewers were convinced the -native packages should be handled the same, see https://lists.openembedded.org/g/openembedded-core/message/175475 . That triggered a set of patches like https://lists.openembedded.org/g/openembedded-core/message/179620 . The result is passing the builds but honestly I'm not surprised about side-effects, sorry for that.

The key question is whether the problematic packages have intrinsic dependency on g-i-native, which can (and should) be fixed by adding the package in DEPENDS directly, or g-i-native is always needed as Ross says, which would imply g-i-native should not be conditioned by GI_DATA_ENABLED in the class.

I'm not an expert on g-i so I didn't have strong arguments but feel free to convince Alex that g-i-native should be always in DEPENDS.

Petr
Ross Burton May 15, 2023, 3:40 p.m. UTC | #4
> On 15 May 2023, at 14:58, Ross Burton via lists.openembedded.org <ross.burton=arm.com@lists.openembedded.org> wrote:
> g-i-native will *always* be needed as a build dependency, because packages built from git may not have any m4 macros in.

I have a patch, just giving it a soak test.

Ross
Ross Burton May 15, 2023, 4:43 p.m. UTC | #5
> On 15 May 2023, at 15:49, Petr Kubizňák - 2N <kubiznak@2n.com> wrote:
> The key question is whether the problematic packages have intrinsic dependency on g-i-native, which can (and should) be fixed by adding the package in DEPENDS directly, or g-i-native is always needed as Ross says, which would imply g-i-native should not be conditioned by GI_DATA_ENABLED in the class.
> 
> I'm not an expert on g-i so I didn't have strong arguments but feel free to convince Alex that g-i-native should be always in DEPENDS.

If we grab a package from git that has a empty m4/ and it has optional support for G-I then it needs a dependency on gobject-introspection-native so that it has introspection.m4 available.

We already inherit gobject-introspection.bbclass, so the dependency can be there.  I don’t see why g-i.bbclass would only pull in fundamental build dependencies when it’s enabled.

Ross
Alexander Kanavin May 16, 2023, 8:21 a.m. UTC | #6
I don't have a strong opinion here. Petr wanted to trim dependencies
down to bare minimum, but if we need to strike a more reasonable
balance, that's fine.

Alex

On Mon, 15 May 2023 at 18:43, Ross Burton <Ross.Burton@arm.com> wrote:
>
>
>
> > On 15 May 2023, at 15:49, Petr Kubizňák - 2N <kubiznak@2n.com> wrote:
> > The key question is whether the problematic packages have intrinsic dependency on g-i-native, which can (and should) be fixed by adding the package in DEPENDS directly, or g-i-native is always needed as Ross says, which would imply g-i-native should not be conditioned by GI_DATA_ENABLED in the class.
> >
> > I'm not an expert on g-i so I didn't have strong arguments but feel free to convince Alex that g-i-native should be always in DEPENDS.
>
> If we grab a package from git that has a empty m4/ and it has optional support for G-I then it needs a dependency on gobject-introspection-native so that it has introspection.m4 available.
>
> We already inherit gobject-introspection.bbclass, so the dependency can be there.  I don’t see why g-i.bbclass would only pull in fundamental build dependencies when it’s enabled.
>
> Ross
diff mbox series

Patch

diff --git a/meta/classes-recipe/gobject-introspection.bbclass b/meta/classes-recipe/gobject-introspection.bbclass
index 0c7b7d200a..98edb93761 100644
--- a/meta/classes-recipe/gobject-introspection.bbclass
+++ b/meta/classes-recipe/gobject-introspection.bbclass
@@ -35,7 +35,7 @@  EXTRA_OEMESON:prepend:class-nativesdk = "${@['', '${GIRMESONBUILD}'][d.getVar('G
 
 # Generating introspection data depends on a combination of native and target
 # introspection tools, and qemu to run the target tools.
-DEPENDS:append:class-target = " gobject-introspection gobject-introspection-native qemu-native"
+DEPENDS:append:class-target = " ${@bb.utils.contains('GI_DATA_ENABLED', 'True', 'gobject-introspection gobject-introspection-native qemu-native', '', d)}"
 
 # Even though introspection is disabled on -native, gobject-introspection package is still
 # needed for m4 macros.
@@ -46,10 +46,12 @@  DEPENDS:append:class-nativesdk = " gobject-introspection-native"
 export XDG_DATA_DIRS = "${STAGING_DATADIR}:${STAGING_LIBDIR}"
 
 do_configure:prepend:class-target () {
-    # introspection.m4 pre-packaged with upstream tarballs does not yet
-    # have our fixes
-    mkdir -p ${S}/m4
-    cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4 ${S}/m4
+    if [ "${@bb.utils.contains('GI_DATA_ENABLED', 'True', '1', '0', d)}" = "1" ] ; then
+        # introspection.m4 pre-packaged with upstream tarballs does not yet
+        # have our fixes
+        mkdir -p ${S}/m4
+        cp ${STAGING_DIR_TARGET}/${datadir}/aclocal/introspection.m4 ${S}/m4
+    fi
 }
 
 # .typelib files are needed at runtime and so they go to the main package (so