scripts: merge_config: Add flag to prevent unsetting config option

[PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Rahul Rameshbabu 2 years, 6 months ago

Overriding a previously defined entry for a config option with 'is not set'
may be undesirable in some fragment configuration setups. Provide a flag to
change the behavior, so 'is not set' is not overridden when a previous
value for the same config option already exists.

Cc: Masahiro Yamada <masahiroy@kernel.org>
Signed-off-by: Rahul Rameshbabu <sergeantsagara@protonmail.com>
---
 scripts/kconfig/merge_config.sh | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index 902eb429b9db..bbe235f2df70 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -30,6 +30,7 @@ usage() {
 	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 "  -Q    disable warning messages for overridden options."
+	echo "  -N    not set entries in fragments will not override options."
 	echo
 	echo "Used prefix: '$CONFIG_PREFIX'. You can redefine it with \$CONFIG_ environment variable."
 }
@@ -42,6 +43,7 @@ OUTPUT=.
 STRICT=false
 CONFIG_PREFIX=${CONFIG_-CONFIG_}
 WARNOVERRIDE=echo
+OVERRIDENOTSET=true
 
 while true; do
 	case $1 in
@@ -89,6 +91,11 @@ while true; do
 		shift
 		continue
 		;;
+	"-N")
+		OVERRIDENOTSET=false
+		shift
+		continue
+		;;
 	*)
 		break
 		;;
@@ -143,13 +150,20 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
 		grep -q -w $CFG $TMP_FILE || continue
 		PREV_VAL=$(grep -w $CFG $TMP_FILE)
 		NEW_VAL=$(grep -w $CFG $MERGE_FILE)
