| Message ID | 1335435324-18792-2-git-send-email-gustavo@zacarias.com.ar |
|---|---|
| State | Accepted |
| Commit | d7616cc9be8b1c7d91df9133dac24a59bb2e19c8 |
| Headers | show |
On 07:15 Thu 26 Apr , Gustavo Zacarias wrote: > Since busybox 1.20+ includes a lsof applet make sure lsof gets built > after busybox so that we get the full-blown version if both are enabled. > Also hide the lsof package unless BUSYBOX_SHOW_OTHERS is true. > > Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> > --- > package/Config.in | 2 ++ > package/lsof/lsof.mk | 3 +++ > 2 files changed, 5 insertions(+), 0 deletions(-) > > diff --git a/package/Config.in b/package/Config.in > index 64fce62..7f9b234 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -24,7 +24,9 @@ source "package/dmalloc/Config.in" > source "package/kexec/Config.in" > source "package/latencytop/Config.in" > source "package/lmbench/Config.in" > +if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > source "package/lsof/Config.in" put a depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS in lsof/Config.in Best Regards, J.
Le Thu, 26 Apr 2012 16:08:40 +0200, Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a écrit : > > +if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > > source "package/lsof/Config.in" > put a depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > > in lsof/Config.in No, that's not what we do for all other packages. Gustavo's patch is correct with regard to the current guidelines. Now, whether we should change those guidelines to something else is a different story. But if we change the guideline, it should be changed for all packages and not just for lsof. Regards, Thomas
On 16:45 Thu 26 Apr , Thomas Petazzoni wrote: > Le Thu, 26 Apr 2012 16:08:40 +0200, > Jean-Christophe PLAGNIOL-VILLARD <plagnioj@jcrosoft.com> a écrit : > > > > +if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > > > source "package/lsof/Config.in" > > put a depends on BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > > > > in lsof/Config.in > > No, that's not what we do for all other packages. Gustavo's patch is > correct with regard to the current guidelines. > > Now, whether we should change those guidelines to something else is a > different story. But if we change the guideline, it should be changed > for all packages and not just for lsof. package/kmod/Config.in is already like this and it's more clean Best Regards, J.
On Friday 27 April 2012 02:55:58 Jean-Christophe PLAGNIOL-VILLARD wrote: > > No, that's not what we do for all other packages. Gustavo's patch is > > correct with regard to the current guidelines. > > > > Now, whether we should change those guidelines to something else is a > > different story. But if we change the guideline, it should be changed > > for all packages and not just for lsof. > package/kmod/Config.in is already like this > > and it's more clean I agree that it's better to do it in the package's Config.in. And changing the guideline doesn't automatically mean that all packages have to change. Only new packages (or patches) must follow the guideline. Regards, Arnout
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes: Hi, >> > Now, whether we should change those guidelines to something else is a >> > different story. But if we change the guideline, it should be changed >> > for all packages and not just for lsof. >> package/kmod/Config.in is already like this The main reason why it's done like this is that it's only the _TOOLS suboption that should depend on the _SHOW_OTHERS option. >> and it's more clean Arnout> I agree that it's better to do it in the package's Config.in. Arnout> And changing the guideline doesn't automatically mean that all Arnout> packages have to change. Only new packages (or patches) must follow Arnout> the guideline. True. But the first thing to decide on is if this _SHOW_OTHERS option is really useful. It was afaik introduced long time ago to encourage people to use the busybox applets instead of the full blown versions, but perhaps we should just let people decide for themselves?
>>>>> "Gustavo" == Gustavo Zacarias <gustavo@zacarias.com.ar> writes:
Gustavo> Since busybox 1.20+ includes a lsof applet make sure lsof gets
Gustavo> built after busybox so that we get the full-blown version if
Gustavo> both are enabled. Also hide the lsof package unless
Gustavo> BUSYBOX_SHOW_OTHERS is true.
Committed, thanks.
On 2012-04-29 05:36, Peter Korsgaard wrote: > True. But the first thing to decide on is if this _SHOW_OTHERS option > is > really useful. It was afaik introduced long time ago to encourage > people > to use the busybox applets instead of the full blown versions, but > perhaps we should just let people decide for themselves? I'd just kill it altogether, i'm not a fan of hidden options since they're prone to noise (people asking where it is) if they can be avoided. And probably use Arnout's idea of appending it to the menu text or help. Regards.
diff --git a/package/Config.in b/package/Config.in index 64fce62..7f9b234 100644 --- a/package/Config.in +++ b/package/Config.in @@ -24,7 +24,9 @@ source "package/dmalloc/Config.in" source "package/kexec/Config.in" source "package/latencytop/Config.in" source "package/lmbench/Config.in" +if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/lsof/Config.in" +endif source "package/ltp-testsuite/Config.in" source "package/lttng-babeltrace/Config.in" source "package/lttng-modules/Config.in" diff --git a/package/lsof/lsof.mk b/package/lsof/lsof.mk index c07fb2d..7bc22c0 100644 --- a/package/lsof/lsof.mk +++ b/package/lsof/lsof.mk @@ -8,6 +8,9 @@ LSOF_VERSION = 4.85 LSOF_SOURCE = lsof_$(LSOF_VERSION).tar.bz2 LSOF_SITE = ftp://lsof.itap.purdue.edu/pub/tools/unix/lsof/ +# Make certain full-blown lsof gets built after the busybox version (1.20+) +LSOF_DEPENDENCIES += $(if $(BR2_PACKAGE_BUSYBOX),busybox) + BR2_LSOF_CFLAGS = ifeq ($(BR2_LARGEFILE),) BR2_LSOF_CFLAGS += -U_FILE_OFFSET_BITS
Since busybox 1.20+ includes a lsof applet make sure lsof gets built after busybox so that we get the full-blown version if both are enabled. Also hide the lsof package unless BUSYBOX_SHOW_OTHERS is true. Signed-off-by: Gustavo Zacarias <gustavo@zacarias.com.ar> --- package/Config.in | 2 ++ package/lsof/lsof.mk | 3 +++ 2 files changed, 5 insertions(+), 0 deletions(-)