Patchwork [5/5] busybox.inc: Install hwclock init script only if rtc is present in MACHINE_FEATURES

login
register
mail settings
Submitter Andrei Gherzan
Date July 10, 2012, 2:59 p.m.
Message ID <d7e0c2813e4002b9dfd8e2d681b93da9ddc5870f.1341932186.git.andrei@gherzan.ro>
Download mbox | patch
Permalink /patch/31639/
State New
Headers show

Comments

Andrei Gherzan - July 10, 2012, 2:59 p.m.
Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
---
 meta/recipes-core/busybox/busybox.inc |   10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
Richard Purdie - July 10, 2012, 4:02 p.m.
On Tue, 2012-07-10 at 17:59 +0300, Andrei Gherzan wrote:
> Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
> ---
>  meta/recipes-core/busybox/busybox.inc |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
> index 82137a3..17ff442 100644
> --- a/meta/recipes-core/busybox/busybox.inc
> +++ b/meta/recipes-core/busybox/busybox.inc
> @@ -180,9 +180,13 @@ do_install () {
>  	if grep "CONFIG_UDHCPD=y" ${WORKDIR}/defconfig; then
>  		install -m 0755 ${WORKDIR}/busybox-udhcpd ${D}${sysconfdir}/init.d/
>  	fi
> -	if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
> -		install -m 0755 ${WORKDIR}/hwclock.sh ${D}${sysconfdir}/init.d/
> -	fi
> +	case "${MACHINE_FEATURES}" in
> +	*rtc*)
> +		if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
> +			install -m 0755 ${WORKDIR}/hwclock.sh ${D}${sysconfdir}/init.d/
> +		fi
> +		;;
> +	esac
>  	if grep "CONFIG_UDHCPC=y" ${WORKDIR}/defconfig; then
>  		install -d ${D}${sysconfdir}/udhcpc.d
>  		install -d ${D}${datadir}/udhcpc

This makes busybox machine specific? Is that really what you want to do?

Cheers,

Richard
Andrei Gherzan - July 10, 2012, 8:53 p.m.
On Tue, Jul 10, 2012 at 7:02 PM, Richard Purdie <
richard.purdie@linuxfoundation.org> wrote:

> On Tue, 2012-07-10 at 17:59 +0300, Andrei Gherzan wrote:
> > Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
> > ---
> >  meta/recipes-core/busybox/busybox.inc |   10 +++++++---
> >  1 file changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/meta/recipes-core/busybox/busybox.inc
> b/meta/recipes-core/busybox/busybox.inc
> > index 82137a3..17ff442 100644
> > --- a/meta/recipes-core/busybox/busybox.inc
> > +++ b/meta/recipes-core/busybox/busybox.inc
> > @@ -180,9 +180,13 @@ do_install () {
> >       if grep "CONFIG_UDHCPD=y" ${WORKDIR}/defconfig; then
> >               install -m 0755 ${WORKDIR}/busybox-udhcpd
> ${D}${sysconfdir}/init.d/
> >       fi
> > -     if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
> > -             install -m 0755 ${WORKDIR}/hwclock.sh
> ${D}${sysconfdir}/init.d/
> > -     fi
> > +     case "${MACHINE_FEATURES}" in
> > +     *rtc*)
> > +             if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
> > +                     install -m 0755 ${WORKDIR}/hwclock.sh
> ${D}${sysconfdir}/init.d/
> > +             fi
> > +             ;;
> > +     esac
> >       if grep "CONFIG_UDHCPC=y" ${WORKDIR}/defconfig; then
> >               install -d ${D}${sysconfdir}/udhcpc.d
> >               install -d ${D}${datadir}/udhcpc
>
> This makes busybox machine specific? Is that really what you want to do?
>
> I want to have hwclock init script only on machines with RTC.

@g
Andrei Gherzan - July 10, 2012, 10:15 p.m.
On Tue, Jul 10, 2012 at 11:53 PM, Andrei Gherzan <andrei@gherzan.ro> wrote:

> On Tue, Jul 10, 2012 at 7:02 PM, Richard Purdie <
> richard.purdie@linuxfoundation.org> wrote:
>
>> On Tue, 2012-07-10 at 17:59 +0300, Andrei Gherzan wrote:
>> > Signed-off-by: Andrei Gherzan <andrei@gherzan.ro>
>> > ---
>> >  meta/recipes-core/busybox/busybox.inc |   10 +++++++---
>> >  1 file changed, 7 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/meta/recipes-core/busybox/busybox.inc
>> b/meta/recipes-core/busybox/busybox.inc
>> > index 82137a3..17ff442 100644
>> > --- a/meta/recipes-core/busybox/busybox.inc
>> > +++ b/meta/recipes-core/busybox/busybox.inc
>> > @@ -180,9 +180,13 @@ do_install () {
>> >       if grep "CONFIG_UDHCPD=y" ${WORKDIR}/defconfig; then
>> >               install -m 0755 ${WORKDIR}/busybox-udhcpd
>> ${D}${sysconfdir}/init.d/
>> >       fi
>> > -     if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
>> > -             install -m 0755 ${WORKDIR}/hwclock.sh
>> ${D}${sysconfdir}/init.d/
>> > -     fi
>> > +     case "${MACHINE_FEATURES}" in
>> > +     *rtc*)
>> > +             if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
>> > +                     install -m 0755 ${WORKDIR}/hwclock.sh
>> ${D}${sysconfdir}/init.d/
>> > +             fi
>> > +             ;;
>> > +     esac
>> >       if grep "CONFIG_UDHCPC=y" ${WORKDIR}/defconfig; then
>> >               install -d ${D}${sysconfdir}/udhcpc.d
>> >               install -d ${D}${datadir}/udhcpc
>>
>> This makes busybox machine specific? Is that really what you want to do?
>>
>> I want to have hwclock init script only on machines with RTC.
>
>
And to move a little further, busybox should be configured
with CONFIG_HWCLOCK=y only if it makes sense for that MACHINE. In my
opinion this is machine specific. So avoiding an init script but still
having useless utils doesn't sound good for me.

@g
Phil Blundell - July 10, 2012, 10:43 p.m.
On Wed, 2012-07-11 at 01:15 +0300, Andrei Gherzan wrote:
> And to move a little further, busybox should be configured with
> CONFIG_HWCLOCK=y only if it makes sense for that MACHINE. In my
> opinion this is machine specific. 

That's a DISTRO decision.  I suspect most that have binary feeds would
prefer to have a single busybox binary per architecture, and accept the
few wasted bytes on machines without RTC, than to make it MACHINE
specific and end up building it multiple times.

Anyway, such policy belongs in your distro layer and not in oe-core.

p.
Martin Jansa - July 11, 2012, 9:11 a.m.
On Tue, Jul 10, 2012 at 11:43:58PM +0100, Phil Blundell wrote:
> On Wed, 2012-07-11 at 01:15 +0300, Andrei Gherzan wrote:
> > And to move a little further, busybox should be configured with
> > CONFIG_HWCLOCK=y only if it makes sense for that MACHINE. In my
> > opinion this is machine specific. 
> 
> That's a DISTRO decision.  I suspect most that have binary feeds would
> prefer to have a single busybox binary per architecture, and accept the
> few wasted bytes on machines without RTC, than to make it MACHINE
> specific and end up building it multiple times.

Agreed.

It would be better to package initscript + hwclock to separate packages
and then pull it to image only for machines with RTC.. but not making
whole busybox machine specific.

Another advantage of this would be option to use busybox-hwclock as utility
and hwclock-systemd instead of busybox-hwclock-init for images which are
using systemd.

Cheers,
Andrei Gherzan - July 25, 2012, 6:13 p.m.
On Wed, Jul 11, 2012 at 12:11 PM, Martin Jansa <martin.jansa@gmail.com>wrote:

> On Tue, Jul 10, 2012 at 11:43:58PM +0100, Phil Blundell wrote:
> > On Wed, 2012-07-11 at 01:15 +0300, Andrei Gherzan wrote:
> > > And to move a little further, busybox should be configured with
> > > CONFIG_HWCLOCK=y only if it makes sense for that MACHINE. In my
> > > opinion this is machine specific.
> >
> > That's a DISTRO decision.  I suspect most that have binary feeds would
> > prefer to have a single busybox binary per architecture, and accept the
> > few wasted bytes on machines without RTC, than to make it MACHINE
> > specific and end up building it multiple times.
>
> Agreed.
>
> It would be better to package initscript + hwclock to separate packages
> and then pull it to image only for machines with RTC.. but not making
> whole busybox machine specific.
>
> Another advantage of this would be option to use busybox-hwclock as utility
> and hwclock-systemd instead of busybox-hwclock-init for images which are
> using systemd.
>
>
Thank you all. It makes sense to keep busybox machine independent. I agree.
Now if i package this init script separately how would it be pulled in by
images? Think this could brake some image which now relay on this being
pulled in by busybox.

ag

Patch

diff --git a/meta/recipes-core/busybox/busybox.inc b/meta/recipes-core/busybox/busybox.inc
index 82137a3..17ff442 100644
--- a/meta/recipes-core/busybox/busybox.inc
+++ b/meta/recipes-core/busybox/busybox.inc
@@ -180,9 +180,13 @@  do_install () {
 	if grep "CONFIG_UDHCPD=y" ${WORKDIR}/defconfig; then
 		install -m 0755 ${WORKDIR}/busybox-udhcpd ${D}${sysconfdir}/init.d/
 	fi
-	if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
-		install -m 0755 ${WORKDIR}/hwclock.sh ${D}${sysconfdir}/init.d/
-	fi
+	case "${MACHINE_FEATURES}" in
+	*rtc*)
+		if grep "CONFIG_HWCLOCK=y" ${WORKDIR}/defconfig; then
+			install -m 0755 ${WORKDIR}/hwclock.sh ${D}${sysconfdir}/init.d/
+		fi
+		;;
+	esac
 	if grep "CONFIG_UDHCPC=y" ${WORKDIR}/defconfig; then
 		install -d ${D}${sysconfdir}/udhcpc.d
 		install -d ${D}${datadir}/udhcpc