Patchwork util-linux: Use u-a for getopt

login
register
mail settings
Submitter Martin Jansa
Date March 20, 2013, 3:09 p.m.
Message ID <1363792150-24988-1-git-send-email-Martin.Jansa@gmail.com>
Download mbox | patch
Permalink /patch/46565/
State Accepted
Commit 27296bd8e6759c2f94477c14787837a761f16163
Headers show

Comments

Martin Jansa - March 20, 2013, 3:09 p.m.
* when enable busybox installs getopt to ${base_bindir} and
  util-linux to ${bindir}, so there is no file conflict, but
  because busybox implementation does not support --long used
  by lsb_release (which RDEPENDS on util-linux) we need to use
  util-linux getopt even when busybox defconfig has it enabled

Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
---
 meta/recipes-core/util-linux/util-linux.inc       | 6 ++++--
 meta/recipes-core/util-linux/util-linux_2.22.2.bb | 2 +-
 2 files changed, 5 insertions(+), 3 deletions(-)
Bernhard Reutner-Fischer - March 22, 2013, 8:08 a.m.
On 20 March 2013 16:09:10 Martin Jansa <martin.jansa@gmail.com> wrote:
> * when enable busybox installs getopt to ${base_bindir} and
>   util-linux to ${bindir}, so there is no file conflict, but
>   because busybox implementation does not support --long used

Do you mean that Busybox' getopt does not support long options?
If so, enable FEATURE_GETOPT_LONG in busybox instead of this patch?

