[U-Boot] [PATCH 0/5] Refactor makefiles for u-boot.cfg rules
The Makefiles are getting cluttered for u-boot.cfg.
I do not think that we should make effort for u-boot.cfg because it is almost the same as include/autoconf.mk.tmp
I was consindering this series as a replacement for Stephen Warren's fixes, but I was late.
Anyway, I do not like code duplication, so here it is.
Masahiro Yamada (5): check-config: fix wrong comment about how to build whitelist kbuild: move no_new_adhoc_configs_check to "all" target command kbuild: make dependencies in scripts/Makefile.autoconf more readable kbuild: generate u-boot.cfg as a byproduct of include/autoconf.mk check-config: allow to complete build even with ad-hoc CONFIG options
Makefile | 29 ++++++----------------------- scripts/Makefile.autoconf | 42 +++++++++++++++++++++++++++--------------- scripts/Makefile.spl | 20 +------------------- scripts/check-config.sh | 12 +++--------- 4 files changed, 37 insertions(+), 66 deletions(-)
The command suggested in this comment block is wrong; it would not rip off CONFIG options that had already been converted to Kconfig.
Instead, we should use the scripts/build-whitelist.sh tool.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
scripts/check-config.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 28c8fe9..6618dfb 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -5,13 +5,8 @@ # Check that the u-boot.cfg file provided does not introduce any new # ad-hoc CONFIG options # -# You can generate the list of current ad-hoc CONFIG options (those which are -# not in Kconfig) with this command: -# -# export LC_ALL=C LC_COLLATE=C -# git grep CONFIG_ |tr ' \t' '\n\n' |sed -n 's/^(CONFIG_[A-Z0-9_]*).*/\1/p' \ -# |sort |uniq >scripts/config_whitelist.txt; -# unset LC_ALL LC_COLLATE +# Use scripts/build-whitelist.sh to generate the list of current ad-hoc +# CONFIG options (those which are not in Kconfig).
# Usage # check-config.sh <path to u-boot.cfg> <path to whitelist file> <source dir>
Hi Masahiro,
On 25 September 2016 at 22:04, Masahiro Yamada yamada.masahiro@socionext.com wrote:
The command suggested in this comment block is wrong; it would not rip off CONFIG options that had already been converted to Kconfig.
Instead, we should use the scripts/build-whitelist.sh tool.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/check-config.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 28c8fe9..6618dfb 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -5,13 +5,8 @@ # Check that the u-boot.cfg file provided does not introduce any new # ad-hoc CONFIG options # -# You can generate the list of current ad-hoc CONFIG options (those which are -# not in Kconfig) with this command: -# -# export LC_ALL=C LC_COLLATE=C -# git grep CONFIG_ |tr ' \t' '\n\n' |sed -n 's/^(CONFIG_[A-Z0-9_]*).*/\1/p' \ -# |sort |uniq >scripts/config_whitelist.txt; -# unset LC_ALL LC_COLLATE +# Use scripts/build-whitelist.sh to generate the list of current ad-hoc +# CONFIG options (those which are not in Kconfig).
For me the LC setup is needed. Does it work correctly without it for you? I found that the sorting was wrong.
# Usage
# check-config.sh <path to u-boot.cfg> <path to whitelist file> <source dir>
1.9.1
Regards, Simon
Hi Simon,
2016-09-27 9:34 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 25 September 2016 at 22:04, Masahiro Yamada yamada.masahiro@socionext.com wrote:
The command suggested in this comment block is wrong; it would not rip off CONFIG options that had already been converted to Kconfig.
Instead, we should use the scripts/build-whitelist.sh tool.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/check-config.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 28c8fe9..6618dfb 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -5,13 +5,8 @@ # Check that the u-boot.cfg file provided does not introduce any new # ad-hoc CONFIG options # -# You can generate the list of current ad-hoc CONFIG options (those which are -# not in Kconfig) with this command: -# -# export LC_ALL=C LC_COLLATE=C -# git grep CONFIG_ |tr ' \t' '\n\n' |sed -n 's/^(CONFIG_[A-Z0-9_]*).*/\1/p' \ -# |sort |uniq >scripts/config_whitelist.txt; -# unset LC_ALL LC_COLLATE +# Use scripts/build-whitelist.sh to generate the list of current ad-hoc +# CONFIG options (those which are not in Kconfig).
For me the LC setup is needed. Does it work correctly without it for you? I found that the sorting was wrong.
I am not quite sure about this, but it worked for me.
I can see
export LC_ALL=C export LC_COLLATE=C
in both check-config.sh and build-whitelist.sh
That's why?
On 26 September 2016 at 19:13, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Hi Simon,
2016-09-27 9:34 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 25 September 2016 at 22:04, Masahiro Yamada yamada.masahiro@socionext.com wrote:
The command suggested in this comment block is wrong; it would not rip off CONFIG options that had already been converted to Kconfig.
Instead, we should use the scripts/build-whitelist.sh tool.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/check-config.sh | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 28c8fe9..6618dfb 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -5,13 +5,8 @@ # Check that the u-boot.cfg file provided does not introduce any new # ad-hoc CONFIG options # -# You can generate the list of current ad-hoc CONFIG options (those which are -# not in Kconfig) with this command: -# -# export LC_ALL=C LC_COLLATE=C -# git grep CONFIG_ |tr ' \t' '\n\n' |sed -n 's/^(CONFIG_[A-Z0-9_]*).*/\1/p' \ -# |sort |uniq >scripts/config_whitelist.txt; -# unset LC_ALL LC_COLLATE +# Use scripts/build-whitelist.sh to generate the list of current ad-hoc +# CONFIG options (those which are not in Kconfig).
For me the LC setup is needed. Does it work correctly without it for you? I found that the sorting was wrong.
I am not quite sure about this, but it worked for me.
I can see
export LC_ALL=C export LC_COLLATE=C
in both check-config.sh and build-whitelist.sh
That's why?
OK thanks. I must have fixed it and forgotten about it.
Reviewed-by: Simon Glass sjg@chromium.org
BTW, what do you think about updating moveconfig.py to remove things from the whitelist as well?
Regards, Simon
2016-10-06 1:50 GMT+09:00 Simon Glass sjg@chromium.org:
BTW, what do you think about updating moveconfig.py to remove things from the whitelist as well?
Yeah, I was also thinking of this.
On Mon, Sep 26, 2016 at 01:04:58PM +0900, Masahiro Yamada wrote:
The command suggested in this comment block is wrong; it would not rip off CONFIG options that had already been converted to Kconfig.
Instead, we should use the scripts/build-whitelist.sh tool.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
I am going to move the build rule of u-boot.cfg. Before that, no_new_adhoc_configs_check must be tweaked to not depend on it.
The ad-hoc option check can be done at the end of build, along with other checks.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Makefile | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/Makefile b/Makefile index c30f90a..2f7a52b 100644 --- a/Makefile +++ b/Makefile @@ -741,7 +741,7 @@ endif
# Always append ALL so that arch config.mk's can add custom ones ALL-y += u-boot.srec u-boot.bin u-boot.sym System.map u-boot.cfg \ - binary_size_check no_new_adhoc_configs_check + binary_size_check
ALL-$(CONFIG_ONENAND_U_BOOT) += u-boot-onenand.bin ifeq ($(CONFIG_SPL_FSL_PBL),y) @@ -820,6 +820,11 @@ ifeq ($(CONFIG_DM_I2C_COMPAT)$(CONFIG_SANDBOX),y) @echo "before sending patches to the mailing list." @echo "====================================================" endif + @# Check that this build does not use CONFIG options that we do not + @# know about unless they are in Kconfig. All the existing CONFIG + @# options are whitelisted, so new ones should not be added. + $(srctree)/scripts/check-config.sh u-boot.cfg \ + $(srctree)/scripts/config_whitelist.txt ${srctree} 1>&2
PHONY += dtbs dtbs: dts/dt.dtb @@ -950,13 +955,6 @@ endif u-boot.cfg: include/config.h FORCE $(call if_changed_dep,cpp_cfg)
-# Check that this build does not use CONFIG options that we don't know about -# unless they are in Kconfig. All the existing CONFIG options are whitelisted, -# so new ones should not be added. -no_new_adhoc_configs_check: u-boot.cfg FORCE - $(srctree)/scripts/check-config.sh $< \ - $(srctree)/scripts/config_whitelist.txt ${srctree} 1>&2 - ifdef CONFIG_TPL SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else
On 25 September 2016 at 22:04, Masahiro Yamada yamada.masahiro@socionext.com wrote:
I am going to move the build rule of u-boot.cfg. Before that, no_new_adhoc_configs_check must be tweaked to not depend on it.
The ad-hoc option check can be done at the end of build, along with other checks.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
Makefile | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Sep 26, 2016 at 01:04:59PM +0900, Masahiro Yamada wrote:
I am going to move the build rule of u-boot.cfg. Before that, no_new_adhoc_configs_check must be tweaked to not depend on it.
The ad-hoc option check can be done at the end of build, along with other checks.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
I do not remember why I wrote the code like this, but let's make it a bit more readable.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
scripts/Makefile.autoconf | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index 01a739d..ba674f8 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -46,7 +46,7 @@ quiet_cmd_autoconf_dep = GEN $@ -MQ include/config/auto.conf $(srctree)/include/common.h > $@ || { \ rm $@; false; \ } -include/autoconf.mk.dep: FORCE +include/autoconf.mk.dep: include/config.h FORCE $(call cmd,autoconf_dep)
# We are migrating from board headers to Kconfig little by little. @@ -71,20 +71,17 @@ quiet_cmd_autoconf = GEN $@ rm $@.tmp; false; \ }
-include/autoconf.mk: FORCE +include/autoconf.mk: include/config.h FORCE $(call cmd,autoconf)
-spl/include/autoconf.mk: FORCE +spl/include/autoconf.mk: include/config.h FORCE $(Q)mkdir -p $(dir $@) $(call cmd,autoconf,-DCONFIG_SPL_BUILD)
-tpl/include/autoconf.mk: FORCE +tpl/include/autoconf.mk: include/config.h FORCE $(Q)mkdir -p $(dir $@) $(call cmd,autoconf,-DCONFIG_SPL_BUILD -DCONFIG_TPL_BUILD)
-include/autoconf.mk include/autoconf.mk.dep \ - spl/include/autoconf.mk tpl/include/autoconf.mk: include/config.h - # include/config.h # Prior to Kconfig, it was generated by mkconfig. Now it is created here. define filechk_config_h
On 25 September 2016 at 22:05, Masahiro Yamada yamada.masahiro@socionext.com wrote:
I do not remember why I wrote the code like this, but let's make it a bit more readable.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/Makefile.autoconf | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Sep 26, 2016 at 01:05:00PM +0900, Masahiro Yamada wrote:
I do not remember why I wrote the code like this, but let's make it a bit more readable.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Our build system still parses ad-hoc CONFIG options in header files and generates include/autoconf.mk so that Makefiles can reference them. This gimmick was introduced in the pre-Kconfig days and will be kept until Kconfig migration is completed.
The include/autoconf.mk is generated like follows:
[1] Preprocess include/common.h with -DDO_DEPS_ONLY and retrieve macros into include/autoconf.mk.tmp [2] Reformat include/autoconf.mk.dep into include/autoconf.mk with tools/scripts/define2mk.sed script [3] Remove include/autoconf.mk.tmp
Here, include/autoconf.mk.tmp is similar to u-boot.cfg, which is also generated by preprocessing include/config.h with -DDO_DEPS_ONLY. In other words, there is much overlap among include/autoconf.mk and u-boot.cfg build rules.
So, the idea is to split the build rule of include/autoconf.mk into two stages. The first preprocesses headers into u-boot.cfg. The second parses the u-boot.cfg into include/autoconf.mk. The build rules of u-boot.cfg in Makefile and spl/Makefile will be gone.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
Makefile | 17 +---------------- scripts/Makefile.autoconf | 37 ++++++++++++++++++++++++++----------- scripts/Makefile.spl | 20 +------------------- 3 files changed, 28 insertions(+), 46 deletions(-)
diff --git a/Makefile b/Makefile index 2f7a52b..095e348 100644 --- a/Makefile +++ b/Makefile @@ -740,8 +740,7 @@ DO_STATIC_RELA = endif
# Always append ALL so that arch config.mk's can add custom ones -ALL-y += u-boot.srec u-boot.bin u-boot.sym System.map u-boot.cfg \ - binary_size_check +ALL-y += u-boot.srec u-boot.bin u-boot.sym System.map binary_size_check
ALL-$(CONFIG_ONENAND_U_BOOT) += u-boot-onenand.bin ifeq ($(CONFIG_SPL_FSL_PBL),y) @@ -941,20 +940,6 @@ u-boot.sha1: u-boot.bin u-boot.dis: u-boot $(OBJDUMP) -d $< > $@
-# If .u-boot.cfg.d is still present, then either: -# a) The previous build used a Makefile that used if_changed rather than -# if_changed_dep when building u-boot.cfg, and hence any later builds will -# be unaware of the dependencies for u-boot.cfg. In this case, we must -# delete u-boot.cfg to force it and .u-boot.cfg.cmd to be rebuilt the -# correct way. -# b) The previous build failed or was interrupted while building u-boot.cfg, -# so deleting u-boot.cfg isn't going to cause any additional work. -ifneq ($(wildcard $(obj)/.u-boot.cfg.d),) - unused := $(shell rm -f $(obj)/u-boot.cfg) -endif -u-boot.cfg: include/config.h FORCE - $(call if_changed_dep,cpp_cfg) - ifdef CONFIG_TPL SPL_PAYLOAD := tpl/u-boot-with-tpl.bin else diff --git a/scripts/Makefile.autoconf b/scripts/Makefile.autoconf index ba674f8..2f85eb9 100644 --- a/scripts/Makefile.autoconf +++ b/scripts/Makefile.autoconf @@ -58,29 +58,44 @@ include/autoconf.mk.dep: include/config.h FORCE # same CONFIG macros quiet_cmd_autoconf = GEN $@ cmd_autoconf = \ - $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM $(srctree)/include/common.h > $@.tmp && { \ - sed -n -f $(srctree)/tools/scripts/define2mk.sed $@.tmp | \ + sed -n -f $(srctree)/tools/scripts/define2mk.sed $< | \ while read line; do \ if [ -n "${KCONFIG_IGNORE_DUPLICATES}" ] || \ ! grep -q "$${line%=*}=" include/config/auto.conf; then \ echo "$$line"; \ fi \ - done > $@; \ - rm $@.tmp; \ - } || { \ - rm $@.tmp; false; \ + done > $@ + +quiet_cmd_u_boot_cfg = CFG $@ + cmd_u_boot_cfg = \ + $(CPP) $(c_flags) $2 -DDO_DEPS_ONLY -dM $(srctree)/include/common.h > $@.tmp && { \ + grep 'define CONFIG_' $@.tmp > $@; \ + rm $@.tmp; \ + } || { \ + rm $@.tmp; false; \ }
-include/autoconf.mk: include/config.h FORCE +u-boot.cfg: include/config.h FORCE + $(call cmd,u_boot_cfg) + +spl/u-boot.cfg: include/config.h FORCE + $(Q)mkdir -p $(dir $@) + $(call cmd,u_boot_cfg,-DCONFIG_SPL_BUILD) + +tpl/u-boot.cfg: include/config.h FORCE + $(Q)mkdir -p $(dir $@) + $(call cmd,u_boot_cfg,-DCONFIG_SPL_BUILD -DCONFIG_TPL_BUILD) + +include/autoconf.mk: u-boot.cfg $(call cmd,autoconf)
-spl/include/autoconf.mk: include/config.h FORCE +spl/include/autoconf.mk: spl/u-boot.cfg $(Q)mkdir -p $(dir $@) - $(call cmd,autoconf,-DCONFIG_SPL_BUILD) + $(call cmd,autoconf)
-tpl/include/autoconf.mk: include/config.h FORCE +tpl/include/autoconf.mk: tpl/u-boot.cfg $(Q)mkdir -p $(dir $@) - $(call cmd,autoconf,-DCONFIG_SPL_BUILD -DCONFIG_TPL_BUILD) + $(call cmd,autoconf)
# include/config.h # Prior to Kconfig, it was generated by mkconfig. Now it is created here. diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl index 5a7f79c..677eef2 100644 --- a/scripts/Makefile.spl +++ b/scripts/Makefile.spl @@ -152,7 +152,7 @@ spl/boot.bin: $(obj)/u-boot-spl.bin FORCE $(call if_changed,mkimage) endif
-ALL-y += $(obj)/$(SPL_BIN).bin $(obj)/$(SPL_BIN).cfg +ALL-y += $(obj)/$(SPL_BIN).bin
ifdef CONFIG_SAMSUNG ALL-y += $(obj)/$(BOARD)-spl.bin @@ -212,24 +212,6 @@ quiet_cmd_fdtgrep = FDTGREP $@ $(obj)/$(SPL_BIN).dtb: dts/dt.dtb $(objtree)/tools/fdtgrep FORCE $(call if_changed,fdtgrep)
-quiet_cmd_cpp_cfg = CFG $@ -cmd_cpp_cfg = $(CPP) -Wp,-MD,$(depfile) $(cpp_flags) $(LDPPFLAGS) -ansi \ - -DDO_DEPS_ONLY -D__ASSEMBLY__ -x assembler-with-cpp -P -dM -E -o $@ $< - -# If .u-boot.cfg.d is still present, then either: -# a) The previous build used a Makefile that used if_changed rather than -# if_changed_dep when building u-boot.cfg, and hence any later builds will -# be unaware of the dependencies for u-boot.cfg. In this case, we must -# delete u-boot.cfg to force it and .u-boot.cfg.cmd to be rebuilt the -# correct way. -# b) The previous build failed or was interrupted while building u-boot.cfg, -# so deleting u-boot.cfg isn't going to cause any additional work. -ifneq ($(wildcard $(obj)/.$(SPL_BIN).d),) - unused := $(shell rm -f $(obj)/$(SPL_BIN).cfg) -endif -$(obj)/$(SPL_BIN).cfg: include/config.h FORCE - $(call if_changed_dep,cpp_cfg) - pythonpath = PYTHONPATH=tools
quiet_cmd_dtocc = DTOC C $@
On 25 September 2016 at 22:05, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Our build system still parses ad-hoc CONFIG options in header files and generates include/autoconf.mk so that Makefiles can reference them. This gimmick was introduced in the pre-Kconfig days and will be kept until Kconfig migration is completed.
The include/autoconf.mk is generated like follows:
[1] Preprocess include/common.h with -DDO_DEPS_ONLY and retrieve macros into include/autoconf.mk.tmp [2] Reformat include/autoconf.mk.dep into include/autoconf.mk with tools/scripts/define2mk.sed script [3] Remove include/autoconf.mk.tmp
Here, include/autoconf.mk.tmp is similar to u-boot.cfg, which is also generated by preprocessing include/config.h with -DDO_DEPS_ONLY. In other words, there is much overlap among include/autoconf.mk and u-boot.cfg build rules.
So, the idea is to split the build rule of include/autoconf.mk into two stages. The first preprocesses headers into u-boot.cfg. The second parses the u-boot.cfg into include/autoconf.mk. The build rules of u-boot.cfg in Makefile and spl/Makefile will be gone.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
Makefile | 17 +---------------- scripts/Makefile.autoconf | 37 ++++++++++++++++++++++++++----------- scripts/Makefile.spl | 20 +------------------- 3 files changed, 28 insertions(+), 46 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
On Mon, Sep 26, 2016 at 01:05:01PM +0900, Masahiro Yamada wrote:
Our build system still parses ad-hoc CONFIG options in header files and generates include/autoconf.mk so that Makefiles can reference them. This gimmick was introduced in the pre-Kconfig days and will be kept until Kconfig migration is completed.
The include/autoconf.mk is generated like follows:
[1] Preprocess include/common.h with -DDO_DEPS_ONLY and retrieve macros into include/autoconf.mk.tmp [2] Reformat include/autoconf.mk.dep into include/autoconf.mk with tools/scripts/define2mk.sed script [3] Remove include/autoconf.mk.tmp
Here, include/autoconf.mk.tmp is similar to u-boot.cfg, which is also generated by preprocessing include/config.h with -DDO_DEPS_ONLY. In other words, there is much overlap among include/autoconf.mk and u-boot.cfg build rules.
So, the idea is to split the build rule of include/autoconf.mk into two stages. The first preprocesses headers into u-boot.cfg. The second parses the u-boot.cfg into include/autoconf.mk. The build rules of u-boot.cfg in Makefile and spl/Makefile will be gone.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com Reviewed-by: Simon Glass sjg@chromium.org
Applied to u-boot/master, thanks!
Currently, the check-config.sh terminates the build when unknown ad-hoc options are detected. I think it is too much because we may want to patch config headers locally in a build/deployment project.
So, let's relax check-config.sh to just warn even if it detects options that are not in the whitelist. Instead, this check can be done at the end of build, along with other checks. It will catch more attention.
Even with this change, the Buildman tool catches new warnings, so Tom can give NACK to new ad-hoc options.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
scripts/check-config.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/scripts/check-config.sh b/scripts/check-config.sh index 6618dfb..9d2cfc6 100755 --- a/scripts/check-config.sh +++ b/scripts/check-config.sh @@ -37,14 +37,13 @@ cat `find ${srctree} -name "Kconfig*"` |sed -n \ -e 's/^menuconfig ([A-Za-z0-9_]*).*$/CONFIG_\1/p' |sort |uniq > ${ok} comm -23 ${suspects} ${ok} >${new_adhoc} if [ -s ${new_adhoc} ]; then - echo "Error: You must add new CONFIG options using Kconfig" + echo "Warning: You must add new CONFIG options using Kconfig" echo "The following new ad-hoc CONFIG options were detected:" cat ${new_adhoc} echo echo "Please add these via Kconfig instead. Find a suitable Kconfig" echo "file and add a 'config' or 'menuconfig' option." # Don't delete the temporary files in case they are useful - exit 1 else rm ${suspects} ${ok} ${new_adhoc} fi
Hi Masahiro,
On 25 September 2016 at 22:05, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Currently, the check-config.sh terminates the build when unknown ad-hoc options are detected. I think it is too much because we may want to patch config headers locally in a build/deployment project.
So, let's relax check-config.sh to just warn even if it detects options that are not in the whitelist. Instead, this check can be done at the end of build, along with other checks. It will catch more attention.
Even with this change, the Buildman tool catches new warnings, so Tom can give NACK to new ad-hoc options.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/check-config.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
I am not sure this is a good idea. We need buildman to fail the build; otherwise patches will make it to mainline with new CONFIGs. Perhaps we should have an ENV variable to skip the check?
Regards, Simon
Hi Simon,
2016-09-27 9:34 GMT+09:00 Simon Glass sjg@chromium.org:
Hi Masahiro,
On 25 September 2016 at 22:05, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Currently, the check-config.sh terminates the build when unknown ad-hoc options are detected. I think it is too much because we may want to patch config headers locally in a build/deployment project.
So, let's relax check-config.sh to just warn even if it detects options that are not in the whitelist. Instead, this check can be done at the end of build, along with other checks. It will catch more attention.
Even with this change, the Buildman tool catches new warnings, so Tom can give NACK to new ad-hoc options.
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
scripts/check-config.sh | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
I am not sure this is a good idea. We need buildman to fail the build; otherwise patches will make it to mainline with new CONFIGs. Perhaps we should have an ENV variable to skip the check?
It is OK for me as long as there is a way to work-around this check, ENV or whatever.
On Mon, Sep 26, 2016 at 01:05:02PM +0900, Masahiro Yamada wrote:
Currently, the check-config.sh terminates the build when unknown ad-hoc options are detected. I think it is too much because we may want to patch config headers locally in a build/deployment project.
I'm not yet convinced this is a good use case. In the not-so-long-now term include/configs/ should go away. So you wouldn't be able to do an ad-hoc thing like this for testing. And tossing stuff ad-hoc into $(BOARDDIR)/Kconfig should be easy enough too, yes?
Hi Tom, Simon,
2016-09-28 2:44 GMT+09:00 Tom Rini trini@konsulko.com:
On Mon, Sep 26, 2016 at 01:05:02PM +0900, Masahiro Yamada wrote:
Currently, the check-config.sh terminates the build when unknown ad-hoc options are detected. I think it is too much because we may want to patch config headers locally in a build/deployment project.
I'm not yet convinced this is a good use case. In the not-so-long-now term include/configs/ should go away. So you wouldn't be able to do an ad-hoc thing like this for testing. And tossing stuff ad-hoc into $(BOARDDIR)/Kconfig should be easy enough too, yes?
You are right.
I tend to add local-hack options into my header file just because it is easier than creating Kconfig entries and enabling them from a defconfig, but it is possible to do so with a little more effort.
So, I retract this patch and marked the Patchwork one as Rejected.
participants (3)
-
Masahiro Yamada -
Simon Glass -
Tom Rini