Message ID | 20230403110646.2555294-1-kubiznak@2n.com |
---|---|
State | New |
Headers | show |
Series | [v5,1/6] gobject-introspection: reduce dependencies | expand |
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
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
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
> 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
> 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
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 --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
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(-)