Thanks,
>   by lsb_release (which RDEPENDS on util-linux) we need to use
>   util-linux getopt even when busybox defconfig has it enabled
>
> Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> ---
>  meta/recipes-core/util-linux/util-linux.inc       | 6 ++++--
>  meta/recipes-core/util-linux/util-linux_2.22.2.bb | 2 +-
>  2 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/meta/recipes-core/util-linux/util-linux.inc 
> b/meta/recipes-core/util-linux/util-linux.inc
> index 41a5471..c75d318 100644
> --- a/meta/recipes-core/util-linux/util-linux.inc
> +++ b/meta/recipes-core/util-linux/util-linux.inc
> @@ -110,7 +110,7 @@ do_install () {
>
>          sbinprogs="agetty ctrlaltdel cfdisk vipw vigr"
>          sbinprogs_a="pivot_root hwclock mkswap mkfs.minix fsck.minix losetup 
>          swapon fdisk readprofile fsck blkid blockdev"
> -        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp 
> renice utmpdump wall setsid chrt flock"
> +        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp 
> renice utmpdump wall setsid chrt flock getopt"
>          binprogs_a="dmesg kill more umount mount login reset"
>
>          if [ "${base_sbindir}" != "${sbindir}" ]; then
> @@ -146,7 +146,7 @@ ALTERNATIVE_PRIORITY = "100"
>
>  ALTERNATIVE_${PN}  = "dmesg kill more mkswap blockdev pivot_root"
>  ALTERNATIVE_${PN} += "mkfs.minix hexdump last logger mesg renice wall"
> -ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject"
> +ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject getopt"
>
>  ALTERNATIVE_LINK_NAME[dmesg] = "${base_bindir}/dmesg"
>  ALTERNATIVE_LINK_NAME[kill] = "${base_bindir}/kill"
> @@ -156,6 +156,8 @@ ALTERNATIVE_LINK_NAME[blockdev] = 
> "${base_sbindir}/blockdev"
>  ALTERNATIVE_LINK_NAME[pivot_root] = "${base_sbindir}/pivot_root"
>  ALTERNATIVE_LINK_NAME[mkfs.minix] = "${base_sbindir}/mkfs.minix"
>  ALTERNATIVE_LINK_NAME[eject] = "${bindir}/eject"
> +ALTERNATIVE_LINK_NAME[getopt] = "${base_bindir}/getopt"
> +ALTERNATIVE_TARGET[getopt] = "${bindir}/getopt"
>
>  ALTERNATIVE_${PN}-doc = "mountpoint.1 last.1 mesg.1 wall.1 sulogin.8 
>  utmpdump.1"
>
> diff --git a/meta/recipes-core/util-linux/util-linux_2.22.2.bb 
> b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> index 9b94a47..43a55a6 100644
> --- a/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> +++ b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> @@ -1,5 +1,5 @@
>  MAJOR_VERSION = "2.22"
> -PR = "r2"
> +PR = "r3"
>  require util-linux.inc
>
>  SRC_URI += "file://util-linux-ng-replace-siginterrupt.patch \
> --
> 1.8.1.5
>
>
> _______________________________________________
> Openembedded-core mailing list
> Openembedded-core@lists.openembedded.org
> http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core


Sent with AquaMail for Android
http://www.aqua-mail.com
Martin Jansa - March 22, 2013, 9:15 a.m.
On Fri, Mar 22, 2013 at 09:08:34AM +0100, Bernhard Reutner-Fischer wrote:
> On 20 March 2013 16:09:10 Martin Jansa <martin.jansa@gmail.com> wrote:
> > * when enable busybox installs getopt to ${base_bindir} and
> >   util-linux to ${bindir}, so there is no file conflict, but
> >   because busybox implementation does not support --long used
> 
> Do you mean that Busybox' getopt does not support long options?
> If so, enable FEATURE_GETOPT_LONG in busybox instead of this patch?

Having lsb working correctly even when someone has bad busybox defconfig
is right thing to do, isn't it?

busybox's getopt supports --longoptions, but util-linux's also supports
--long alias for that and that's what lsb is using

$ getopt --longoptions
getopt: option '--longoptions' requires an argument
Try `getopt --help' for more information.
$ getopt --long
getopt: option '--longoptions' requires an argument
Try `getopt --help' for more information.

busybox (IIRC even with FEATURE_GETOPT_LONG enabled) reports
getopt: unrecognized option '--long'
BusyBox v1.20.2 (2013-03-16 17:45:30 PDT) multi-call binary.

lsb already RDEPENDS on util-linux, changing lsb to use --longoptions
would be good, but having /bin/getopt and /usr/bin/getopt without u-a 
to select preferred one is bad, that's why I used this solution instead.

> >   by lsb_release (which RDEPENDS on util-linux) we need to use
> >   util-linux getopt even when busybox defconfig has it enabled
> >
> > Signed-off-by: Martin Jansa <Martin.Jansa@gmail.com>
> > ---
> >  meta/recipes-core/util-linux/util-linux.inc       | 6 ++++--
> >  meta/recipes-core/util-linux/util-linux_2.22.2.bb | 2 +-
> >  2 files changed, 5 insertions(+), 3 deletions(-)
> >
> > diff --git a/meta/recipes-core/util-linux/util-linux.inc 
> > b/meta/recipes-core/util-linux/util-linux.inc
> > index 41a5471..c75d318 100644
> > --- a/meta/recipes-core/util-linux/util-linux.inc
> > +++ b/meta/recipes-core/util-linux/util-linux.inc
> > @@ -110,7 +110,7 @@ do_install () {
> >
> >          sbinprogs="agetty ctrlaltdel cfdisk vipw vigr"
> >          sbinprogs_a="pivot_root hwclock mkswap mkfs.minix fsck.minix losetup 
> >          swapon fdisk readprofile fsck blkid blockdev"
> > -        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp 
> > renice utmpdump wall setsid chrt flock"
> > +        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp 
> > renice utmpdump wall setsid chrt flock getopt"
> >          binprogs_a="dmesg kill more umount mount login reset"
> >
> >          if [ "${base_sbindir}" != "${sbindir}" ]; then
> > @@ -146,7 +146,7 @@ ALTERNATIVE_PRIORITY = "100"
> >
> >  ALTERNATIVE_${PN}  = "dmesg kill more mkswap blockdev pivot_root"
> >  ALTERNATIVE_${PN} += "mkfs.minix hexdump last logger mesg renice wall"
> > -ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject"
> > +ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject getopt"
> >
> >  ALTERNATIVE_LINK_NAME[dmesg] = "${base_bindir}/dmesg"
> >  ALTERNATIVE_LINK_NAME[kill] = "${base_bindir}/kill"
> > @@ -156,6 +156,8 @@ ALTERNATIVE_LINK_NAME[blockdev] = 
> > "${base_sbindir}/blockdev"
> >  ALTERNATIVE_LINK_NAME[pivot_root] = "${base_sbindir}/pivot_root"
> >  ALTERNATIVE_LINK_NAME[mkfs.minix] = "${base_sbindir}/mkfs.minix"
> >  ALTERNATIVE_LINK_NAME[eject] = "${bindir}/eject"
> > +ALTERNATIVE_LINK_NAME[getopt] = "${base_bindir}/getopt"
> > +ALTERNATIVE_TARGET[getopt] = "${bindir}/getopt"
> >
> >  ALTERNATIVE_${PN}-doc = "mountpoint.1 last.1 mesg.1 wall.1 sulogin.8 
> >  utmpdump.1"
> >
> > diff --git a/meta/recipes-core/util-linux/util-linux_2.22.2.bb 
> > b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> > index 9b94a47..43a55a6 100644
> > --- a/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> > +++ b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
> > @@ -1,5 +1,5 @@
> >  MAJOR_VERSION = "2.22"
> > -PR = "r2"
> > +PR = "r3"
> >  require util-linux.inc
> >
> >  SRC_URI += "file://util-linux-ng-replace-siginterrupt.patch \
> > --
> > 1.8.1.5
> >
> >
> > _______________________________________________
> > Openembedded-core mailing list
> > Openembedded-core@lists.openembedded.org
> > http://lists.linuxtogo.org/cgi-bin/mailman/listinfo/openembedded-core
> 
> 
> Sent with AquaMail for Android
> http://www.aqua-mail.com
> 
>
Bernhard Reutner-Fischer - March 22, 2013, 11:51 a.m.
On 22 March 2013 10:15, Martin Jansa <martin.jansa@gmail.com> wrote:
> On Fri, Mar 22, 2013 at 09:08:34AM +0100, Bernhard Reutner-Fischer wrote:
>> On 20 March 2013 16:09:10 Martin Jansa <martin.jansa@gmail.com> wrote:
>> > * when enable busybox installs getopt to ${base_bindir} and
>> >   util-linux to ${bindir}, so there is no file conflict, but
>> >   because busybox implementation does not support --long used
>>
>> Do you mean that Busybox' getopt does not support long options?
>> If so, enable FEATURE_GETOPT_LONG in busybox instead of this patch?
>
> Having lsb working correctly even when someone has bad busybox defconfig
> is right thing to do, isn't it?

Probably. That aside, --long being rejected smells like a bug.
>
> busybox's getopt supports --longoptions, but util-linux's also supports
> --long alias for that and that's what lsb is using
>
> $ getopt --longoptions
> getopt: option '--longoptions' requires an argument
> Try `getopt --help' for more information.
> $ getopt --long
> getopt: option '--longoptions' requires an argument
> Try `getopt --help' for more information.
>
> busybox (IIRC even with FEATURE_GETOPT_LONG enabled) reports
> getopt: unrecognized option '--long'
> BusyBox v1.20.2 (2013-03-16 17:45:30 PDT) multi-call binary.

That's odd, it seems to work for me?
$ grep GETOPT .config
CONFIG_GETOPT=y
CONFIG_FEATURE_GETOPT_LONG=y
CONFIG_ASH_GETOPTS=y
$ ./busybox getopt --long 2>&1 | head -n2
getopt: option '--longoptions' requires an argument
BusyBox v1.22.0.git (2013-03-22 11:20:17 CET) multi-call binary.

>
> lsb already RDEPENDS on util-linux, changing lsb to use --longoptions
> would be good, but having /bin/getopt and /usr/bin/getopt without u-a
> to select preferred one is bad, that's why I used this solution instead.

Yes, i see what you mean. Still,
a) it should not be required for lsb to RDEPEND on util-linux since
   busybox supposedly should work fine too, iff configured correctly, of course.
b) busybox's getopt --lo (or other substrings of the
-l,--longoptions=LOPT[,...] opt)
   should work correctly.

