Rebuild packages when their external config is updated

Hello all,

this is a follow up to the previous discussion at
http://lists.busybox.net/pipermail/buildroot/2014-May/095532.html
(Rebuild busybox when an external config is updated).

With this updated patch, I try to address the concerns raised there. The concerns were:

1) Buildroot doesn't try to be smart to detect what needs to be rebuilt.

That's true, but buildroot is smart enough to rebuild the package after "make $(pkg)-menuconfig". This patch extends the smartness a bit by considering not only the -menuconfig invocation but also the change of the external config.

2) This must be implemented consistelntly for all packages where the user can provide a .config.

The functionality is now implemented in pkg-generic.mk so it applies to all packages.

If people agree with this I'll update the manual as well.

Best regards,
-Michal

Michal Sojka (1):
Rebuild packages when their external config is updated

 boot/barebox/barebox.mk    |  4 ++--
 linux/linux.mk             | 10 +++++-----
 package/busybox/busybox.mk |  2 +-
 package/pkg-generic.mk     | 17 +++++++++++++++++
 package/uclibc/uclibc.mk   |  2 +-
 5 files changed, 26 insertions(+), 9 deletions(-)

2.0.0.rc2

Packages like busybox, Linux kernel or uclibc can "import" their configuration files from external location specified either in buildroot's .config or in an environment variable. Currently, this import happens only when the package is built for the first time. When the external config changes later, the package is not rebuilt with the updated configuration until the corresponding .stamp_configured file is manually deleted. This patch changes this to automatically rebuild the package, when its external configuration files is newer than .stamp_configured.

The change is implemented in pkg-generic.mk so it automatically applies to all packages that use external config.

This patch also changes $(pkg)-update-config targets, which export the internally used config files to external locations, to not change the modification time of the external config. This ensures that the package is not rebuilt just because its config file was exported.

Finally, linux.mk used variable KERNEL_SOURCE_CONFIG to hold the file name of the external config. This was renamed to LINUX_SOURCE_CONFIG, in order to be compatible with the change in pkg-generic.mk.

---
 boot/barebox/barebox.mk    |  4 ++--
 linux/linux.mk             | 10 +++++-----
 package/busybox/busybox.mk |  2 +-
 package/pkg-generic.mk     | 17 +++++++++++++++++
 package/uclibc/uclibc.mk   |  2 +-
 5 files changed, 26 insertions(+), 9 deletions(-)

index f57d297..a49f8dc 100644
@@ -127,10 +127,10 @@ barebox-savedefconfig: barebox-configure
 
 ifeq ($(BR2_TARGET_BAREBOX_USE_CUSTOM_CONFIG),y)
 barebox-update-config: barebox-configure $(BAREBOX_DIR)/.config
-	cp -f $(BAREBOX_DIR)/.config $(BR2_TARGET_BAREBOX_CUSTOM_CONFIG_FILE)
+	cp -fa $(BAREBOX_DIR)/.config $(BR2_TARGET_BAREBOX_CUSTOM_CONFIG_FILE)
 
 barebox-update-defconfig: barebox-savedefconfig
-	cp -f $(BAREBOX_DIR)/defconfig $(BR2_TARGET_BAREBOX_CUSTOM_CONFIG_FILE)
+	cp -fa $(BAREBOX_DIR)/defconfig $(BR2_TARGET_BAREBOX_CUSTOM_CONFIG_FILE)
 else
 barebox-update-config: ;
 barebox-update-defconfig: ;

index bd3f2ac..18254ca 100644
@@ -161,13 +161,13 @@ LINUX_POST_PATCH_HOOKS += LINUX_APPLY_PATCHES
 
 
 ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
-KERNEL_SOURCE_CONFIG = $(KERNEL_ARCH_PATH)/configs/$(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG))_defconfig
+LINUX_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)
+LINUX_SOURCE_CONFIG = $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
 endif
 
 define LINUX_CONFIGURE_CMDS
