core/kconfig: fix display of custom error messages (branch yem/misc)

PATCH Superseded Yann E. MORIN 3 participants 13 messages Jul 12, 2015 – Jul 13, 2015 Arm/Microchip (AT91) SoC support

lore

Share & tools

lore

Yann E. MORIN

author

2015-07-12 12:11 UTC #0 lore raw

cover core/kconfig: fix display of custom error messages (branch yem/misc)

Hello All!

This series fixes the display of our custom error messages when defconfig/.config files do not exist, and also adds such messages to busybox and uClibc.

Regards,
Yann E. MORIN.

The following changes since commit 22ed697d11e033914cab7631fba899ba436a5a5b:

  packages: do not use TAR_STRIP_COMPONENTS, but directly --strip-components (2015-07-12 13:51:59 +0200)

are available in the git repository at:

git://git.busybox.net/~ymorin/git/buildroot yem/misc

for you to fetch changes up to 62fed5cb399404addb30c60c3dc128f46333c9ec:

package/uclibc: check for config file before calling kconfig-package (2015-07-12 14:02:05 +0200)

----------------------------------------------------------------
Yann E. MORIN (7):
core/pkg-kconfig: don't enforce check for config file when not building package/linux: don't enforce check for DTS when not building boot/at91boostrap3: check for config file before calling kconfig-package boot/barebox: check for config file before calling kconfig-package package/linux: check for config file before calling kconfig-package package/busybox: check for config file before calling kconfig-package package/uclibc: check for config file before calling kconfig-package

 boot/at91bootstrap3/at91bootstrap3.mk |  7 +++++--
 boot/barebox/barebox.mk               |  6 ++++--
 linux/linux.mk                        | 38 ++++++++++++++++++-----------------
 package/busybox/busybox.mk            |  6 ++++++
 package/pkg-kconfig.mk                |  2 +-
 package/uclibc/uclibc.mk              |  6 ++++++
 6 files changed, 42 insertions(+), 23 deletions(-)

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2015-07-12 12:11 UTC #1 lore raw

PATCH 1/7 core/pkg-kconfig: don't enforce check for config file when not building

Currently, this is triggering the error message:
make randconfig
make source

Only limit the check to enforce a config file being set to when we are actually building, like is done in the various kconfig-using packages.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/pkg-kconfig.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

package/pkg-kconfig.mk +1 -1

index c86c340..41c6fd5 100644
@@ -90,7 +90,7 @@ $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done
 ifeq ($$($$($(2)_KCONFIG_VAR)),y)
 
 # FOO_KCONFIG_FILE is required
-ifeq ($$($(2)_KCONFIG_FILE),)
+ifeq ($(BR_BUILDING)$$($(2)_KCONFIG_FILE),y)
 $$(error Internal error: no value specified for $(2)_KCONFIG_FILE)
 endif

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-07-12 12:11 UTC #2 lore raw

PATCH 2/7 package/linux: don't enforce check for DTS when not building

In reply to Yann E. MORIN

Currently, this is triggering the error message:
make randconfig
make source

Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 linux/linux.mk | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

linux/linux.mk +2 -2

index eca1450..b2c9481 100644
@@ -88,12 +88,12 @@ else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y)
 KERNEL_DTS_NAME = $(basename $(filter %.dts,$(notdir $(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_DTS_PATH)))))
 endif
 
-ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy)
 $(error No kernel device tree source specified, check your \
 BR2_LINUX_KERNEL_USE_INTREE_DTS / BR2_LINUX_KERNEL_USE_CUSTOM_DTS settings)
 endif
 
-ifeq ($(BR2_LINUX_KERNEL_APPENDED_DTB),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_APPENDED_DTB),yy)
 ifneq ($(words $(KERNEL_DTS_NAME)),1)
 $(error Kernel with appended device tree needs exactly one DTS source. \
 	Check BR2_LINUX_KERNEL_INTREE_DTS_NAME or BR2_LINUX_KERNEL_CUSTOM_DTS_PATH.)

