[PROPOSAL] Package feature switches, redux.

[PROPOSAL] Package feature switches, redux.

[Chris Elston]

Since responses to my previous mail were generally positive, I've
reworked the package feature switches so that the interface is as RP
suggested.

In the recipe for foo you would have a set of features defined like
this:

PACKAGE_CONFIG[bar] = "--enable-bar, --disable-bar, libbar"
PACKAGE_CONFIG[baz] = "--enable-baz, --disable-baz, libbaz"

The default set of features for the package would be defined with:

PACKAGE_FEATURES ?= "bar baz" 

Perhaps this set of features could go into a metadata field in the .ipk
- would this be helpful for feed users?

The package features can then be tailored in a config/layer with
something like:

PACKAGE_FEATURES_pn-foo = "baz pop"

If a layer requests a feature not supported by the recipe, you get a
warning (should help distro maintainers detect bitrot in their layer):

WARNING: foo: Unknown feature 'pop' requested

The patch below uses gstreamer as an example of something which would
benefit from this:

diff --git a/meta/classes/base.bbclass b/meta/classes/base.bbclass
index 8f4ef1e..ee8e914 100644
--- a/meta/classes/base.bbclass
+++ b/meta/classes/base.bbclass
@@ -396,6 +396,30 @@ python () {
             break
 
     bb.data.setVar('MULTIMACH_ARCH', multiarch, d)
+
+    features = bb.data.getVar('PACKAGE_FEATURES', d, True)
+    if features:
+        config = list(bb.data.getVarFlags('PACKAGE_CONFIG', d) or {})
+        oeconf = [ (bb.data.getVar('EXTRA_OECONF', d, True) or '') ]
+        depends = [ (bb.data.getVar('DEPENDS', d, True) or '') ]
+        for feature in features.split():
+            if feature in config:
+                settings = bb.data.getVarFlag('PACKAGE_CONFIG', feature, d).split(',')
+                oeconf.append(settings[0])
+                depends.append(settings[2])
+                config.remove(feature) 
+            else:
+                bb.warn("%s: Unknown feature '%s' requested" % (pn, feature))
+
+        for feature in config:
+            settings = bb.data.getVarFlag('PACKAGE_CONFIG', feature, d).split(',')
+            oeconf.append(settings[1])
+
+        if len(oeconf) > 1:
+            bb.data.setVar('EXTRA_OECONF', ' '.join(oeconf), d)
+
+        if len(depends) > 1:
+            bb.data.setVar('DEPENDS', ' '.join(depends), d)
 }
 
 def check_gcc3(data):
diff --git a/meta/recipes-multimedia/gstreamer/gst-plugins-base_0.10.32.bb b/meta/recipes-multimedia/gstreamer/gst-plugins-base_0.10.32.bb
index f81011f..70f0171 100644
--- a/meta/recipes-multimedia/gstreamer/gst-plugins-base_0.10.32.bb
+++ b/meta/recipes-multimedia/gstreamer/gst-plugins-base_0.10.32.bb
@@ -6,10 +6,16 @@ LIC_FILES_CHKSUM = "file://COPYING;md5=0636e73ff0215e8d672dc4c32c317bb3 \
                     file://COPYING.LIB;md5=55ca817ccb7d5b5b66355690e9abc605 \
                     file://gst/ffmpegcolorspace/utils.c;beginline=1;endline=20;md5=9c83a200b8e597b26ca29df20fc6ecd0"
 
-DEPENDS += "virtual/libx11 alsa-lib freetype gnome-vfs liboil libogg libvorbis libxv libtheora avahi util-linux tremor"
+DEPENDS += "virtual/libx11 alsa-lib freetype gnome-vfs liboil libxv avahi util-linux tremor"
 RDEPENDS_${PN} += "gnome-vfs-plugin-file gnome-vfs-plugin-http gnome-vfs-plugin-ftp \
              gnome-vfs-plugin-sftp"
 
