diff mbox

[10/13] Add update-all-config target

Message ID 20121013231438.17317.13801.stgit@localhost
State RFC
Headers show

Commit Message

Arnout Vandecappelle Oct. 13, 2012, 11:14 p.m. UTC 
From: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

The update-all-config target updates all the external configuration
file with their current values.  This includes:
- buildroot
- busybox
- linux
- crosstool-ng
- uClibc

Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
Barebox is missing because there is no BR2_TARGET_BAREBOX_CONFIG option.
---
 Makefile                                         |    6 ++++++
 linux/linux.mk                                   |    2 ++
 package/busybox/busybox.mk                       |    4 ++++
 toolchain/toolchain-crosstool-ng/crosstool-ng.mk |    7 +++++++
 toolchain/uClibc/uclibc.mk                       |    4 ++++
 5 files changed, 23 insertions(+)

Comments

Thomas De Schampheleire Oct. 14, 2012, 6:45 p.m. UTC | #1 
Hi,

On Sun, Oct 14, 2012 at 1:14 AM, Arnout Vandecappelle (Essensium/Mind)
<arnout@mind.be> wrote:
> From: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>
> The update-all-config target updates all the external configuration
> file with their current values.  This includes:
> - buildroot
> - busybox
> - linux
> - crosstool-ng
> - uClibc
>
> Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

I like this idea, and would propose to go through with this regardless
of the outcome of the series.
See below for some comments though.

> ---
> Barebox is missing because there is no BR2_TARGET_BAREBOX_CONFIG option.
> ---
>  Makefile                                         |    6 ++++++
>  linux/linux.mk                                   |    2 ++
>  package/busybox/busybox.mk                       |    4 ++++
>  toolchain/toolchain-crosstool-ng/crosstool-ng.mk |    7 +++++++
>  toolchain/uClibc/uclibc.mk                       |    4 ++++
>  5 files changed, 23 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index 1f42141..0e5b28e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -660,6 +660,10 @@ savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
>                 --savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
>                 $(CONFIG_CONFIG_IN)
>
> +# UPDATE_ALL_CONFIG_TARGETS is set by the individual packages that have a
> +# save*config target.
> +update-all-config: savedefconfig $(UPDATE_ALL_CONFIG_TARGETS)
> +
>  # check if download URLs are outdated
>  source-check:
>         $(MAKE) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
> @@ -714,6 +718,8 @@ help:
>         @echo '  defconfig              - New config with default answer to all options'
>         @echo '                             BR2_DEFCONFIG, if set, is used as input'
>         @echo '  savedefconfig          - Save current config as ./defconfig (minimal config)'
> +       @echo '  update-all-config      - Update all configuration targets that have an input:'
> +       @echo '                           buildroot, busybox, linux, crosstool-ng, uClibc'
>         @echo '  allyesconfig           - New config where all options are accepted with yes'
>         @echo '  allnoconfig            - New config where all options are answered with no'
>         @echo '  randpackageconfig      - New config with random answer to package options'
> diff --git a/linux/linux.mk b/linux/linux.mk
> index c4bdf90..98ffe44 100644
> --- a/linux/linux.mk
> +++ b/linux/linux.mk
> @@ -151,6 +151,8 @@ ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
>  KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
>  else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
>  KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
> +# savedefconfig requires a kernel >= 2.6.33
> +UPDATE_ALL_CONFIG_TARGETS += linux-update-defconfig
>  endif
>
>  define LINUX_CONFIGURE_CMDS
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index 33f8633..5063a8b 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -217,3 +217,7 @@ busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch
>
>  busybox-update-config:
>         cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
> +
> +ifneq ($(BUSYBOX_CONFIG_FILE),)
> +UPDATE_ALL_CONFIG_TARGETS += busybox-update-config
> +endif
> diff --git a/toolchain/toolchain-crosstool-ng/crosstool-ng.mk b/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
> index d18f5d5..688bcfe 100644
> --- a/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
> +++ b/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
> @@ -397,3 +397,10 @@ ctng-menuconfig: $(CTNG_DIR)/.config
>         $(call ctng-oldconfig,$<)
>         $(call ctng-check-config-changed,$<,$<.timestamp)
>         $(Q)rm -f $<.timestamp
> +
> +ctng-update-config: $(CTNG_DIR)/.config
> +       $(Q)cp $< $(CTNG_CONFIG_FILE)
> +
> +ifneq ($(CTNG_CONFIG_FILE),)
> +UPDATE_ALL_CONFIG_TARGETS += ctng-update-config
> +endif

I would personally have split the addition of an update-config feature
for crosstool-ng from this patch that wraps the different
update-config targets in one update-all-config.

I wonder why ctng-update-config uses a different type of recipe than
their counterparts in busybox, uclibc, ...

For busybox it is:
>  busybox-update-config:
>         cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)

while for ctng, as proposed above:
> +ctng-update-config: $(CTNG_DIR)/.config
> +       $(Q)cp $< $(CTNG_CONFIG_FILE)

Differences:

- target dependency
- $(Q)
- forced copy

I would say that since all these packages are using the same kind of
config system, the way to save their config files should also line up.
It's possible that the new ctng-update-config is better in some ways,
but in this case I think the other ones should be updated as well


> diff --git a/toolchain/uClibc/uclibc.mk b/toolchain/uClibc/uclibc.mk
> index d1cd718..0faaf18 100644
> --- a/toolchain/uClibc/uclibc.mk
> +++ b/toolchain/uClibc/uclibc.mk
> @@ -508,6 +508,10 @@ uclibc-oldconfig: $(UCLIBC_DIR)/.oldconfig
>  uclibc-update-config: uclibc-config
>         cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
>
> +ifneq ($(UCLIBC_CONFIG_FILE),)
> +UPDATE_ALL_CONFIG_TARGETS += uclibc-update-config
> +endif
> +
>  uclibc-configured: gcc_initial kernel-headers $(UCLIBC_DIR)/.configured
>
>  uclibc-configured-source: uclibc-source
>

Best regards,
Thomas
Arnout Vandecappelle Oct. 20, 2012, 4:47 p.m. UTC | #2 
On 14/10/12 20:45, Thomas De Schampheleire wrote:
>> +
>> >  +ctng-update-config: $(CTNG_DIR)/.config
>> >  +       $(Q)cp $<  $(CTNG_CONFIG_FILE)
>> >  +
>> >  +ifneq ($(CTNG_CONFIG_FILE),)
>> >  +UPDATE_ALL_CONFIG_TARGETS += ctng-update-config
>> >  +endif
> I would personally have split the addition of an update-config feature
> for crosstool-ng from this patch that wraps the different
> update-config targets in one update-all-config.

  Good point.


> I wonder why ctng-update-config uses a different type of recipe than
> their counterparts in busybox, uclibc, ...
>
> For busybox it is:
>> >    busybox-update-config:
>> >           cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
> while for ctng, as proposed above:
>> >  +ctng-update-config: $(CTNG_DIR)/.config
>> >  +       $(Q)cp $<  $(CTNG_CONFIG_FILE)
> Differences:
>
> - target dependency

  linux and uclibc have it, only busybox is missing it.

> - $(Q)

  My bad.

> - forced copy

  Ah, except that cp -f doesn't mean forced copy; it means: remove an
already existing file before the copy. That means that permissions are
lost and if it's a symlink, it's not the file it points to that gets
updated. So I think cp -f is _not_ what we want here...


> I would say that since all these packages are using the same kind of
> config system, the way to save their config files should also line up.
> It's possible that the new ctng-update-config is better in some ways,
> but in this case I think the other ones should be updated as well

  True, so I'll fix the use of -f in a separate patch. And add the
dependency for busybox as well.


  Regards,
  Arnout
Arnout Vandecappelle Oct. 20, 2012, 4:52 p.m. UTC | #3 
On 20/10/12 18:47, Arnout Vandecappelle wrote:
>> - forced copy
>
>   Ah, except that cp -f doesn't mean forced copy; it means: remove an
> already existing file before the copy. That means that permissions are
> lost and if it's a symlink, it's not the file it points to that gets
> updated. So I think cp -f is _not_ what we want here...

  Wrong again, Arnout :-(  The -f only removes the symlink it the file
couldn't be written to in the first place, so it's OK to have it.

  Regards,
  Arnout
Stephan Hoffmann Dec. 3, 2012, 2:18 p.m. UTC | #4 
Am 14.10.2012 20:45, schrieb Thomas De Schampheleire:
> Hi,
>
> On Sun, Oct 14, 2012 at 1:14 AM, Arnout Vandecappelle (Essensium/Mind)
> <arnout@mind.be> wrote:
>> > From: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
>> >
>> > The update-all-config target updates all the external configuration
>> > file with their current values.  This includes:
>> > - buildroot
>> > - busybox
>> > - linux
>> > - crosstool-ng
>> > - uClibc
>> >
>> > Signed-off-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
> I like this idea, and would propose to go through with this regardless
> of the outcome of the series.
> See below for some comments though
Hi,

I also like this idea much.

I'd like to go one step further and put buildroot's default config files
in the board/company/product hierarchy, too. Then everything would be in
one place.

This approach would be similar to that used by uClinux where all
relevant board specific files are in vendors/company/product.

Kind regards

Stephan
Thomas Petazzoni Dec. 3, 2012, 4:41 p.m. UTC | #5 
Dear Stephan Hoffmann,

On Mon, 03 Dec 2012 15:18:11 +0100, Stephan Hoffmann wrote:

> I'd like to go one step further and put buildroot's default config files
> in the board/company/product hierarchy, too. Then everything would be in
> one place.

This is what we had a long time ago, but it was changed for the current
approach.

See the discussion at
http://lists.busybox.net/pipermail/buildroot/2009-October/029556.html
(note: the discussion starts on October 2009, but continues on November
2009, and the mailing list archive doesn't show it as one single
thread).

Best regards,

Thomas
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 1f42141..0e5b28e 100644
--- a/Makefile
+++ b/Makefile
@@ -660,6 +660,10 @@  savedefconfig: $(BUILD_DIR)/buildroot-config/conf outputmakefile
 		--savedefconfig=$(if $(DEFCONFIG),$(DEFCONFIG),$(CONFIG_DIR)/defconfig) \
 		$(CONFIG_CONFIG_IN)
 
+# UPDATE_ALL_CONFIG_TARGETS is set by the individual packages that have a
+# save*config target.
+update-all-config: savedefconfig $(UPDATE_ALL_CONFIG_TARGETS)
+
 # check if download URLs are outdated
 source-check:
 	$(MAKE) DL_MODE=SOURCE_CHECK $(EXTRAMAKEARGS) source
@@ -714,6 +718,8 @@  help:
 	@echo '  defconfig              - New config with default answer to all options'
 	@echo '                             BR2_DEFCONFIG, if set, is used as input'
 	@echo '  savedefconfig          - Save current config as ./defconfig (minimal config)'
+	@echo '  update-all-config      - Update all configuration targets that have an input:'
+	@echo '                           buildroot, busybox, linux, crosstool-ng, uClibc'
 	@echo '  allyesconfig           - New config where all options are accepted with yes'
 	@echo '  allnoconfig            - New config where all options are answered with no'
 	@echo '  randpackageconfig      - New config with random answer to package options'
diff --git a/linux/linux.mk b/linux/linux.mk
index c4bdf90..98ffe44 100644
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -151,6 +151,8 @@  ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
 KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
 else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
 KERNEL_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
+# savedefconfig requires a kernel >= 2.6.33
+UPDATE_ALL_CONFIG_TARGETS += linux-update-defconfig
 endif
 
 define LINUX_CONFIGURE_CMDS
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 33f8633..5063a8b 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -217,3 +217,7 @@  busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch
 
 busybox-update-config:
 	cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
+
+ifneq ($(BUSYBOX_CONFIG_FILE),)
+UPDATE_ALL_CONFIG_TARGETS += busybox-update-config
+endif
diff --git a/toolchain/toolchain-crosstool-ng/crosstool-ng.mk b/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
index d18f5d5..688bcfe 100644
--- a/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
+++ b/toolchain/toolchain-crosstool-ng/crosstool-ng.mk
@@ -397,3 +397,10 @@  ctng-menuconfig: $(CTNG_DIR)/.config
 	$(call ctng-oldconfig,$<)
 	$(call ctng-check-config-changed,$<,$<.timestamp)
 	$(Q)rm -f $<.timestamp
+
+ctng-update-config: $(CTNG_DIR)/.config
+	$(Q)cp $< $(CTNG_CONFIG_FILE)
+
+ifneq ($(CTNG_CONFIG_FILE),)
+UPDATE_ALL_CONFIG_TARGETS += ctng-update-config
+endif
diff --git a/toolchain/uClibc/uclibc.mk b/toolchain/uClibc/uclibc.mk
index d1cd718..0faaf18 100644
--- a/toolchain/uClibc/uclibc.mk
+++ b/toolchain/uClibc/uclibc.mk
@@ -508,6 +508,10 @@  uclibc-oldconfig: $(UCLIBC_DIR)/.oldconfig
 uclibc-update-config: uclibc-config
 	cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
 
+ifneq ($(UCLIBC_CONFIG_FILE),)
+UPDATE_ALL_CONFIG_TARGETS += uclibc-update-config
+endif
+
 uclibc-configured: gcc_initial kernel-headers $(UCLIBC_DIR)/.configured
 
 uclibc-configured-source: uclibc-source