Signature · 1 line

1.9.1

Baruch Siach

2015-07-12 20:03 UTC #3 lore raw

PATCH 2/7 package/linux: don't enforce check for DTS when not building

Hi Yann,

Quoted 9 lines · Yann E. MORIN

Currently, this is triggering the error message:
make randconfig
make source
Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables.
Signed-off-by "Yann E. MORIN" <[email protected]>

[snip]

linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) · line 91

Quoted 2 lines

linux/linux.mk +1 -1

-ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy)

yy means that exactly two of these must be 'y'. Is that intended?

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Arnout Vandecappelle

2015-07-12 23:44 UTC #4 lore raw

PATCH 2/7 package/linux: don't enforce check for DTS when not building

Quoted 11 lines · Baruch Siach

Hi Yann,
Quoted 8 lines · Yann E. MORIN
Currently, this is triggering the error message:
make randconfig
make source
Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables.

Quoted 8 lines

Quoted 1 line
Signed-off-by "Yann E. MORIN" <[email protected]>
[snip]
linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) · line 91
Quoted 2 lines
linux/linux.mk +1 -1
-ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy)
yy means that exactly two of these must be 'y'. Is that intended?

Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it
essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly it's not very transparent, it would make sense to add delimiters in-between at least.

Regards,
Arnout

Signature · 6 lines

Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

Baruch Siach

2015-07-13 03:37 UTC #5 lore raw

PATCH 2/7 package/linux: don't enforce check for DTS when not building

Hi Arnout,

Quoted 19 lines · Arnout Vandecappelle

Quoted 9 lines · Baruch Siach
Quoted 8 lines · Yann E. MORIN
Currently, this is triggering the error message:
make randconfig
make source
Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables.
Quoted 8 lines
Quoted 1 line
Signed-off-by "Yann E. MORIN" <[email protected]>
[snip]
linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) · line 91
Quoted 2 lines
linux/linux.mk +1 -1
-ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy)
yy means that exactly two of these must be 'y'. Is that intended?

Quoted 4 lines

Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it
essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly it's not very transparent, it would make sense to add delimiters in-between at least.

Thanks for the explanation. A comment would be helpful here, I guess.

Shouldn't we protect the code against someone naming a DTS just 'y'?

baruch

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -

Yann E. MORIN

author

2015-07-13 07:18 UTC #6 lore raw

PATCH 2/7 package/linux: don't enforce check for DTS when not building

Baruch, All,

On 2015-07-13 06:37 +0300, Baruch Siach spake thusly:

Quoted 20 lines · Arnout Vandecappelle

Quoted 19 lines · Baruch Siach
Quoted 17 lines · Yann E. MORIN
Quoted 9 lines
Currently, this is triggering the error message:
make randconfig
make source
Limit the checks that enforce a DTS is set and at most one DTB is appended to when we are actually building, like is done for the configuration-file variables.
Signed-off-by "Yann E. MORIN" <[email protected]>
[snip]
linux/linux.mk else ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_DTS),y) · line 91
Quoted 2 lines
linux/linux.mk +1 -1
-ifeq ($(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),y)
+ifeq ($(BR_BUILDING)$(BR2_LINUX_KERNEL_DTS_SUPPORT)$(KERNEL_DTS_NAME),yy)
yy means that exactly two of these must be 'y'. Is that intended?

Quoted 8 lines

Quoted 4 lines
Yes it is, because KERNEL_DTS_NAME is a string, not a boolean, so it
essentially checks that it is empty and DTS_SUPPORT is selected. But admittedly it's not very transparent, it would make sense to add delimiters in-between at least.
Thanks for the explanation. A comment would be helpful here, I guess.
Shouldn't we protect the code against someone naming a DTS just 'y'?

Yeah, we already discussed that with Thomas (live, we're in the same place for a few days), and we've come to the conclusion that this construct is indeed too tricky, and that we need a simpler code.

Your comments are now confirming this. ;-)

I'll rework the series shortly.

Thanks! :-)

