diff mbox

[5/5] check-config: allow to complete build even with ad-hoc CONFIG options

Message ID 1474862702-16580-6-git-send-email-yamada.masahiro@socionext.com
State New
Headers show

Commit Message

Masahiro Yamada Sept. 26, 2016, 4:05 a.m. UTC
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(-)

-- 
1.9.1

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Comments

Masahiro Yamada Sept. 27, 2016, 1:16 a.m. UTC | #1
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.





-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Masahiro Yamada Sept. 28, 2016, 5:23 a.m. UTC | #2
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.



-- 
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
diff mbox

Patch

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