-	$(INSTALL) -m 0644 $(KERNEL_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig
+	$(INSTALL) -m 0644 $(LINUX_SOURCE_CONFIG) $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig
 	$(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) buildroot_defconfig
 	rm $(KERNEL_ARCH_PATH)/configs/buildroot_defconfig
 	$(if $(BR2_arm)$(BR2_armeb),

@@ -317,10 +317,10 @@ linux-savedefconfig linux26-savedefconfig: linux-configure
 
 ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
 linux-update-config linux26-update-config: linux-configure $(LINUX_DIR)/.config
-	cp -f $(LINUX_DIR)/.config $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
+	cp -fa $(LINUX_DIR)/.config $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
 
 linux-update-defconfig linux26-update-defconfig: linux-savedefconfig
-	cp -f $(LINUX_DIR)/defconfig $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
+	cp -fa $(LINUX_DIR)/defconfig $(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)
 else
 linux-update-config linux26-update-config: ;
 linux-update-defconfig linux26-update-defconfig: ;

index 150100b..a5405db 100644
@@ -231,4 +231,4 @@ busybox-menuconfig busybox-xconfig busybox-gconfig: busybox-patch
 	rm -f $(BUSYBOX_DIR)/.stamp_target_installed
 
 busybox-update-config: busybox-configure
-	cp -f $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)
+	cp -fa $(BUSYBOX_BUILD_CONFIG) $(BUSYBOX_CONFIG_FILE)

index 6eca6d4..7440d2d 100644
@@ -466,6 +466,23 @@ $$($(2)_TARGET_BUILD):	$$($(2)_TARGET_CONFIGURE)
 $(1)-configure:			$$($(2)_TARGET_CONFIGURE)
 $$($(2)_TARGET_CONFIGURE):	| $$($(2)_DEPENDENCIES)
 
+# We want a package to be reconfigured whenever an "external" config
+# (if any) is updated. The name of the variable holding the external
+# config file name is not unified in the recipes so we add
+# dependencies to both commonly used variables.
+#
+# Additionally, we want the commands like
+#     make BUSYBOX_CONFIG_FILE=xxx busybox-update-config
+# to succeed even if file 'xxx' does not exist before. Therefore, we
+# condition the dependency by the existence of the config file.
+
+ifneq ($$(wildcard $$(call qstrip,$$($(2)_CONFIG_FILE))),)
+$$($(2)_TARGET_CONFIGURE):	$$(call qstrip,$$($(2)_CONFIG_FILE))
+endif
+ifneq ($$(wildcard $$(call qstrip,$$($(2)_SOURCE_CONFIG))),)
+$$($(2)_TARGET_CONFIGURE):	$$(call qstrip,$$($(2)_SOURCE_CONFIG))
+endif
+
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dirs prepare
 ifeq ($(filter $(1),$(DEPENDENCIES_HOST_PREREQ)),)
 $$($(2)_TARGET_SOURCE) $$($(2)_TARGET_RSYNC): | dependencies

index 717cf53..6401f3d 100644
@@ -554,7 +554,7 @@ uclibc-menuconfig: uclibc-patch
 $(eval $(generic-package))
 
 uclibc-update-config: $(UCLIBC_DIR)/.stamp_configured
-	cp -f $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
+	cp -fa $(UCLIBC_DIR)/.config $(UCLIBC_CONFIG_FILE)
 
 # Before uClibc is built, we must have the second stage cross-compiler
 $(UCLIBC_TARGET_BUILD): | host-gcc-intermediate

2.0.0.rc2

This is an extremely bad idea for uclibc, ABI compatibility is not guaranteed and changes to the configuration make it change in incompatible ways, hence you'd need to rebuild everything if this happens. Unless i'm missing something you're not considering this scenario. Regards.

Michal, All,

On 2014-05-16 17:22 +0200, Michal Sojka spake thusly:

I think this is still too far-reaching.

If you were to change a source file of your package, then you are supposed to tell Buildroot that it has to rebiuild the package with either of:
make PKG-reconfigure
make PKG-rebuild

The same goes if you modify the package's .mk file in Buildroot, or changing an option of the package in Buildroot's own .config.

I don't see how different changing a .config is from changing a source file, changing the .mk of the package, or changing an option in the Buildroot's menuconfig.

Changing the .config is like if you were changing the package source tree, and telling Buildroot to use that with _OVERRIDE_SRCDIR, and we explicitly state that this should be handled with either 'reconfigure' or 'rebuild'.

Really, the .config is part of the sources of a package.

If we were to automatically rebuild a package because its .config did change, then I don.t see why we would not do it when a source file, its .mk or an option did change as well. And surely we do not want to go that route, or it would be a mess, and pratically impossible to detect correctly.

And as Gustavo pointed out, this would even break a system in the case of uClibc anyway, since reconfiguring uClibc is most probably an ABI breakage, and would require much more than only rebuilding uClibc alone.

So, I'm still not convinced by this change.

Regards,
Yann E. MORIN.

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

Dear Michal Sojka,

Thanks for your patch. But following Gustavo's and Yann's feedback, we decided to reject this approach. The user should start the rebuild of the package manually after changing the configuration.

Note that there is currently an on-going work by Thomas De Schampheleire on creating a kconfig-package infrastructure. You're welcome to raise your use cases and participate to the discussion.

Thanks,

Thomas

Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Hi all,

At first glance, some of the issues raised by Michal are already implicitly covered by my patches changing uclibc:
http://patchwork.ozlabs.org/patch/371701/
http://patchwork.ozlabs.org/patch/371702/
http://patchwork.ozlabs.org/patch/371703/

The other series I sent, that introduce a kconfig-package infrastructure, merely make the uclibc kconfig rules generic, the behavior is defined by this first series.

Regarding the uclibc ABI argument: I may not be understanding it completely, but if someone changes the uclibc configuration with uclibc-menuconfig (which we support) and then types 'make', then uclibc is rebuilt, but any real packages are not, right? So the ABI may be broken anyway, already today.

Best regards,
Thomas