Regards,
Yann E. MORIN.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

Yann E. MORIN

author

2015-07-12 12:11 UTC #7 lore raw

PATCH 3/7 boot/at91boostrap3: check for config file before calling kconfig-package

In reply to Yann E. MORIN

If we check that the user provides a config file after we call to the kconfig-package infra, the error message we get is the one for the kconfig-package infra, not the custom error message we want to show to the user.

So, only call kconfig-package after we do the check.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 boot/at91bootstrap3/at91bootstrap3.mk | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

boot/at91bootstrap3/at91bootstrap3.mk +5 -2

index 1c68094..fa67ea6 100644
@@ -49,10 +49,11 @@ endif
 AT91BOOTSTRAP3_KCONFIG_FILE = $(AT91BOOTSTRAP3_SOURCE_CONFIG)
 AT91BOOTSTRAP3_KCONFIG_EDITORS = menuconfig xconfig gconfig
 AT91BOOTSTRAP3_KCONFIG_OPTS = $(AT91BOOTSTRAP3_MAKE_OPTS)
-$(eval $(kconfig-package))
 
 # Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
 ifeq ($(BR_BUILDING),y)
+
 ifeq ($(BR2_TARGET_AT91BOOTSTRAP3_USE_DEFCONFIG),y)
 ifeq ($(call qstrip,$(BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG)),)
 $(error No at91bootstrap3 defconfig name specified, check your BR2_TARGET_AT91BOOTSTRAP3_DEFCONFIG setting)

@@ -74,4 +75,6 @@ $(error No custom at91bootstrap3 repository version specified. Check your BR2_TA
 endif
 endif
 
-endif
+endif # BR_BUILDING
+
+$(eval $(kconfig-package))

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-07-12 12:11 UTC #8 lore raw

PATCH 4/7 boot/barebox: check for config file before calling kconfig-package

In reply to Yann E. MORIN

So, only call kconfig-package after we do the check.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 boot/barebox/barebox.mk | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

boot/barebox/barebox.mk +4 -2

index 7e53037..e45976d 100644
@@ -109,10 +109,12 @@ define BAREBOX_INSTALL_TARGET_CMDS
 endef
 endif
 
-$(eval $(kconfig-package))
-
+# Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
 ifeq ($(BR2_TARGET_BAREBOX)$(BR_BUILDING),yy)
 ifeq ($(BAREBOX_SOURCE_CONFIG),)
 $(error No Barebox config file. Check your BR2_TARGET_BAREBOX_BOARD_DEFCONFIG or BR2_TARGET_BAREBOX_CUSTOM_CONFIG_FILE settings)
 endif
 endif
+
+$(eval $(kconfig-package))

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-07-12 12:11 UTC #9 lore raw

PATCH 5/7 package/linux: check for config file before calling kconfig-package

In reply to Yann E. MORIN

So, only call kconfig-package after we do the check.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 linux/linux.mk | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

linux/linux.mk +18 ~~-16~~

index b2c9481..e9866b8 100644
@@ -342,6 +342,24 @@ LINUX_PRE_PATCH_HOOKS += $(foreach ext,$(LINUX_EXTENSIONS),\
 	$(if $(BR2_LINUX_KERNEL_EXT_$(call UPPERCASE,$(ext))),\
 		$(call UPPERCASE,$(ext))_PREPARE_KERNEL))
 
+# Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
+ifeq ($(BR_BUILDING),y)
+
+ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
+ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG)),)
+$(error No kernel defconfig name specified, check your BR2_LINUX_KERNEL_DEFCONFIG setting)
+endif
+endif
+
+ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
+ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)),)
+$(error No kernel configuration file specified, check your BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE setting)
+endif
+endif
+
+endif # BR_BUILDIING
+
 $(eval $(kconfig-package))
 
 # Support for rebuilding the kernel after the cpio archive has

