diff mbox series

scripts: merge_config: add strict mode to fail upon any redefinition

Message ID 20201202151238.3776616-1-maennich@google.com (mailing list archive)
State New, archived
Headers show
Series scripts: merge_config: add strict mode to fail upon any redefinition | expand

Commit Message

Matthias Männich Dec. 2, 2020, 3:12 p.m. UTC
When merging configuration fragments, it might be of interest to
identify mismatches (redefinitions) programmatically. Hence add the
option -s (strict mode) to instruct merge_config.sh to bail out in
case any redefinition has been detected.

With strict mode, warnings are emitted as before, but the script
terminates with rc=1. If -y is set to define "builtin having
precedence over modules", fragments are still allowed to set =m (while
the base config has =y). Strict mode will tolerate that as demotions
from =y to =m are ignored when setting -y.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Matthias Maennich <maennich@google.com>
---
 scripts/kconfig/merge_config.sh | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Comments

Masahiro Yamada July 24, 2021, 1:56 p.m. UTC | #1
On Thu, Jul 22, 2021 at 7:13 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> Masahiro,
>
> On Fri, 21 May 2021 at 10:29, Lee Jones <lee.jones@linaro.org> wrote:
>>
>> On Wed, 2 Dec 2020 at 15:13, Matthias Maennich <maennich@google.com> wrote:
>>>
>>> When merging configuration fragments, it might be of interest to
>>> identify mismatches (redefinitions) programmatically. Hence add the
>>> option -s (strict mode) to instruct merge_config.sh to bail out in
>>> case any redefinition has been detected.
>>>
>>> With strict mode, warnings are emitted as before, but the script
>>> terminates with rc=1. If -y is set to define "builtin having
>>> precedence over modules", fragments are still allowed to set =m (while
>>> the base config has =y). Strict mode will tolerate that as demotions
>>> from =y to =m are ignored when setting -y.
>>>
>>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>>> Signed-off-by: Matthias Maennich <maennich@google.com>
>>> ---
>>>  scripts/kconfig/merge_config.sh | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>
>>
>> Reviewed-by: Lee Jones <lee.jones@linaro.org>
>
>
> Any idea what's holding this up please?

Simply because I hate this script.


merge_config.sh itself is a bad hack.
I do not like to extend it further.

Not only this one.
I saw more people with
"hey, I came up with a new option for merge_config.sh"
to do whatever they like to do.

However, it might be too late anyway.

So, I can merge this patch if people believe
it is useful.
Masahiro Yamada Aug. 1, 2021, 1:51 a.m. UTC | #2
On Mon, Jul 26, 2021 at 8:01 PM Lee Jones <lee.jones@linaro.org> wrote:
>
> On Sat, 24 Jul 2021, 14:57 Masahiro Yamada, <masahiroy@kernel.org> wrote:
>>
>> On Thu, Jul 22, 2021 at 7:13 PM Lee Jones <lee.jones@linaro.org> wrote:
>> >
>> > Masahiro,
>> >
>> > On Fri, 21 May 2021 at 10:29, Lee Jones <lee.jones@linaro.org> wrote:
>> >>
>> >> On Wed, 2 Dec 2020 at 15:13, Matthias Maennich <maennich@google.com> wrote:
>> >>>
>> >>> When merging configuration fragments, it might be of interest to
>> >>> identify mismatches (redefinitions) programmatically. Hence add the
>> >>> option -s (strict mode) to instruct merge_config.sh to bail out in
>> >>> case any redefinition has been detected.
>> >>>
>> >>> With strict mode, warnings are emitted as before, but the script
>> >>> terminates with rc=1. If -y is set to define "builtin having
>> >>> precedence over modules", fragments are still allowed to set =m (while
>> >>> the base config has =y). Strict mode will tolerate that as demotions
>> >>> from =y to =m are ignored when setting -y.
>> >>>
>> >>> Cc: Masahiro Yamada <masahiroy@kernel.org>
>> >>> Signed-off-by: Matthias Maennich <maennich@google.com>
>> >>> ---
>> >>>  scripts/kconfig/merge_config.sh | 15 +++++++++++++++
>> >>>  1 file changed, 15 insertions(+)
>> >>
>> >>
>> >> Reviewed-by: Lee Jones <lee.jones@linaro.org>
>> >
>> >
>> > Any idea what's holding this up please?
>>
>> Simply because I hate this script.
>>
>>
>> merge_config.sh itself is a bad hack.
>> I do not like to extend it further.
>
>
> Sorry you feel that way Masahiro.


I do not know why merge_config was implemented as
a separate shell script while all the other
functions were contained in the kconfig binary.

Anyway...


>> Not only this one.
>> I saw more people with
>> "hey, I came up with a new option for merge_config.sh"
>> to do whatever they like to do.
>>
>> However, it might be too late anyway.
>>
>>
>> So, I can merge this patch if people believe
>> it is useful.
>
>
> I know of multiple entities who make good use of the script and this extension.
>
> My vote is to merge it, but ultimately this is your train set.


OK, I decided to not care about it too much.

Now applied to linux-kbuild. Thanks.
Lee Jones Aug. 2, 2021, 7:50 a.m. UTC | #3
> > I know of multiple entities who make good use of the script and this extension.
> >
> > My vote is to merge it, but ultimately this is your train set.
> 
> OK, I decided to not care about it too much.
> 
> Now applied to linux-kbuild. Thanks.

Thanks Masahiro.  Much appreciated.
diff mbox series

Patch

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 63c8565206a4..d0b957b20cbf 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -28,6 +28,7 @@  usage() {
 	echo "  -r    list redundant entries when merging fragments"
 	echo "  -y    make builtin have precedence over modules"
 	echo "  -O    dir to put generated output files.  Consider setting \$KCONFIG_CONFIG instead."
+	echo "  -s    strict mode. Fail if the fragment redefines any value."
 	echo
 	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable."
 }
@@ -37,6 +38,7 @@  ALLTARGET=alldefconfig
 WARNREDUN=false
 BUILTIN=false
 OUTPUT=.
+STRICT=false
 CONFIG_PREFIX=${CONFIG_-CONFIG_}
 
 while true; do
@@ -75,6 +77,11 @@  while true; do
 		shift 2
 		continue
 		;;
+	"-s")
+		STRICT=true
+		shift
+		continue
+		;;
 	*)
 		break
 		;;
@@ -141,6 +148,9 @@  for ORIG_MERGE_FILE in $MERGE_LIST ; do
 			echo Previous  value: $PREV_VAL
 			echo New value:       $NEW_VAL
 			echo
+			if [ "$STRICT" = "true" ]; then
+				STRICT_MODE_VIOLATED=true
+			fi
 		elif [ "$WARNREDUN" = "true" ]; then
 			echo Value of $CFG is redundant by fragment $ORIG_MERGE_FILE:
 		fi
@@ -153,6 +163,11 @@  for ORIG_MERGE_FILE in $MERGE_LIST ; do
 	cat $MERGE_FILE >> $TMP_FILE
 done
 
+if [ "$STRICT_MODE_VIOLATED" = "true" ]; then
+	echo "The fragment redefined a value and strict mode had been passed."
+	exit 1
+fi
+
 if [ "$RUNMAKE" = "false" ]; then
 	cp -T -- "$TMP_FILE" "$KCONFIG_CONFIG"
 	echo "#"