linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] kconfig: change .config format to use =n instead of "is not set"
@ 2022-02-26 12:37 Masahiro Yamada
  2022-02-26 21:37 ` Sedat Dilek
  2022-03-02 13:16 ` Boris Kolpackov
  0 siblings, 2 replies; 7+ messages in thread
From: Masahiro Yamada @ 2022-02-26 12:37 UTC (permalink / raw)
  To: linux-kbuild; +Cc: linux-kernel, Masahiro Yamada

The .config file uses "# CONFIG_FOO is not set" form to represent
disabled options. In the old days, it was useful because the .config
was directly included from Makefiles. For example, you can use
"ifdef CONFIG_FOO" in Makefiles to check if the option is enabled.

Commit c955ccafc38e ("kconfig: fix .config dependencies") introduced
include/config/auto.conf, which mirrors the .config, but trims down
all disabled options.

Since then, include/config/auto.conf defines CONFIG options during the
build. The .config is used just for storing the user's configuration.
I do not see a strong reason to use a particular pattern of comment
for disabled options.

With this commit, Kconfig will output disable options in a more natural
form, "CONFIG_FOO=n".

Kconfig accepts both "# CONFIG_FOO is not set" and "CONFIG_FOO=n" as a
valid input. You do not need to update arch/*/configs/*_defconfig files
for now. "git bisect" should be able to cross the commit in both ways
without any issue.

A problem may occur if you parse the .config for the "# ... is not set"
patterns.

I adjusted streamline_config.pl, merge_config.sh, scripts/kconfig/tests/.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/kconfig/confdata.c                    | 15 +++++----------
 scripts/kconfig/merge_config.sh               | 19 +++++++++++--------
 scripts/kconfig/streamline_config.pl          |  2 +-
 .../tests/choice/alldef_expected_config       |  6 +++---
 .../tests/choice/allmod_expected_config       |  4 ++--
 .../tests/choice/allno_expected_config        |  6 +++---
 .../tests/choice/allyes_expected_config       |  8 ++++----
 scripts/kconfig/tests/choice/oldask1_config   |  2 +-
 .../tests/inter_choice/expected_config        |  2 +-
 .../kconfig/tests/new_choice_with_dep/config  |  2 +-
 .../tests/no_write_if_dep_unmet/__init__.py   |  7 +++----
 .../no_write_if_dep_unmet/expected_config     |  2 +-
 12 files changed, 36 insertions(+), 39 deletions(-)

diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
index c4340c90e172..00f93c03aa57 100644
--- a/scripts/kconfig/confdata.c
+++ b/scripts/kconfig/confdata.c
@@ -658,9 +658,7 @@ static char *escape_string_value(const char *in)
 	return out;
 }
 
-enum output_n { OUTPUT_N, OUTPUT_N_AS_UNSET, OUTPUT_N_NONE };
-
-static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
+static void __print_symbol(FILE *fp, struct symbol *sym, bool output_n,
 			   bool escape_string)
 {
 	const char *val;
@@ -672,11 +670,8 @@ static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
 	val = sym_get_string_value(sym);
 
 	if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
-	    output_n != OUTPUT_N && *val == 'n') {
-		if (output_n == OUTPUT_N_AS_UNSET)
-			fprintf(fp, "# %s%s is not set\n", CONFIG_, sym->name);
+	    !output_n && *val == 'n')
 		return;
-	}
 
 	if (sym->type == S_STRING && escape_string) {
 		escaped = escape_string_value(val);
@@ -690,17 +685,17 @@ static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
 
 static void print_symbol_for_dotconfig(FILE *fp, struct symbol *sym)
 {
-	__print_symbol(fp, sym, OUTPUT_N_AS_UNSET, true);
+	__print_symbol(fp, sym, true, true);
 }
 
 static void print_symbol_for_autoconf(FILE *fp, struct symbol *sym)
 {
-	__print_symbol(fp, sym, OUTPUT_N_NONE, false);
+	__print_symbol(fp, sym, false, false);
 }
 
 void print_symbol_for_listconfig(struct symbol *sym)
 {
-	__print_symbol(stdout, sym, OUTPUT_N, true);
+	print_symbol_for_dotconfig(stdout, sym);
 }
 
 static void print_symbol_for_c(FILE *fp, struct symbol *sym)
diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
index e5b46980c22a..aac60de3b7c7 100755
--- a/scripts/kconfig/merge_config.sh
+++ b/scripts/kconfig/merge_config.sh
@@ -110,8 +110,11 @@ if [ ! -r "$INITFILE" ]; then
 fi
 
 MERGE_LIST=$*
-SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
-SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
+SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
+
+# Disabled options were previously written as "# CONFIG_... is not set", but
+# now is "CONFIG_...=n". Convert the format before the merge steps.
+SED_CONVERT_NOT_SET="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1=n/"
 
 TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
 MERGE_FILE=$(mktemp ./.merge_tmp.config.XXXXXXXXXX)
@@ -120,7 +123,7 @@ echo "Using $INITFILE as base"
 
 trap clean_up EXIT
 
-cat $INITFILE > $TMP_FILE
+sed "$SED_CONVERT_NOT_SET" $INITFILE > $TMP_FILE
 
 # Merge files, printing warnings on overridden values
 for ORIG_MERGE_FILE in $MERGE_LIST ; do
@@ -129,8 +132,8 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
 		echo "The merge file '$ORIG_MERGE_FILE' does not exist.  Exit." >&2
 		exit 1
 	fi
-	cat $ORIG_MERGE_FILE > $MERGE_FILE
-	CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $MERGE_FILE)
+	sed "$SED_CONVERT_NOT_SET" $ORIG_MERGE_FILE > $MERGE_FILE
+	CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP" $MERGE_FILE)
 
 	for CFG in $CFG_LIST ; do
 		grep -q -w $CFG $TMP_FILE || continue
@@ -155,9 +158,9 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
 			echo Value of $CFG is redundant by fragment $ORIG_MERGE_FILE:
 		fi
 		if [ "$BUILTIN_FLAG" = "false" ]; then
-			sed -i "/$CFG[ =]/d" $TMP_FILE
+			sed -i "/$CFG=/d" $TMP_FILE
 		else
-			sed -i "/$CFG[ =]/d" $MERGE_FILE
+			sed -i "/$CFG=/d" $MERGE_FILE
 		fi
 	done
 	cat $MERGE_FILE >> $TMP_FILE
@@ -191,7 +194,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
 
 
 # Check all specified config values took (might have missed-dependency issues)
-for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
+for CFG in $(sed -n -e "$SED_CONFIG_EXP" $TMP_FILE); do
 
 	REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
 	ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || true)
diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
index 3387ad7508f7..0f20142764f2 100755
--- a/scripts/kconfig/streamline_config.pl
+++ b/scripts/kconfig/streamline_config.pl
@@ -617,7 +617,7 @@ foreach my $line (@config_file) {
     $_ = $line;
 
     if (/CONFIG_IKCONFIG/) {
-	if (/# CONFIG_IKCONFIG is not set/) {
+	if (/# CONFIG_IKCONFIG is not set/ || /CONFIG_IKCONFIG=n/) {
 	    # enable IKCONFIG at least as a module
 	    print "CONFIG_IKCONFIG=m\n";
 	    # don't ask about PROC
diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config
index 7a754bf4be94..75d98d488488 100644
--- a/scripts/kconfig/tests/choice/alldef_expected_config
+++ b/scripts/kconfig/tests/choice/alldef_expected_config
@@ -1,5 +1,5 @@
 CONFIG_MODULES=y
-# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE0=n
 CONFIG_BOOL_CHOICE1=y
-# CONFIG_TRI_CHOICE0 is not set
-# CONFIG_TRI_CHOICE1 is not set
+CONFIG_TRI_CHOICE0=n
+CONFIG_TRI_CHOICE1=n
diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config
index f1f5dcdb7923..ed8ffcb18a3a 100644
--- a/scripts/kconfig/tests/choice/allmod_expected_config
+++ b/scripts/kconfig/tests/choice/allmod_expected_config
@@ -1,7 +1,7 @@
 CONFIG_MODULES=y
-# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE0=n
 CONFIG_BOOL_CHOICE1=y
-# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE0=n
 CONFIG_OPT_BOOL_CHOICE1=y
 CONFIG_TRI_CHOICE0=m
 CONFIG_TRI_CHOICE1=m
diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config
index b88ee7a43136..37b2749277dd 100644
--- a/scripts/kconfig/tests/choice/allno_expected_config
+++ b/scripts/kconfig/tests/choice/allno_expected_config
@@ -1,5 +1,5 @@
-# CONFIG_MODULES is not set
-# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_MODULES=n
+CONFIG_BOOL_CHOICE0=n
 CONFIG_BOOL_CHOICE1=y
-# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE0=n
 CONFIG_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config
index e5a062a1157c..a2b36c017ffb 100644
--- a/scripts/kconfig/tests/choice/allyes_expected_config
+++ b/scripts/kconfig/tests/choice/allyes_expected_config
@@ -1,9 +1,9 @@
 CONFIG_MODULES=y
-# CONFIG_BOOL_CHOICE0 is not set
+CONFIG_BOOL_CHOICE0=n
 CONFIG_BOOL_CHOICE1=y
-# CONFIG_OPT_BOOL_CHOICE0 is not set
+CONFIG_OPT_BOOL_CHOICE0=n
 CONFIG_OPT_BOOL_CHOICE1=y
-# CONFIG_TRI_CHOICE0 is not set
+CONFIG_TRI_CHOICE0=n
 CONFIG_TRI_CHOICE1=y
-# CONFIG_OPT_TRI_CHOICE0 is not set
+CONFIG_OPT_TRI_CHOICE0=n
 CONFIG_OPT_TRI_CHOICE1=y
diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config
index b67bfe3c641f..f0a4e5e9f2c1 100644
--- a/scripts/kconfig/tests/choice/oldask1_config
+++ b/scripts/kconfig/tests/choice/oldask1_config
@@ -1,2 +1,2 @@
-# CONFIG_MODULES is not set
+CONFIG_MODULES=n
 CONFIG_OPT_BOOL_CHOICE0=y
diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config
index 5dceefb054e3..4cf72e16f300 100644
--- a/scripts/kconfig/tests/inter_choice/expected_config
+++ b/scripts/kconfig/tests/inter_choice/expected_config
@@ -1,4 +1,4 @@
 CONFIG_MODULES=y
 CONFIG_CHOICE_VAL0=y
-# CONFIG_CHOIVE_VAL1 is not set
+CONFIG_CHOIVE_VAL1=n
 CONFIG_DUMMY=y
diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config
index 47ef95d567fd..47599c856033 100644
--- a/scripts/kconfig/tests/new_choice_with_dep/config
+++ b/scripts/kconfig/tests/new_choice_with_dep/config
@@ -1,3 +1,3 @@
 CONFIG_CHOICE_B=y
-# CONFIG_CHOICE_D is not set
+CONFIG_CHOICE_D=n
 CONFIG_CHOICE_E=y
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
index ffd469d1f226..9ba7542d47d5 100644
--- a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
@@ -2,14 +2,13 @@
 """
 Do not write choice values to .config if the dependency is unmet.
 
-"# CONFIG_... is not set" should not be written into the .config file
-for symbols with unmet dependency.
+=n should not be written into the .config file for symbols with unmet
+dependency.
 
 This was not working correctly for choice values because choice needs
 a bit different symbol computation.
 
-This checks that no unneeded "# COFIG_... is not set" is contained in
-the .config file.
+This checks that no unneeded =n is contained in the .config file.
 
 Related Linux commit: cb67ab2cd2b8abd9650292c986c79901e3073a59
 """
diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
index 473228810c35..9d057fad723c 100644
--- a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
+++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
@@ -2,4 +2,4 @@
 # Automatically generated file; DO NOT EDIT.
 # Main menu
 #
-# CONFIG_A is not set
+CONFIG_A=n
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-02-26 12:37 [PATCH] kconfig: change .config format to use =n instead of "is not set" Masahiro Yamada
@ 2022-02-26 21:37 ` Sedat Dilek
  2022-02-27  4:23   ` Masahiro Yamada
  2022-03-02 13:16 ` Boris Kolpackov
  1 sibling, 1 reply; 7+ messages in thread
From: Sedat Dilek @ 2022-02-26 21:37 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel

On Sat, Feb 26, 2022 at 2:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> The .config file uses "# CONFIG_FOO is not set" form to represent
> disabled options. In the old days, it was useful because the .config
> was directly included from Makefiles. For example, you can use
> "ifdef CONFIG_FOO" in Makefiles to check if the option is enabled.
>
> Commit c955ccafc38e ("kconfig: fix .config dependencies") introduced
> include/config/auto.conf, which mirrors the .config, but trims down
> all disabled options.
>
> Since then, include/config/auto.conf defines CONFIG options during the
> build. The .config is used just for storing the user's configuration.
> I do not see a strong reason to use a particular pattern of comment
> for disabled options.
>
> With this commit, Kconfig will output disable options in a more natural
> form, "CONFIG_FOO=n".
>
> Kconfig accepts both "# CONFIG_FOO is not set" and "CONFIG_FOO=n" as a
> valid input. You do not need to update arch/*/configs/*_defconfig files
> for now. "git bisect" should be able to cross the commit in both ways
> without any issue.
>

Good.

Lot of people use/used the notation CONFIG_FOO=n, so did I.

Thanks for keeping the "compatibility" with old usage "# CONFIG_FOO is not set".

Normally, I use git diff (or scripts/diffconfig in Git tree) to
compare two kernel-configs, so seeing

-CONFIG_FOO=y
+CONFIG_FOO=n

...might be at first view unfamiliar/unusual.
With the old notation it was easier to see that Kconfig is unset.

Is this patch on top of kbuild-next Git?

( Untested )

- Sedat -

> A problem may occur if you parse the .config for the "# ... is not set"
> patterns.
>
> I adjusted streamline_config.pl, merge_config.sh, scripts/kconfig/tests/.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/kconfig/confdata.c                    | 15 +++++----------
>  scripts/kconfig/merge_config.sh               | 19 +++++++++++--------
>  scripts/kconfig/streamline_config.pl          |  2 +-
>  .../tests/choice/alldef_expected_config       |  6 +++---
>  .../tests/choice/allmod_expected_config       |  4 ++--
>  .../tests/choice/allno_expected_config        |  6 +++---
>  .../tests/choice/allyes_expected_config       |  8 ++++----
>  scripts/kconfig/tests/choice/oldask1_config   |  2 +-
>  .../tests/inter_choice/expected_config        |  2 +-
>  .../kconfig/tests/new_choice_with_dep/config  |  2 +-
>  .../tests/no_write_if_dep_unmet/__init__.py   |  7 +++----
>  .../no_write_if_dep_unmet/expected_config     |  2 +-
>  12 files changed, 36 insertions(+), 39 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index c4340c90e172..00f93c03aa57 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -658,9 +658,7 @@ static char *escape_string_value(const char *in)
>         return out;
>  }
>
> -enum output_n { OUTPUT_N, OUTPUT_N_AS_UNSET, OUTPUT_N_NONE };
> -
> -static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
> +static void __print_symbol(FILE *fp, struct symbol *sym, bool output_n,
>                            bool escape_string)
>  {
>         const char *val;
> @@ -672,11 +670,8 @@ static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
>         val = sym_get_string_value(sym);
>
>         if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
> -           output_n != OUTPUT_N && *val == 'n') {
> -               if (output_n == OUTPUT_N_AS_UNSET)
> -                       fprintf(fp, "# %s%s is not set\n", CONFIG_, sym->name);
> +           !output_n && *val == 'n')
>                 return;
> -       }
>
>         if (sym->type == S_STRING && escape_string) {
>                 escaped = escape_string_value(val);
> @@ -690,17 +685,17 @@ static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
>
>  static void print_symbol_for_dotconfig(FILE *fp, struct symbol *sym)
>  {
> -       __print_symbol(fp, sym, OUTPUT_N_AS_UNSET, true);
> +       __print_symbol(fp, sym, true, true);
>  }
>
>  static void print_symbol_for_autoconf(FILE *fp, struct symbol *sym)
>  {
> -       __print_symbol(fp, sym, OUTPUT_N_NONE, false);
> +       __print_symbol(fp, sym, false, false);
>  }
>
>  void print_symbol_for_listconfig(struct symbol *sym)
>  {
> -       __print_symbol(stdout, sym, OUTPUT_N, true);
> +       print_symbol_for_dotconfig(stdout, sym);
>  }
>
>  static void print_symbol_for_c(FILE *fp, struct symbol *sym)
> diff --git a/scripts/kconfig/merge_config.sh b/scripts/kconfig/merge_config.sh
> index e5b46980c22a..aac60de3b7c7 100755
> --- a/scripts/kconfig/merge_config.sh
> +++ b/scripts/kconfig/merge_config.sh
> @@ -110,8 +110,11 @@ if [ ! -r "$INITFILE" ]; then
>  fi
>
>  MERGE_LIST=$*
> -SED_CONFIG_EXP1="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
> -SED_CONFIG_EXP2="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1/p"
> +SED_CONFIG_EXP="s/^\(${CONFIG_PREFIX}[a-zA-Z0-9_]*\)=.*/\1/p"
> +
> +# Disabled options were previously written as "# CONFIG_... is not set", but
> +# now is "CONFIG_...=n". Convert the format before the merge steps.
> +SED_CONVERT_NOT_SET="s/^# \(${CONFIG_PREFIX}[a-zA-Z0-9_]*\) is not set$/\1=n/"
>
>  TMP_FILE=$(mktemp ./.tmp.config.XXXXXXXXXX)
>  MERGE_FILE=$(mktemp ./.merge_tmp.config.XXXXXXXXXX)
> @@ -120,7 +123,7 @@ echo "Using $INITFILE as base"
>
>  trap clean_up EXIT
>
> -cat $INITFILE > $TMP_FILE
> +sed "$SED_CONVERT_NOT_SET" $INITFILE > $TMP_FILE
>
>  # Merge files, printing warnings on overridden values
>  for ORIG_MERGE_FILE in $MERGE_LIST ; do
> @@ -129,8 +132,8 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
>                 echo "The merge file '$ORIG_MERGE_FILE' does not exist.  Exit." >&2
>                 exit 1
>         fi
> -       cat $ORIG_MERGE_FILE > $MERGE_FILE
> -       CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $MERGE_FILE)
> +       sed "$SED_CONVERT_NOT_SET" $ORIG_MERGE_FILE > $MERGE_FILE
> +       CFG_LIST=$(sed -n -e "$SED_CONFIG_EXP" $MERGE_FILE)
>
>         for CFG in $CFG_LIST ; do
>                 grep -q -w $CFG $TMP_FILE || continue
> @@ -155,9 +158,9 @@ for ORIG_MERGE_FILE in $MERGE_LIST ; do
>                         echo Value of $CFG is redundant by fragment $ORIG_MERGE_FILE:
>                 fi
>                 if [ "$BUILTIN_FLAG" = "false" ]; then
> -                       sed -i "/$CFG[ =]/d" $TMP_FILE
> +                       sed -i "/$CFG=/d" $TMP_FILE
>                 else
> -                       sed -i "/$CFG[ =]/d" $MERGE_FILE
> +                       sed -i "/$CFG=/d" $MERGE_FILE
>                 fi
>         done
>         cat $MERGE_FILE >> $TMP_FILE
> @@ -191,7 +194,7 @@ make KCONFIG_ALLCONFIG=$TMP_FILE $OUTPUT_ARG $ALLTARGET
>
>
>  # Check all specified config values took (might have missed-dependency issues)
> -for CFG in $(sed -n -e "$SED_CONFIG_EXP1" -e "$SED_CONFIG_EXP2" $TMP_FILE); do
> +for CFG in $(sed -n -e "$SED_CONFIG_EXP" $TMP_FILE); do
>
>         REQUESTED_VAL=$(grep -w -e "$CFG" $TMP_FILE)
>         ACTUAL_VAL=$(grep -w -e "$CFG" "$KCONFIG_CONFIG" || true)
> diff --git a/scripts/kconfig/streamline_config.pl b/scripts/kconfig/streamline_config.pl
> index 3387ad7508f7..0f20142764f2 100755
> --- a/scripts/kconfig/streamline_config.pl
> +++ b/scripts/kconfig/streamline_config.pl
> @@ -617,7 +617,7 @@ foreach my $line (@config_file) {
>      $_ = $line;
>
>      if (/CONFIG_IKCONFIG/) {
> -       if (/# CONFIG_IKCONFIG is not set/) {
> +       if (/# CONFIG_IKCONFIG is not set/ || /CONFIG_IKCONFIG=n/) {
>             # enable IKCONFIG at least as a module
>             print "CONFIG_IKCONFIG=m\n";
>             # don't ask about PROC
> diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config
> index 7a754bf4be94..75d98d488488 100644
> --- a/scripts/kconfig/tests/choice/alldef_expected_config
> +++ b/scripts/kconfig/tests/choice/alldef_expected_config
> @@ -1,5 +1,5 @@
>  CONFIG_MODULES=y
> -# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE0=n
>  CONFIG_BOOL_CHOICE1=y
> -# CONFIG_TRI_CHOICE0 is not set
> -# CONFIG_TRI_CHOICE1 is not set
> +CONFIG_TRI_CHOICE0=n
> +CONFIG_TRI_CHOICE1=n
> diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config
> index f1f5dcdb7923..ed8ffcb18a3a 100644
> --- a/scripts/kconfig/tests/choice/allmod_expected_config
> +++ b/scripts/kconfig/tests/choice/allmod_expected_config
> @@ -1,7 +1,7 @@
>  CONFIG_MODULES=y
> -# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE0=n
>  CONFIG_BOOL_CHOICE1=y
> -# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE0=n
>  CONFIG_OPT_BOOL_CHOICE1=y
>  CONFIG_TRI_CHOICE0=m
>  CONFIG_TRI_CHOICE1=m
> diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config
> index b88ee7a43136..37b2749277dd 100644
> --- a/scripts/kconfig/tests/choice/allno_expected_config
> +++ b/scripts/kconfig/tests/choice/allno_expected_config
> @@ -1,5 +1,5 @@
> -# CONFIG_MODULES is not set
> -# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_MODULES=n
> +CONFIG_BOOL_CHOICE0=n
>  CONFIG_BOOL_CHOICE1=y
> -# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE0=n
>  CONFIG_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config
> index e5a062a1157c..a2b36c017ffb 100644
> --- a/scripts/kconfig/tests/choice/allyes_expected_config
> +++ b/scripts/kconfig/tests/choice/allyes_expected_config
> @@ -1,9 +1,9 @@
>  CONFIG_MODULES=y
> -# CONFIG_BOOL_CHOICE0 is not set
> +CONFIG_BOOL_CHOICE0=n
>  CONFIG_BOOL_CHOICE1=y
> -# CONFIG_OPT_BOOL_CHOICE0 is not set
> +CONFIG_OPT_BOOL_CHOICE0=n
>  CONFIG_OPT_BOOL_CHOICE1=y
> -# CONFIG_TRI_CHOICE0 is not set
> +CONFIG_TRI_CHOICE0=n
>  CONFIG_TRI_CHOICE1=y
> -# CONFIG_OPT_TRI_CHOICE0 is not set
> +CONFIG_OPT_TRI_CHOICE0=n
>  CONFIG_OPT_TRI_CHOICE1=y
> diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config
> index b67bfe3c641f..f0a4e5e9f2c1 100644
> --- a/scripts/kconfig/tests/choice/oldask1_config
> +++ b/scripts/kconfig/tests/choice/oldask1_config
> @@ -1,2 +1,2 @@
> -# CONFIG_MODULES is not set
> +CONFIG_MODULES=n
>  CONFIG_OPT_BOOL_CHOICE0=y
> diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config
> index 5dceefb054e3..4cf72e16f300 100644
> --- a/scripts/kconfig/tests/inter_choice/expected_config
> +++ b/scripts/kconfig/tests/inter_choice/expected_config
> @@ -1,4 +1,4 @@
>  CONFIG_MODULES=y
>  CONFIG_CHOICE_VAL0=y
> -# CONFIG_CHOIVE_VAL1 is not set
> +CONFIG_CHOIVE_VAL1=n
>  CONFIG_DUMMY=y
> diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config
> index 47ef95d567fd..47599c856033 100644
> --- a/scripts/kconfig/tests/new_choice_with_dep/config
> +++ b/scripts/kconfig/tests/new_choice_with_dep/config
> @@ -1,3 +1,3 @@
>  CONFIG_CHOICE_B=y
> -# CONFIG_CHOICE_D is not set
> +CONFIG_CHOICE_D=n
>  CONFIG_CHOICE_E=y
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> index ffd469d1f226..9ba7542d47d5 100644
> --- a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py
> @@ -2,14 +2,13 @@
>  """
>  Do not write choice values to .config if the dependency is unmet.
>
> -"# CONFIG_... is not set" should not be written into the .config file
> -for symbols with unmet dependency.
> +=n should not be written into the .config file for symbols with unmet
> +dependency.
>
>  This was not working correctly for choice values because choice needs
>  a bit different symbol computation.
>
> -This checks that no unneeded "# COFIG_... is not set" is contained in
> -the .config file.
> +This checks that no unneeded =n is contained in the .config file.
>
>  Related Linux commit: cb67ab2cd2b8abd9650292c986c79901e3073a59
>  """
> diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> index 473228810c35..9d057fad723c 100644
> --- a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config
> @@ -2,4 +2,4 @@
>  # Automatically generated file; DO NOT EDIT.
>  # Main menu
>  #
> -# CONFIG_A is not set
> +CONFIG_A=n
> --
> 2.32.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-02-26 21:37 ` Sedat Dilek
@ 2022-02-27  4:23   ` Masahiro Yamada
  2022-02-27  7:54     ` Sedat Dilek
  0 siblings, 1 reply; 7+ messages in thread
From: Masahiro Yamada @ 2022-02-27  4:23 UTC (permalink / raw)
  To: Sedat Dilek; +Cc: Linux Kbuild mailing list, Linux Kernel Mailing List

On Sun, Feb 27, 2022 at 6:38 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sat, Feb 26, 2022 at 2:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > The .config file uses "# CONFIG_FOO is not set" form to represent
> > disabled options. In the old days, it was useful because the .config
> > was directly included from Makefiles. For example, you can use
> > "ifdef CONFIG_FOO" in Makefiles to check if the option is enabled.
> >
> > Commit c955ccafc38e ("kconfig: fix .config dependencies") introduced
> > include/config/auto.conf, which mirrors the .config, but trims down
> > all disabled options.
> >
> > Since then, include/config/auto.conf defines CONFIG options during the
> > build. The .config is used just for storing the user's configuration.
> > I do not see a strong reason to use a particular pattern of comment
> > for disabled options.
> >
> > With this commit, Kconfig will output disable options in a more natural
> > form, "CONFIG_FOO=n".
> >
> > Kconfig accepts both "# CONFIG_FOO is not set" and "CONFIG_FOO=n" as a
> > valid input. You do not need to update arch/*/configs/*_defconfig files
> > for now. "git bisect" should be able to cross the commit in both ways
> > without any issue.
> >
>
> Good.
>
> Lot of people use/used the notation CONFIG_FOO=n, so did I.
>
> Thanks for keeping the "compatibility" with old usage "# CONFIG_FOO is not set".
>
> Normally, I use git diff (or scripts/diffconfig in Git tree) to
> compare two kernel-configs, so seeing
>
> -CONFIG_FOO=y
> +CONFIG_FOO=n
>
> ...might be at first view unfamiliar/unusual.
> With the old notation it was easier to see that Kconfig is unset.

I agree on this point.

"is not set" stands out much better than "=n",
and our eyes are accustomed to this notation for 20 years.

However, real comments do not stand out since
we already (ab)use comments for disabled options.

This is related thread
https://patchwork.kernel.org/project/linux-kbuild/patch/20211213100043.45645-3-arielmarcovitch@gmail.com/



>
> Is this patch on top of kbuild-next Git?
>

Yes.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-02-27  4:23   ` Masahiro Yamada
@ 2022-02-27  7:54     ` Sedat Dilek
  2022-03-05  7:18       ` Sedat Dilek
  0 siblings, 1 reply; 7+ messages in thread
From: Sedat Dilek @ 2022-02-27  7:54 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, Linux Kernel Mailing List

On Sun, Feb 27, 2022 at 5:24 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Sun, Feb 27, 2022 at 6:38 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> >
> > On Sat, Feb 26, 2022 at 2:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > The .config file uses "# CONFIG_FOO is not set" form to represent
> > > disabled options. In the old days, it was useful because the .config
> > > was directly included from Makefiles. For example, you can use
> > > "ifdef CONFIG_FOO" in Makefiles to check if the option is enabled.
> > >
> > > Commit c955ccafc38e ("kconfig: fix .config dependencies") introduced
> > > include/config/auto.conf, which mirrors the .config, but trims down
> > > all disabled options.
> > >
> > > Since then, include/config/auto.conf defines CONFIG options during the
> > > build. The .config is used just for storing the user's configuration.
> > > I do not see a strong reason to use a particular pattern of comment
> > > for disabled options.
> > >
> > > With this commit, Kconfig will output disable options in a more natural
> > > form, "CONFIG_FOO=n".
> > >
> > > Kconfig accepts both "# CONFIG_FOO is not set" and "CONFIG_FOO=n" as a
> > > valid input. You do not need to update arch/*/configs/*_defconfig files
> > > for now. "git bisect" should be able to cross the commit in both ways
> > > without any issue.
> > >
> >
> > Good.
> >
> > Lot of people use/used the notation CONFIG_FOO=n, so did I.
> >
> > Thanks for keeping the "compatibility" with old usage "# CONFIG_FOO is not set".
> >
> > Normally, I use git diff (or scripts/diffconfig in Git tree) to
> > compare two kernel-configs, so seeing
> >
> > -CONFIG_FOO=y
> > +CONFIG_FOO=n
> >
> > ...might be at first view unfamiliar/unusual.
> > With the old notation it was easier to see that Kconfig is unset.
>
> I agree on this point.
>
> "is not set" stands out much better than "=n",
> and our eyes are accustomed to this notation for 20 years.
>
> However, real comments do not stand out since
> we already (ab)use comments for disabled options.
>
> This is related thread
> https://patchwork.kernel.org/project/linux-kbuild/patch/20211213100043.45645-3-arielmarcovitch@gmail.com/
>

Thanks for the link.

> >
> > Is this patch on top of kbuild-next Git?
> >
>
> Yes.

Let me see if I will try kbuild-next with this patch on top of
upcoming Linux v5.17-rc6.

- Sedat -

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-02-26 12:37 [PATCH] kconfig: change .config format to use =n instead of "is not set" Masahiro Yamada
  2022-02-26 21:37 ` Sedat Dilek
@ 2022-03-02 13:16 ` Boris Kolpackov
  2022-03-03  0:01   ` Masahiro Yamada
  1 sibling, 1 reply; 7+ messages in thread
From: Boris Kolpackov @ 2022-03-02 13:16 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: linux-kbuild, linux-kernel

Masahiro Yamada <masahiroy@kernel.org> writes:

> With this commit, Kconfig will output disable options in a more natural
> form, "CONFIG_FOO=n".

While I agree with the overall direction, I was wondering how this
relates to string and int/hex options. It appears that they either
have a value or are not written to the .config at all, for example,
if the option is disabled via the `depends on` attribute. At first
I thought this would be inconsistent (i.e., =n for bool and omitted
for string), but it appears a disabled bool option is also omitted.
Hopefully the fact that sometimes a false bool option is =n and
sometimes it's omitted won't surprise anyone.

Reviewed-by: Boris Kolpackov <boris@codesynthesis.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-03-02 13:16 ` Boris Kolpackov
@ 2022-03-03  0:01   ` Masahiro Yamada
  0 siblings, 0 replies; 7+ messages in thread
From: Masahiro Yamada @ 2022-03-03  0:01 UTC (permalink / raw)
  To: Boris Kolpackov; +Cc: Linux Kbuild mailing list, Linux Kernel Mailing List

On Wed, Mar 2, 2022 at 10:17 PM Boris Kolpackov <boris@codesynthesis.com> wrote:
>
> Masahiro Yamada <masahiroy@kernel.org> writes:
>
> > With this commit, Kconfig will output disable options in a more natural
> > form, "CONFIG_FOO=n".
>
> While I agree with the overall direction, I was wondering how this
> relates to string and int/hex options. It appears that they either
> have a value or are not written to the .config at all, for example,
> if the option is disabled via the `depends on` attribute. At first
> I thought this would be inconsistent (i.e., =n for bool and omitted
> for string), but it appears a disabled bool option is also omitted.
> Hopefully the fact that sometimes a false bool option is =n and
> sometimes it's omitted won't surprise anyone.


Options with unmet 'depends on' are all omitted in the .config
It is consistent for all types.





>
> Reviewed-by: Boris Kolpackov <boris@codesynthesis.com>




--
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] kconfig: change .config format to use =n instead of "is not set"
  2022-02-27  7:54     ` Sedat Dilek
@ 2022-03-05  7:18       ` Sedat Dilek
  0 siblings, 0 replies; 7+ messages in thread
From: Sedat Dilek @ 2022-03-05  7:18 UTC (permalink / raw)
  To: Masahiro Yamada; +Cc: Linux Kbuild mailing list, Linux Kernel Mailing List

On Sun, Feb 27, 2022 at 8:54 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
>
> On Sun, Feb 27, 2022 at 5:24 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Sun, Feb 27, 2022 at 6:38 AM Sedat Dilek <sedat.dilek@gmail.com> wrote:
> > >
> > > On Sat, Feb 26, 2022 at 2:34 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > The .config file uses "# CONFIG_FOO is not set" form to represent
> > > > disabled options. In the old days, it was useful because the .config
> > > > was directly included from Makefiles. For example, you can use
> > > > "ifdef CONFIG_FOO" in Makefiles to check if the option is enabled.
> > > >
> > > > Commit c955ccafc38e ("kconfig: fix .config dependencies") introduced
> > > > include/config/auto.conf, which mirrors the .config, but trims down
> > > > all disabled options.
> > > >
> > > > Since then, include/config/auto.conf defines CONFIG options during the
> > > > build. The .config is used just for storing the user's configuration.
> > > > I do not see a strong reason to use a particular pattern of comment
> > > > for disabled options.
> > > >
> > > > With this commit, Kconfig will output disable options in a more natural
> > > > form, "CONFIG_FOO=n".
> > > >
> > > > Kconfig accepts both "# CONFIG_FOO is not set" and "CONFIG_FOO=n" as a
> > > > valid input. You do not need to update arch/*/configs/*_defconfig files
> > > > for now. "git bisect" should be able to cross the commit in both ways
> > > > without any issue.
> > > >
> > >
> > > Good.
> > >
> > > Lot of people use/used the notation CONFIG_FOO=n, so did I.
> > >
> > > Thanks for keeping the "compatibility" with old usage "# CONFIG_FOO is not set".
> > >
> > > Normally, I use git diff (or scripts/diffconfig in Git tree) to
> > > compare two kernel-configs, so seeing
> > >
> > > -CONFIG_FOO=y
> > > +CONFIG_FOO=n
> > >
> > > ...might be at first view unfamiliar/unusual.
> > > With the old notation it was easier to see that Kconfig is unset.
> >
> > I agree on this point.
> >
> > "is not set" stands out much better than "=n",
> > and our eyes are accustomed to this notation for 20 years.
> >
> > However, real comments do not stand out since
> > we already (ab)use comments for disabled options.
> >
> > This is related thread
> > https://patchwork.kernel.org/project/linux-kbuild/patch/20211213100043.45645-3-arielmarcovitch@gmail.com/
> >
>
> Thanks for the link.
>
> > >
> > > Is this patch on top of kbuild-next Git?
> > >
> >
> > Yes.
>
> Let me see if I will try kbuild-next with this patch on top of
> upcoming Linux v5.17-rc6.
>

I was not able to apply your patch on top of latest kbuild-next.git:

$ git log --oneline v5.17-rc6..
3a8276b1ae7e (HEAD -> for-5.17/kbuild-next-2022022) Merge branch
'for-next' of https://git.kernel.org/pub/scm/linux/kernel/git/masahiroy/linux-kbuild
into for-5.17/kbuild-next-2022022
d4c858643263 kallsyms: ignore all local labels prefixed by '.L'
a7d4f58e99dd kconfig: fix missing '# end of' for empty menu
868653f421cd kconfig: add fflush() before ferror() check
5c8166419acf kbuild: replace $(if A,A,B) with $(or A,B)
f67695c9962e kbuild: Add environment variables for userprogs flags
a5575df58004 kbuild: unify cmd_copy and cmd_shipped

$ LC_ALL=C git apply --check --verbose
../20220226_masahiroy_kconfig_change_config_format_to_use_n_instead_of_is_not_set.mbx
Checking patch scripts/kconfig/confdata.c...
error: while searching for:
       return out;
}

enum output_n { OUTPUT_N, OUTPUT_N_AS_UNSET, OUTPUT_N_NONE };

static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
                          bool escape_string)
{
       const char *val;

error: patch failed: scripts/kconfig/confdata.c:658
error: scripts/kconfig/confdata.c: patch does not apply
Checking patch scripts/kconfig/merge_config.sh...
Checking patch scripts/kconfig/streamline_config.pl...
Checking patch scripts/kconfig/tests/choice/alldef_expected_config...
Checking patch scripts/kconfig/tests/choice/allmod_expected_config...
Checking patch scripts/kconfig/tests/choice/allno_expected_config...
Checking patch scripts/kconfig/tests/choice/allyes_expected_config...
Checking patch scripts/kconfig/tests/choice/oldask1_config...
Checking patch scripts/kconfig/tests/inter_choice/expected_config...
Checking patch scripts/kconfig/tests/new_choice_with_dep/config...
Checking patch scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py...
Checking patch scripts/kconfig/tests/no_write_if_dep_unmet/expected_config...

$ git log --oneline v5.17-rc6.. scripts/kconfig/confdata.c
a7d4f58e99dd kconfig: fix missing '# end of' for empty menu
868653f421cd kconfig: add fflush() before ferror() check

- Sedat -

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-03-05  7:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-26 12:37 [PATCH] kconfig: change .config format to use =n instead of "is not set" Masahiro Yamada
2022-02-26 21:37 ` Sedat Dilek
2022-02-27  4:23   ` Masahiro Yamada
2022-02-27  7:54     ` Sedat Dilek
2022-03-05  7:18       ` Sedat Dilek
2022-03-02 13:16 ` Boris Kolpackov
2022-03-03  0:01   ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).