[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+icZUUm1zpbSyOW3xKUsqo9bBjAehw6KvVBjGxpYy4XBjO4yw@mail.gmail.com>
Date: Sat, 26 Feb 2022 22:37:40 +0100
From: Sedat Dilek <sedat.dilek@...il.com>
To: Masahiro Yamada <masahiroy@...nel.org>
Cc: linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] kconfig: change .config format to use =n instead of "is
not set"
On Sat, Feb 26, 2022 at 2:34 PM Masahiro Yamada <masahiroy@...nel.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@...nel.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
>
Powered by blists - more mailing lists