I'd like to know why you trip b) above.
Do you have CONFIG_FEATURE_GETOPT_LONG set?
If so, what libc are you using?

Thanks,
Martin Jansa - March 22, 2013, 1:06 p.m.
On Fri, Mar 22, 2013 at 12:51:42PM +0100, Bernhard Reutner-Fischer wrote:
> On 22 March 2013 10:15, Martin Jansa <martin.jansa@gmail.com> wrote:
> > On Fri, Mar 22, 2013 at 09:08:34AM +0100, Bernhard Reutner-Fischer wrote:
> >> On 20 March 2013 16:09:10 Martin Jansa <martin.jansa@gmail.com> wrote:
> >> > * when enable busybox installs getopt to ${base_bindir} and
> >> >   util-linux to ${bindir}, so there is no file conflict, but
> >> >   because busybox implementation does not support --long used
> >>
> >> Do you mean that Busybox' getopt does not support long options?
> >> If so, enable FEATURE_GETOPT_LONG in busybox instead of this patch?
> >
> > Having lsb working correctly even when someone has bad busybox defconfig
> > is right thing to do, isn't it?
> 
> Probably. That aside, --long being rejected smells like a bug.
> >
> > busybox's getopt supports --longoptions, but util-linux's also supports
> > --long alias for that and that's what lsb is using
> >
> > $ getopt --longoptions
> > getopt: option '--longoptions' requires an argument
> > Try `getopt --help' for more information.
> > $ getopt --long
> > getopt: option '--longoptions' requires an argument
> > Try `getopt --help' for more information.
> >
> > busybox (IIRC even with FEATURE_GETOPT_LONG enabled) reports
> > getopt: unrecognized option '--long'
> > BusyBox v1.20.2 (2013-03-16 17:45:30 PDT) multi-call binary.
> 
> That's odd, it seems to work for me?
> $ grep GETOPT .config
> CONFIG_GETOPT=y
> CONFIG_FEATURE_GETOPT_LONG=y
> CONFIG_ASH_GETOPTS=y
> $ ./busybox getopt --long 2>&1 | head -n2
> getopt: option '--longoptions' requires an argument
> BusyBox v1.22.0.git (2013-03-22 11:20:17 CET) multi-call binary.

