Message-ID: <CAK7LNAT79Vgo4f335_XAfS36xqNcTm08=oz8aNqkj0LiJFbn4Q@mail.gmail.com>
Date: Sat, 24 Aug 2024 02:54:31 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Luis Chamberlain <mcgrof@...nel.org>
Cc: kdevops@...ts.linux.dev, linux-kbuild@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [RFC] kconfig: add optional selective yaml output support

On Fri, Aug 23, 2024 at 12:45 PM Luis Chamberlain <mcgrof@...nel.org> wrote:
>
> kconfig is used outside of Linux, and one of the uses of Kconfig is to
> also allow kconfig to be used for automation on kdevops by leveraging
> a smaller subset of variables for yaml run time for ansible runs.
> There is no need to clutter a full yaml file with every single config
> we have as we do in the kernel, and so this lets users decide if they
> want all or just a few select key symbols as part of the yaml output.
>
> What this will do is save us the pain of doing the selective transformation
> we currently do and let's us only annotate what we need for runtime with
> ansible.
>
> You can test with the Linux kernel config (that's not what we use):
>
> export KCONFIG_YAMLCFG=".yaml"
> export KCONFIG_YAMLCFG_ALL="y"
> rm -f .config .yaml
> make defconfig
> head -10 .yaml
> ---
> cc_version_text: "gcc (Debian 13.3.0-1) 13.3.0"
> cc_is_gcc: True
> gcc_version: 130300
> clang_version: 0
> as_is_gnu: True
> as_version: 24250
> ld_is_bfd: True
> ld_version: 24250
> lld_version: 0
>
> You can also use the selective mechanism "output yaml" on any symbol,
> so that we only output those. This also paves the way to let us later
> use kconfig for direct json transformations directly from the same
> kconfig logic.
>
> Signed-off-by: Luis Chamberlain <mcgrof@...nel.org>
> ---
>
> Long ago, I envisioned we could do this so to simplify the addition of
> new workflows and remove all the stupid Makefile transformations we have
> in kdevops today to generate extra_vars.yaml.
>
> Feedback welcome.
>
> I completley understand if this is not desirable upstream.


Agree.
Not desirable upstream.



> However
> kdevops does aim to track kconfig upstream using a git sub tree already,
> it follows linux-next, and so getting support upstream is easier rather
> than going with a branch for our git subtree.
>
> The only puzzle I have is why when we use the selective method, we end
> up with tons of empty lines.. Any ideas? Example of how one can use this
> this on random symbols in case it is not clear, with the selective
> method:
>
> If we use this for example:
>
>   diff --git a/fs/efivarfs/Kconfig b/fs/efivarfs/Kconfig
>   index edec8a19c894..2faf651725dc 100644
>   --- a/fs/efivarfs/Kconfig
>   +++ b/fs/efivarfs/Kconfig
>   @@ -3,6 +3,7 @@ config EFIVAR_FS
>         tristate "EFI Variable filesystem"
>         depends on EFI
>         default m
>   +     output yaml
>         help
>           efivarfs is a replacement filesystem for the old EFI
>           variable support via sysfs, as it doesn't suffer from the
>
> In this case we'd end up with just:
>
> export KCONFIG_YAMLCFG=".yaml"
> unset KCONFIG_YAMLCFG_ALL
> rm -f .config .yaml
> make defconfig
> cat .yaml | cat -s
> ---
>
> efivar_fs: m
>
> Thoughts?
>
>  scripts/kconfig/confdata.c | 152 ++++++++++++++++++++++++++++++++++++-
>  scripts/kconfig/expr.h     |   1 +
>  scripts/kconfig/lexer.l    |   2 +
>  scripts/kconfig/parser.y   |  11 +++
>  4 files changed, 163 insertions(+), 3 deletions(-)
>
> diff --git a/scripts/kconfig/confdata.c b/scripts/kconfig/confdata.c
> index 76193ce5a792..78d188320040 100644
> --- a/scripts/kconfig/confdata.c
> +++ b/scripts/kconfig/confdata.c
> @@ -233,6 +233,20 @@ static const char *conf_get_rustccfg_name(void)
>         return name ? name : "include/generated/rustc_cfg";
>  }
>
> +static bool conf_yaml_enable_all(void)
> +{
> +       char *name = getenv("KCONFIG_YAMLCFG_ALL");
> +
> +       return name ? true: false;
> +}
> +
> +static const char *conf_get_yaml_config_name(void)
> +{
> +       char *name = getenv("KCONFIG_YAMLCFG");
> +
> +       return name ? name : NULL;
> +}
> +
>  static int conf_set_sym_val(struct symbol *sym, int def, int def_flags, char *p)
>  {
>         char *p2;
> @@ -623,9 +637,103 @@ static void __print_symbol(FILE *fp, struct symbol *sym, enum output_n output_n,
>         free(escaped);
>  }
>
> -static void print_symbol_for_dotconfig(FILE *fp, struct symbol *sym)
> +static char *conf_name_to_yaml(struct symbol *sym)
> +{
> +       const char *name = sym->name;
> +       size_t len = strlen(name);
> +       size_t i, j = 0;
> +       char *yaml_name = (char *) malloc(len + 1);
> +
> +       if (!yaml_name)
> +               return NULL;
> +
> +       for (i = 0; i < len; i++) {
> +               if (name[i] == '_')
> +                       yaml_name[j++] = '_';
> +               else
> +                       yaml_name[j++] = tolower(name[i]);
> +       }
> +
> +       yaml_name[j] = '\0';
> +
> +    return yaml_name;
> +}
> +
> +static char *conf_value_to_yaml(struct symbol *sym, const char *val)
> +{
> +       char *yaml_value = NULL;
> +
> +       switch (sym->type) {
> +       case S_INT:
> +               yaml_value = strdup(val);
> +               break;
> +       case S_HEX:
> +            asprintf(&yaml_value, "0x%s", val);
> +            break;
> +        case S_STRING:
> +           /* Wrap strings in quotes */
> +            asprintf(&yaml_value, "\"%s\"", val);
> +            break;
> +        case S_BOOLEAN:
> +        case S_TRISTATE:
> +               if (strcmp(val, "y") == 0)
> +                       yaml_value = strdup("True");
> +               else if (strcmp(val, "n") == 0)
> +                       yaml_value = strdup("False");
> +               else
> +                       yaml_value = strdup(val); /* m in tristate */
> +               break;
> +        default:
> +               /* In case type is unknown */
> +               yaml_value = strdup(val);
> +               break;
> +       }
> +
> +       return yaml_value;
> +}
> +
> +static void __print_yaml_symbol(FILE *fp, struct symbol *sym,
> +                               enum output_n output_n,
> +                               bool escape_string)
> +{
> +       const char *val;
> +       char *yaml_config = NULL;
> +       char *yaml_config_value = NULL;
> +
> +       if (!fp || sym->type == S_UNKNOWN)
> +               return;
> +       if (!conf_yaml_enable_all() && !(sym->flags & SYMBOL_YAML))
> +               return;
> +
> +       val = sym_get_string_value(sym);
> +
> +       yaml_config = conf_name_to_yaml(sym);
> +       if (!yaml_config)
> +               return;
> +
> +       yaml_config_value = conf_value_to_yaml(sym, val);
> +       if (!yaml_config_value) {
> +               free(yaml_config);
> +               return;
> +       }
> +
> +       if ((sym->type == S_BOOLEAN || sym->type == S_TRISTATE) &&
> +           output_n != OUTPUT_N && *val == 'n') {
> +               if (output_n == OUTPUT_N_AS_UNSET && conf_yaml_enable_all())
> +                       fprintf(fp, "# %s: False\n", yaml_config);
> +               return;
> +       }
> +
> +       fprintf(fp, "%s: %s\n", yaml_config, yaml_config_value);
> +
> +       free(yaml_config);
> +       free(yaml_config_value);
> +}
> +
> +static void print_symbol_for_dotconfig(FILE *fp, FILE *yaml, struct symbol *sym)
>  {
>         __print_symbol(fp, sym, OUTPUT_N_AS_UNSET, true);
> +       __print_yaml_symbol(yaml, sym, OUTPUT_N_AS_UNSET, true);
>  }
>
>  static void print_symbol_for_autoconf(FILE *fp, struct symbol *sym)
> @@ -748,11 +856,24 @@ int conf_write_defconfig(const char *filename)
>         struct symbol *sym;
>         struct menu *menu;
>         FILE *out;
> +       FILE *yaml_out = NULL;
> +       const char *yaml_config = NULL;
> +
> +       yaml_config = conf_get_yaml_config_name();
>
>         out = fopen(filename, "w");
>         if (!out)
>                 return 1;
>
> +       if (yaml_config) {
> +               yaml_out = fopen(yaml_config, "w");
> +               if (!yaml_out) {
> +                       fclose(out);
> +                       return 1;
> +               }
> +               fprintf(yaml_out, "---\n");
> +       }
> +
>         sym_clear_all_valid();
>
>         menu_for_each_entry(menu) {
> @@ -783,21 +904,25 @@ int conf_write_defconfig(const char *filename)
>                         if (sym == ds && sym_get_tristate_value(sym) == yes)
>                                 continue;
>                 }
> -               print_symbol_for_dotconfig(out, sym);
> +               print_symbol_for_dotconfig(out, yaml_out, sym);
>         }
>         fclose(out);
> +       if (yaml_out)
> +               fclose(yaml_out);
>         return 0;
>  }
>
>  int conf_write(const char *name)
>  {
>         FILE *out;
> +       FILE *yaml_out = NULL;
>         struct symbol *sym;
>         struct menu *menu;
>         const char *str;
>         char tmpname[PATH_MAX + 1], oldname[PATH_MAX + 1];
>         char *env;
>         bool need_newline = false;
> +       const char *yaml_config;
>
>         if (!name)
>                 name = conf_get_configname();
> @@ -815,18 +940,33 @@ int conf_write(const char *name)
>         if (make_parent_dir(name))
>                 return -1;
>
> +       yaml_config = conf_get_yaml_config_name();
> +
>         env = getenv("KCONFIG_OVERWRITECONFIG");
>         if (env && *env) {
>                 *tmpname = 0;
>                 out = fopen(name, "w");
> +               if (yaml_config)
> +                       yaml_out = fopen(yaml_config, "w");
>         } else {
>                 snprintf(tmpname, sizeof(tmpname), "%s.%d.tmp",
>                          name, (int)getpid());
>                 out = fopen(tmpname, "w");
> +               if (yaml_config)
> +                       yaml_out = fopen(yaml_config, "w");
>         }
>         if (!out)
>                 return 1;
>
> +       if (yaml_config) {
> +               if (!yaml_out) {
> +                       fclose(out);
> +                       return 1;
> +               }
> +               fprintf(yaml_out, "---\n");
> +       }
> +
> +
>         conf_write_heading(out, &comment_style_pound);
>
>         if (!conf_get_changed())
> @@ -852,9 +992,11 @@ int conf_write(const char *name)
>                         if (need_newline) {
>                                 fprintf(out, "\n");
>                                 need_newline = false;
> +                               if (yaml_config)
> +                                       fprintf(yaml_out, "\n");
>                         }
>                         sym->flags |= SYMBOL_WRITTEN;
> -                       print_symbol_for_dotconfig(out, sym);
> +                       print_symbol_for_dotconfig(out, yaml_out, sym);
>                 }
>
>  next:
> @@ -879,6 +1021,8 @@ int conf_write(const char *name)
>                 }
>         }
>         fclose(out);
> +       if (yaml_out)
> +               fclose(yaml_out);
>
>         for_all_symbols(sym)
>                 sym->flags &= ~SYMBOL_WRITTEN;
> @@ -898,6 +1042,8 @@ int conf_write(const char *name)
>         }
>
>         conf_message("configuration written to %s", name);
> +       if (yaml_config)
> +               conf_message("yaml configuration written to %s", yaml_config);
>
>         conf_set_changed(false);
>
> diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
> index 2bc96cd28253..88e8a2a06f67 100644
> --- a/scripts/kconfig/expr.h
> +++ b/scripts/kconfig/expr.h
> @@ -132,6 +132,7 @@ struct symbol {
>  #define SYMBOL_CHECK      0x0008  /* used during dependency checking */
>  #define SYMBOL_VALID      0x0080  /* set when symbol.curr is calculated */
>  #define SYMBOL_WRITE      0x0200  /* write symbol to file (KCONFIG_CONFIG) */
> +#define SYMBOL_YAML       0x0400  /* write symbol to file (KCONFIG_YAMLCFG) */
>  #define SYMBOL_WRITTEN    0x0800  /* track info to avoid double-write to .config */
>  #define SYMBOL_CHECKED    0x2000  /* used during dependency checking */
>  #define SYMBOL_WARNED     0x8000  /* warning has been issued */
> diff --git a/scripts/kconfig/lexer.l b/scripts/kconfig/lexer.l
> index 8dd597c4710d..190937070fb1 100644
> --- a/scripts/kconfig/lexer.l
> +++ b/scripts/kconfig/lexer.l
> @@ -120,6 +120,7 @@ n   [A-Za-z0-9_-]
>  "menuconfig"           return T_MENUCONFIG;
>  "modules"              return T_MODULES;
>  "on"                   return T_ON;
> +"output"               return T_OUTPUT;
>  "prompt"               return T_PROMPT;
>  "range"                        return T_RANGE;
>  "select"               return T_SELECT;
> @@ -127,6 +128,7 @@ n   [A-Za-z0-9_-]
>  "string"               return T_STRING;
>  "tristate"             return T_TRISTATE;
>  "visible"              return T_VISIBLE;
> +"yaml"                 return T_YAML;
>  "||"                   return T_OR;
>  "&&"                   return T_AND;
>  "="                    return T_EQUAL;
> diff --git a/scripts/kconfig/parser.y b/scripts/kconfig/parser.y
> index 61900feb4254..f298f052dddc 100644
> --- a/scripts/kconfig/parser.y
> +++ b/scripts/kconfig/parser.y
> @@ -69,6 +69,7 @@ struct menu *current_menu, *current_entry, *current_choice;
>  %token T_MODULES
>  %token T_ON
>  %token T_OPEN_PAREN
> +%token T_OUTPUT
>  %token T_PLUS_EQUAL
>  %token T_PROMPT
>  %token T_RANGE
> @@ -77,6 +78,7 @@ struct menu *current_menu, *current_entry, *current_choice;
>  %token T_STRING
>  %token T_TRISTATE
>  %token T_VISIBLE
> +%token T_YAML
>  %token T_EOL
>  %token <string> T_ASSIGN_VAL
>
> @@ -234,6 +236,15 @@ config_option: T_MODULES T_EOL
>         modules_sym = current_entry->sym;
>  };
>
> +/* When we want to output symbols as part of an additional output formats */
> +
> +config_option: T_OUTPUT T_YAML T_EOL
> +{
> +       printd(DEBUG_PARSE, "%s will be part of the yaml output file %s:%d:\n",
> +              current_entry->sym->name, cur_filename, cur_lineno);
> +       current_entry->sym->flags |= SYMBOL_YAML;
> +};
> +
>  /* choice entry */
>
>  choice: T_CHOICE T_EOL
> --
> 2.43.0
>


-- 
Best Regards
Masahiro Yamada

Powered by blists - more mailing lists