Patchwork [1/3] busybox: enable mdev by default

login
register
mail settings
Submitter Otavio Salvador
Date June 1, 2011, 8:09 p.m.
Message ID <e96c2e88aaf1cd22faac3b7ae61ccf34adfe92ba.1306958862.git.otavio@ossystems.com.br>
Download mbox | patch
Permalink /patch/5247/
State New, archived
Headers show

Comments

Otavio Salvador - June 1, 2011, 8:09 p.m.
Since we do not require a configuration file by default we don't force
it's addition on the package. If the a layer wants to have it enabled
it should also set the CONFFILES for busybox-mdev package.

Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
 meta/recipes-core/busybox/busybox-1.18.4/defconfig |    2 +-
 meta/recipes-core/busybox/busybox.inc              |    1 -
 meta/recipes-core/busybox/busybox_1.18.4.bb        |    2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)
Phil Blundell - June 1, 2011, 8:37 p.m.
On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
> -# CONFIG_MDEV is not set
> +CONFIG_MDEV=y

Per previous discussion, I am still uneasy about this change.  I think
we really need some sort of coherent policy for what exactly the default
busybox configuration in oe-core is meant to be doing, and then (if
necessary) a set of patches to make it match the policy.  Just flipping
random features on and off does not seem like a good way to proceed.

p.
Otavio Salvador - June 1, 2011, 8:40 p.m.
On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
>> -# CONFIG_MDEV is not set
>> +CONFIG_MDEV=y
>
> Per previous discussion, I am still uneasy about this change.  I think
> we really need some sort of coherent policy for what exactly the default
> busybox configuration in oe-core is meant to be doing, and then (if
> necessary) a set of patches to make it match the policy.  Just flipping
> random features on and off does not seem like a good way to proceed.

OE-core has support to mdev as device handling mechanism as such this
ought to be enabled by default IMO.

Personally it doesn't matter since I have already overriden it in my
internal layer.
Richard Purdie - June 2, 2011, 4:37 p.m.
On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
> > On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
> >> -# CONFIG_MDEV is not set
> >> +CONFIG_MDEV=y
> >
> > Per previous discussion, I am still uneasy about this change.  I think
> > we really need some sort of coherent policy for what exactly the default
> > busybox configuration in oe-core is meant to be doing, and then (if
> > necessary) a set of patches to make it match the policy.  Just flipping
> > random features on and off does not seem like a good way to proceed.
> 
> OE-core has support to mdev as device handling mechanism as such this
> ought to be enabled by default IMO.
> 
> Personally it doesn't matter since I have already overriden it in my
> internal layer.

I'm afraid I'm with Phil on this. I don't like the idea of enabling
something we don't actually use. This really needs to become some kind
of configure option which would at the same time disable/replace udev so
the patch in its currently form isn't acceptable.

Cheers,

Richard
Otavio Salvador - June 2, 2011, 4:40 p.m.
On Thu, Jun 2, 2011 at 16:37, Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
> I'm afraid I'm with Phil on this. I don't like the idea of enabling
> something we don't actually use. This really needs to become some kind
> of configure option which would at the same time disable/replace udev so
> the patch in its currently form isn't acceptable.

I'll drop it from my tree since I am using an bbappend on our local layer.

Cheers,
Khem Raj - June 3, 2011, 1:06 a.m.
On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
> > On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
> > > On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
> > >> -# CONFIG_MDEV is not set
> > >> +CONFIG_MDEV=y
> > > 
> > > Per previous discussion, I am still uneasy about this change.  I think
> > > we really need some sort of coherent policy for what exactly the
> > > default busybox configuration in oe-core is meant to be doing, and
> > > then (if necessary) a set of patches to make it match the policy. 
> > > Just flipping random features on and off does not seem like a good way
> > > to proceed.
> > 
> > OE-core has support to mdev as device handling mechanism as such this
> > ought to be enabled by default IMO.
> > 
> > Personally it doesn't matter since I have already overriden it in my
> > internal layer.
> 
> I'm afraid I'm with Phil on this. I don't like the idea of enabling
> something we don't actually use. This really needs to become some kind
> of configure option which would at the same time disable/replace udev so
> the patch in its currently form isn't acceptable.
> 

mdev or udev are image features so probably should be controlled by 
IMAGE_FEATURES or some such
> Cheers,
> 
> Richard
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Koen Kooi - June 3, 2011, 6:37 a.m.
Op 3 jun 2011, om 03:06 heeft Khem Raj het volgende geschreven:

