| Message ID | 20250210154958.1119250-1-ecordonnier@snap.com |
|---|---|
| State | New |
| Headers | show |
| Series | gdb-cross: remove duplicated PACKAGECONFIG | expand |
On Mon, 2025-02-10 at 16:49 +0100, Etienne Cordonnier via lists.openembedded.org wrote: > From: Etienne Cordonnier <ecordonnier@snap.com> > > Duplicating the PACKAGECONFIG variable in gdb-cross and gdb-common makes > it easy to forget updating one definition without updating the other. > However at the moment there is no need for two different definitions. > > xz is also useful for gdb-cross when minidebuginfo is enabled, so it also needs > to be in the definition of PACKAGECONFIG in gdb-cross.inc > > Let's remove the definition from gdb-cross.inc to make maintenance easier. > This enables xz for gdb-cross as side-effect. > > Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com> > --- > meta/recipes-devtools/gdb/gdb-cross.inc | 1 - > 1 file changed, 1 deletion(-) At the very least this commit message needs to be a bit clearer as there is a change of behaviour here. To be clear, this change adds: ${@bb.utils.contains('DISTRO_FEATURES', 'minidebuginfo', 'xz', '', d)} to the default PACKAGECONFIG for gdb-cross. You can't tell that from the patch. I'd also mention that there is still a different definition for gdb- cross-canadian and if we do this, they should all probably match. Whether we should do this, I'm unsure. It might be worth just adding xz unconditionally at this point by default to be honest, it isn't a huge dependency. gdb is a debugging tool, not for a minimal system. Cheers, Richard
On Mon, Feb 10, 2025 at 8:21 AM Richard Purdie via lists.openembedded.org <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > > On Mon, 2025-02-10 at 16:49 +0100, Etienne Cordonnier via lists.openembedded.org wrote: > > From: Etienne Cordonnier <ecordonnier@snap.com> > > > > Duplicating the PACKAGECONFIG variable in gdb-cross and gdb-common makes > > it easy to forget updating one definition without updating the other. > > However at the moment there is no need for two different definitions. > > > > xz is also useful for gdb-cross when minidebuginfo is enabled, so it also needs > > to be in the definition of PACKAGECONFIG in gdb-cross.inc > > > > Let's remove the definition from gdb-cross.inc to make maintenance easier. > > This enables xz for gdb-cross as side-effect. > > > > Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com> > > --- > > meta/recipes-devtools/gdb/gdb-cross.inc | 1 - > > 1 file changed, 1 deletion(-) > > At the very least this commit message needs to be a bit clearer as > there is a change of behaviour here. > > To be clear, this change adds: > > ${@bb.utils.contains('DISTRO_FEATURES', 'minidebuginfo', 'xz', '', d)} > > to the default PACKAGECONFIG for gdb-cross. You can't tell that from > the patch. > > I'd also mention that there is still a different definition for gdb- > cross-canadian and if we do this, they should all probably match. > > Whether we should do this, I'm unsure. It might be worth just adding xz > unconditionally at this point by default to be honest, it isn't a huge > dependency. gdb is a debugging tool, not for a minimal system. target gdb where the size and dependencies might matter, for cross and cross-canadian IIRC it won't matter as much from size perspective. we can let the defaults be controlled from bb itself or from inc and then add/remove as needed in bb files. Status quo is a mix of both so we should do something. Perhaps this patch with updated commit log is a good one > > Cheers, > > Richard > > -=-=-=-=-=-=-=-=-=-=-=- > Links: You receive all messages sent to this group. > View/Reply Online (#211104): https://lists.openembedded.org/g/openembedded-core/message/211104 > Mute This Topic: https://lists.openembedded.org/mt/111104210/1997914 > Group Owner: openembedded-core+owner@lists.openembedded.org > Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [raj.khem@gmail.com] > -=-=-=-=-=-=-=-=-=-=-=- >
> To be clear, this change adds: > > ${@bb.utils.contains('DISTRO_FEATURES', 'minidebuginfo', 'xz', '', d)} > > to the default PACKAGECONFIG for gdb-cross. You can't tell that from > the patch. That's what I was referring to in the commit message with "This enables xz for gdb-cross as side-effect.", but I've reworded it now so that it is hopefully clearer. I've sent a new version of the patch as "[PATCH v2] gdb: enable xz in gdb-cross and gdb-cross-canadian" Étienne On Mon, Feb 10, 2025 at 6:31 PM Khem Raj <raj.khem@gmail.com> wrote: > On Mon, Feb 10, 2025 at 8:21 AM Richard Purdie via > lists.openembedded.org > <richard.purdie=linuxfoundation.org@lists.openembedded.org> wrote: > > > > On Mon, 2025-02-10 at 16:49 +0100, Etienne Cordonnier via > lists.openembedded.org wrote: > > > From: Etienne Cordonnier <ecordonnier@snap.com> > > > > > > Duplicating the PACKAGECONFIG variable in gdb-cross and gdb-common > makes > > > it easy to forget updating one definition without updating the other. > > > However at the moment there is no need for two different definitions. > > > > > > xz is also useful for gdb-cross when minidebuginfo is enabled, so it > also needs > > > to be in the definition of PACKAGECONFIG in gdb-cross.inc > > > > > > Let's remove the definition from gdb-cross.inc to make maintenance > easier. > > > This enables xz for gdb-cross as side-effect. > > > > > > Signed-off-by: Etienne Cordonnier <ecordonnier@snap.com> > > > --- > > > meta/recipes-devtools/gdb/gdb-cross.inc | 1 - > > > 1 file changed, 1 deletion(-) > > > > At the very least this commit message needs to be a bit clearer as > > there is a change of behaviour here. > > > > To be clear, this change adds: > > > > ${@bb.utils.contains('DISTRO_FEATURES', 'minidebuginfo', 'xz', '', d)} > > > > to the default PACKAGECONFIG for gdb-cross. You can't tell that from > > the patch. > > > > I'd also mention that there is still a different definition for gdb- > > cross-canadian and if we do this, they should all probably match. > > > > Whether we should do this, I'm unsure. It might be worth just adding xz > > unconditionally at this point by default to be honest, it isn't a huge > > dependency. gdb is a debugging tool, not for a minimal system. > > target gdb where the size and dependencies might matter, for cross and > cross-canadian > IIRC it won't matter as much from size perspective. we can let the > defaults be controlled from > bb itself or from inc and then add/remove as needed in bb files. > Status quo is a mix of both > so we should do something. Perhaps this patch with updated commit log > is a good one > > > > > Cheers, > > > > Richard > > > > -=-=-=-=-=-=-=-=-=-=-=- > > Links: You receive all messages sent to this group. > > View/Reply Online (#211104): > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.openembedded.org_g_openembedded-2Dcore_message_211104&d=DwIFaQ&c=ncDTmphkJTvjIDPh0hpF_4vCHvabgGkICC2epckfdiw&r=AhkbNonVuMIGRfPx_Qj9TsRih1DULJTKUkSGa66m67E&m=B9ScRVJzxS9xTwss3tO6LOaEc25WeGEM_0kgqZqT1CxQ8-mnKsbdAMJWNa1exXOj&s=6q0oPee_RTrM7WSHcfvWsw3iIjGaAulZ0SKtycMXofQ&e= > > Mute This Topic: > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.openembedded.org_mt_111104210_1997914&d=DwIFaQ&c=ncDTmphkJTvjIDPh0hpF_4vCHvabgGkICC2epckfdiw&r=AhkbNonVuMIGRfPx_Qj9TsRih1DULJTKUkSGa66m67E&m=B9ScRVJzxS9xTwss3tO6LOaEc25WeGEM_0kgqZqT1CxQ8-mnKsbdAMJWNa1exXOj&s=yfjmYBaSzY87H0_XcTS-BwM3HH6O9x6XrowE4qs0AiE&e= > > Group Owner: openembedded-core+owner@lists.openembedded.org > > Unsubscribe: > https://urldefense.proofpoint.com/v2/url?u=https-3A__lists.openembedded.org_g_openembedded-2Dcore_unsub&d=DwIFaQ&c=ncDTmphkJTvjIDPh0hpF_4vCHvabgGkICC2epckfdiw&r=AhkbNonVuMIGRfPx_Qj9TsRih1DULJTKUkSGa66m67E&m=B9ScRVJzxS9xTwss3tO6LOaEc25WeGEM_0kgqZqT1CxQ8-mnKsbdAMJWNa1exXOj&s=LzjhLaGMsXHO9k8oG2EZBslP7bQuVNQresguoGJre7E&e= > [raj.khem@gmail.com] > > -=-=-=-=-=-=-=-=-=-=-=- > > >
diff --git a/meta/recipes-devtools/gdb/gdb-cross.inc b/meta/recipes-devtools/gdb/gdb-cross.inc index 399f4bba97a..4f16deabe3e 100644 --- a/meta/recipes-devtools/gdb/gdb-cross.inc +++ b/meta/recipes-devtools/gdb/gdb-cross.inc @@ -5,7 +5,6 @@ DEPENDS = "expat-native gmp-native mpfr-native ncurses-native flex-native bison- inherit python3native pkgconfig # Overrides PACKAGECONFIG variables in gdb-common.inc -PACKAGECONFIG ??= "python readline ${@bb.utils.filter('DISTRO_FEATURES', 'debuginfod', d)}" PACKAGECONFIG[python] = "--with-python=${PYTHON},--without-python,python3-native" PACKAGECONFIG[readline] = "--with-system-readline,--without-system-readline,readline-native" PACKAGECONFIG[debuginfod] = "--with-debuginfod, --without-debuginfod, elfutils-native"