FWIW I have older version (from danny), but the same problem about not
using u-a is with master.

> > lsb already RDEPENDS on util-linux, changing lsb to use --longoptions
> > would be good, but having /bin/getopt and /usr/bin/getopt without u-a
> > to select preferred one is bad, that's why I used this solution instead.
> 
> Yes, i see what you mean. Still,
> a) it should not be required for lsb to RDEPEND on util-linux since
>    busybox supposedly should work fine too, iff configured correctly, of course.

Yes but I'm not changing that in this commit :) All I want is to make
getopt provider deterministic in image. default busybox defconfig does
not have GETOPT enabled at all:
meta/recipes-core/busybox/busybox-1.20.2/defconfig:# CONFIG_GETOPT is not set
meta/recipes-core/busybox/busybox-1.20.2/defconfig:# CONFIG_FEATURE_GETOPT_LONG is not set

so if someone wants to remove util-linux from lsb RDEPENDS, then he
should also update default config.

> b) busybox's getopt --lo (or other substrings of the
> -l,--longoptions=LOPT[,...] opt)
>    should work correctly.
> 
> I'd like to know why you trip b) above.
> Do you have CONFIG_FEATURE_GETOPT_LONG set?

I've rechecked and CONFIG_FEATURE_GETOPT_LONG is not set, only
CONFIG_GETOPT is. But this commit is still valid, I want to use getopt
from util-linux when util-linux is installed.

> If so, what libc are you using?

private external
Bernhard Reutner-Fischer - March 23, 2013, 8:54 a.m.
On 22 March 2013 14:06:48 Martin Jansa <martin.jansa@gmail.com> wrote:
> On Fri, Mar 22, 2013 at 12:51:42PM +0100, Bernhard Reutner-Fischer wrote:
> > On 22 March 2013 10:15, Martin Jansa <martin.jansa@gmail.com> wrote:
> > > On Fri, Mar 22, 2013 at 09:08:34AM +0100, Bernhard Reutner-Fischer wrote:
> > >> On 20 March 2013 16:09:10 Martin Jansa <martin.jansa@gmail.com> wrote:
> > >> > * when enable busybox installs getopt to ${base_bindir} and
> > >> >   util-linux to ${bindir}, so there is no file conflict, but
> > >> >   because busybox implementation does not support --long used
> > >>
> > >> Do you mean that Busybox' getopt does not support long options?
> > >> If so, enable FEATURE_GETOPT_LONG in busybox instead of this patch?
> > >
> > > Having lsb working correctly even when someone has bad busybox defconfig
> > > is right thing to do, isn't it?
> >
> > Probably. That aside, --long being rejected smells like a bug.
> > >
> > > busybox's getopt supports --longoptions, but util-linux's also supports
> > > --long alias for that and that's what lsb is using
> > >
> > > $ getopt --longoptions
> > > getopt: option '--longoptions' requires an argument
> > > Try `getopt --help' for more information.
> > > $ getopt --long
> > > getopt: option '--longoptions' requires an argument
> > > Try `getopt --help' for more information.
> > >
> > > busybox (IIRC even with FEATURE_GETOPT_LONG enabled) reports
> > > getopt: unrecognized option '--long'
> > > BusyBox v1.20.2 (2013-03-16 17:45:30 PDT) multi-call binary.
> >
> > That's odd, it seems to work for me?
> > $ grep GETOPT .config
> > CONFIG_GETOPT=y
> > CONFIG_FEATURE_GETOPT_LONG=y
> > CONFIG_ASH_GETOPTS=y
> > $ ./busybox getopt --long 2>&1 | head -n2
> > getopt: option '--longoptions' requires an argument
> > BusyBox v1.22.0.git (2013-03-22 11:20:17 CET) multi-call binary.
>
> FWIW I have older version (from danny), but the same problem about not
> using u-a is with master.
>
> > > lsb already RDEPENDS on util-linux, changing lsb to use --longoptions
> > > would be good, but having /bin/getopt and /usr/bin/getopt without u-a
> > > to select preferred one is bad, that's why I used this solution instead.
> >
> > Yes, i see what you mean. Still,
> > a) it should not be required for lsb to RDEPEND on util-linux since
> >    busybox supposedly should work fine too, iff configured correctly, 
> of course.
>
> Yes but I'm not changing that in this commit :) All I want is to make

