* [Buildroot] [PATCH] linux.mk: check for y explicitly
@ 2012-03-14 16:36 Thomas De Schampheleire
2012-03-14 19:21 ` Thomas Petazzoni
0 siblings, 1 reply; 4+ messages in thread
From: Thomas De Schampheleire @ 2012-03-14 16:36 UTC (permalink / raw)
To: buildroot
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.
Signed-off-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
---
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!
diff --git a/linux/linux.mk b/linux/linux.mk
--- a/linux/linux.mk
+++ b/linux/linux.mk
@@ -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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH] linux.mk: check for y explicitly
2012-03-14 16:36 [Buildroot] [PATCH] linux.mk: check for y explicitly Thomas De Schampheleire
@ 2012-03-14 19:21 ` Thomas Petazzoni
2012-03-14 19:30 ` Thomas De Schampheleire
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Petazzoni @ 2012-03-14 19:21 UTC (permalink / raw)
To: buildroot
Le Wed, 14 Mar 2012 17:36:49 +0100,
Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a ?crit :
> LINUX_CONFIGURE_CMDS uses the construct
> $(if $(BR2_x), ...
> which don't work as expected when BR2_x=n.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH] linux.mk: check for y explicitly
2012-03-14 19:21 ` Thomas Petazzoni
@ 2012-03-14 19:30 ` Thomas De Schampheleire
2012-03-14 19:33 ` Thomas Petazzoni
0 siblings, 1 reply; 4+ messages in thread
From: Thomas De Schampheleire @ 2012-03-14 19:30 UTC (permalink / raw)
To: buildroot
Hi Thomas, Arnout,
On Wed, Mar 14, 2012 at 8:21 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Le Wed, 14 Mar 2012 17:36:49 +0100,
> Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a ?crit :
>
>> LINUX_CONFIGURE_CMDS uses the construct
>> $(if $(BR2_x), ...
>> which don't work as expected when BR2_x=n.
>
> 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.
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [Buildroot] [PATCH] linux.mk: check for y explicitly
2012-03-14 19:30 ` Thomas De Schampheleire
@ 2012-03-14 19:33 ` Thomas Petazzoni
0 siblings, 0 replies; 4+ messages in thread
From: Thomas Petazzoni @ 2012-03-14 19:33 UTC (permalink / raw)
To: buildroot
Le Wed, 14 Mar 2012 20:30:58 +0100,
Thomas De Schampheleire <patrickdepinguin+buildroot@gmail.com> a ?crit :
> 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...
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-03-14 19:33 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-14 16:36 [Buildroot] [PATCH] linux.mk: check for y explicitly Thomas De Schampheleire
2012-03-14 19:21 ` Thomas Petazzoni
2012-03-14 19:30 ` Thomas De Schampheleire
2012-03-14 19:33 ` Thomas Petazzoni
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox