Patchwork [PATCHv2,07/10] busybox: update-all-config shouldn't update default busybox config

login
register
mail settings
Submitter Arnout Vandecappelle
Date Oct. 24, 2012, 7:17 a.m.
Message ID <1351063027-13800-8-git-send-email-arnout@mind.be>
Download mbox | patch
Permalink /patch/193696/
State Superseded
Headers show

Comments

Arnout Vandecappelle - Oct. 24, 2012, 7:17 a.m.
From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>

The new update-all-config target will update the busybox config file
if BR2_PACKAGE_BUSYBOX_CONFIG is set, even if it is set to the
default value in package/busybox/busybox-xxx.config.

To avoid this, set the default BR2_PACKAGE_BUSYBOX_CONFIG to empty,
and select a default to use in the .mk file.

Note that busybox-update-config will still overwrite the default
file in package/busybox/busybox-xxx.config - presumably it's
intentional.

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 package/busybox/Config.in  |    7 ++-----
 package/busybox/busybox.mk |    9 +++++----
 2 files changed, 7 insertions(+), 9 deletions(-)
Luca Ceresoli - Nov. 7, 2012, 8:13 a.m.
Arnout Vandecappelle (Essensium/Mind) wrote:
> From: "Arnout Vandecappelle (Essensium/Mind)" <arnout@mind.be>
>
> The new update-all-config target will update the busybox config file
> if BR2_PACKAGE_BUSYBOX_CONFIG is set, even if it is set to the
> default value in package/busybox/busybox-xxx.config.
>
> To avoid this, set the default BR2_PACKAGE_BUSYBOX_CONFIG to empty,
> and select a default to use in the .mk file.
>
> Note that busybox-update-config will still overwrite the default
> file in package/busybox/busybox-xxx.config - presumably it's
> intentional.
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> ---
>   package/busybox/Config.in  |    7 ++-----
>   package/busybox/busybox.mk |    9 +++++----
>   2 files changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/package/busybox/Config.in b/package/busybox/Config.in
> index dedcf18..0b77872 100644
> --- a/package/busybox/Config.in
> +++ b/package/busybox/Config.in
> @@ -40,15 +40,12 @@ config BR2_BUSYBOX_VERSION
>
>   config BR2_PACKAGE_BUSYBOX_CONFIG
>   	string "BusyBox configuration file to use?"
> -	default "package/busybox/busybox-1.20.x.config" if BR2_PACKAGE_BUSYBOX_SNAPSHOT
> -	default "package/busybox/busybox-1.18.x.config" if BR2_BUSYBOX_VERSION_1_18_X
> -	default "package/busybox/busybox-1.19.x.config" if BR2_BUSYBOX_VERSION_1_19_X
> -	default "package/busybox/busybox-1.20.x.config" if BR2_BUSYBOX_VERSION_1_20_X
> +	default ""
>   	help
>   	  Some people may wish to use their own modified BusyBox configuration
>   	  file, and will specify their config file location with this option.
>
> -	  Most people will just use the default BusyBox configuration file.
> +	  If left empty, a default configuration file is used.
>
>   config BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
>   	bool "Show packages that are also provided by busybox"
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index c73d0d0..5c54319 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -6,9 +6,11 @@
>
>   ifeq ($(BR2_PACKAGE_BUSYBOX_SNAPSHOT),y)
>   BUSYBOX_VERSION = snapshot
> +BUSYBOX_CONFIG_VERSION = 1.20
>   BUSYBOX_SITE = http://www.busybox.net/downloads/snapshots
>   else
>   BUSYBOX_VERSION = $(call qstrip,$(BR2_BUSYBOX_VERSION))
> +BUSYBOX_CONFIG_VERSION = $(subst $(space),.,$(wordlist 1,2,$(subst .,$(space),$(BUSYBOX_VERSION))))

Smart! But maybe not obvious at first sight, so I'd add a brief
explanatory comment such as:
# strip bugfix number, e.g.: 1.20.3 -> 1.20

Up to here you're automating the config filename handling, not doing
anything related to update-config. It may be worth splitting the patch,
although the changed lines are very few.