Yep, OK. :)

> getopt provider deterministic in image. default busybox defconfig does

It should be deterministic, agree.

> not have GETOPT enabled at all:
> meta/recipes-core/busybox/busybox-1.20.2/defconfig:# CONFIG_GETOPT is not set
> meta/recipes-core/busybox/busybox-1.20.2/defconfig:# 
> CONFIG_FEATURE_GETOPT_LONG is not set


>
> so if someone wants to remove util-linux from lsb RDEPENDS, then he
> should also update default config.
>
> > b) busybox's getopt --lo (or other substrings of the
> > -l,--longoptions=LOPT[,...] opt)
> >    should work correctly.
> >
> > I'd like to know why you trip b) above.
> > Do you have CONFIG_FEATURE_GETOPT_LONG set?
>
> I've rechecked and CONFIG_FEATURE_GETOPT_LONG is not set, only
> CONFIG_GETOPT is. But this commit is still valid, I want to use getopt

Given how "fat" any OE distro (except micro) is,  I would strongly 
suggest to turn on FEATURE_GETOPT_LONG anyway.

That is of course not relevant for this very patch of yours, just a 
general remark.
Cheers,

> from util-linux when util-linux is installed.
>
> > If so, what libc are you using?
>
> private external
>
> --
> Martin 'JaMa' Jansa     jabber: Martin.Jansa@gmail.com


Sent with AquaMail for Android
http://www.aqua-mail.com

Patch

diff --git a/meta/recipes-core/util-linux/util-linux.inc b/meta/recipes-core/util-linux/util-linux.inc
index 41a5471..c75d318 100644
--- a/meta/recipes-core/util-linux/util-linux.inc
+++ b/meta/recipes-core/util-linux/util-linux.inc
@@ -110,7 +110,7 @@  do_install () {
 
         sbinprogs="agetty ctrlaltdel cfdisk vipw vigr"
         sbinprogs_a="pivot_root hwclock mkswap mkfs.minix fsck.minix losetup swapon fdisk readprofile fsck blkid blockdev"
-        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp renice utmpdump wall setsid chrt flock"
+        usrbinprogs_a="chfn chsh hexdump last logger mesg newgrp renice utmpdump wall setsid chrt flock getopt"
         binprogs_a="dmesg kill more umount mount login reset"
 
         if [ "${base_sbindir}" != "${sbindir}" ]; then
@@ -146,7 +146,7 @@  ALTERNATIVE_PRIORITY = "100"
 
 ALTERNATIVE_${PN}  = "dmesg kill more mkswap blockdev pivot_root"
 ALTERNATIVE_${PN} += "mkfs.minix hexdump last logger mesg renice wall"
-ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject"
+ALTERNATIVE_${PN} += "setsid chrt flock hwclock utmpdump eject getopt"
 
 ALTERNATIVE_LINK_NAME[dmesg] = "${base_bindir}/dmesg"
 ALTERNATIVE_LINK_NAME[kill] = "${base_bindir}/kill"
@@ -156,6 +156,8 @@  ALTERNATIVE_LINK_NAME[blockdev] = "${base_sbindir}/blockdev"
 ALTERNATIVE_LINK_NAME[pivot_root] = "${base_sbindir}/pivot_root"
 ALTERNATIVE_LINK_NAME[mkfs.minix] = "${base_sbindir}/mkfs.minix"
 ALTERNATIVE_LINK_NAME[eject] = "${bindir}/eject"
+ALTERNATIVE_LINK_NAME[getopt] = "${base_bindir}/getopt"
+ALTERNATIVE_TARGET[getopt] = "${bindir}/getopt"
 
 ALTERNATIVE_${PN}-doc = "mountpoint.1 last.1 mesg.1 wall.1 sulogin.8 utmpdump.1"
 
diff --git a/meta/recipes-core/util-linux/util-linux_2.22.2.bb b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
index 9b94a47..43a55a6 100644
--- a/meta/recipes-core/util-linux/util-linux_2.22.2.bb
+++ b/meta/recipes-core/util-linux/util-linux_2.22.2.bb
@@ -1,5 +1,5 @@ 
 MAJOR_VERSION = "2.22"
-PR = "r2"
+PR = "r3"
 require util-linux.inc
 
 SRC_URI += "file://util-linux-ng-replace-siginterrupt.patch \