> On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
>> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
>>> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
>>>> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
>>>>> -# CONFIG_MDEV is not set
>>>>> +CONFIG_MDEV=y
>>>> 
>>>> Per previous discussion, I am still uneasy about this change.  I think
>>>> we really need some sort of coherent policy for what exactly the
>>>> default busybox configuration in oe-core is meant to be doing, and
>>>> then (if necessary) a set of patches to make it match the policy. 
>>>> Just flipping random features on and off does not seem like a good way
>>>> to proceed.
>>> 
>>> OE-core has support to mdev as device handling mechanism as such this
>>> ought to be enabled by default IMO.
>>> 
>>> Personally it doesn't matter since I have already overriden it in my
>>> internal layer.
>> 
>> I'm afraid I'm with Phil on this. I don't like the idea of enabling
>> something we don't actually use. This really needs to become some kind
>> of configure option which would at the same time disable/replace udev so
>> the patch in its currently form isn't acceptable.
>> 
> 
> mdev or udev are image features so probably should be controlled by 
> IMAGE_FEATURES or some such

You can't put IMAGE_FEATURES in the equivalent of EXTRA_OECONF since the packages in the feeds don't magically change when being installed in a different image.
Richard Purdie - June 3, 2011, 8:24 a.m.
On Fri, 2011-06-03 at 08:37 +0200, Koen Kooi wrote:
> Op 3 jun 2011, om 03:06 heeft Khem Raj het volgende geschreven:
> 
> > On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
> >> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
> >>> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
> >>>> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
> >>>>> -# CONFIG_MDEV is not set
> >>>>> +CONFIG_MDEV=y
> >>>> 
> >>>> Per previous discussion, I am still uneasy about this change.  I think
> >>>> we really need some sort of coherent policy for what exactly the
> >>>> default busybox configuration in oe-core is meant to be doing, and
> >>>> then (if necessary) a set of patches to make it match the policy. 
> >>>> Just flipping random features on and off does not seem like a good way
> >>>> to proceed.
> >>> 
> >>> OE-core has support to mdev as device handling mechanism as such this
> >>> ought to be enabled by default IMO.
> >>> 
> >>> Personally it doesn't matter since I have already overriden it in my
> >>> internal layer.
> >> 
> >> I'm afraid I'm with Phil on this. I don't like the idea of enabling
> >> something we don't actually use. This really needs to become some kind
> >> of configure option which would at the same time disable/replace udev so
> >> the patch in its currently form isn't acceptable.
> >> 
> > 
> > mdev or udev are image features so probably should be controlled by 
> > IMAGE_FEATURES or some such
> 
> You can't put IMAGE_FEATURES in the equivalent of EXTRA_OECONF since
> the packages in the feeds don't magically change when being installed
> in a different image.

Right, it would have to be a DISTRO_FEATURE for that reason...

Cheers,

Richard
Koen Kooi - June 3, 2011, 8:59 a.m.
Op 3 jun 2011, om 10:24 heeft Richard Purdie het volgende geschreven:

> On Fri, 2011-06-03 at 08:37 +0200, Koen Kooi wrote:
>> Op 3 jun 2011, om 03:06 heeft Khem Raj het volgende geschreven:
>> 
>>> On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
>>>> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
>>>>> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
>>>>>> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
>>>>>>> -# CONFIG_MDEV is not set
>>>>>>> +CONFIG_MDEV=y
>>>>>> 
>>>>>> Per previous discussion, I am still uneasy about this change.  I think
>>>>>> we really need some sort of coherent policy for what exactly the
>>>>>> default busybox configuration in oe-core is meant to be doing, and
>>>>>> then (if necessary) a set of patches to make it match the policy. 
>>>>>> Just flipping random features on and off does not seem like a good way
>>>>>> to proceed.
>>>>> 
>>>>> OE-core has support to mdev as device handling mechanism as such this
>>>>> ought to be enabled by default IMO.
>>>>> 
>>>>> Personally it doesn't matter since I have already overriden it in my
>>>>> internal layer.
>>>> 
>>>> I'm afraid I'm with Phil on this. I don't like the idea of enabling
>>>> something we don't actually use. This really needs to become some kind
>>>> of configure option which would at the same time disable/replace udev so
>>>> the patch in its currently form isn't acceptable.
>>>> 
>>> 
>>> mdev or udev are image features so probably should be controlled by 
>>> IMAGE_FEATURES or some such
>> 
>> You can't put IMAGE_FEATURES in the equivalent of EXTRA_OECONF since
>> the packages in the feeds don't magically change when being installed
>> in a different image.
> 
> Right, it would have to be a DISTRO_FEATURE for that reason...

Right, but you can build mdev and choose not to use it, which is what .dev currently does. So while the distro could in theory set a DISTRO_FEATURE, we shouldn't make other decisions based on that, like deciding that if mdev is in DISTRO_FEATURES every image will use it.

regards,

Koen
Richard Purdie - June 3, 2011, 9:50 a.m.
On Fri, 2011-06-03 at 10:59 +0200, Koen Kooi wrote:
> Op 3 jun 2011, om 10:24 heeft Richard Purdie het volgende geschreven:
> 
> > On Fri, 2011-06-03 at 08:37 +0200, Koen Kooi wrote:
> >> Op 3 jun 2011, om 03:06 heeft Khem Raj het volgende geschreven:
> >> 
> >>> On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
> >>>> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
> >>>>> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
> >>>>>> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
> >>>>>>> -# CONFIG_MDEV is not set
> >>>>>>> +CONFIG_MDEV=y
> >>>>>> 
> >>>>>> Per previous discussion, I am still uneasy about this change.  I think
> >>>>>> we really need some sort of coherent policy for what exactly the
> >>>>>> default busybox configuration in oe-core is meant to be doing, and
> >>>>>> then (if necessary) a set of patches to make it match the policy. 
> >>>>>> Just flipping random features on and off does not seem like a good way
> >>>>>> to proceed.
> >>>>> 
> >>>>> OE-core has support to mdev as device handling mechanism as such this
> >>>>> ought to be enabled by default IMO.
> >>>>> 
> >>>>> Personally it doesn't matter since I have already overriden it in my
> >>>>> internal layer.
> >>>> 
> >>>> I'm afraid I'm with Phil on this. I don't like the idea of enabling
> >>>> something we don't actually use. This really needs to become some kind
> >>>> of configure option which would at the same time disable/replace udev so
> >>>> the patch in its currently form isn't acceptable.
> >>>> 
> >>> 
> >>> mdev or udev are image features so probably should be controlled by 
> >>> IMAGE_FEATURES or some such
> >> 
> >> You can't put IMAGE_FEATURES in the equivalent of EXTRA_OECONF since
> >> the packages in the feeds don't magically change when being installed
> >> in a different image.
> > 
> > Right, it would have to be a DISTRO_FEATURE for that reason...
> 
> Right, but you can build mdev and choose not to use it, which is
> what .dev currently does. So while the distro could in theory set a
> DISTRO_FEATURE, we shouldn't make other decisions based on that, like
> deciding that if mdev is in DISTRO_FEATURES every image will use it.

Why we wouldn't enable that functionality based on the option?

Cheers,

Richard
Phil Blundell - June 3, 2011, 9:57 a.m.
On Thu, 2011-06-02 at 18:06 -0700, Khem Raj wrote:
> mdev or udev are image features so probably should be controlled by 
> IMAGE_FEATURES or some such

Well, yeah, but this is the thing with busybox: it is a monolithic
executable, so you need to decide at configure time what features it's
going to include.  Although there is a separate busybox-mdev package, it
just contains the auxiliary scripting and the bulk of the mdev code goes
into busybox itself.

This means that making mdev a pure IMAGE_FEATURE isn't practical, since
it would basically require mdev to be always compiled in on the
offchance that some image might want it.  That would lead to code bloat
for those images (probably the majority) which don't.

Equally, having a single straightforward DISTRO_FEATURE to control mdev
is not terribly satisfactory, since some distros might want to configure
busybox with mdev enabled but actually use udev (or nothing) for some or
all of their images.

So, it seems that we really need both: a way for distros to configure
whether busybox is built with mdev on or not, and then an IMAGE_FEATURE
to control whether any given image gets mdev, or udev, or nothing.  The
former of those could be done as a DISTRO_FEATURE but, since it's only
going to affect busybox, I'm not sure that there is much benefit in
doing that; it seems like the distros could just as easily take care of
that by direct manipulation of the busybox configuration.

p.
Koen Kooi - June 3, 2011, 10:42 a.m.
Op 3 jun 2011, om 11:50 heeft Richard Purdie het volgende geschreven:

> On Fri, 2011-06-03 at 10:59 +0200, Koen Kooi wrote:
>> Op 3 jun 2011, om 10:24 heeft Richard Purdie het volgende geschreven:
>> 
>>> On Fri, 2011-06-03 at 08:37 +0200, Koen Kooi wrote:
>>>> Op 3 jun 2011, om 03:06 heeft Khem Raj het volgende geschreven:
>>>> 
>>>>> On Thursday, June 02, 2011 09:37:41 AM Richard Purdie wrote:
>>>>>> On Wed, 2011-06-01 at 20:40 +0000, Otavio Salvador wrote:
>>>>>>> On Wed, Jun 1, 2011 at 20:37, Phil Blundell <pb@pbcl.net> wrote:
>>>>>>>> On Wed, 2011-06-01 at 20:09 +0000, Otavio Salvador wrote:
>>>>>>>>> -# CONFIG_MDEV is not set
>>>>>>>>> +CONFIG_MDEV=y
>>>>>>>> 
>>>>>>>> Per previous discussion, I am still uneasy about this change.  I think
>>>>>>>> we really need some sort of coherent policy for what exactly the
>>>>>>>> default busybox configuration in oe-core is meant to be doing, and
>>>>>>>> then (if necessary) a set of patches to make it match the policy. 
>>>>>>>> Just flipping random features on and off does not seem like a good way
>>>>>>>> to proceed.
>>>>>>> 
>>>>>>> OE-core has support to mdev as device handling mechanism as such this
>>>>>>> ought to be enabled by default IMO.
>>>>>>> 
>>>>>>> Personally it doesn't matter since I have already overriden it in my
>>>>>>> internal layer.
>>>>>> 
>>>>>> I'm afraid I'm with Phil on this. I don't like the idea of enabling
>>>>>> something we don't actually use. This really needs to become some kind
>>>>>> of configure option which would at the same time disable/replace udev so
>>>>>> the patch in its currently form isn't acceptable.
>>>>>> 
>>>>> 
>>>>> mdev or udev are image features so probably should be controlled by 
>>>>> IMAGE_FEATURES or some such
>>>> 
>>>> You can't put IMAGE_FEATURES in the equivalent of EXTRA_OECONF since
>>>> the packages in the feeds don't magically change when being installed
>>>> in a different image.
>>> 
>>> Right, it would have to be a DISTRO_FEATURE for that reason...
>> 
>> Right, but you can build mdev and choose not to use it, which is
>> what .dev currently does. So while the distro could in theory set a
>> DISTRO_FEATURE, we shouldn't make other decisions based on that, like
>> deciding that if mdev is in DISTRO_FEATURES every image will use it.
> 
> Why we wouldn't enable that functionality based on the option?

I was going to write a lengthy response, but Phil summed it up way too well in http://lists.linuxtogo.org/pipermail/openembedded-core/2011-June/003702.html :)

Patch

diff --git a/meta/recipes-core/busybox/busybox-1.18.4/defconfig b/meta/recipes-core/busybox/busybox-1.18.4/defconfig
index 5327a64..6b48cc1 100644
--- a/meta/recipes-core/busybox/busybox-1.18.4/defconfig
+++ b/meta/recipes-core/busybox/busybox-1.18.4/defconfig
@@ -518,7 +518,7 @@  CONFIG_FEATURE_HWCLOCK_ADJTIME_FHS=y
 CONFIG_LOSETUP=y
 # CONFIG_LSPCI is not set
 # CONFIG_LSUSB is not set
-# CONFIG_MDEV is not set
+CONFIG_MDEV=y
 # CONFIG_FEATURE_MDEV_CONF is not set
 # CONFIG_FEATURE_MDEV_RENAME is not set
 # CONFIG_FEATURE_MDEV_RENAME_REGEXP is not set
diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
index 86fbdae..9b874c5 100644
--- a/meta/recipes-core/busybox/busybox.inc
+++ b/meta/recipes-core/busybox/busybox.inc
@@ -29,7 +29,6 @@  INITSCRIPT_PARAMS_${PN}-mdev = "start 06 S ."
 INITSCRIPT_NAME_${PN}-udhcpd = "busybox-udhcpd" 
 INITSCRIPT_NAME_${PN}-udhcpc = "busybox-udhcpc" 
 CONFFILES_${PN}-syslog = "${sysconfdir}/syslog.conf.${PN}"
-CONFFILES_${PN}-mdev = "${sysconfdir}/mdev.conf"
 
 RRECOMMENDS_${PN} = "${PN}-syslog ${PN}-udhcpc"
 
diff --git a/meta/recipes-core/busybox/busybox_1.18.4.bb b/meta/recipes-core/busybox/busybox_1.18.4.bb
index a5080d5..a999490 100644
--- a/meta/recipes-core/busybox/busybox_1.18.4.bb
+++ b/meta/recipes-core/busybox/busybox_1.18.4.bb
@@ -1,5 +1,5 @@ 
 require busybox.inc
-PR = "r2"
+PR = "r3"
 
 SRC_URI = "http://www.busybox.net/downloads/busybox-${PV}.tar.bz2;name=tarball \
            file://udhcpscript.patch \