+PACKAGE_CONFIG[ogg] = "--enable-ogg, --disable-ogg, libogg"
+PACKAGE_CONFIG[vorbis] = "--enable-vorbis, --disable-vorbis, libvorbis"
+PACKAGE_CONFIG[theora] = "--enable-theora, --disable-theora, libtheora"
+
+PACKAGE_FEATURES ?= "ogg vorbis theora" 
+
 SRC_URI += " file://gst-plugins-base-tremor.patch"
 
 SRC_URI[md5sum] = "2920af2b3162f3d9fbaa7fabc8ed4d38"
---

Cheers,

Chris.

Re: [PROPOSAL] Package feature switches, redux.

[Richard Purdie]

On Mon, 2011-07-04 at 12:54 +0100, Chris Elston wrote:
> Since responses to my previous mail were generally positive, I've
> reworked the package feature switches so that the interface is as RP
> suggested.
> 
> In the recipe for foo you would have a set of features defined like
> this:
> 
> PACKAGE_CONFIG[bar] = "--enable-bar, --disable-bar, libbar"
> PACKAGE_CONFIG[baz] = "--enable-baz, --disable-baz, libbaz"
> 
> The default set of features for the package would be defined with:
> 
> PACKAGE_FEATURES ?= "bar baz" 
> 
> Perhaps this set of features could go into a metadata field in the .ipk
> - would this be helpful for feed users?
> 
> The package features can then be tailored in a config/layer with
> something like:
> 
> PACKAGE_FEATURES_pn-foo = "baz pop"
> 
> If a layer requests a feature not supported by the recipe, you get a
> warning (should help distro maintainers detect bitrot in their layer):
> 
> WARNING: foo: Unknown feature 'pop' requested
> 
> The patch below uses gstreamer as an example of something which would
> benefit from this:

Looks good, thanks.

My main concern is still the PACKAGE_FEATURES variable. I've been
meaning to reply to your original email about this.

I understand your issue that you want to be able to do this on a per
package basis. I suspect you also see my concern about maintain this
centrally as a distro decision primarily rather than letting things
descend into more of a free for all.

FWIW, even if done centrally using DISTRO_FEATURES, you can customise on
a per recipe basis if you ever needed to, e.g.:

DISTRO_FEATURES = "a b c ${MYDISTROTWEAKS}"
MYDISTROTWEAKS = "d e f"
MYDISTROTWEAKS_pn-gstreamer = "e"

Now I'd agree this is a bit ugly but I think it would encourage less
misuse of the variable.

Any thoughts on that?

Cheers,

Richard

Re: [PROPOSAL] Package feature switches, redux.

[Graeme Gregory]

On 07/04/2011 12:54 PM, Chris Elston wrote:
> Since responses to my previous mail were generally positive, I've
> reworked the package feature switches so that the interface is as RP
> suggested.
>
> In the recipe for foo you would have a set of features defined like
> this:
>
> PACKAGE_CONFIG[bar] = "--enable-bar, --disable-bar, libbar"
> PACKAGE_CONFIG[baz] = "--enable-baz, --disable-baz, libbaz"
>
> The default set of features for the package would be defined with:
>
> PACKAGE_FEATURES ?= "bar baz" 
>
> Perhaps this set of features could go into a metadata field in the .ipk
> - would this be helpful for feed users?
>
> The package features can then be tailored in a config/layer with
> something like:
>
> PACKAGE_FEATURES_pn-foo = "baz pop"
>
> If a layer requests a feature not supported by the recipe, you get a
> warning (should help distro maintainers detect bitrot in their layer):
>
Hi, with my Angstrom cap on I like this syntax and I think it will be
really useful.

A second level concern I have is about conflicting features, its not
something we will come across probably in DISTRO land as we are sensible
enough not to select them. But users could select them in local.conf.

Graeme

Re: [PROPOSAL] Package feature switches, redux.

[Chris Elston]

> Hi, with my Angstrom cap on I like this syntax and I think it will be
> really useful.
> 
> A second level concern I have is about conflicting features, its not
> something we will come across probably in DISTRO land as we are sensible
> enough not to select them. But users could select them in local.conf.
> 
> Graeme

As a new developer, I've discovered that there are plenty of things that
you can set in local.conf which break things :D

Could you please give an example of conflicting features that could
cause problems, I'm not experienced enough with OE to have encountered
that problem yet.

Cheers,

Chris.

Re: [PROPOSAL] Package feature switches, redux.

[Graeme Gregory]