-		BUILTIN_FLAG=false
-		if [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then
+		NO_OVERRIDE_FLAG=false
+		if [ "$OVERRIDENOTSET" = "false" ] && [ "${NEW_VAL#\# CONFIG_* }" = "is not set" ] &&
+			   [ "$PREV_VAL" != "" ] && [ "${PREV_VAL#\# CONFIG_* }" != "is not set" ]; then
+			${WARNOVERRIDE} Previous  value: $PREV_VAL
+			${WARNOVERRIDE} New value:       $NEW_VAL
+			${WARNOVERRIDE} -N passed, will not unset option
+			${WARNOVERRIDE}
+			NO_OVERRIDE_FLAG=true
+		elif [ "$BUILTIN" = "true" ] && [ "${NEW_VAL#CONFIG_*=}" = "m" ] && [ "${PREV_VAL#CONFIG_*=}" = "y" ]; then
 			${WARNOVERRIDE} Previous  value: $PREV_VAL
 			${WARNOVERRIDE} New value:       $NEW_VAL
 			${WARNOVERRIDE} -y passed, will not demote y to m
 			${WARNOVERRIDE}
-			BUILTIN_FLAG=true
+			NO_OVERRIDE_FLAG=true
 		elif [ "x$PREV_VAL" != "x$NEW_VAL" ] ; then
 			${WARNOVERRIDE} Value of $CFG is redefined by fragment $ORIG_MERGE_FILE:
 			${WARNOVERRIDE} Previous  value: $PREV_VAL
@@ -161,7 +175,7 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
 		elif [ "$WARNREDUN" = "true" ]; then
 			${WARNOVERRIDE} Value of $CFG is redundant by fragment $ORIG_MERGE_FILE:
 		fi
-		if [ "$BUILTIN_FLAG" = "false" ]; then
+		if [ "$NO_OVERRIDE_FLAG" = "false" ]; then
 			sed -i "/$CFG[ =]/d" $TMP_FILE
 		else
 			sed -i "/$CFG[ =]/d" $MERGE_FILE
-- 
2.40.1

Re: [PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Masahiro Yamada 2 years, 6 months ago

On Sun, Jul 30, 2023 at 6:42 AM Rahul Rameshbabu
<sergeantsagara@protonmail.com> wrote:
>
> Overriding a previously defined entry for a config option with 'is not set'
> may be undesirable in some fragment configuration setups.

Then, you should remove the 'is not set' entry from the fragment.






-- 
Best Regards
Masahiro Yamada

Re: [PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Rahul Rameshbabu 2 years, 6 months ago

On Sun, 06 Aug, 2023 23:19:55 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
> On Sun, Jul 30, 2023 at 6:42 AM Rahul Rameshbabu
> <sergeantsagara@protonmail.com> wrote:
>>
>> Overriding a previously defined entry for a config option with 'is not set'
>> may be undesirable in some fragment configuration setups.
>
> Then, you should remove the 'is not set' entry from the fragment.

I had a feeling that was the expectation. Just for reference, my flow
for generating fragments looks like the following.

  1. make allnoconfig
  2. make menuconfig   # select the options that I desire for the fragment

We can drop this patch if this is the expected developer flow. I
realized that overriding with 'is not set' entries in a fragment is
likely desirable, so I made this behavior change as part of a flag that
can be set by the user.

--
Thanks,

Rahul Rameshbabu

Re: [PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Masahiro Yamada 2 years, 6 months ago

On Mon, Aug 7, 2023 at 1:13 PM Rahul Rameshbabu
<sergeantsagara@protonmail.com> wrote:
>
>
> On Sun, 06 Aug, 2023 23:19:55 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
> > On Sun, Jul 30, 2023 at 6:42 AM Rahul Rameshbabu
> > <sergeantsagara@protonmail.com> wrote:
> >>
> >> Overriding a previously defined entry for a config option with 'is not set'
> >> may be undesirable in some fragment configuration setups.
> >
> > Then, you should remove the 'is not set' entry from the fragment.
>
> I had a feeling that was the expectation. Just for reference, my flow
> for generating fragments looks like the following.
>
>   1. make allnoconfig
>   2. make menuconfig   # select the options that I desire for the fragment


Sorry, I could not understand
how these steps generate a fragment file.

You will get a full .config file
after 'make menuconfig'.





> We can drop this patch if this is the expected developer flow. I
> realized that overriding with 'is not set' entries in a fragment is
> likely desirable, so I made this behavior change as part of a flag that
> can be set by the user.
>
> --
> Thanks,
>
> Rahul Rameshbabu
>


-- 
Best Regards
Masahiro Yamada

Re: [PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Rahul Rameshbabu 2 years, 5 months ago

On Tue, 08 Aug, 2023 04:04:37 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
> On Mon, Aug 7, 2023 at 1:13 PM Rahul Rameshbabu
> <sergeantsagara@protonmail.com> wrote:
>>
>>
>> On Sun, 06 Aug, 2023 23:19:55 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
>> > On Sun, Jul 30, 2023 at 6:42 AM Rahul Rameshbabu
>> > <sergeantsagara@protonmail.com> wrote:
>> >>
>> >> Overriding a previously defined entry for a config option with 'is not set'
>> >> may be undesirable in some fragment configuration setups.
>> >
>> > Then, you should remove the 'is not set' entry from the fragment.
>>
>> I had a feeling that was the expectation. Just for reference, my flow
>> for generating fragments looks like the following.
>>
>>   1. make allnoconfig
>>   2. make menuconfig   # select the options that I desire for the fragment
>
>
> Sorry, I could not understand
> how these steps generate a fragment file.
>
> You will get a full .config file
> after 'make menuconfig'.

Yep, this is right. I am not really generating a fragment this way but
rather full configs with minimal options that I end up wanting to merge
together. What's your process for generating fragments you need? Just
dumping the options you want in fragment files and letting make properly
select the dependencies?

>
>
>
>
>
>> We can drop this patch if this is the expected developer flow. I
>> realized that overriding with 'is not set' entries in a fragment is
>> likely desirable, so I made this behavior change as part of a flag that
>> can be set by the user.
>>
>> --
>> Thanks,
>>
>> Rahul Rameshbabu
>>


--
Thanks,

Rahul Rameshbabu

Re: [PATCH] scripts: merge_config: Add flag to prevent unsetting config option

Posted by Masahiro Yamada 2 years, 5 months ago

On Mon, Aug 14, 2023 at 12:00 AM Rahul Rameshbabu
<sergeantsagara@protonmail.com> wrote:
>
>
> On Tue, 08 Aug, 2023 04:04:37 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
> > On Mon, Aug 7, 2023 at 1:13 PM Rahul Rameshbabu
> > <sergeantsagara@protonmail.com> wrote:
> >>
> >>
> >> On Sun, 06 Aug, 2023 23:19:55 +0900 "Masahiro Yamada" <masahiroy@kernel.org> wrote:
> >> > On Sun, Jul 30, 2023 at 6:42 AM Rahul Rameshbabu
> >> > <sergeantsagara@protonmail.com> wrote:
> >> >>
> >> >> Overriding a previously defined entry for a config option with 'is not set'
> >> >> may be undesirable in some fragment configuration setups.
> >> >
> >> > Then, you should remove the 'is not set' entry from the fragment.
> >>
> >> I had a feeling that was the expectation. Just for reference, my flow
> >> for generating fragments looks like the following.
> >>
> >>   1. make allnoconfig
> >>   2. make menuconfig   # select the options that I desire for the fragment
> >
> >
> > Sorry, I could not understand
> > how these steps generate a fragment file.
> >
> > You will get a full .config file
> > after 'make menuconfig'.
>
> Yep, this is right. I am not really generating a fragment this way but
> rather full configs with minimal options that I end up wanting to merge
> together. What's your process for generating fragments you need? Just
> dumping the options you want in fragment files and letting make properly
> select the dependencies?


I would manually write a fragment file.


I see comment lines in

kernel/configs/debug.config
kernel/configs/xen.config

I believe they were written by hand.





-- 
Best Regards
Masahiro Yamada