Patchwork [1/1] powertop: inherit update-alternatives and use a higher priority than busybox

login
register
mail settings
Submitter Dexuan Cui
Date July 7, 2011, 8:39 a.m.
Message ID <cb76f021571c5f35239192ff05eb89cab4713d04.1310027619.git.dexuan.cui@intel.com>
Download mbox | patch
Permalink /patch/7117/
State New, archived
Headers show

Comments

Dexuan Cui - July 7, 2011, 8:39 a.m.
busybox-1.18.4 installs /bin/powertop and the powertop recipe installs
/usr/bin/powertop. So, in PATH, if /bin appears before /usr/bin, we would
run the version offered by busybox, which has a very limited function (e.g.,
no parameter is accepted) and this causes trouble to eclipse plugin.

We can use update-alternatives for powertop with higher priority to resolve
the issue.

Fixes [YOCTO #1208]

Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
---
 meta/recipes-kernel/powertop/powertop_1.13.bb |    8 +++++++-
 1 files changed, 7 insertions(+), 1 deletions(-)
Tom Rini - July 7, 2011, 3:40 p.m.
On 07/07/2011 01:39 AM, Dexuan Cui wrote:
> busybox-1.18.4 installs /bin/powertop and the powertop recipe installs
> /usr/bin/powertop. So, in PATH, if /bin appears before /usr/bin, we would
> run the version offered by busybox, which has a very limited function (e.g.,
> no parameter is accepted) and this causes trouble to eclipse plugin.
> 
> We can use update-alternatives for powertop with higher priority to resolve
> the issue.
> 
> Fixes [YOCTO #1208]
> 
> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>

This fix seems a bit incomplete.  Why is busybox putting powertop into
/bin when it almost certainly belongs in /usr/bin like the real recipe
was placing it.  busybox needs a fix here too.
Dexuan Cui - July 8, 2011, 1:40 a.m.
Tom Rini wrote:
> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
>> /usr/bin, we would run the version offered by busybox, which has a
>> very limited function (e.g., no parameter is accepted) and this
>> causes trouble to eclipse plugin. 
>> 
>> We can use update-alternatives for powertop with higher priority to
>> resolve the issue. 
>> 
>> Fixes [YOCTO #1208]
>> 
>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> 
> This fix seems a bit incomplete.  Why is busybox putting powertop into
> /bin when it almost certainly belongs in /usr/bin like the real recipe
> was placing it.  busybox needs a fix here too.
Thanks for the comment! 
I was hesitant about fixing busybox as I wasn't sure if it's worthy to make a patch to only fix the path for busybox. I don't know why busybox puts it into /bin. I think the best place may be /usr/sbin/. 
A little unluckily this patch to powertop has been already in poky master... So maybe we could try to fix the recipes in future, e.g., when upgrading them.

Thanks,
-- Dexuan
Koen Kooi - July 8, 2011, 7:25 a.m.
Op 8 jul. 2011 om 02:40 heeft "Cui, Dexuan" <dexuan.cui@intel.com> het volgende geschreven:

> Tom Rini wrote:
>> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
>>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
>>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
>>> /usr/bin, we would run the version offered by busybox, which has a
>>> very limited function (e.g., no parameter is accepted) and this
>>> causes trouble to eclipse plugin. 
>>> 
>>> We can use update-alternatives for powertop with higher priority to
>>> resolve the issue. 
>>> 
>>> Fixes [YOCTO #1208]
>>> 
>>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
>> 
>> This fix seems a bit incomplete.  Why is busybox putting powertop into
>> /bin when it almost certainly belongs in /usr/bin like the real recipe
>> was placing it.  busybox needs a fix here too.
> Thanks for the comment! 
> I was hesitant about fixing busybox as I wasn't sure if it's worthy to make a patch to only fix the path for busybox. I don't know why busybox puts it into /bin. I think the best place may be /usr/sbin/. 
> A little unluckily this patch to powertop has been already in poky master... So maybe we could try to fix the recipes in future, e.g., when upgrading them.

we should do the right thing in oe-core,  the poky people can clean up on their own. 




> 
> Thanks,
> -- Dexuan
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Richard Purdie - July 8, 2011, 3 p.m.
On Fri, 2011-07-08 at 08:25 +0100, Koen Kooi wrote:
> 
> Op 8 jul. 2011 om 02:40 heeft "Cui, Dexuan" <dexuan.cui@intel.com> het volgende geschreven:
> 
> > Tom Rini wrote:
> >> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
> >>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
> >>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
> >>> /usr/bin, we would run the version offered by busybox, which has a
> >>> very limited function (e.g., no parameter is accepted) and this
> >>> causes trouble to eclipse plugin. 
> >>> 
> >>> We can use update-alternatives for powertop with higher priority to
> >>> resolve the issue. 
> >>> 
> >>> Fixes [YOCTO #1208]
> >>> 
> >>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> >> 
> >> This fix seems a bit incomplete.  Why is busybox putting powertop into
> >> /bin when it almost certainly belongs in /usr/bin like the real recipe
> >> was placing it.  busybox needs a fix here too.
> > Thanks for the comment! 
> > I was hesitant about fixing busybox as I wasn't sure if it's worthy
> to make a patch to only fix the path for busybox. I don't know why
> busybox puts it into /bin. I think the best place may be /usr/sbin/. 
> > A little unluckily this patch to powertop has been already in poky
> master... So maybe we could try to fix the recipes in future, e.g.,
> when upgrading them.
> 
> we should do the right thing in oe-core,  the poky people can clean up on their own. 

I don't think anyone is suggesting we shouldn't do the right thing in
OE-Core? :)

I merged the original patch on the grounds that its was an improvement
to the situation. We've identified a better improvement so can someone
please send me the patch and I'll likely merge that too.

FWIW, I agree that powertop should really be under /usr/...

Cheers,

Richard
Koen Kooi - July 8, 2011, 3:57 p.m.
Op 8 jul. 2011 om 16:00 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:

> On Fri, 2011-07-08 at 08:25 +0100, Koen Kooi wrote:
>> 
>> Op 8 jul. 2011 om 02:40 heeft "Cui, Dexuan" <dexuan.cui@intel.com> het volgende geschreven:
>> 
>>> Tom Rini wrote:
>>>> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
>>>>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
>>>>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
>>>>> /usr/bin, we would run the version offered by busybox, which has a
>>>>> very limited function (e.g., no parameter is accepted) and this
>>>>> causes trouble to eclipse plugin. 
>>>>> 
>>>>> We can use update-alternatives for powertop with higher priority to
>>>>> resolve the issue. 
>>>>> 
>>>>> Fixes [YOCTO #1208]
>>>>> 
>>>>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
>>>> 
>>>> This fix seems a bit incomplete.  Why is busybox putting powertop into
>>>> /bin when it almost certainly belongs in /usr/bin like the real recipe
>>>> was placing it.  busybox needs a fix here too.
>>> Thanks for the comment! 
>>> I was hesitant about fixing busybox as I wasn't sure if it's worthy
>> to make a patch to only fix the path for busybox. I don't know why
>> busybox puts it into /bin. I think the best place may be /usr/sbin/. 
>>> A little unluckily this patch to powertop has been already in poky
>> master... So maybe we could try to fix the recipes in future, e.g.,
>> when upgrading them.
>> 
>> we should do the right thing in oe-core,  the poky people can clean up on their own. 
> 
> I don't think anyone is suggesting we shouldn't do the right thing in
> OE-Core? :)
> 
> I merged the original patch on the grounds that its was an improvement
> to the situation. We've identified a better improvement so can someone
> please send me the patch and I'll likely merge that too.

the email makes it seem that the patch was merged into poky, but not oe-core.  When reading it like that the proposal involved merging the 'incomplete' patch for the sake of keeping poky and oe-core in sync

> 
> FWIW, I agree that powertop should really be under /usr/...
> 
> Cheers,
> 
> Richard
> 
> 
> 
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
Richard Purdie - July 8, 2011, 4:43 p.m.
On Fri, 2011-07-08 at 16:57 +0100, Koen Kooi wrote:
> 
> Op 8 jul. 2011 om 16:00 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
> 
> > On Fri, 2011-07-08 at 08:25 +0100, Koen Kooi wrote:
> >> 
> >> Op 8 jul. 2011 om 02:40 heeft "Cui, Dexuan" <dexuan.cui@intel.com> het volgende geschreven:
> >> 
> >>> Tom Rini wrote:
> >>>> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
> >>>>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
> >>>>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
> >>>>> /usr/bin, we would run the version offered by busybox, which has a
> >>>>> very limited function (e.g., no parameter is accepted) and this
> >>>>> causes trouble to eclipse plugin. 
> >>>>> 
> >>>>> We can use update-alternatives for powertop with higher priority to
> >>>>> resolve the issue. 
> >>>>> 
> >>>>> Fixes [YOCTO #1208]
> >>>>> 
> >>>>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
> >>>> 
> >>>> This fix seems a bit incomplete.  Why is busybox putting powertop into
> >>>> /bin when it almost certainly belongs in /usr/bin like the real recipe
> >>>> was placing it.  busybox needs a fix here too.
> >>> Thanks for the comment! 
> >>> I was hesitant about fixing busybox as I wasn't sure if it's worthy
> >> to make a patch to only fix the path for busybox. I don't know why
> >> busybox puts it into /bin. I think the best place may be /usr/sbin/. 
> >>> A little unluckily this patch to powertop has been already in poky
> >> master... So maybe we could try to fix the recipes in future, e.g.,
> >> when upgrading them.
> >> 
> >> we should do the right thing in oe-core,  the poky people can clean up on their own. 
> > 
> > I don't think anyone is suggesting we shouldn't do the right thing in
> > OE-Core? :)
> > 
> > I merged the original patch on the grounds that its was an improvement
> > to the situation. We've identified a better improvement so can someone
> > please send me the patch and I'll likely merge that too.
> 
> the email makes it seem that the patch was merged into poky, but not
> oe-core.  When reading it like that the proposal involved merging the
> 'incomplete' patch for the sake of keeping poky and oe-core in sync

The OE-Core component of Poky always stays in sync now...

Cheers,

Richard
Koen Kooi - July 8, 2011, 8:45 p.m.
Op 8 jul. 2011 om 18:43 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:

> On Fri, 2011-07-08 at 16:57 +0100, Koen Kooi wrote:
>> 
>> Op 8 jul. 2011 om 16:00 heeft Richard Purdie <richard.purdie@linuxfoundation.org> het volgende geschreven:
>> 
>>> On Fri, 2011-07-08 at 08:25 +0100, Koen Kooi wrote:
>>>> 
>>>> Op 8 jul. 2011 om 02:40 heeft "Cui, Dexuan" <dexuan.cui@intel.com> het volgende geschreven:
>>>> 
>>>>> Tom Rini wrote:
>>>>>> On 07/07/2011 01:39 AM, Dexuan Cui wrote:
>>>>>>> busybox-1.18.4 installs /bin/powertop and the powertop recipe
>>>>>>> installs /usr/bin/powertop. So, in PATH, if /bin appears before
>>>>>>> /usr/bin, we would run the version offered by busybox, which has a
>>>>>>> very limited function (e.g., no parameter is accepted) and this
>>>>>>> causes trouble to eclipse plugin. 
>>>>>>> 
>>>>>>> We can use update-alternatives for powertop with higher priority to
>>>>>>> resolve the issue. 
>>>>>>> 
>>>>>>> Fixes [YOCTO #1208]
>>>>>>> 
>>>>>>> Signed-off-by: Dexuan Cui <dexuan.cui@intel.com>
>>>>>> 
>>>>>> This fix seems a bit incomplete.  Why is busybox putting powertop into
>>>>>> /bin when it almost certainly belongs in /usr/bin like the real recipe
>>>>>> was placing it.  busybox needs a fix here too.
>>>>> Thanks for the comment! 
>>>>> I was hesitant about fixing busybox as I wasn't sure if it's worthy
>>>> to make a patch to only fix the path for busybox. I don't know why
>>>> busybox puts it into /bin. I think the best place may be /usr/sbin/. 
>>>>> A little unluckily this patch to powertop has been already in poky
>>>> master... So maybe we could try to fix the recipes in future, e.g.,
>>>> when upgrading them.
>>>> 
>>>> we should do the right thing in oe-core,  the poky people can clean up on their own. 
>>> 
>>> I don't think anyone is suggesting we shouldn't do the right thing in
>>> OE-Core? :)
>>> 
>>> I merged the original patch on the grounds that its was an improvement
>>> to the situation. We've identified a better improvement so can someone
>>> please send me the patch and I'll likely merge that too.
>> 
>> the email makes it seem that the patch was merged into poky, but not
>> oe-core.  When reading it like that the proposal involved merging the
>> 'incomplete' patch for the sake of keeping poky and oe-core in sync
> 
> The OE-Core component of Poky always stays in sync now...

I realized that later, I'm way too tired to think properly. sorry about the fuss


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

Patch

diff --git a/meta/recipes-kernel/powertop/powertop_1.13.bb b/meta/recipes-kernel/powertop/powertop_1.13.bb
index dff4fcd..472a2bb 100644
--- a/meta/recipes-kernel/powertop/powertop_1.13.bb
+++ b/meta/recipes-kernel/powertop/powertop_1.13.bb
@@ -11,7 +11,7 @@  DEPENDS = "virtual/libintl ncurses"
 # powertop 1.13 needs lspci
 RDEPENDS_${PN} = "pciutils"
 
-PR = "r0"
+PR = "r1"
 
 SRC_URI = "http://www.lesswatts.org/projects/powertop/download/powertop-${PV}.tar.gz \
            file://stub_out_the_ncurses_calls_in_dump_mode.patch"
@@ -22,6 +22,12 @@  SRC_URI[sha256sum] = "2bc866089496877dd26d2d316ad5763ab8ecb5e28aefba44bc5d355dcd
 CFLAGS += "${LDFLAGS}"
 EXTRA_OEMAKE = "VERSION=\"${PV}\""
 
+inherit update-alternatives
+ALTERNATIVE_NAME = "powertop"
+ALTERNATIVE_PATH = "${bindir}/powertop"
+ALTERNATIVE_LINK = "${base_bindir}/powertop"
+ALTERNATIVE_PRIORITY = "100"
+
 do_configure() {
 	# We do not build ncurses with wide char support
 	sed -i -e "s/lncursesw/lncurses/" ${S}/Makefile