diff mbox

[1/1] busybox: ncurses progs equiv as default

Message ID 1492654408-25342-1-git-send-email-matthew.weber@rockwellcollins.com
State Rejected
Headers show

Commit Message

Matt Weber April 20, 2017, 2:13 a.m. UTC
By default, enable the busybox equivalents of the ncurses
clear and reset if ncurses progs are not enabled.

Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
---
 package/busybox/busybox.mk | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Baruch Siach April 20, 2017, 4 a.m. UTC | #1
Hi Matt,

On Wed, Apr 19, 2017 at 09:13:28PM -0500, Matt Weber wrote:
> By default, enable the busybox equivalents of the ncurses
> clear and reset if ncurses progs are not enabled.

Our default busybox.config enables these applets. If the user chooses to 
disable them in a custom config, why should we force them back on?

baruch

> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> ---
>  package/busybox/busybox.mk | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 9d9fcc2..56ec939 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -153,10 +153,15 @@ ifeq ($(BR2_PACKAGE_NCURSES_TARGET_PROGS),y)
>  #     /usr/bin/clear
>  #     /usr/bin/reset -> /usr/bin/tset (symlink)
>  #
> -define BUSYBOX_DISABLE_NCURSES_PROGS
> +define BUSYBOX_NCURSES_PROGS
>  	$(call KCONFIG_DISABLE_OPT,CONFIG_CLEAR,$(BUSYBOX_BUILD_CONFIG))
>  	$(call KCONFIG_DISABLE_OPT,CONFIG_RESET,$(BUSYBOX_BUILD_CONFIG))
>  endef
> +else
> +define BUSYBOX_NCURSES_PROGS
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_CLEAR,$(BUSYBOX_BUILD_CONFIG))
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_RESET,$(BUSYBOX_BUILD_CONFIG))
> +endef
>  endif
>  
>  define BUSYBOX_INSTALL_UDHCPC_SCRIPT
> @@ -240,7 +245,7 @@ define BUSYBOX_KCONFIG_FIXUP_CMDS
>  	$(BUSYBOX_SET_WATCHDOG)
>  	$(BUSYBOX_SET_SELINUX)
>  	$(BUSYBOX_MUSL_TWEAKS)
> -	$(BUSYBOX_DISABLE_NCURSES_PROGS)
> +	$(BUSYBOX_NCURSES_PROGS)
>  endef
>  
>  define BUSYBOX_CONFIGURE_CMDS
Matt Weber April 20, 2017, 1:01 p.m. UTC | #2
Baruch,

On Wed, Apr 19, 2017 at 11:00 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> Hi Matt,
>
> On Wed, Apr 19, 2017 at 09:13:28PM -0500, Matt Weber wrote:
>> By default, enable the busybox equivalents of the ncurses
>> clear and reset if ncurses progs are not enabled.
>
> Our default busybox.config enables these applets. If the user chooses to
> disable them in a custom config, why should we force them back on?
>

True, I was thinking the use case of build time changing (then doing
incremental build)if you use ncurses progs vs not and forcing the
busybox reconfig to have it update the target folder respectively.

I agree this isn't' ideal for all situations, but with these being
terminal tools, would/should you ever turn them off and the amount of
code is small?

Other discussion related to this:
https://patchwork.ozlabs.org/patch/750264/
https://patchwork.ozlabs.org/patch/750721/

-Matt
Arnout Vandecappelle April 23, 2017, 9:32 p.m. UTC | #3
On 20-04-17 15:01, Matthew Weber wrote:
> Baruch,
> 
> On Wed, Apr 19, 2017 at 11:00 PM, Baruch Siach <baruch@tkos.co.il> wrote:
>> Hi Matt,
>>
>> On Wed, Apr 19, 2017 at 09:13:28PM -0500, Matt Weber wrote:
>>> By default, enable the busybox equivalents of the ncurses
>>> clear and reset if ncurses progs are not enabled.
>>
>> Our default busybox.config enables these applets. If the user chooses to
>> disable them in a custom config, why should we force them back on?
>>
> 
> True, I was thinking the use case of build time changing (then doing
> incremental build)if you use ncurses progs vs not and forcing the
> busybox reconfig to have it update the target folder respectively.

 This is a use case we certainly don't want to support. On the other hand, we
*do* want to support the use case where the user has a custom busybox config,
doesn't have ncurses, and doesn't want to have clear and reset in his system.

 Regards,
 Arnout

> 
> I agree this isn't' ideal for all situations, but with these being
> terminal tools, would/should you ever turn them off and the amount of
> code is small?
> 
> Other discussion related to this:
> https://patchwork.ozlabs.org/patch/750264/
> https://patchwork.ozlabs.org/patch/750721/
> 
> -Matt
>
Matt Weber April 24, 2017, 12:40 p.m. UTC | #4
Arnout,

On Sun, Apr 23, 2017 at 4:32 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
>
>
>
> On 20-04-17 15:01, Matthew Weber wrote:
> > Baruch,
> >
> > On Wed, Apr 19, 2017 at 11:00 PM, Baruch Siach <baruch@tkos.co.il> wrote:
> >> Hi Matt,
> >>
> >> On Wed, Apr 19, 2017 at 09:13:28PM -0500, Matt Weber wrote:
> >>> By default, enable the busybox equivalents of the ncurses
> >>> clear and reset if ncurses progs are not enabled.
> >>
> >> Our default busybox.config enables these applets. If the user chooses to
> >> disable them in a custom config, why should we force them back on?
> >>
> >
> > True, I was thinking the use case of build time changing (then doing
> > incremental build)if you use ncurses progs vs not and forcing the
> > busybox reconfig to have it update the target folder respectively.
>
>  This is a use case we certainly don't want to support. On the other hand, we
> *do* want to support the use case where the user has a custom busybox config,
> doesn't have ncurses, and doesn't want to have clear and reset in his system.

Thanks for the feedback, this was more of a RFC to tie-off my previous
patches. I'm good with it being rejected.
-Matt
diff mbox

Patch

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 9d9fcc2..56ec939 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -153,10 +153,15 @@  ifeq ($(BR2_PACKAGE_NCURSES_TARGET_PROGS),y)
 #     /usr/bin/clear
 #     /usr/bin/reset -> /usr/bin/tset (symlink)
 #
-define BUSYBOX_DISABLE_NCURSES_PROGS
+define BUSYBOX_NCURSES_PROGS
 	$(call KCONFIG_DISABLE_OPT,CONFIG_CLEAR,$(BUSYBOX_BUILD_CONFIG))
 	$(call KCONFIG_DISABLE_OPT,CONFIG_RESET,$(BUSYBOX_BUILD_CONFIG))
 endef
+else
+define BUSYBOX_NCURSES_PROGS
+	$(call KCONFIG_ENABLE_OPT,CONFIG_CLEAR,$(BUSYBOX_BUILD_CONFIG))
+	$(call KCONFIG_ENABLE_OPT,CONFIG_RESET,$(BUSYBOX_BUILD_CONFIG))
+endef
 endif
 
 define BUSYBOX_INSTALL_UDHCPC_SCRIPT
@@ -240,7 +245,7 @@  define BUSYBOX_KCONFIG_FIXUP_CMDS
 	$(BUSYBOX_SET_WATCHDOG)
 	$(BUSYBOX_SET_SELINUX)
 	$(BUSYBOX_MUSL_TWEAKS)
-	$(BUSYBOX_DISABLE_NCURSES_PROGS)
+	$(BUSYBOX_NCURSES_PROGS)
 endef
 
 define BUSYBOX_CONFIGURE_CMDS