>   BUSYBOX_SITE = http://www.busybox.net/downloads
>   endif
>   BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2
> @@ -27,9 +29,8 @@ BUSYBOX_MAKE_OPTS = \
>   	CONFIG_PREFIX="$(TARGET_DIR)" \
>   	SKIP_STRIP=y
>
> -ifndef BUSYBOX_CONFIG_FILE
> -	BUSYBOX_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
> -endif
> +BUSYBOX_CONFIG = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
> +BUSYBOX_CONFIG_FILE = $(or $(wildcard $(BUSYBOX_CONFIG)),package/busybox/busybox-$(BUSYBOX_CONFIG_VERSION).x.config)
>
>   define BUSYBOX_PERMISSIONS
>   /bin/busybox			 f 4755	0 0 - - - - -
> @@ -218,6 +219,6 @@ busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch
>   busybox-update-config: busybox-configure
>   	cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
>
> -ifneq ($(BUSYBOX_CONFIG_FILE),)
> +ifneq ($(BUSYBOX_CONFIG),)
>   UPDATE_ALL_CONFIG_TARGETS += busybox-update-config
>   endif
>

Luca
Arnout Vandecappelle - Nov. 7, 2012, 7:20 p.m.
On 11/07/12 09:13, Luca Ceresoli wrote:
> Up to here you're automating the config filename handling, not doing
> anything related to update-config. It may be worth splitting the patch,
> although the changed lines are very few.

  Since there's no reason for automating the config filename handling except
to enable the update-config, I don't think it's worth to split it.

  Regards,
  Arnout

Patch

diff --git a/package/busybox/Config.in b/package/busybox/Config.in
index dedcf18..0b77872 100644
--- a/package/busybox/Config.in
+++ b/package/busybox/Config.in
@@ -40,15 +40,12 @@  config BR2_BUSYBOX_VERSION
 
 config BR2_PACKAGE_BUSYBOX_CONFIG
 	string "BusyBox configuration file to use?"
-	default "package/busybox/busybox-1.20.x.config" if BR2_PACKAGE_BUSYBOX_SNAPSHOT
-	default "package/busybox/busybox-1.18.x.config" if BR2_BUSYBOX_VERSION_1_18_X
-	default "package/busybox/busybox-1.19.x.config" if BR2_BUSYBOX_VERSION_1_19_X
-	default "package/busybox/busybox-1.20.x.config" if BR2_BUSYBOX_VERSION_1_20_X
+	default ""
 	help
 	  Some people may wish to use their own modified BusyBox configuration
 	  file, and will specify their config file location with this option.
 
-	  Most people will just use the default BusyBox configuration file.
+	  If left empty, a default configuration file is used.
 
 config BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
 	bool "Show packages that are also provided by busybox"
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index c73d0d0..5c54319 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -6,9 +6,11 @@ 
 
 ifeq ($(BR2_PACKAGE_BUSYBOX_SNAPSHOT),y)
 BUSYBOX_VERSION = snapshot
+BUSYBOX_CONFIG_VERSION = 1.20
 BUSYBOX_SITE = http://www.busybox.net/downloads/snapshots
 else
 BUSYBOX_VERSION = $(call qstrip,$(BR2_BUSYBOX_VERSION))
+BUSYBOX_CONFIG_VERSION = $(subst $(space),.,$(wordlist 1,2,$(subst .,$(space),$(BUSYBOX_VERSION))))
 BUSYBOX_SITE = http://www.busybox.net/downloads
 endif
 BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2
@@ -27,9 +29,8 @@  BUSYBOX_MAKE_OPTS = \
 	CONFIG_PREFIX="$(TARGET_DIR)" \
 	SKIP_STRIP=y
 
-ifndef BUSYBOX_CONFIG_FILE
-	BUSYBOX_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
-endif
+BUSYBOX_CONFIG = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
+BUSYBOX_CONFIG_FILE = $(or $(wildcard $(BUSYBOX_CONFIG)),package/busybox/busybox-$(BUSYBOX_CONFIG_VERSION).x.config)
 
 define BUSYBOX_PERMISSIONS
 /bin/busybox			 f 4755	0 0 - - - - -
@@ -218,6 +219,6 @@  busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch
 busybox-update-config: busybox-configure
 	cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
 
-ifneq ($(BUSYBOX_CONFIG_FILE),)
+ifneq ($(BUSYBOX_CONFIG),)
 UPDATE_ALL_CONFIG_TARGETS += busybox-update-config
 endif