linux.mk: check for y explicitly

LINUX_CONFIGURE_CMDS uses the construct
$(if $(BR2_x), ...
which don't work as expected when BR2_x=n.

This patch modifies linux.mk to use the construct
$(if $(filter y,$(BR2_x)), ...

See http://lists.busybox.net/pipermail/buildroot/2012-March/051523.html
for more information about a specific problem case.

---
 linux/linux.mk |  8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

Note: the problematic construct seems to be present in many files in buildroot. I'd say we need to fix them all, but I'm not sure if you agree. Feedback welcome!

@@ -128,14 +128,14 @@ define LINUX_CONFIGURE_CMDS
 	cp $(KERNEL_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_EABI),
+	$(if $(filter y,$(BR2_ARM_EABI)),
 		$(call KCONFIG_ENABLE_OPT,CONFIG_AEABI,$(@D)/.config),
 		$(call KCONFIG_DISABLE_OPT,CONFIG_AEABI,$(@D)/.config))
 	# As the kernel gets compiled before root filesystems are
 	# built, we create a fake cpio file. It'll be
 	# replaced later by the real cpio archive, and the kernel will be
 	# rebuilt using the linux26-rebuild-with-initramfs target.
-	$(if $(BR2_TARGET_ROOTFS_INITRAMFS),
+	$(if $(filter y,$(BR2_TARGET_ROOTFS_INITRAMFS)),
 		touch $(BINARIES_DIR)/rootfs.cpio
 		$(call KCONFIG_ENABLE_OPT,CONFIG_BLK_DEV_INITRD,$(@D)/.config)
 		$(call KCONFIG_SET_OPT,CONFIG_INITRAMFS_SOURCE,\"$(BINARIES_DIR)/rootfs.cpio\",$(@D)/.config)

@@ -143,10 +143,10 @@ define LINUX_CONFIGURE_CMDS
 		$(call KCONFIG_SET_OPT,CONFIG_INITRAMFS_ROOT_GID,0,$(@D)/.config)
 		$(call KCONFIG_DISABLE_OPT,CONFIG_INITRAMFS_COMPRESSION_NONE,$(@D)/.config)
 		$(call KCONFIG_ENABLE_OPT,CONFIG_INITRAMFS_COMPRESSION_GZIP,$(@D)/.config))
-	$(if $(BR2_ROOTFS_DEVICE_CREATION_STATIC),,
+	$(if $(filter y,$(BR2_ROOTFS_DEVICE_CREATION_STATIC)),,
 		$(call KCONFIG_ENABLE_OPT,CONFIG_DEVTMPFS,$(@D)/.config)
 		$(call KCONFIG_ENABLE_OPT,CONFIG_DEVTMPFS_MOUNT,$(@D)/.config))
-	$(if $(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),
+	$(if $(filter y,$(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV)),
 		$(call KCONFIG_SET_OPT,CONFIG_UEVENT_HELPER_PATH,\"/sbin/mdev\",$(@D)/.config))
 	yes '' | $(TARGET_MAKE_ENV) $(MAKE1) $(LINUX_MAKE_FLAGS) -C $(@D) oldconfig
 endef

Le Wed, 14 Mar 2012 17:36:49 +0100,
Thomas De Schampheleire <[email protected]> a ?crit :

This is an invalid construct in my opinion. Unset options should look like:

# BR2_SOMETHING is not set

in the .config file.

There are many other places in Buildroot which assumes that an unselected option has an empty value:

xenomai/xenomai.mk:ifeq ($(XENOMAI_VERSION),) xenomai/xenomai.mk:ifeq ($(BR2_PACKAGE_XENOMAI_SMP),y) xenomai/xenomai.mk:ifeq ($(BR2_HAVE_DEVFILES),) xenomai/xenomai.mk:ifeq ($(BR2_PACKAGE_XENOMAI_TESTSUITE),) xenomai/xenomai.mk:ifeq ($(BR2_PACKAGE_XENOMAI_RTCAN),) xenomai/xenomai.mk:ifeq ($(BR2_PACKAGE_XENOMAI_ANALOGY),)

connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_THREADS),--enable-threads,--disable-threads)         \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_DEBUG),--enable-debug,--disable-debug)               \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_ETHERNET),--enable-ethernet,--disable-ethernet)      \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_WIFI),--enable-wifi,--disable-wifi)                  \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_BLUETOOTH),--enable-bluetooth,--disable-bluetooth)   \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_LOOPBACK),--enable-loopback,--disable-loopback)      \
connman/connman.mk:     $(if $(BR2_PACKAGE_CONNMAN_NTPD),--enable-ntpd,--disable-ntpd)

And many, many, many more. Just grep for '(if' in the Buildroot code. So definitely not my Ack on this patch.

Best regards,

Thomas

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

Hi Thomas, Arnout,

I was giving this on the command-line, but as Arnout hinted I could give BR2_x= instead.

Although I find this behavior counter-intuitive (if you can give =y you should be able to give =n as well) I can understand that you don't like such changes...

Best regards,
Thomas

Le Wed, 14 Mar 2012 20:30:58 +0100,
Thomas De Schampheleire <[email protected]> a ?crit :

Well, kconfig is not done by us, so I think it's much better to stick with what the kernel is doing (i.e, unselected options have an empty value).

Thomas

Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com