On 07/04/2011 05:12 PM, Chris Elston wrote:
>> Hi, with my Angstrom cap on I like this syntax and I think it will be
>> really useful.
>>
>> A second level concern I have is about conflicting features, its not
>> something we will come across probably in DISTRO land as we are sensible
>> enough not to select them. But users could select them in local.conf.
>>
>> Graeme
> As a new developer, I've discovered that there are plenty of things that
> you can set in local.conf which break things :D
>
> Could you please give an example of conflicting features that could
> cause problems, I'm not experienced enough with OE to have encountered
> that problem yet.
>
>
Cant think of a solid one off the top of my head, but I mean the cases where

--enable-feature means that --disable-another-feature is done.

This is why I listed it as a secondary issue.

Graeme

Re: [PROPOSAL] Package feature switches, redux.

[Chris Elston]

On Mon, 2011-07-04 at 17:44 +0100, Graeme Gregory wrote:
> On 07/04/2011 05:12 PM, Chris Elston wrote:
> >> Hi, with my Angstrom cap on I like this syntax and I think it will be
> >> really useful.
> >>
> >> A second level concern I have is about conflicting features, its not
> >> something we will come across probably in DISTRO land as we are sensible
> >> enough not to select them. But users could select them in local.conf.
> >>
> >> Graeme
> > As a new developer, I've discovered that there are plenty of things that
> > you can set in local.conf which break things :D
> >
> > Could you please give an example of conflicting features that could
> > cause problems, I'm not experienced enough with OE to have encountered
> > that problem yet.
> >
> >
> Cant think of a solid one off the top of my head, but I mean the cases where
> 
> --enable-feature means that --disable-another-feature is done.
> 
> This is why I listed it as a secondary issue.
> 
> Graeme

I understand. We could capture that information with an optional extra
field in the config info something like:

PACKAGE_CONFIG[foo] = "--enable-foo, --disable-foo, libfoo, <options
which conflict with foo>"

And then detect the conflict.  It's a trade off of time to handle that
conflicts field, versus how many potential conflicts there are I guess.
I think we could add it later if it turned out to be a common problem.

Cheers,

Chris.

Re: [PROPOSAL] Package feature switches, redux.

[Chris Elston]

On Mon, 2011-07-04 at 14:58 +0100, Richard Purdie wrote:
> On Mon, 2011-07-04 at 12:54 +0100, Chris Elston wrote:
> > Since responses to my previous mail were generally positive, I've
> > reworked the package feature switches so that the interface is as RP
> > suggested.
> > 
> > In the recipe for foo you would have a set of features defined like
> > this:
> > 
> > PACKAGE_CONFIG[bar] = "--enable-bar, --disable-bar, libbar"
> > PACKAGE_CONFIG[baz] = "--enable-baz, --disable-baz, libbaz"
> > 
> > The default set of features for the package would be defined with:
> > 
> > PACKAGE_FEATURES ?= "bar baz" 
> > 
> > Perhaps this set of features could go into a metadata field in the .ipk
> > - would this be helpful for feed users?
> > 
> > The package features can then be tailored in a config/layer with
> > something like:
> > 
> > PACKAGE_FEATURES_pn-foo = "baz pop"
> > 
> > If a layer requests a feature not supported by the recipe, you get a
> > warning (should help distro maintainers detect bitrot in their layer):
> > 
> > WARNING: foo: Unknown feature 'pop' requested
> > 
> > The patch below uses gstreamer as an example of something which would
> > benefit from this:
> 
> Looks good, thanks.
> 
> My main concern is still the PACKAGE_FEATURES variable. I've been
> meaning to reply to your original email about this.
> 
> I understand your issue that you want to be able to do this on a per
> package basis. I suspect you also see my concern about maintain this
> centrally as a distro decision primarily rather than letting things
> descend into more of a free for all.


> FWIW, even if done centrally using DISTRO_FEATURES, you can customise on
> a per recipe basis if you ever needed to, e.g.:
> 
> DISTRO_FEATURES = "a b c ${MYDISTROTWEAKS}"
> MYDISTROTWEAKS = "d e f"
> MYDISTROTWEAKS_pn-gstreamer = "e"
> 
> Now I'd agree this is a bit ugly but I think it would encourage less
> misuse of the variable.