@@ -360,19 +378,3 @@ $(LINUX_DIR)/.stamp_initramfs_rebuilt: $(LINUX_DIR)/.stamp_target_installed $(LI
 # The initramfs building code must make sure this target gets called
 # after it generated the initramfs list of files.
 linux-rebuild-with-initramfs: $(LINUX_DIR)/.stamp_initramfs_rebuilt
-
-# Checks to give errors that the user can understand
-ifeq ($(BR_BUILDING),y)
-ifeq ($(BR2_LINUX_KERNEL_USE_DEFCONFIG),y)
-ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_DEFCONFIG)),)
-$(error No kernel defconfig name specified, check your BR2_LINUX_KERNEL_DEFCONFIG setting)
-endif
-endif
-
-ifeq ($(BR2_LINUX_KERNEL_USE_CUSTOM_CONFIG),y)
-ifeq ($(call qstrip,$(BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE)),)
-$(error No kernel configuration file specified, check your BR2_LINUX_KERNEL_CUSTOM_CONFIG_FILE setting)
-endif
-endif
-
-endif

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-07-12 12:11 UTC #10 lore raw

PATCH 6/7 package/busybox: check for config file before calling kconfig-package

In reply to Yann E. MORIN

So, only call kconfig-package after we do the check.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/busybox/busybox.mk | 6 ++++++
 1 file changed, 6 insertions(+)

package/busybox/busybox.mk +6 -0

index 92874cd..6b57fd7 100644
@@ -207,4 +207,10 @@ define BUSYBOX_INSTALL_INIT_SYSV
 	$(BUSYBOX_INSTALL_WATCHDOG_SCRIPT)
 endef
 
+# Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
+ifeq ($(BR_BUILDING)$(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
+$(error No BusyBox configuration file specified, check your BR2_PACKAGE_BUSYBOX_CONFIG setting)
+endif
+
 $(eval $(kconfig-package))

Signature · 1 line

1.9.1

Yann E. MORIN

author

2015-07-12 12:11 UTC #11 lore raw

PATCH 7/7 package/uclibc: check for config file before calling kconfig-package

In reply to Yann E. MORIN

So, only call kconfig-package after we do the check.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/uclibc/uclibc.mk | 6 ++++++
 1 file changed, 6 insertions(+)

package/uclibc/uclibc.mk +6 -0

index ecbfa11..7dee6b3 100644
@@ -474,4 +474,10 @@ define UCLIBC_INSTALL_STAGING_CMDS
 	$(UCLIBC_INSTALL_UTILS_STAGING)
 endef
 
+# Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
+ifeq ($(BR_BUILDING)$(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
+$(error No uClibc configuration file specified, check your BR2_UCLIBC_CONFIG setting)
+endif
+
 $(eval $(kconfig-package))

Signature · 1 line

1.9.1

Baruch Siach

2015-07-12 20:06 UTC #12 lore raw

PATCH 7/7 package/uclibc: check for config file before calling kconfig-package

Hi Yann,

Quoted 6 lines · Yann E. MORIN

If we check that the user provides a config file after we call to the kconfig-package infra, the error message we get is the one for the kconfig-package infra, not the custom error message we want to show to the user.
So, only call kconfig-package after we do the check.

The commit log doesn't match the patch. There was no error message before this patch, AFAICS.

baruch

Quoted 20 lines

Signed-off-by "Yann E. MORIN" <[email protected]>

---
 package/uclibc/uclibc.mk | 6 ++++++
 1 file changed, 6 insertions(+)

package/uclibc/uclibc.mk +6 -0

index ecbfa11..7dee6b3 100644
@@ -474,4 +474,10 @@ define UCLIBC_INSTALL_STAGING_CMDS
 	$(UCLIBC_INSTALL_UTILS_STAGING)
 endef
 
+# Checks to give errors that the user can understand
+# Must be before we call to kconfig-package
+ifeq ($(BR_BUILDING)$(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
+$(error No uClibc configuration file specified, check your BR2_UCLIBC_CONFIG setting)
+endif
+
 $(eval $(kconfig-package))

Signature · 3 lines

     http://baruch.siach.name/blog/                  ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -