[OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Laurentiu Palcu
A normal user does not have /sbin in its PATH, by default, so having the
entire path here allows the correct execution when run as regular user.

[YOCTO #4345]

Signed-off-by: Laurentiu Palcu laurentiu.pa...@intel.com
---
 .../shutdown-desktop/shutdown-desktop.bb   |2 ++
 1 file changed, 2 insertions(+)

diff --git a/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb 
b/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
index c5096c1..9e283e4 100644
--- a/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
+++ b/meta/recipes-sato/shutdown-desktop/shutdown-desktop.bb
@@ -18,6 +18,8 @@ pkg_postinst_${PN} () {
 grep -q qemuarm $D${sysconfdir}/hostname  \
 sed -i $D${datadir}/applications/shutdown.desktop -e 
's/^Exec=halt/Exec=reboot/' \
 || true
+
+sed -i $D${datadir}/applications/shutdown.desktop -e 
's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'
 }
 
 inherit allarch
-- 
1.7.9.5

___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Burton, Ross
On 4 July 2013 11:58, Laurentiu Palcu laurentiu.pa...@intel.com wrote:
 +sed -i $D${datadir}/applications/shutdown.desktop -e 
 's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'

Doing this in postinst is pretty nasty, instead change the desktop
file to contain something like @SBIN@ and run it through sed in
do_install.

Ross
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Laurentiu Palcu


On 07/04/2013 05:58 PM, Burton, Ross wrote:
 On 4 July 2013 11:58, Laurentiu Palcu laurentiu.pa...@intel.com wrote:
 +sed -i $D${datadir}/applications/shutdown.desktop -e 
 's#^Exec=\(.*\)#Exec=${base_sbindir}/\1#'
 
 Doing this in postinst is pretty nasty, 
Nasty, in what way? How is this line different from the previous line in
the same postinstall? That one is also using sed to change 'halt' to
'reboot' for qemuarm. Is it the sed you're worried about? This
postinstall is executed on host anyway, at do_rootfs().

 instead change the desktop
 file to contain something like @SBIN@ and run it through sed in
 do_install.
Even though the change you're proposing is OK, involves changing the
desktop file to add the @SBIN@ pattern and move the sed line to
do_install(). Compared to having one single line added in the
postinstall... I would choose the latter, unless you elaborate on what
do you actually mean by nasty.

Laurentiu
 
 Ross
 
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Burton, Ross
On 4 July 2013 16:39, Laurentiu Palcu laurentiu.pa...@intel.com wrote:
 Doing this in postinst is pretty nasty,
 Nasty, in what way? How is this line different from the previous line in
 the same postinstall? That one is also using sed to change 'halt' to
 'reboot' for qemuarm. Is it the sed you're worried about? This
 postinstall is executed on host anyway, at do_rootfs().

The existing sed to manipulate the exec line is arguably a better
solution to making this package machine-specific and special-casing
just qemuarm.  (I'm not sure why it special-cases qemuarm, and
751212d5effdceab91d95705e647cf07e6820940 where it was introduced
doesn't clarify matters, but we'll ignore that for now).  Arguably,
but I'm not sure I agree.  Anyway, you've a path that is known at
build time so we can fix it at build time.  Instead of using install,
you can use sed.  Just because we *can* run postinst scripts when
generating the rootfs doesn't mean we *should*, do_install is for
installing the files we're going to package, and postinst is for
runtime changes that can't happen at any other time.

 instead change the desktop
 file to contain something like @SBIN@ and run it through sed in
 do_install.
 Even though the change you're proposing is OK, involves changing the
 desktop file to add the @SBIN@ pattern and move the sed line to
 do_install(). Compared to having one single line added in the
 postinstall... I would choose the latter, unless you elaborate on what
 do you actually mean by nasty.

We're not being invoiced based on the number of patch hunks, and
changing /sbin to @SBIN@ and install to sed isn't exactly a massive
time sink.

Ross
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Tomas Frydrych
On 04/07/13 16:39, Laurentiu Palcu wrote:
 On 07/04/2013 05:58 PM, Burton, Ross wrote:
 On 4 July 2013 11:58, Laurentiu Palcu laurentiu.pa...@intel.com wrote:
 Even though the change you're proposing is OK, involves changing the
 desktop file to add the @SBIN@ pattern and move the sed line to
 do_install(). Compared to having one single line added in the
 postinstall... I would choose the latter, unless you elaborate on what
 do you actually mean by nasty.

Post install scripts should be avoided whenever possible and used only
for things that cannot be done at build or install time. This clearly
can be done at install time, so it should.

Tomas

-- 
http://sleepfive.com
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core


Re: [OE-core] [PATCH v2 5/5] shutdown-desktop: give entire path in Exec field

2013-07-04 Thread Laurentiu Palcu
On Thu, Jul 04, 2013 at 04:56:45PM +0100, Tomas Frydrych wrote:
 On 04/07/13 16:39, Laurentiu Palcu wrote:
  On 07/04/2013 05:58 PM, Burton, Ross wrote:
  On 4 July 2013 11:58, Laurentiu Palcu laurentiu.pa...@intel.com wrote:
  Even though the change you're proposing is OK, involves changing the
  desktop file to add the @SBIN@ pattern and move the sed line to
  do_install(). Compared to having one single line added in the
  postinstall... I would choose the latter, unless you elaborate on what
  do you actually mean by nasty.
 
 Post install scripts should be avoided whenever possible and used only
 for things that cannot be done at build or install time. This clearly
 can be done at install time, so it should.
I agree. However, in this particular case I'm just being consistent with
what was already there. I would have personally moved the entire
postinstall to do_install, but the previous sed cannot be moved. So,
since we're stuck with that sed there, this one is just a harmless
addition. This is what I mean!

Laurentiu

 
 Tomas
 
 -- 
 http://sleepfive.com
 ___
 Openembedded-core mailing list
 Openembedded-core@lists.openembedded.org
 http://lists.openembedded.org/mailman/listinfo/openembedded-core
___
Openembedded-core mailing list
Openembedded-core@lists.openembedded.org
http://lists.openembedded.org/mailman/listinfo/openembedded-core