> Any thoughts on that?

Apologies for the delay in this response.  For those not present, we
discussed this a little on IRC the other day, and I'd like to resurrect
the on-list discussion.

I don't really understand the way in which you're worried it could be
misused if implemented as PACKAGE_FEATURES.  I'd imagined that
PACKAGE_FEATURES would have it's canonical setting in the recipe itself,
and would only be overridden in private layers.  I accept that my lack
of understanding is probably due to my inexperience with OE :)

I'm hopeful that we could find a technical solution that you're happy
won't cause maintenance problems down the line.

Cheers,

Chris.

Re: [PROPOSAL] Package feature switches, redux.

[Mark Hatle]

On 7/4/11 11:44 AM, Graeme Gregory wrote:
> On 07/04/2011 05:12 PM, Chris Elston wrote:
>>> Hi, with my Angstrom cap on I like this syntax and I think it will be
>>> really useful.
>>>
>>> A second level concern I have is about conflicting features, its not
>>> something we will come across probably in DISTRO land as we are sensible
>>> enough not to select them. But users could select them in local.conf.
>>>
>>> Graeme
>> As a new developer, I've discovered that there are plenty of things that
>> you can set in local.conf which break things :D
>>
>> Could you please give an example of conflicting features that could
>> cause problems, I'm not experienced enough with OE to have encountered
>> that problem yet.
>>
>>
> Cant think of a solid one off the top of my head, but I mean the cases where
> 
> --enable-feature means that --disable-another-feature is done.
> 
> This is why I listed it as a secondary issue.

I remember seeing similar issues as well.

I really like this syntax described for the options.  If there is some way to
list a conflicting option -- or even simple some python syntax useful within a
recipe itself to say "hey you can't specify these two things and use this
recipe", that would be all that I think is needed.

I rarely find packages where it's likely someone will try to configure a
conflicting option set, but it does happen.. if we find it.. we should be able
to note it within the recipe and have it flag the user before do_configure is
run.  (Preferably even before that!)

--Mark

> Graeme
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core

Re: [PROPOSAL] Package feature switches, redux.

[Martin Jansa]

[-- Attachment #1: Type: text/plain, Size: 2017 bytes --]

On Mon, Jul 04, 2011 at 05:49:37PM +0100, Chris Elston wrote:
> On Mon, 2011-07-04 at 17:44 +0100, Graeme Gregory wrote:
> > On 07/04/2011 05:12 PM, Chris Elston wrote:
> > >> Hi, with my Angstrom cap on I like this syntax and I think it will be
> > >> really useful.
> > >>
> > >> A second level concern I have is about conflicting features, its not
> > >> something we will come across probably in DISTRO land as we are sensible
> > >> enough not to select them. But users could select them in local.conf.
> > >>
> > >> Graeme
> > > As a new developer, I've discovered that there are plenty of things that
> > > you can set in local.conf which break things :D
> > >
> > > Could you please give an example of conflicting features that could
> > > cause problems, I'm not experienced enough with OE to have encountered
> > > that problem yet.
> > >
> > >
> > Cant think of a solid one off the top of my head, but I mean the cases where
> > 
> > --enable-feature means that --disable-another-feature is done.
> > 
> > This is why I listed it as a secondary issue.
> > 
> > Graeme
> 
> I understand. We could capture that information with an optional extra
> field in the config info something like:
> 
> PACKAGE_CONFIG[foo] = "--enable-foo, --disable-foo, libfoo, <options
> which conflict with foo>"

Maybe I'm influenced a lot by gentoo, but most options are set with
--enable-foo/--disable-foo or --with-foo/--without-foo so shortcuts for
such cases would be nice instead of repeating "foo" 3 times on same line.

Also again from gentoo world.. can I say in recipe bar.bb

DEPENDS = "abc[foo]"
or
DEPENDS = "abc[-foo]"

because if some recipe depends on gstreamer with vorbis enabled it
should be easy to say it in DEPENDS then some magic python fce and if
someone tries to build it in distribution with vorbis disabled then it
should fail with message like "No provider for gstreamer[vorbis]"

Cheers,

-- 
Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 205 bytes --]