From 86adf1a07e896f19a799abe65e3f4a1a26ac142e Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Fri, 16 Feb 2018 21:22:53 +0100 Subject: [PATCH 01/30] checkpatch: kconfig: recognize more prompts when checking help texts The check for a missing or short help text only considers symbols with a prompt, but doesn't recognize any of the following as a prompt: bool 'foo' tristate 'foo' prompt "foo" prompt 'foo' Make the check recognize those too. Signed-off-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/checkpatch.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3d4040322ae1..2b404317daea 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2812,7 +2812,7 @@ sub process { next if ($f =~ /^-/); last if (!$file && $f =~ /^\@\@/); - if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate)\s*\"/) { + if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { $is_start = 1; } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { $length = -1; From 678ae162dd2fdda97879680426029dffc3e96041 Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Fri, 16 Feb 2018 21:22:54 +0100 Subject: [PATCH 02/30] checkpatch: kconfig: check help texts for menuconfig and choice Currently, only Kconfig symbols are checked for a missing or short help text, and are only checked if they are defined with the 'config' keyword. To make the check more general, extend it to also check help texts for choices and for symbols defined with the 'menuconfig' keyword. This increases the accuracy of the check for symbols that would already have been checked as well, since e.g. a 'menuconfig' symbol after a help text will be recognized as ending the preceding symbol/choice definition. To increase the accuracy of the check further, also recognize 'if', 'endif', 'menu', 'endmenu', 'endchoice', and 'source' as ending a symbol/choice definition. Signed-off-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/checkpatch.pl | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 2b404317daea..54b782fab4fd 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2797,7 +2797,10 @@ sub process { # Only applies when adding the entry originally, after that we do not have # sufficient context to determine whether it is indeed long enough. if ($realfile =~ /Kconfig/ && - $line =~ /^\+\s*config\s+/) { + # 'choice' is usually the last thing on the line (though + # Kconfig supports named choices), so use a word boundary + # (\b) rather than a whitespace character (\s) + $line =~ /^\+\s*(?:config|menuconfig|choice)\b/) { my $length = 0; my $cnt = $realcnt; my $ln = $linenr + 1; @@ -2822,7 +2825,13 @@ sub process { $f =~ s/#.*//; $f =~ s/^\s+//; next if ($f =~ /^$/); - if ($f =~ /^\s*config\s/) { + + # This only checks context lines in the patch + # and so hopefully shouldn't trigger false + # positives, even though some of these are + # common words in help texts + if ($f =~ /^\s*(?:config|menuconfig|choice|endchoice| + if|endif|menu|endmenu|source)\b/x) { $is_end = 1; last; } From 84af7a6194e493fae312a2b7fa5a3b51f76d9282 Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Fri, 16 Feb 2018 21:22:55 +0100 Subject: [PATCH 03/30] checkpatch: kconfig: prefer 'help' over '---help---' IMO, we should discourage '---help---' for new help texts, even in cases where it would be consistent with other help texts in the file. This will help if we ever want to get rid of '---help---' in the future. Also simplify the code to only check for exactly '---help---'. Since commit c2264564df3d ("kconfig: warn of unhandled characters in Kconfig commands"), '---help---' is a proper keyword and can only appear in that form. Prior to that commit, '---help---' working was more of a syntactic quirk. Signed-off-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/checkpatch.pl | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 54b782fab4fd..2784f6ab309f 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2817,7 +2817,11 @@ sub process { if ($lines[$ln - 1] =~ /^\+\s*(?:bool|tristate|prompt)\s*["']/) { $is_start = 1; - } elsif ($lines[$ln - 1] =~ /^\+\s*(?:---)?help(?:---)?$/) { + } elsif ($lines[$ln - 1] =~ /^\+\s*(?:help|---help---)\s*$/) { + if ($lines[$ln - 1] =~ "---help---") { + WARN("CONFIG_DESCRIPTION", + "prefer 'help' over '---help---' for new help texts\n" . $herecurr); + } $length = -1; } From 9a47ceec543bfb703fbe2f8d584850b582caf1a6 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 20 Feb 2018 17:18:47 +0900 Subject: [PATCH 04/30] kconfig: clean-up reverse dependency help implementation This commit splits out the special E_OR handling ('-' instead of '||') into a dedicated helper expr_print_revdev(). Restore the original expr_print() prior to commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" readable"). This makes sense because: - We need to chop those expressions only when printing the reverse dependency, and only when E_OR is encountered - Otherwise, it should be printed as before, so fall back to expr_print() This also improves the behavior; for a single line, it was previously displayed in the same line as "Selected by", like this: Selected by: A [=n] && B [=n] This will be displayed in a new line, consistently: Selected by: - A [=n] && B [=n] Signed-off-by: Masahiro Yamada Reviewed-by: Petr Vorel --- scripts/kconfig/expr.c | 36 +++++++++++++++++++++--------------- scripts/kconfig/menu.c | 4 ++-- 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index d45381986ac7..cd3a8f501f38 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1179,7 +1179,9 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2) return expr_get_leftmost_symbol(ret); } -static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken, bool revdep) +void expr_print(struct expr *e, + void (*fn)(void *, struct symbol *, const char *), + void *data, int prevtoken) { if (!e) { fn(data, NULL, "y"); @@ -1234,14 +1236,9 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con fn(data, e->right.sym, e->right.sym->name); break; case E_OR: - if (revdep && e->left.expr->type != E_OR) - fn(data, NULL, "\n - "); - __expr_print(e->left.expr, fn, data, E_OR, revdep); - if (revdep) - fn(data, NULL, "\n - "); - else - fn(data, NULL, " || "); - __expr_print(e->right.expr, fn, data, E_OR, revdep); + expr_print(e->left.expr, fn, data, E_OR); + fn(data, NULL, " || "); + expr_print(e->right.expr, fn, data, E_OR); break; case E_AND: expr_print(e->left.expr, fn, data, E_AND); @@ -1274,11 +1271,6 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con fn(data, NULL, ")"); } -void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken) -{ - __expr_print(e, fn, data, prevtoken, false); -} - static void expr_print_file_helper(void *data, struct symbol *sym, const char *str) { xfwrite(str, strlen(str), 1, data); @@ -1329,7 +1321,21 @@ void expr_gstr_print(struct expr *e, struct gstr *gs) * line with a minus. This makes expressions much easier to read. * Suitable for reverse dependency expressions. */ +static void expr_print_revdep(struct expr *e, + void (*fn)(void *, struct symbol *, const char *), + void *data) +{ + if (e->type == E_OR) { + expr_print_revdep(e->left.expr, fn, data); + expr_print_revdep(e->right.expr, fn, data); + } else { + fn(data, NULL, " - "); + expr_print(e, fn, data, E_NONE); + fn(data, NULL, "\n"); + } +} + void expr_gstr_print_revdep(struct expr *e, struct gstr *gs) { - __expr_print(e, expr_print_gstr_helper, gs, E_NONE, true); + expr_print_revdep(e, expr_print_gstr_helper, gs); } diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 36cd3e1f1c28..02f46813e732 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -829,15 +829,15 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, get_symbol_props_str(r, sym, P_SELECT, _(" Selects: ")); if (sym->rev_dep.expr) { str_append(r, _(" Selected by: ")); - expr_gstr_print_revdep(sym->rev_dep.expr, r); str_append(r, "\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r); } get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: ")); if (sym->implied.expr) { str_append(r, _(" Implied by: ")); - expr_gstr_print_revdep(sym->implied.expr, r); str_append(r, "\n"); + expr_gstr_print_revdep(sym->implied.expr, r); } str_append(r, "\n\n"); From d9119b5925a03b9a3191fa3e93b4091651d8ad25 Mon Sep 17 00:00:00 2001 From: Eugeniu Rosca Date: Sat, 24 Feb 2018 16:24:18 +0100 Subject: [PATCH 05/30] kconfig: Print reverse dependencies in groups Surprisingly or not, disabling a CONFIG option (which is assumed to be unneeded) may be not so trivial. Especially it is not trivial, when this CONFIG option is selected by a dozen of other configs. Before the moment commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:" readable") popped up in v4.16-rc1, it was an absolute pain to break down the "Selected by" reverse dependency expression in order to identify all those configs which select (IOW *do not allow disabling*) a certain feature (assumed to be not needed). This patch tries to make one step further by putting at users' fingertips the revdep top level OR sub-expressions grouped/clustered by the tristate value they evaluate to. This should allow the users to directly concentrate on and tackle the _active_ reverse dependencies. To give some numbers and quantify the complexity of certain reverse dependencies, assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 and vanilla arm64 defconfig, here is the top 10 CONFIG options with the highest amount of top level "||" sub-expressions/tokens that make up the final "Selected by" reverse dependency expression. | Config | All revdep | Active revdep | |-------------------|------------|---------------| | REGMAP_I2C | 212 | 9 | | CRC32 | 167 | 25 | | FW_LOADER | 128 | 5 | | MFD_CORE | 124 | 9 | | FB_CFB_IMAGEBLIT | 114 | 2 | | FB_CFB_COPYAREA | 111 | 2 | | FB_CFB_FILLRECT | 110 | 2 | | SND_PCM | 103 | 2 | | CRYPTO_HASH | 87 | 19 | | WATCHDOG_CORE | 86 | 6 | The story behind the above is that users need to visually review/evaluate 212 expressions which *potentially* select REGMAP_I2C in order to identify the expressions which *actually* select REGMAP_I2C, for a particular ARCH and for a particular defconfig used. To make this experience smoother, change the way reverse dependencies are displayed to the user from [1] to [2]. [1] Old representation of DMA_ENGINE_RAID: Selected by: - AMCC_PPC440SPE_ADMA [=n] && DMADEVICES [=y] && (440SPe || 440SP) - BCM_SBA_RAID [=m] && DMADEVICES [=y] && (ARM64 [=y] || ... - FSL_RAID [=n] && DMADEVICES [=y] && FSL_SOC && ... - INTEL_IOATDMA [=n] && DMADEVICES [=y] && PCI [=y] && X86_64 - MV_XOR [=n] && DMADEVICES [=y] && (PLAT_ORION || ARCH_MVEBU [=y] ... - MV_XOR_V2 [=y] && DMADEVICES [=y] && ARM64 [=y] - XGENE_DMA [=n] && DMADEVICES [=y] && (ARCH_XGENE [=y] || ... - DMATEST [=n] && DMADEVICES [=y] && DMA_ENGINE [=y] [2] New representation of DMA_ENGINE_RAID: Selected by [y]: - MV_XOR_V2 [=y] && DMADEVICES [=y] && ARM64 [=y] Selected by [m]: - BCM_SBA_RAID [=m] && DMADEVICES [=y] && (ARM64 [=y] || ... Selected by [n]: - AMCC_PPC440SPE_ADMA [=n] && DMADEVICES [=y] && (440SPe || ... - FSL_RAID [=n] && DMADEVICES [=y] && FSL_SOC && ... - INTEL_IOATDMA [=n] && DMADEVICES [=y] && PCI [=y] && X86_64 - MV_XOR [=n] && DMADEVICES [=y] && (PLAT_ORION || ARCH_MVEBU [=y] ... - XGENE_DMA [=n] && DMADEVICES [=y] && (ARCH_XGENE [=y] || ... - DMATEST [=n] && DMADEVICES [=y] && DMA_ENGINE [=y] Suggested-by: Masahiro Yamada Signed-off-by: Eugeniu Rosca Reviewed-by: Petr Vorel Reviewed-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/kconfig/expr.c | 18 ++++++++++++------ scripts/kconfig/expr.h | 3 ++- scripts/kconfig/menu.c | 12 ++++++------ 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index cd3a8f501f38..49376e12fa30 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1323,19 +1323,25 @@ void expr_gstr_print(struct expr *e, struct gstr *gs) */ static void expr_print_revdep(struct expr *e, void (*fn)(void *, struct symbol *, const char *), - void *data) + void *data, tristate pr_type, const char **title) { if (e->type == E_OR) { - expr_print_revdep(e->left.expr, fn, data); - expr_print_revdep(e->right.expr, fn, data); - } else { + expr_print_revdep(e->left.expr, fn, data, pr_type, title); + expr_print_revdep(e->right.expr, fn, data, pr_type, title); + } else if (expr_calc_value(e) == pr_type) { + if (*title) { + fn(data, NULL, *title); + *title = NULL; + } + fn(data, NULL, " - "); expr_print(e, fn, data, E_NONE); fn(data, NULL, "\n"); } } -void expr_gstr_print_revdep(struct expr *e, struct gstr *gs) +void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, + tristate pr_type, const char *title) { - expr_print_revdep(e, expr_print_gstr_helper, gs); + expr_print_revdep(e, expr_print_gstr_helper, gs, pr_type, &title); } diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index c16e82e302a2..8dbf2a4cdae1 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -310,7 +310,8 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2); void expr_fprint(struct expr *e, FILE *out); struct gstr; /* forward */ void expr_gstr_print(struct expr *e, struct gstr *gs); -void expr_gstr_print_revdep(struct expr *e, struct gstr *gs); +void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, + tristate pr_type, const char *title); static inline int expr_is_yes(struct expr *e) { diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c index 02f46813e732..5c5c1374b151 100644 --- a/scripts/kconfig/menu.c +++ b/scripts/kconfig/menu.c @@ -828,16 +828,16 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, get_symbol_props_str(r, sym, P_SELECT, _(" Selects: ")); if (sym->rev_dep.expr) { - str_append(r, _(" Selected by: ")); - str_append(r, "\n"); - expr_gstr_print_revdep(sym->rev_dep.expr, r); + expr_gstr_print_revdep(sym->rev_dep.expr, r, yes, " Selected by [y]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r, mod, " Selected by [m]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, r, no, " Selected by [n]:\n"); } get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: ")); if (sym->implied.expr) { - str_append(r, _(" Implied by: ")); - str_append(r, "\n"); - expr_gstr_print_revdep(sym->implied.expr, r); + expr_gstr_print_revdep(sym->implied.expr, r, yes, " Implied by [y]:\n"); + expr_gstr_print_revdep(sym->implied.expr, r, mod, " Implied by [m]:\n"); + expr_gstr_print_revdep(sym->implied.expr, r, no, " Implied by [n]:\n"); } str_append(r, "\n\n"); From f467c5640c29ad258c3cd8186a776c82fc3b8057 Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Fri, 23 Feb 2018 12:49:01 +0100 Subject: [PATCH 06/30] kconfig: only write '# CONFIG_FOO is not set' for visible symbols === Background === - Visible n-valued bool/tristate symbols generate a '# CONFIG_FOO is not set' line in the .config file. The idea is to remember the user selection without having to set a Makefile variable. Having n correspond to the variable being undefined in the Makefiles makes for easy CONFIG_* tests. - Invisible n-valued bool/tristate symbols normally do not generate a '# CONFIG_FOO is not set' line, because user values from .config files have no effect on invisible symbols anyway. Currently, there is one exception to this rule: Any bool/tristate symbol that gets the value n through a 'default' property generates a '# CONFIG_FOO is not set' line, even if the symbol is invisible. Note that this only applies to explicitly given defaults, and not when the symbol implicitly defaults to n (like bool/tristate symbols without 'default' properties do). This is inconsistent, and seems redundant: - As mentioned, the '# CONFIG_FOO is not set' won't affect the symbol once the .config is read back in. - Even if the symbol is invisible at first but becomes visible later, there shouldn't be any harm in recalculating the default value rather than viewing the '# CONFIG_FOO is not set' as a previous user value of n. === Changes === Change sym_calc_value() to only set SYMBOL_WRITE (write to .config) for non-n-valued 'default' properties. Note that SYMBOL_WRITE is always set for visible symbols regardless of whether they have 'default' properties or not, so this change only affects invisible symbols. This reduces the size of the x86 .config on my system by about 1% (due to removed '# CONFIG_FOO is not set' entries). One side effect of (and the main motivation for) this change is making the following two definitions behave exactly the same: config FOO bool config FOO bool default n With this change, neither of these will generate a '# CONFIG_FOO is not set' line (assuming FOO isn't selected/implied). That might make it clearer to people that a bare 'default n' is redundant. This change only affects generated .config files and not autoconf.h: autoconf.h only includes #defines for non-n bool/tristate symbols. === Testing === The following testing was done with the x86 Kconfigs: - .config files generated before and after the change were compared to verify that the only difference is some '# CONFIG_FOO is not set' entries disappearing. A couple of these were inspected manually, and most turned out to be from redundant 'default n/def_bool n' properties. - The generated include/generated/autoconf.h was compared before and after the change and verified to be identical. - As a sanity check, the same modification was done to Kconfiglib. The Kconfiglib test suite was then run to check for any mismatches against the output of the C implementation. Signed-off-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/kconfig/symbol.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 2220bc4b051b..0f7eba7d472a 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -403,9 +403,10 @@ void sym_calc_value(struct symbol *sym) if (!sym_is_choice(sym)) { prop = sym_get_default_prop(sym); if (prop) { - sym->flags |= SYMBOL_WRITE; newval.tri = EXPR_AND(expr_calc_value(prop->expr), prop->visible.tri); + if (newval.tri != no) + sym->flags |= SYMBOL_WRITE; } if (sym->implied.tri != no) { sym->flags |= SYMBOL_WRITE; From 59a80b5e892ddee09139f7d7942effdf1ca532e4 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 28 Feb 2018 09:15:21 +0900 Subject: [PATCH 07/30] kconfig: do not call check_conf() for olddefconfig check_conf() traverses the menu tree, but it is completely no-op for olddefconfig because the following if-else block does nothing. if (input_mode == listnewconfig) { ... } else if (input_mode != olddefconfig) { ... } As the help message says, olddefconfig automatically sets new symbols to their default value. There is no room for manual intervention. So, calling check_conf() for olddefconfig is odd in the first place. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/conf.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 822dc51923d6..4227d3ba5ed2 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -424,7 +424,7 @@ static void check_conf(struct menu *menu) if (sym->name && !sym_is_choice_value(sym)) { printf("%s%s\n", CONFIG_, sym->name); } - } else if (input_mode != olddefconfig) { + } else { if (!conf_cnt++) printf(_("*\n* Restart config...\n*\n")); rootEntry = menu_get_parent_menu(menu); @@ -666,15 +666,15 @@ int main(int ac, char **av) /* fall through */ case oldconfig: case listnewconfig: - case olddefconfig: case silentoldconfig: /* Update until a loop caused no more changes */ do { conf_cnt = 0; check_conf(&rootmenu); - } while (conf_cnt && - (input_mode != listnewconfig && - input_mode != olddefconfig)); + } while (conf_cnt && input_mode != listnewconfig); + break; + case olddefconfig: + default: break; } From 4bb3a5b085cd6f98aad10be9bd87532ff26ae086 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 28 Feb 2018 09:15:22 +0900 Subject: [PATCH 08/30] kconfig: remove unneeded input_mode test in conf() conf() is never called for listnewconfig / olddefconfig. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/conf.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 4227d3ba5ed2..1faa55f93a62 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -358,9 +358,7 @@ static void conf(struct menu *menu) switch (prop->type) { case P_MENU: - if ((input_mode == silentoldconfig || - input_mode == listnewconfig || - input_mode == olddefconfig) && + if (input_mode == silentoldconfig && rootEntry != menu) { check_conf(menu); return; From 99f0b6578bab440586befe2cdb6db8e38fec3acd Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 28 Feb 2018 09:15:23 +0900 Subject: [PATCH 09/30] kconfig: remove redundant input_mode test for check_conf() loop check_conf() never increments conf_cnt for listnewconfig, so conf_cnt is always zero. In other words, conf_cnt is not zero, "input_mode != listnewconfig" is met. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 1faa55f93a62..59656d39b671 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -669,7 +669,7 @@ int main(int ac, char **av) do { conf_cnt = 0; check_conf(&rootmenu); - } while (conf_cnt && input_mode != listnewconfig); + } while (conf_cnt); break; case olddefconfig: default: From 2aad9b896213860bd169ebb3ae70caf82d471f3f Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Wed, 28 Feb 2018 09:15:24 +0900 Subject: [PATCH 10/30] kconfig: hide irrelevant sub-menus for oldconfig Historically, "make oldconfig" has changed its behavior several times, quieter or louder. (I attached the history below.) Currently, it is not as quiet as it should be. This commit addresses it. Test Case --------- ---------------------------(Kconfig)---------------------------- menu "menu" config FOO bool "foo" menu "sub menu" config BAR bool "bar" endmenu endmenu menu "sibling menu" config BAZ bool "baz" endmenu ---------------------------------------------------------------- ---------------------------(.config)---------------------------- CONFIG_BAR=y CONFIG_BAZ=y ---------------------------------------------------------------- With the Kconfig and .config above, "make silentoldconfig" and "make oldconfig" work differently, like follows: $ make silentoldconfig scripts/kconfig/conf --silentoldconfig Kconfig * * Restart config... * * * menu * foo (FOO) [N/y/?] (NEW) y # # configuration written to .config # $ make oldconfig scripts/kconfig/conf --oldconfig Kconfig * * Restart config... * * * menu * foo (FOO) [N/y/?] (NEW) y * * sub menu * bar (BAR) [Y/n/?] y # # configuration written to .config # Both hide "sibling node" since it is irrelevant. The difference is that silentoldconfig hides "sub menu" whereas oldconfig does not. The behavior of silentoldconfig is preferred since the "sub menu" does not contain any new symbol. The root cause is in conf(). There are three input modes that can call conf(); oldaskconfig, oldconfig, and silentoldconfig. Everytime conf() encounters a menu entry, it calls check_conf() to check if it contains new symbols. If no new symbol is found, the menu is just skipped. Currently, this happens only when input_mode == silentoldconfig. The oldaskconfig enters into the check_conf() loop as silentoldconfig, so oldaskconfig works likewise for the second loop or later, but it never happens for oldconfig. So, irrelevant sub-menus are shown for oldconfig. Change the test condition to "input_mode != oldaskconfig". This is false only for the first loop of oldaskconfig; it must ask the user all symbols, so no need to call check_conf(). History of oldconfig -------------------- [0] Originally, "make oldconfig" was as loud as "make config" (It showed the entire .config file) [1] Commit cd9140e1e73a ("kconfig: make oldconfig is now less chatty") made oldconfig quieter, but it was still less quieter than silentoldconfig. (oldconfig did not hide sub-menus) [2] Commit 204c96f60904 ("kconfig: fix silentoldconfig") changed the input_mode of oldconfig to "ask_silent" from "ask_new". So, oldconfig really became as quiet as silentoldconfig. (oldconfig hided irrelevant sub-menus) [3] Commit 4062f1a4c030 ("kconfig: use long options in conf") made oldconfig as loud as [0] due to misconversion. [4] Commit 14828349719a ("kconfig: fix make oldconfig") addressed the misconversion of [3], but it made oldconfig quieter only to the same level as [1], not [2]. This commit is restoring the behavior of [2]. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/conf.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 59656d39b671..11a4e4509833 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -358,8 +358,11 @@ static void conf(struct menu *menu) switch (prop->type) { case P_MENU: - if (input_mode == silentoldconfig && - rootEntry != menu) { + /* + * Except in oldaskconfig mode, we show only menus that + * contain new symbols. + */ + if (input_mode != oldaskconfig && rootEntry != menu) { check_conf(menu); return; } @@ -660,7 +663,7 @@ int main(int ac, char **av) case oldaskconfig: rootEntry = &rootmenu; conf(&rootmenu); - input_mode = silentoldconfig; + input_mode = oldconfig; /* fall through */ case oldconfig: case listnewconfig: From 81d2bc2273052e49c46a791f45aeb09802d76b93 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 1 Mar 2018 15:34:36 +0900 Subject: [PATCH 11/30] kconfig: invoke oldconfig instead of silentoldconfig from local*config The purpose of local{yes,mod}config is to arrange the .config file based on actually loaded modules. It is unnecessary to update include/generated/autoconf.h and include/config/* stuff here. They will be updated as needed during the build. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/Makefile | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index cb3ec53a7c29..41e2a9f5c85a 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -49,11 +49,11 @@ localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf cmp -s .tmp.config .config || \ (mv -f .config .config.old.1; \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ mv -f .config.old.1 .config.old) \ else \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --silentoldconfig $(Kconfig); \ + $(obj)/conf $(silent) --oldconfig $(Kconfig); \ fi $(Q)rm -f .tmp.config From 911a91c39cabcb6adb2a78f4f9777abb4c032b75 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 1 Mar 2018 15:34:37 +0900 Subject: [PATCH 12/30] kconfig: rename silentoldconfig to syncconfig As commit cedd55d49dee ("kconfig: Remove silentoldconfig from help and docs; fix kconfig/conf's help") mentioned, 'silentoldconfig' is a historical misnomer. That commit removed it from help and docs since it is an internal interface. If so, it should be allowed to rename it to something more intuitive. 'syncconfig' is the one I came up with because it updates the .config if necessary, then synchronize include/generated/autoconf.h and include/config/* with it. You should not manually invoke 'silentoldcofig'. Display warning if used in case existing scripts are doing wrong. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- Documentation/kbuild/kconfig.txt | 2 +- Documentation/networking/i40e.txt | 2 +- Makefile | 2 +- scripts/kconfig/Makefile | 13 ++++++++++--- scripts/kconfig/conf.c | 20 ++++++++++---------- 5 files changed, 23 insertions(+), 16 deletions(-) diff --git a/Documentation/kbuild/kconfig.txt b/Documentation/kbuild/kconfig.txt index bbc99c0c1094..7233118f3a05 100644 --- a/Documentation/kbuild/kconfig.txt +++ b/Documentation/kbuild/kconfig.txt @@ -119,7 +119,7 @@ Examples: 15% of tristates will be set to 'y', 15% to 'm', 70% to 'n' ______________________________________________________________________ -Environment variables for 'silentoldconfig' +Environment variables for 'syncconfig' KCONFIG_NOSILENTUPDATE -------------------------------------------------- diff --git a/Documentation/networking/i40e.txt b/Documentation/networking/i40e.txt index 57e616ed10b0..c2d6e1824b29 100644 --- a/Documentation/networking/i40e.txt +++ b/Documentation/networking/i40e.txt @@ -32,7 +32,7 @@ Enabling the driver The driver is enabled via the standard kernel configuration system, using the make command: - Make oldconfig/silentoldconfig/menuconfig/etc. + make config/oldconfig/menuconfig/etc. The driver is located in the menu structure at: diff --git a/Makefile b/Makefile index e02d092bc2d6..5675aa15ae84 100644 --- a/Makefile +++ b/Makefile @@ -588,7 +588,7 @@ $(KCONFIG_CONFIG) include/config/auto.conf.cmd: ; # include/generated/ and include/config/. Update them if .config is newer than # include/config/auto.conf (which mirrors .config). include/config/%.conf: $(KCONFIG_CONFIG) include/config/auto.conf.cmd - $(Q)$(MAKE) -f $(srctree)/Makefile silentoldconfig + $(Q)$(MAKE) -f $(srctree)/Makefile syncconfig else # external modules needs include/generated/autoconf.h and include/config/auto.conf # but do not care if they are up-to-date. Use auto.conf to trigger the test diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index 41e2a9f5c85a..753a6de4b7ae 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -3,7 +3,7 @@ # Kernel configuration targets # These targets are used from top-level makefile -PHONY += xconfig gconfig menuconfig config silentoldconfig update-po-config \ +PHONY += xconfig gconfig menuconfig config syncconfig update-po-config \ localmodconfig localyesconfig ifdef KBUILD_KCONFIG @@ -36,7 +36,7 @@ nconfig: $(obj)/nconf # This has become an internal implementation detail and is now deprecated # for external use. -silentoldconfig: $(obj)/conf +syncconfig: $(obj)/conf $(Q)mkdir -p include/config include/generated $(Q)test -e include/generated/autoksyms.h || \ touch include/generated/autoksyms.h @@ -88,7 +88,7 @@ PHONY += $(simple-targets) $(simple-targets): $(obj)/conf $< $(silent) --$@ $(Kconfig) -PHONY += oldnoconfig savedefconfig defconfig +PHONY += oldnoconfig silentoldconfig savedefconfig defconfig # oldnoconfig is an alias of olddefconfig, because people already are dependent # on its behavior (sets new symbols to their default value but not 'n') with the @@ -97,6 +97,13 @@ oldnoconfig: olddefconfig @echo " WARNING: \"oldnoconfig\" target will be removed after Linux 4.19" @echo " Please use \"olddefconfig\" instead, which is an alias." +# We do not expect manual invokcation of "silentoldcofig" (or "syncconfig"). +silentoldconfig: syncconfig + @echo " WARNING: \"silentoldconfig\" has been renamed to \"syncconfig\"" + @echo " and is now an internal implementation detail." + @echo " What you want is probably \"oldconfig\"." + @echo " \"silentoldconfig\" will be removed after Linux 4.19" + savedefconfig: $(obj)/conf $< $(silent) --$@=defconfig $(Kconfig) diff --git a/scripts/kconfig/conf.c b/scripts/kconfig/conf.c index 11a4e4509833..4e08121a35fb 100644 --- a/scripts/kconfig/conf.c +++ b/scripts/kconfig/conf.c @@ -23,7 +23,7 @@ static void check_conf(struct menu *menu); enum input_mode { oldaskconfig, - silentoldconfig, + syncconfig, oldconfig, allnoconfig, allyesconfig, @@ -100,7 +100,7 @@ static int conf_askvalue(struct symbol *sym, const char *def) switch (input_mode) { case oldconfig: - case silentoldconfig: + case syncconfig: if (sym_has_value(sym)) { printf("%s\n", def); return 0; @@ -293,7 +293,7 @@ static int conf_choice(struct menu *menu) printf("[1-%d?]: ", cnt); switch (input_mode) { case oldconfig: - case silentoldconfig: + case syncconfig: if (!is_new) { cnt = def; printf("%d\n", cnt); @@ -441,7 +441,7 @@ static void check_conf(struct menu *menu) static struct option long_opts[] = { {"oldaskconfig", no_argument, NULL, oldaskconfig}, {"oldconfig", no_argument, NULL, oldconfig}, - {"silentoldconfig", no_argument, NULL, silentoldconfig}, + {"syncconfig", no_argument, NULL, syncconfig}, {"defconfig", optional_argument, NULL, defconfig}, {"savedefconfig", required_argument, NULL, savedefconfig}, {"allnoconfig", no_argument, NULL, allnoconfig}, @@ -468,8 +468,8 @@ static void conf_usage(const char *progname) printf(" --listnewconfig List new options\n"); printf(" --oldaskconfig Start a new configuration using a line-oriented program\n"); printf(" --oldconfig Update a configuration using a provided .config as base\n"); - printf(" --silentoldconfig Similar to oldconfig but generates configuration in\n" - " include/{generated/,config/} (oldconfig used to be more verbose)\n"); + printf(" --syncconfig Similar to oldconfig but generates configuration in\n" + " include/{generated/,config/}\n"); printf(" --olddefconfig Same as oldconfig but sets new symbols to their default value\n"); printf(" --oldnoconfig An alias of olddefconfig\n"); printf(" --defconfig New config with default defined in \n"); @@ -501,7 +501,7 @@ int main(int ac, char **av) } input_mode = (enum input_mode)opt; switch (opt) { - case silentoldconfig: + case syncconfig: sync_kconfig = 1; break; case defconfig: @@ -583,7 +583,7 @@ int main(int ac, char **av) } break; case savedefconfig: - case silentoldconfig: + case syncconfig: case oldaskconfig: case oldconfig: case listnewconfig: @@ -667,7 +667,7 @@ int main(int ac, char **av) /* fall through */ case oldconfig: case listnewconfig: - case silentoldconfig: + case syncconfig: /* Update until a loop caused no more changes */ do { conf_cnt = 0; @@ -680,7 +680,7 @@ int main(int ac, char **av) } if (sync_kconfig) { - /* silentoldconfig is used during the build so we shall update autoconf. + /* syncconfig is used during the build so we shall update autoconf. * All other commands are only used to generate a config. */ if (conf_get_changed() && conf_write(NULL)) { From 2a61625835c7c89c5f00de458a213d59ac54db21 Mon Sep 17 00:00:00 2001 From: Ulf Magnusson Date: Thu, 1 Mar 2018 12:18:01 +0100 Subject: [PATCH 13/30] kconfig: remove redundant streamline_config.pl prerequisite The local{yes,mod}config targets currently have streamline_config.pl as a prerequisite. This is redundant, because streamline_config.pl is a checked-in file with no prerequisites. Remove the prerequisite and reference streamline_config.pl directly in the recipe of the rule instead. Signed-off-by: Ulf Magnusson Signed-off-by: Masahiro Yamada --- scripts/kconfig/Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index 753a6de4b7ae..a9f325ada4cb 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -42,18 +42,18 @@ syncconfig: $(obj)/conf touch include/generated/autoksyms.h $< $(silent) --$@ $(Kconfig) -localyesconfig localmodconfig: $(obj)/streamline_config.pl $(obj)/conf +localyesconfig localmodconfig: $(obj)/conf $(Q)mkdir -p include/config include/generated - $(Q)perl $< --$@ $(srctree) $(Kconfig) > .tmp.config + $(Q)perl $(srctree)/$(src)/streamline_config.pl --$@ $(srctree) $(Kconfig) > .tmp.config $(Q)if [ -f .config ]; then \ cmp -s .tmp.config .config || \ (mv -f .config .config.old.1; \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --oldconfig $(Kconfig); \ + $< $(silent) --oldconfig $(Kconfig); \ mv -f .config.old.1 .config.old) \ else \ mv -f .tmp.config .config; \ - $(obj)/conf $(silent) --oldconfig $(Kconfig); \ + $< $(silent) --oldconfig $(Kconfig); \ fi $(Q)rm -f .tmp.config From e9781b52d4e0e3381351d3483cfd173a968dcbe6 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:02 +0900 Subject: [PATCH 14/30] kbuild: add PYTHON2 and PYTHON3 variables The variable 'PYTHON' allows users to specify a proper executable name in case the default 'python' does not work. However, this does not address the case where both Python 2.x and 3.x scripts are used in one source tree. PEP 394 (https://www.python.org/dev/peps/pep-0394/) provides a convention for Python scripts portability. Here is a quotation: In order to tolerate differences across platforms, all new code that needs to invoke the Python interpreter should not specify 'python', but rather should specify either 'python2' or 'python3'. This distinction should be made in shebangs, when invoking from a shell script, when invoking via the system() call, or when invoking in any other context. One exception to this is scripts that are deliberately written to be source compatible with both Python 2.x and 3.x. Such scripts may continue to use python on their shebang line without affecting their portability. To meet this requirement, this commit adds new variables 'PYTHON2' and 'PYTHON3'. arch/ia64/scripts/unwcheck.py is the only script that has ever used $(PYTHON). Recent commit bd5edbe67794 ("ia64: convert unwcheck.py to python3") converted it to be compatible with both Python 2.x and 3.x, so this is the exceptional case where the use of 'python' is allowed. So, I did not touch arch/ia64/Makefile. tools/perf/Makefile.config sets PYTHON and PYTHON2 by itself, so it is not affected by this commit. Signed-off-by: Masahiro Yamada --- Makefile | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 5675aa15ae84..de2c8f7cb9d7 100644 --- a/Makefile +++ b/Makefile @@ -385,6 +385,8 @@ INSTALLKERNEL := installkernel DEPMOD = /sbin/depmod PERL = perl PYTHON = python +PYTHON2 = python2 +PYTHON3 = python3 CHECK = sparse CHECKFLAGS := -D__linux__ -Dlinux -D__STDC__ -Dunix -D__unix__ \ @@ -430,7 +432,7 @@ GCC_PLUGINS_CFLAGS := export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES -export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON UTS_MACHINE +export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON PYTHON2 PYTHON3 UTS_MACHINE export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS export KBUILD_CPPFLAGS NOSTDINC_FLAGS LINUXINCLUDE OBJCOPYFLAGS LDFLAGS From 022a4bf6b59dfdb192ca8aef291c7346f984e511 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:03 +0900 Subject: [PATCH 15/30] kconfig: tests: add framework for Kconfig unit testing Many parts in Kconfig are so cryptic and need refactoring. However, its complexity prevents us from moving forward. There are several naive corner cases where it is difficult to notice breakage. If those are covered by unit tests, we will be able to touch the code with more confidence. Here is a simple test framework based on pytest. The conftest.py provides a fixture useful to run commands such as 'oldaskconfig' etc. and to compare the resulted .config, stdout, stderr with expectations. How to add test cases? ---------------------- For each test case, you should create a subdirectory under scripts/kconfig/tests/ (so test cases are separated from each other). Every test case directory should contain the following files: - __init__.py: describes test functions - Kconfig: the top level Kconfig file for the test To do a useful job, test cases generally need additional data like input .config and information about expected results. How to run tests? ----------------- You need python3 and pytest. Then, run "make testconfig". O= option is supported. If V=1 is given, detailed logs captured during tests are displayed. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/Makefile | 8 + scripts/kconfig/tests/conftest.py | 291 ++++++++++++++++++++++++++++++ scripts/kconfig/tests/pytest.ini | 7 + 3 files changed, 306 insertions(+) create mode 100644 scripts/kconfig/tests/conftest.py create mode 100644 scripts/kconfig/tests/pytest.ini diff --git a/scripts/kconfig/Makefile b/scripts/kconfig/Makefile index a9f325ada4cb..78c96aabd00f 100644 --- a/scripts/kconfig/Makefile +++ b/scripts/kconfig/Makefile @@ -142,6 +142,14 @@ PHONY += tinyconfig tinyconfig: $(Q)$(MAKE) -f $(srctree)/Makefile allnoconfig tiny.config +# CHECK: -o cache_dir= working? +PHONY += testconfig +testconfig: $(obj)/conf + $(PYTHON3) -B -m pytest $(srctree)/$(src)/tests \ + -o cache_dir=$(abspath $(obj)/tests/.cache) \ + $(if $(findstring 1,$(KBUILD_VERBOSE)),--capture=no) +clean-dirs += tests/.cache + # Help text used by make help help: @echo ' config - Update current config utilising a line-oriented program' diff --git a/scripts/kconfig/tests/conftest.py b/scripts/kconfig/tests/conftest.py new file mode 100644 index 000000000000..0345ef6e3273 --- /dev/null +++ b/scripts/kconfig/tests/conftest.py @@ -0,0 +1,291 @@ +# SPDX-License-Identifier: GPL-2.0 +# +# Copyright (C) 2018 Masahiro Yamada +# + +""" +Kconfig unit testing framework. + +This provides fixture functions commonly used from test files. +""" + +import os +import pytest +import shutil +import subprocess +import tempfile + +CONF_PATH = os.path.abspath(os.path.join('scripts', 'kconfig', 'conf')) + + +class Conf: + """Kconfig runner and result checker. + + This class provides methods to run text-based interface of Kconfig + (scripts/kconfig/conf) and retrieve the resulted configuration, + stdout, and stderr. It also provides methods to compare those + results with expectations. + """ + + def __init__(self, request): + """Create a new Conf instance. + + request: object to introspect the requesting test module + """ + # the directory of the test being run + self._test_dir = os.path.dirname(str(request.fspath)) + + # runners + def _run_conf(self, mode, dot_config=None, out_file='.config', + interactive=False, in_keys=None, extra_env={}): + """Run text-based Kconfig executable and save the result. + + mode: input mode option (--oldaskconfig, --defconfig= etc.) + dot_config: .config file to use for configuration base + out_file: file name to contain the output config data + interactive: flag to specify the interactive mode + in_keys: key inputs for interactive modes + extra_env: additional environments + returncode: exit status of the Kconfig executable + """ + command = [CONF_PATH, mode, 'Kconfig'] + + # Override 'srctree' environment to make the test as the top directory + extra_env['srctree'] = self._test_dir + + # Run Kconfig in a temporary directory. + # This directory is automatically removed when done. + with tempfile.TemporaryDirectory() as temp_dir: + + # if .config is given, copy it to the working directory + if dot_config: + shutil.copyfile(os.path.join(self._test_dir, dot_config), + os.path.join(temp_dir, '.config')) + + ps = subprocess.Popen(command, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + cwd=temp_dir, + env=dict(os.environ, **extra_env)) + + # If input key sequence is given, feed it to stdin. + if in_keys: + ps.stdin.write(in_keys.encode('utf-8')) + + while ps.poll() is None: + # For interactive modes such as oldaskconfig, oldconfig, + # send 'Enter' key until the program finishes. + if interactive: + ps.stdin.write(b'\n') + + self.retcode = ps.returncode + self.stdout = ps.stdout.read().decode() + self.stderr = ps.stderr.read().decode() + + # Retrieve the resulted config data only when .config is supposed + # to exist. If the command fails, the .config does not exist. + # 'listnewconfig' does not produce .config in the first place. + if self.retcode == 0 and out_file: + with open(os.path.join(temp_dir, out_file)) as f: + self.config = f.read() + else: + self.config = None + + # Logging: + # Pytest captures the following information by default. In failure + # of tests, the captured log will be displayed. This will be useful to + # figure out what has happened. + + print("[command]\n{}\n".format(' '.join(command))) + + print("[retcode]\n{}\n".format(self.retcode)) + + print("[stdout]") + print(self.stdout) + + print("[stderr]") + print(self.stderr) + + if self.config is not None: + print("[output for '{}']".format(out_file)) + print(self.config) + + return self.retcode + + def oldaskconfig(self, dot_config=None, in_keys=None): + """Run oldaskconfig. + + dot_config: .config file to use for configuration base (optional) + in_key: key inputs (optional) + returncode: exit status of the Kconfig executable + """ + return self._run_conf('--oldaskconfig', dot_config=dot_config, + interactive=True, in_keys=in_keys) + + def oldconfig(self, dot_config=None, in_keys=None): + """Run oldconfig. + + dot_config: .config file to use for configuration base (optional) + in_key: key inputs (optional) + returncode: exit status of the Kconfig executable + """ + return self._run_conf('--oldconfig', dot_config=dot_config, + interactive=True, in_keys=in_keys) + + def olddefconfig(self, dot_config=None): + """Run olddefconfig. + + dot_config: .config file to use for configuration base (optional) + returncode: exit status of the Kconfig executable + """ + return self._run_conf('--olddefconfig', dot_config=dot_config) + + def defconfig(self, defconfig): + """Run defconfig. + + defconfig: defconfig file for input + returncode: exit status of the Kconfig executable + """ + defconfig_path = os.path.join(self._test_dir, defconfig) + return self._run_conf('--defconfig={}'.format(defconfig_path)) + + def _allconfig(self, mode, all_config): + if all_config: + all_config_path = os.path.join(self._test_dir, all_config) + extra_env = {'KCONFIG_ALLCONFIG': all_config_path} + else: + extra_env = {} + + return self._run_conf('--{}config'.format(mode), extra_env=extra_env) + + def allyesconfig(self, all_config=None): + """Run allyesconfig. + + all_config: fragment config file for KCONFIG_ALLCONFIG (optional) + returncode: exit status of the Kconfig executable + """ + return self._allconfig('allyes', all_config) + + def allmodconfig(self, all_config=None): + """Run allmodconfig. + + all_config: fragment config file for KCONFIG_ALLCONFIG (optional) + returncode: exit status of the Kconfig executable + """ + return self._allconfig('allmod', all_config) + + def allnoconfig(self, all_config=None): + """Run allnoconfig. + + all_config: fragment config file for KCONFIG_ALLCONFIG (optional) + returncode: exit status of the Kconfig executable + """ + return self._allconfig('allno', all_config) + + def alldefconfig(self, all_config=None): + """Run alldefconfig. + + all_config: fragment config file for KCONFIG_ALLCONFIG (optional) + returncode: exit status of the Kconfig executable + """ + return self._allconfig('alldef', all_config) + + def randconfig(self, all_config=None): + """Run randconfig. + + all_config: fragment config file for KCONFIG_ALLCONFIG (optional) + returncode: exit status of the Kconfig executable + """ + return self._allconfig('rand', all_config) + + def savedefconfig(self, dot_config): + """Run savedefconfig. + + dot_config: .config file for input + returncode: exit status of the Kconfig executable + """ + return self._run_conf('--savedefconfig', out_file='defconfig') + + def listnewconfig(self, dot_config=None): + """Run listnewconfig. + + dot_config: .config file to use for configuration base (optional) + returncode: exit status of the Kconfig executable + """ + return self._run_conf('--listnewconfig', dot_config=dot_config, + out_file=None) + + # checkers + def _read_and_compare(self, compare, expected): + """Compare the result with expectation. + + compare: function to compare the result with expectation + expected: file that contains the expected data + """ + with open(os.path.join(self._test_dir, expected)) as f: + expected_data = f.read() + return compare(self, expected_data) + + def _contains(self, attr, expected): + return self._read_and_compare( + lambda s, e: getattr(s, attr).find(e) >= 0, + expected) + + def _matches(self, attr, expected): + return self._read_and_compare(lambda s, e: getattr(s, attr) == e, + expected) + + def config_contains(self, expected): + """Check if resulted configuration contains expected data. + + expected: file that contains the expected data + returncode: True if result contains the expected data, False otherwise + """ + return self._contains('config', expected) + + def config_matches(self, expected): + """Check if resulted configuration exactly matches expected data. + + expected: file that contains the expected data + returncode: True if result matches the expected data, False otherwise + """ + return self._matches('config', expected) + + def stdout_contains(self, expected): + """Check if resulted stdout contains expected data. + + expected: file that contains the expected data + returncode: True if result contains the expected data, False otherwise + """ + return self._contains('stdout', expected) + + def stdout_matches(self, expected): + """Check if resulted stdout exactly matches expected data. + + expected: file that contains the expected data + returncode: True if result matches the expected data, False otherwise + """ + return self._matches('stdout', expected) + + def stderr_contains(self, expected): + """Check if resulted stderr contains expected data. + + expected: file that contains the expected data + returncode: True if result contains the expected data, False otherwise + """ + return self._contains('stderr', expected) + + def stderr_matches(self, expected): + """Check if resulted stderr exactly matches expected data. + + expected: file that contains the expected data + returncode: True if result matches the expected data, False otherwise + """ + return self._matches('stderr', expected) + + +@pytest.fixture(scope="module") +def conf(request): + """Create a Conf instance and provide it to test functions.""" + return Conf(request) diff --git a/scripts/kconfig/tests/pytest.ini b/scripts/kconfig/tests/pytest.ini new file mode 100644 index 000000000000..85d7ce8e448b --- /dev/null +++ b/scripts/kconfig/tests/pytest.ini @@ -0,0 +1,7 @@ +[pytest] +addopts = --verbose + +# Pytest requires that test files have unique names, because pytest imports +# them as top-level modules. It is silly to prefix or suffix a test file with +# the directory name that contains it. Use __init__.py for all test files. +python_files = __init__.py From 1903c511905984685e0a299421bc4c8b6fc1344b Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:04 +0900 Subject: [PATCH 16/30] kconfig: tests: add basic choice tests The calculation of 'choice' is a bit complicated part in Kconfig. The behavior of 'y' choice is intuitive. If choice values are tristate, the choice can be 'm' where each value can be enabled independently. Also, if a choice is marked as 'optional', the whole choice can be invisible. Test basic functionality of choice. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/tests/choice/Kconfig | 54 +++++++++++++++++++ scripts/kconfig/tests/choice/__init__.py | 40 ++++++++++++++ .../tests/choice/alldef_expected_config | 5 ++ .../tests/choice/allmod_expected_config | 9 ++++ .../tests/choice/allno_expected_config | 5 ++ .../tests/choice/allyes_expected_config | 9 ++++ .../tests/choice/oldask0_expected_stdout | 10 ++++ scripts/kconfig/tests/choice/oldask1_config | 2 + .../tests/choice/oldask1_expected_stdout | 15 ++++++ 9 files changed, 149 insertions(+) create mode 100644 scripts/kconfig/tests/choice/Kconfig create mode 100644 scripts/kconfig/tests/choice/__init__.py create mode 100644 scripts/kconfig/tests/choice/alldef_expected_config create mode 100644 scripts/kconfig/tests/choice/allmod_expected_config create mode 100644 scripts/kconfig/tests/choice/allno_expected_config create mode 100644 scripts/kconfig/tests/choice/allyes_expected_config create mode 100644 scripts/kconfig/tests/choice/oldask0_expected_stdout create mode 100644 scripts/kconfig/tests/choice/oldask1_config create mode 100644 scripts/kconfig/tests/choice/oldask1_expected_stdout diff --git a/scripts/kconfig/tests/choice/Kconfig b/scripts/kconfig/tests/choice/Kconfig new file mode 100644 index 000000000000..cc60e9ce2c03 --- /dev/null +++ b/scripts/kconfig/tests/choice/Kconfig @@ -0,0 +1,54 @@ +config MODULES + bool "Enable loadable module support" + option modules + default y + +choice + prompt "boolean choice" + default BOOL_CHOICE1 + +config BOOL_CHOICE0 + bool "choice 0" + +config BOOL_CHOICE1 + bool "choice 1" + +endchoice + +choice + prompt "optional boolean choice" + optional + default OPT_BOOL_CHOICE1 + +config OPT_BOOL_CHOICE0 + bool "choice 0" + +config OPT_BOOL_CHOICE1 + bool "choice 1" + +endchoice + +choice + prompt "tristate choice" + default TRI_CHOICE1 + +config TRI_CHOICE0 + tristate "choice 0" + +config TRI_CHOICE1 + tristate "choice 1" + +endchoice + +choice + prompt "optional tristate choice" + optional + default OPT_TRI_CHOICE1 + +config OPT_TRI_CHOICE0 + tristate "choice 0" + +config OPT_TRI_CHOICE1 + tristate "choice 1" + +endchoice diff --git a/scripts/kconfig/tests/choice/__init__.py b/scripts/kconfig/tests/choice/__init__.py new file mode 100644 index 000000000000..9edcc5262134 --- /dev/null +++ b/scripts/kconfig/tests/choice/__init__.py @@ -0,0 +1,40 @@ +""" +Basic choice tests. + +The handling of 'choice' is a bit complicated part in Kconfig. + +The behavior of 'y' choice is intuitive. If choice values are tristate, +the choice can be 'm' where each value can be enabled independently. +Also, if a choice is marked as 'optional', the whole choice can be +invisible. +""" + + +def test_oldask0(conf): + assert conf.oldaskconfig() == 0 + assert conf.stdout_contains('oldask0_expected_stdout') + + +def test_oldask1(conf): + assert conf.oldaskconfig('oldask1_config') == 0 + assert conf.stdout_contains('oldask1_expected_stdout') + + +def test_allyes(conf): + assert conf.allyesconfig() == 0 + assert conf.config_contains('allyes_expected_config') + + +def test_allmod(conf): + assert conf.allmodconfig() == 0 + assert conf.config_contains('allmod_expected_config') + + +def test_allno(conf): + assert conf.allnoconfig() == 0 + assert conf.config_contains('allno_expected_config') + + +def test_alldef(conf): + assert conf.alldefconfig() == 0 + assert conf.config_contains('alldef_expected_config') diff --git a/scripts/kconfig/tests/choice/alldef_expected_config b/scripts/kconfig/tests/choice/alldef_expected_config new file mode 100644 index 000000000000..7a754bf4be94 --- /dev/null +++ b/scripts/kconfig/tests/choice/alldef_expected_config @@ -0,0 +1,5 @@ +CONFIG_MODULES=y +# CONFIG_BOOL_CHOICE0 is not set +CONFIG_BOOL_CHOICE1=y +# CONFIG_TRI_CHOICE0 is not set +# CONFIG_TRI_CHOICE1 is not set diff --git a/scripts/kconfig/tests/choice/allmod_expected_config b/scripts/kconfig/tests/choice/allmod_expected_config new file mode 100644 index 000000000000..f1f5dcdb7923 --- /dev/null +++ b/scripts/kconfig/tests/choice/allmod_expected_config @@ -0,0 +1,9 @@ +CONFIG_MODULES=y +# CONFIG_BOOL_CHOICE0 is not set +CONFIG_BOOL_CHOICE1=y +# CONFIG_OPT_BOOL_CHOICE0 is not set +CONFIG_OPT_BOOL_CHOICE1=y +CONFIG_TRI_CHOICE0=m +CONFIG_TRI_CHOICE1=m +CONFIG_OPT_TRI_CHOICE0=m +CONFIG_OPT_TRI_CHOICE1=m diff --git a/scripts/kconfig/tests/choice/allno_expected_config b/scripts/kconfig/tests/choice/allno_expected_config new file mode 100644 index 000000000000..b88ee7a43136 --- /dev/null +++ b/scripts/kconfig/tests/choice/allno_expected_config @@ -0,0 +1,5 @@ +# CONFIG_MODULES is not set +# CONFIG_BOOL_CHOICE0 is not set +CONFIG_BOOL_CHOICE1=y +# CONFIG_TRI_CHOICE0 is not set +CONFIG_TRI_CHOICE1=y diff --git a/scripts/kconfig/tests/choice/allyes_expected_config b/scripts/kconfig/tests/choice/allyes_expected_config new file mode 100644 index 000000000000..e5a062a1157c --- /dev/null +++ b/scripts/kconfig/tests/choice/allyes_expected_config @@ -0,0 +1,9 @@ +CONFIG_MODULES=y +# CONFIG_BOOL_CHOICE0 is not set +CONFIG_BOOL_CHOICE1=y +# CONFIG_OPT_BOOL_CHOICE0 is not set +CONFIG_OPT_BOOL_CHOICE1=y +# CONFIG_TRI_CHOICE0 is not set +CONFIG_TRI_CHOICE1=y +# CONFIG_OPT_TRI_CHOICE0 is not set +CONFIG_OPT_TRI_CHOICE1=y diff --git a/scripts/kconfig/tests/choice/oldask0_expected_stdout b/scripts/kconfig/tests/choice/oldask0_expected_stdout new file mode 100644 index 000000000000..b251bba9698b --- /dev/null +++ b/scripts/kconfig/tests/choice/oldask0_expected_stdout @@ -0,0 +1,10 @@ +Enable loadable module support (MODULES) [Y/n/?] (NEW) +boolean choice + 1. choice 0 (BOOL_CHOICE0) (NEW) +> 2. choice 1 (BOOL_CHOICE1) (NEW) +choice[1-2?]: +optional boolean choice [N/y/?] (NEW) +tristate choice [M/y/?] (NEW) + choice 0 (TRI_CHOICE0) [N/m/?] (NEW) + choice 1 (TRI_CHOICE1) [N/m/?] (NEW) +optional tristate choice [N/m/y/?] (NEW) diff --git a/scripts/kconfig/tests/choice/oldask1_config b/scripts/kconfig/tests/choice/oldask1_config new file mode 100644 index 000000000000..b67bfe3c641f --- /dev/null +++ b/scripts/kconfig/tests/choice/oldask1_config @@ -0,0 +1,2 @@ +# CONFIG_MODULES is not set +CONFIG_OPT_BOOL_CHOICE0=y diff --git a/scripts/kconfig/tests/choice/oldask1_expected_stdout b/scripts/kconfig/tests/choice/oldask1_expected_stdout new file mode 100644 index 000000000000..c2125e9bf96a --- /dev/null +++ b/scripts/kconfig/tests/choice/oldask1_expected_stdout @@ -0,0 +1,15 @@ +Enable loadable module support (MODULES) [N/y/?] +boolean choice + 1. choice 0 (BOOL_CHOICE0) (NEW) +> 2. choice 1 (BOOL_CHOICE1) (NEW) +choice[1-2?]: +optional boolean choice [Y/n/?] (NEW) +optional boolean choice +> 1. choice 0 (OPT_BOOL_CHOICE0) + 2. choice 1 (OPT_BOOL_CHOICE1) (NEW) +choice[1-2?]: +tristate choice + 1. choice 0 (TRI_CHOICE0) (NEW) +> 2. choice 1 (TRI_CHOICE1) (NEW) +choice[1-2?]: +optional tristate choice [N/y/?] From 49ac3c0c3aa3b7636961b72a40ddb03e6609f594 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:05 +0900 Subject: [PATCH 17/30] kconfig: tests: test automatic submenu creation If a symbols has dependency on the preceding symbol, the menu entry should become the submenu of the preceding one, and displayed with deeper indentation. This is done by restructuring the menu tree in menu_finalize(). It is a bit complicated computation, so let's add a test case. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/tests/auto_submenu/Kconfig | 50 +++++++++++++++++++ .../kconfig/tests/auto_submenu/__init__.py | 12 +++++ .../tests/auto_submenu/expected_stdout | 10 ++++ 3 files changed, 72 insertions(+) create mode 100644 scripts/kconfig/tests/auto_submenu/Kconfig create mode 100644 scripts/kconfig/tests/auto_submenu/__init__.py create mode 100644 scripts/kconfig/tests/auto_submenu/expected_stdout diff --git a/scripts/kconfig/tests/auto_submenu/Kconfig b/scripts/kconfig/tests/auto_submenu/Kconfig new file mode 100644 index 000000000000..c17bf2caa7e6 --- /dev/null +++ b/scripts/kconfig/tests/auto_submenu/Kconfig @@ -0,0 +1,50 @@ +config A + bool "A" + default y + +config A0 + bool "A0" + depends on A + default y + help + This depends on A, so should be a submenu of A. + +config A0_0 + bool "A1_0" + depends on A0 + help + Submenus are created recursively. + This should be a submenu of A0. + +config A1 + bool "A1" + depends on A + default y + help + This should line up with A0. + +choice + prompt "choice" + depends on A1 + help + Choice should become a submenu as well. + +config A1_0 + bool "A1_0" + +config A1_1 + bool "A1_1" + +endchoice + +config B + bool "B" + help + This is independent of A. + +config C + bool "C" + depends on A + help + This depends on A, but not a consecutive item, so can/should not + be a submenu. diff --git a/scripts/kconfig/tests/auto_submenu/__init__.py b/scripts/kconfig/tests/auto_submenu/__init__.py new file mode 100644 index 000000000000..32e79b85faeb --- /dev/null +++ b/scripts/kconfig/tests/auto_submenu/__init__.py @@ -0,0 +1,12 @@ +""" +Create submenu for symbols that depend on the preceding one. + +If a symbols has dependency on the preceding symbol, the menu entry +should become the submenu of the preceding one, and displayed with +deeper indentation. +""" + + +def test(conf): + assert conf.oldaskconfig() == 0 + assert conf.stdout_contains('expected_stdout') diff --git a/scripts/kconfig/tests/auto_submenu/expected_stdout b/scripts/kconfig/tests/auto_submenu/expected_stdout new file mode 100644 index 000000000000..bf5236f39a56 --- /dev/null +++ b/scripts/kconfig/tests/auto_submenu/expected_stdout @@ -0,0 +1,10 @@ +A (A) [Y/n/?] (NEW) + A0 (A0) [Y/n/?] (NEW) + A1_0 (A0_0) [N/y/?] (NEW) + A1 (A1) [Y/n/?] (NEW) + choice + > 1. A1_0 (A1_0) (NEW) + 2. A1_1 (A1_1) (NEW) + choice[1-2?]: +B (B) [N/y/?] (NEW) +C (C) [N/y/?] (NEW) From b76960c0f6b25d447a1493c4388defb9e8e76c63 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:06 +0900 Subject: [PATCH 18/30] kconfig: tests: test if new symbols in choice are asked If new choice values are added with new dependency, and they become visible during user configuration, oldconfig should recognize them as (NEW), and ask the user for choice. This issue was fixed by commit 5d09598d488f ("kconfig: fix new choices being skipped upon config update"). This is a subtle corner case. Add a test case to avoid breakage. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- .../kconfig/tests/new_choice_with_dep/Kconfig | 37 +++++++++++++++++++ .../tests/new_choice_with_dep/__init__.py | 14 +++++++ .../kconfig/tests/new_choice_with_dep/config | 3 ++ .../tests/new_choice_with_dep/expected_stdout | 10 +++++ 4 files changed, 64 insertions(+) create mode 100644 scripts/kconfig/tests/new_choice_with_dep/Kconfig create mode 100644 scripts/kconfig/tests/new_choice_with_dep/__init__.py create mode 100644 scripts/kconfig/tests/new_choice_with_dep/config create mode 100644 scripts/kconfig/tests/new_choice_with_dep/expected_stdout diff --git a/scripts/kconfig/tests/new_choice_with_dep/Kconfig b/scripts/kconfig/tests/new_choice_with_dep/Kconfig new file mode 100644 index 000000000000..53ef1b86e7bf --- /dev/null +++ b/scripts/kconfig/tests/new_choice_with_dep/Kconfig @@ -0,0 +1,37 @@ +config A + bool "A" + help + This is a new symbol. + +choice + prompt "Choice ?" + depends on A + help + "depends on A" has been newly added. + +config CHOICE_B + bool "Choice B" + +config CHOICE_C + bool "Choice C" + help + This is a new symbol, so should be asked. + +endchoice + +choice + prompt "Choice2 ?" + +config CHOICE_D + bool "Choice D" + +config CHOICE_E + bool "Choice E" + +config CHOICE_F + bool "Choice F" + depends on A + help + This is a new symbol, so should be asked. + +endchoice diff --git a/scripts/kconfig/tests/new_choice_with_dep/__init__.py b/scripts/kconfig/tests/new_choice_with_dep/__init__.py new file mode 100644 index 000000000000..f0e0ead0f32f --- /dev/null +++ b/scripts/kconfig/tests/new_choice_with_dep/__init__.py @@ -0,0 +1,14 @@ +""" +Ask new choice values when they become visible. + +If new choice values are added with new dependency, and they become +visible during user configuration, oldconfig should recognize them +as (NEW), and ask the user for choice. + +Related Linux commit: 5d09598d488f081e3be23f885ed65cbbe2d073b5 +""" + + +def test(conf): + assert conf.oldconfig('config', 'y') == 0 + assert conf.stdout_contains('expected_stdout') diff --git a/scripts/kconfig/tests/new_choice_with_dep/config b/scripts/kconfig/tests/new_choice_with_dep/config new file mode 100644 index 000000000000..47ef95d567fd --- /dev/null +++ b/scripts/kconfig/tests/new_choice_with_dep/config @@ -0,0 +1,3 @@ +CONFIG_CHOICE_B=y +# CONFIG_CHOICE_D is not set +CONFIG_CHOICE_E=y diff --git a/scripts/kconfig/tests/new_choice_with_dep/expected_stdout b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout new file mode 100644 index 000000000000..74dc0bcb22bc --- /dev/null +++ b/scripts/kconfig/tests/new_choice_with_dep/expected_stdout @@ -0,0 +1,10 @@ +A (A) [N/y/?] (NEW) y + Choice ? + > 1. Choice B (CHOICE_B) + 2. Choice C (CHOICE_C) (NEW) + choice[1-2?]: +Choice2 ? + 1. Choice D (CHOICE_D) +> 2. Choice E (CHOICE_E) + 3. Choice F (CHOICE_F) (NEW) +choice[1-3?]: From 930c429a656fdba0c7f76b9a36ec2698f946e8e3 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:07 +0900 Subject: [PATCH 19/30] kconfig: tests: check unneeded "is not set" with unmet dependency Commit cb67ab2cd2b8 ("kconfig: do not write choice values when their dependency becomes n") fixed a problem where "# CONFIG_... is not set" for choice values are wrongly written into the .config file when they are once visible, then become invisible later. Add a test for this naive case. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- .../tests/no_write_if_dep_unmet/Kconfig | 14 ++++++++++++++ .../tests/no_write_if_dep_unmet/__init__.py | 19 +++++++++++++++++++ .../tests/no_write_if_dep_unmet/config | 1 + .../no_write_if_dep_unmet/expected_config | 5 +++++ 4 files changed, 39 insertions(+) create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/config create mode 100644 scripts/kconfig/tests/no_write_if_dep_unmet/expected_config diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig new file mode 100644 index 000000000000..c00b8fe54f45 --- /dev/null +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/Kconfig @@ -0,0 +1,14 @@ +config A + bool "A" + +choice + prompt "Choice ?" + depends on A + +config CHOICE_B + bool "Choice B" + +config CHOICE_C + bool "Choice C" + +endchoice diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py new file mode 100644 index 000000000000..207261b0fe00 --- /dev/null +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/__init__.py @@ -0,0 +1,19 @@ +""" +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. + +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. + +Related Linux commit: cb67ab2cd2b8abd9650292c986c79901e3073a59 +""" + + +def test(conf): + assert conf.oldaskconfig('config', 'n') == 0 + assert conf.config_matches('expected_config') diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/config b/scripts/kconfig/tests/no_write_if_dep_unmet/config new file mode 100644 index 000000000000..abd280e2f616 --- /dev/null +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/config @@ -0,0 +1 @@ +CONFIG_A=y diff --git a/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config new file mode 100644 index 000000000000..0d15e41da475 --- /dev/null +++ b/scripts/kconfig/tests/no_write_if_dep_unmet/expected_config @@ -0,0 +1,5 @@ +# +# Automatically generated file; DO NOT EDIT. +# Linux Kernel Configuration +# +# CONFIG_A is not set From ee236610653ede74c91f4c35f731047f5b76d667 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:08 +0900 Subject: [PATCH 20/30] kconfig: tests: check visibility of tristate choice values in y choice If tristate choice values depend on symbols set to 'm', they should be hidden when the choice containing them is changed from 'm' to 'y' (i.e. exclusive choice). This issue was fixed by commit fa64e5f6a35e ("kconfig/symbol.c: handle choice_values that depend on 'm' symbols"). Add a test case to avoid regression. For the input in this unit test, there is a room for argument if "# CONFIG_CHOICE1 is not set" should be written to the .config file. After commit fa64e5f6a35e, this line was written to the .config file. With commit cb67ab2cd2b8 ("kconfig: do not write choice values when their dependency becomes n"), it is not written now. In this test, "# CONFIG_CHOICE1 is not set" is don't care. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- .../tests/choice_value_with_m_dep/Kconfig | 19 +++++++++++++++++++ .../tests/choice_value_with_m_dep/__init__.py | 15 +++++++++++++++ .../tests/choice_value_with_m_dep/config | 2 ++ .../choice_value_with_m_dep/expected_config | 3 +++ .../choice_value_with_m_dep/expected_stdout | 4 ++++ 5 files changed, 43 insertions(+) create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/Kconfig create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/__init__.py create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/config create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_config create mode 100644 scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig new file mode 100644 index 000000000000..11ac25c26040 --- /dev/null +++ b/scripts/kconfig/tests/choice_value_with_m_dep/Kconfig @@ -0,0 +1,19 @@ +config MODULES + def_bool y + option modules + +config DEP + tristate + default m + +choice + prompt "Tristate Choice" + +config CHOICE0 + tristate "Choice 0" + +config CHOICE1 + tristate "Choice 1" + depends on DEP + +endchoice diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py new file mode 100644 index 000000000000..f8d728c7b101 --- /dev/null +++ b/scripts/kconfig/tests/choice_value_with_m_dep/__init__.py @@ -0,0 +1,15 @@ +""" +Hide tristate choice values with mod dependency in y choice. + +If tristate choice values depend on symbols set to 'm', they should be +hidden when the choice containing them is changed from 'm' to 'y' +(i.e. exclusive choice). + +Related Linux commit: fa64e5f6a35efd5e77d639125d973077ca506074 +""" + + +def test(conf): + assert conf.oldaskconfig('config', 'y') == 0 + assert conf.config_contains('expected_config') + assert conf.stdout_contains('expected_stdout') diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/config b/scripts/kconfig/tests/choice_value_with_m_dep/config new file mode 100644 index 000000000000..3a126b7a2546 --- /dev/null +++ b/scripts/kconfig/tests/choice_value_with_m_dep/config @@ -0,0 +1,2 @@ +CONFIG_CHOICE0=m +CONFIG_CHOICE1=m diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_config b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config new file mode 100644 index 000000000000..4d07b449540e --- /dev/null +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_config @@ -0,0 +1,3 @@ +CONFIG_MODULES=y +CONFIG_DEP=m +CONFIG_CHOICE0=y diff --git a/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout new file mode 100644 index 000000000000..2b50ab65c86a --- /dev/null +++ b/scripts/kconfig/tests/choice_value_with_m_dep/expected_stdout @@ -0,0 +1,4 @@ +Tristate Choice [M/y/?] y +Tristate Choice +> 1. Choice 0 (CHOICE0) +choice[1]: 1 From beaaddb625400e4a561d2d8443f9df2aa3f9a899 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:09 +0900 Subject: [PATCH 21/30] kconfig: tests: test defconfig when two choices interact Commit fbe98bb9ed3d ("kconfig: Fix defconfig when one choice menu selects options that another choice menu depends on") fixed defconfig when two choices interact (i.e. calculating the visibility of a choice requires to calculate another choice). The test code in that commit log was based on the real world example, and complicated. So, I shrunk it down to the following: defconfig.choice: ---8<--- CONFIG_CHOICE_VAL0=y ---8<--- ---8<--- config MODULES def_bool y option modules choice prompt "Choice" config CHOICE_VAL0 tristate "Choice 0" config CHOICE_VAL1 tristate "Choice 1" endchoice choice prompt "Another choice" depends on CHOICE_VAL0 config DUMMY bool "dummy" endchoice ---8<--- Prior to commit fbe98bb9ed3d, $ scripts/kconfig/conf --defconfig=defconfig.choice Kconfig.choice resulted in: CONFIG_MODULES=y CONFIG_CHOICE_VAL0=m # CONFIG_CHOICE_VAL1 is not set CONFIG_DUMMY=y where the expected result would be: CONFIG_MODULES=y CONFIG_CHOICE_VAL0=y # CONFIG_CHOICE_VAL1 is not set CONFIG_DUMMY=y Roughly, this weird behavior happened like this: Symbols are calculated a couple of times. First, all symbols are calculated in conf_read(). The first 'choice' is evaluated to 'y' due to the SYMBOL_DEF_USER flag, but sym_calc_choice() clears it unless all of its choice values are explicitly set by the user. conf_set_all_new_symbols() clears all SYMBOL_VALID flags. Then, only choices are calculated. Here, the SYMBOL_DEF_USER for the first choice has been forgotten, so it is evaluated to 'm'. set_all_choice_values() sets SYMBOL_DEF_USER again to choice symbols. When calculating the second choice, due to 'depends on CHOICE_VAL0', it triggers the calculation of CHOICE_VAL0. As a result, SYMBOL_VALID is set for CHOICE_VAL0. Symbols except choices get the final chance of re-calculation in conf_write(). In a normal case, CHOICE_VAL0 would be re-calculated, then the first choice would be indirectly re-calculated with the SYMBOL_DEF_USER which has been recalled by set_all_choice_values(), which would be evaluated to 'y'. But, in this case, CHOICE_VAL0 has already been marked as SYMBOL_VALID, so this re-calculation does not happen. Then, =m from the conf_set_all_new_symbols() phase is written out to the .config file. Add a unit test for this naive case. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/tests/inter_choice/Kconfig | 23 +++++++++++++++++++ .../kconfig/tests/inter_choice/__init__.py | 14 +++++++++++ scripts/kconfig/tests/inter_choice/defconfig | 1 + .../tests/inter_choice/expected_config | 4 ++++ 4 files changed, 42 insertions(+) create mode 100644 scripts/kconfig/tests/inter_choice/Kconfig create mode 100644 scripts/kconfig/tests/inter_choice/__init__.py create mode 100644 scripts/kconfig/tests/inter_choice/defconfig create mode 100644 scripts/kconfig/tests/inter_choice/expected_config diff --git a/scripts/kconfig/tests/inter_choice/Kconfig b/scripts/kconfig/tests/inter_choice/Kconfig new file mode 100644 index 000000000000..e44449f075df --- /dev/null +++ b/scripts/kconfig/tests/inter_choice/Kconfig @@ -0,0 +1,23 @@ +config MODULES + def_bool y + option modules + +choice + prompt "Choice" + +config CHOICE_VAL0 + tristate "Choice 0" + +config CHOIVE_VAL1 + tristate "Choice 1" + +endchoice + +choice + prompt "Another choice" + depends on CHOICE_VAL0 + +config DUMMY + bool "dummy" + +endchoice diff --git a/scripts/kconfig/tests/inter_choice/__init__.py b/scripts/kconfig/tests/inter_choice/__init__.py new file mode 100644 index 000000000000..5c7fc365ed40 --- /dev/null +++ b/scripts/kconfig/tests/inter_choice/__init__.py @@ -0,0 +1,14 @@ +""" +Do not affect user-assigned choice value by another choice. + +Handling of state flags for choices is complecated. In old days, +the defconfig result of a choice could be affected by another choice +if those choices interact by 'depends on', 'select', etc. + +Related Linux commit: fbe98bb9ed3dae23e320c6b113e35f129538d14a +""" + + +def test(conf): + assert conf.defconfig('defconfig') == 0 + assert conf.config_contains('expected_config') diff --git a/scripts/kconfig/tests/inter_choice/defconfig b/scripts/kconfig/tests/inter_choice/defconfig new file mode 100644 index 000000000000..162c4148e2a5 --- /dev/null +++ b/scripts/kconfig/tests/inter_choice/defconfig @@ -0,0 +1 @@ +CONFIG_CHOICE_VAL0=y diff --git a/scripts/kconfig/tests/inter_choice/expected_config b/scripts/kconfig/tests/inter_choice/expected_config new file mode 100644 index 000000000000..5dceefb054e3 --- /dev/null +++ b/scripts/kconfig/tests/inter_choice/expected_config @@ -0,0 +1,4 @@ +CONFIG_MODULES=y +CONFIG_CHOICE_VAL0=y +# CONFIG_CHOIVE_VAL1 is not set +CONFIG_DUMMY=y From 3e4888c2e3d77d70edb905364cd70059a52a1343 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:10 +0900 Subject: [PATCH 22/30] kconfig: tests: test randconfig for choice in choice Commit 3b9a19e08960 ("kconfig: loop as long as we changed some symbols in randconfig") fixed randconfig where a choice contains a sub-choice. Prior to that commit, the sub-choice values were not set. I am not sure whether this is an intended feature or just something people discovered works, but it is used in the real world; drivers/usb/gadget/legacy/Kconfig is source'd in a choice context, then creates a sub-choice in it. For the test case in this commit, there are 3 possible results. Case 1: CONFIG_A=y # CONFIG_B is not set Case 2: # CONFIG_A is not set CONFIG_B=y CONFIG_C=y # CONFIG_D is not set Case 3: # CONFIG_A is not set CONFIG_B=y # CONFIG_C is not set CONFIG_D=y CONFIG_E=y So, this test iterates several times, and checks if the result is either of the three. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- .../kconfig/tests/rand_nested_choice/Kconfig | 33 +++++++++++++++++++ .../tests/rand_nested_choice/__init__.py | 16 +++++++++ .../tests/rand_nested_choice/expected_stdout0 | 2 ++ .../tests/rand_nested_choice/expected_stdout1 | 4 +++ .../tests/rand_nested_choice/expected_stdout2 | 5 +++ 5 files changed, 60 insertions(+) create mode 100644 scripts/kconfig/tests/rand_nested_choice/Kconfig create mode 100644 scripts/kconfig/tests/rand_nested_choice/__init__.py create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout0 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout1 create mode 100644 scripts/kconfig/tests/rand_nested_choice/expected_stdout2 diff --git a/scripts/kconfig/tests/rand_nested_choice/Kconfig b/scripts/kconfig/tests/rand_nested_choice/Kconfig new file mode 100644 index 000000000000..c591d512929f --- /dev/null +++ b/scripts/kconfig/tests/rand_nested_choice/Kconfig @@ -0,0 +1,33 @@ +choice + prompt "choice" + +config A + bool "A" + +config B + bool "B" + +if B +choice + prompt "sub choice" + +config C + bool "C" + +config D + bool "D" + +if D +choice + prompt "subsub choice" + +config E + bool "E" + +endchoice +endif # D + +endchoice +endif # B + +endchoice diff --git a/scripts/kconfig/tests/rand_nested_choice/__init__.py b/scripts/kconfig/tests/rand_nested_choice/__init__.py new file mode 100644 index 000000000000..e729a4e85218 --- /dev/null +++ b/scripts/kconfig/tests/rand_nested_choice/__init__.py @@ -0,0 +1,16 @@ +""" +Set random values recursively in nested choices. + +Kconfig can create a choice-in-choice structure by using 'if' statement. +randconfig should correctly set random choice values. + +Related Linux commit: 3b9a19e08960e5cdad5253998637653e592a3c29 +""" + + +def test(conf): + for i in range(20): + assert conf.randconfig() == 0 + assert (conf.config_contains('expected_stdout0') or + conf.config_contains('expected_stdout1') or + conf.config_contains('expected_stdout2')) diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout0 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0 new file mode 100644 index 000000000000..05450f3d4eb5 --- /dev/null +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout0 @@ -0,0 +1,2 @@ +CONFIG_A=y +# CONFIG_B is not set diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout1 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1 new file mode 100644 index 000000000000..37ab29584157 --- /dev/null +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout1 @@ -0,0 +1,4 @@ +# CONFIG_A is not set +CONFIG_B=y +CONFIG_C=y +# CONFIG_D is not set diff --git a/scripts/kconfig/tests/rand_nested_choice/expected_stdout2 b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2 new file mode 100644 index 000000000000..849ff47e9848 --- /dev/null +++ b/scripts/kconfig/tests/rand_nested_choice/expected_stdout2 @@ -0,0 +1,5 @@ +# CONFIG_A is not set +CONFIG_B=y +# CONFIG_C is not set +CONFIG_D=y +CONFIG_E=y From 29c434f367ea7b95bea7057105507c05abdc1297 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:11 +0900 Subject: [PATCH 23/30] kconfig: tests: test if recursive dependencies are detected Recursive dependency should be detected and warned. Test this. This indirectly tests the line number increments. Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- .../kconfig/tests/warn_recursive_dep/Kconfig | 62 +++++++++++++++++++ .../tests/warn_recursive_dep/__init__.py | 9 +++ .../tests/warn_recursive_dep/expected_stderr | 30 +++++++++ 3 files changed, 101 insertions(+) create mode 100644 scripts/kconfig/tests/warn_recursive_dep/Kconfig create mode 100644 scripts/kconfig/tests/warn_recursive_dep/__init__.py create mode 100644 scripts/kconfig/tests/warn_recursive_dep/expected_stderr diff --git a/scripts/kconfig/tests/warn_recursive_dep/Kconfig b/scripts/kconfig/tests/warn_recursive_dep/Kconfig new file mode 100644 index 000000000000..a65bfcb7137e --- /dev/null +++ b/scripts/kconfig/tests/warn_recursive_dep/Kconfig @@ -0,0 +1,62 @@ +# depends on itself + +config A + bool "A" + depends on A + +# select itself + +config B + bool + select B + +# depends on each other + +config C1 + bool "C1" + depends on C2 + +config C2 + bool "C2" + depends on C1 + +# depends on and select + +config D1 + bool "D1" + depends on D2 + select D2 + +config D2 + bool + +# depends on and imply +# This is not recursive dependency + +config E1 + bool "E1" + depends on E2 + imply E2 + +config E2 + bool "E2" + +# property + +config F1 + bool "F1" + default F2 + +config F2 + bool "F2" + depends on F1 + +# menu + +menu "menu depending on its content" + depends on G + +config G + bool "G" + +endmenu diff --git a/scripts/kconfig/tests/warn_recursive_dep/__init__.py b/scripts/kconfig/tests/warn_recursive_dep/__init__.py new file mode 100644 index 000000000000..adb21951ba41 --- /dev/null +++ b/scripts/kconfig/tests/warn_recursive_dep/__init__.py @@ -0,0 +1,9 @@ +""" +Warn recursive inclusion. + +Recursive dependency should be warned. +""" + +def test(conf): + assert conf.oldaskconfig() == 0 + assert conf.stderr_contains('expected_stderr') diff --git a/scripts/kconfig/tests/warn_recursive_dep/expected_stderr b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr new file mode 100644 index 000000000000..3de807dd9cb2 --- /dev/null +++ b/scripts/kconfig/tests/warn_recursive_dep/expected_stderr @@ -0,0 +1,30 @@ +Kconfig:9:error: recursive dependency detected! +Kconfig:9: symbol B is selected by B +For a resolution refer to Documentation/kbuild/kconfig-language.txt +subsection "Kconfig recursive dependency limitations" + +Kconfig:3:error: recursive dependency detected! +Kconfig:3: symbol A depends on A +For a resolution refer to Documentation/kbuild/kconfig-language.txt +subsection "Kconfig recursive dependency limitations" + +Kconfig:15:error: recursive dependency detected! +Kconfig:15: symbol C1 depends on C2 +Kconfig:19: symbol C2 depends on C1 +For a resolution refer to Documentation/kbuild/kconfig-language.txt +subsection "Kconfig recursive dependency limitations" + +Kconfig:30:error: recursive dependency detected! +Kconfig:30: symbol D2 is selected by D1 +Kconfig:25: symbol D1 depends on D2 +For a resolution refer to Documentation/kbuild/kconfig-language.txt +subsection "Kconfig recursive dependency limitations" + +Kconfig:59:error: recursive dependency detected! +Kconfig:59: symbol G depends on G +For a resolution refer to Documentation/kbuild/kconfig-language.txt +subsection "Kconfig recursive dependency limitations" + +Kconfig:50:error: recursive dependency detected! +Kconfig:50: symbol F2 depends on F1 +Kconfig:48: symbol F1 default value contains F2 From e2c75e7667c737148de6cdd522edf2163b4c61d3 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:12:12 +0900 Subject: [PATCH 24/30] kconfig: tests: test if recursive inclusion is detected If recursive inclusion is detected, it should fail with error messages. Test this. This also tests the line numbers in the error message, fixed by commit 5ae6fcc4bb82 ("kconfig: fix line number in recursive inclusion error message"). Signed-off-by: Masahiro Yamada Reviewed-by: Ulf Magnusson --- scripts/kconfig/tests/err_recursive_inc/Kconfig | 1 + scripts/kconfig/tests/err_recursive_inc/Kconfig.inc1 | 4 ++++ scripts/kconfig/tests/err_recursive_inc/Kconfig.inc2 | 3 +++ scripts/kconfig/tests/err_recursive_inc/Kconfig.inc3 | 1 + scripts/kconfig/tests/err_recursive_inc/__init__.py | 10 ++++++++++ .../kconfig/tests/err_recursive_inc/expected_stderr | 5 +++++ 6 files changed, 24 insertions(+) create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc1 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc2 create mode 100644 scripts/kconfig/tests/err_recursive_inc/Kconfig.inc3 create mode 100644 scripts/kconfig/tests/err_recursive_inc/__init__.py create mode 100644 scripts/kconfig/tests/err_recursive_inc/expected_stderr diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig b/scripts/kconfig/tests/err_recursive_inc/Kconfig new file mode 100644 index 000000000000..0e4c8750ab65 --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig @@ -0,0 +1 @@ +source "Kconfig.inc1" diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc1 b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc1 new file mode 100644 index 000000000000..00e408d653fc --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc1 @@ -0,0 +1,4 @@ + + + +source "Kconfig.inc2" diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc2 b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc2 new file mode 100644 index 000000000000..349ea2db15dc --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc2 @@ -0,0 +1,3 @@ + + +source "Kconfig.inc3" diff --git a/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc3 b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc3 new file mode 100644 index 000000000000..0e4c8750ab65 --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/Kconfig.inc3 @@ -0,0 +1 @@ +source "Kconfig.inc1" diff --git a/scripts/kconfig/tests/err_recursive_inc/__init__.py b/scripts/kconfig/tests/err_recursive_inc/__init__.py new file mode 100644 index 000000000000..0e4c839c54aa --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/__init__.py @@ -0,0 +1,10 @@ +""" +Detect recursive inclusion error. + +If recursive inclusion is detected, it should fail with error messages. +""" + + +def test(conf): + assert conf.oldaskconfig() != 0 + assert conf.stderr_contains('expected_stderr') diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr new file mode 100644 index 000000000000..a15dbedd253a --- /dev/null +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr @@ -0,0 +1,5 @@ +Kconfig.inc1:4: recursive inclusion detected. Inclusion path: + current file : 'Kconfig.inc1' + included from: 'Kconfig.inc3:1' + included from: 'Kconfig.inc2:3' + included from: 'Kconfig.inc1:4' From f622f8279581626170154d77d67e67bcb227ca2f Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:56:07 +0900 Subject: [PATCH 25/30] kconfig: warn unmet direct dependency of tristate symbols selected by y Commit 246cf9c26bf1 ("kbuild: Warn on selecting symbols with unmet direct dependencies") forcibly promoted ->dir_dep.tri to yes from mod. So, the unmet direct dependencies of tristate symbols are not reported. [Test Case] config MODULES def_bool y option modules config A def_bool y select B config B tristate "B" depends on m This causes unmet dependency because 'B' is forced 'y' ignoring 'depends on m'. This should be warned. On the other hand, the following case ('B' is bool) should not be warned, so 'depends on m' for bool symbols should be naturally treated as 'depends on y'. [Test Case2 (not unmet dependency)] config MODULES def_bool y option modules config A def_bool y select B config B bool "B" depends on m Signed-off-by: Masahiro Yamada --- scripts/kconfig/symbol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 0f7eba7d472a..6acf536c9726 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -243,7 +243,7 @@ static void sym_calc_visibility(struct symbol *sym) tri = yes; if (sym->dir_dep.expr) tri = expr_calc_value(sym->dir_dep.expr); - if (tri == mod) + if (tri == mod && sym_get_type(sym) == S_BOOLEAN) tri = yes; if (sym->dir_dep.tri != tri) { sym->dir_dep.tri = tri; @@ -414,7 +414,7 @@ void sym_calc_value(struct symbol *sym) } } calc_newval: - if (sym->dir_dep.tri == no && sym->rev_dep.tri != no) { + if (sym->dir_dep.tri < sym->rev_dep.tri) { struct expr *e; e = expr_simplify_unmet_dep(sym->rev_dep.expr, sym->dir_dep.expr); From f8f69dc0b4e070d4a4d39889a85424913cc922d5 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Tue, 13 Mar 2018 18:56:08 +0900 Subject: [PATCH 26/30] kconfig: make unmet dependency warnings readable Currently, the unmet dependency warnings end up with endlessly long expressions, most of which are false positives. Here is test code to demonstrate how it currently works. [Test Case] config DEP1 def_bool y config DEP2 bool "DEP2" config A bool "A" select E config B bool "B" depends on DEP2 select E config C bool "C" depends on DEP1 && DEP2 select E config D def_bool n select E config E bool depends on DEP1 && DEP2 [Result] $ make config scripts/kconfig/conf --oldaskconfig Kconfig * * Linux Kernel Configuration * DEP2 (DEP2) [N/y/?] (NEW) n A (A) [N/y/?] (NEW) y warning: (A && B && D) selects E which has unmet direct dependencies (DEP1 && DEP2) Here, I see some points to be improved. First, '(A || B || D)' would make more sense than '(A && B && D)'. I am not sure if this is intentional, but expr_simplify_unmet_dep() turns OR expressions into AND, like follows: case E_OR: return expr_alloc_and( Second, we see false positives. 'A' is a real unmet dependency. 'B' is false positive because 'DEP1' is fixed to 'y', and 'B' depends on 'DEP2'. 'C' was correctly dropped by expr_simplify_unmet_dep(). 'D' is also false positive because it has no chance to be enabled. Current expr_simplify_unmet_dep() cannot avoid those false positives. After all, I decided to use the same helpers as used for printing reverse dependencies in the help. With this commit, unreadable warnings (most of the reported symbols are false positives) in the real world: $ make ARCH=score allyesconfig scripts/kconfig/conf --allyesconfig Kconfig warning: (HWSPINLOCK_QCOM && AHCI_MTK && STMMAC_PLATFORM && DWMAC_IPQ806X && DWMAC_LPC18XX && DWMAC_OXNAS && DWMAC_ROCKCHIP && DWMAC_SOCFPGA && DWMAC_STI && TI_CPSW && PINCTRL_GEMINI && PINCTRL_OXNAS && PINCTRL_ROCKCHIP && PINCTRL_DOVE && PINCTRL_ARMADA_37XX && PINCTRL_STM32 && S3C2410_WATCHDOG && VIDEO_OMAP3 && VIDEO_S5P_FIMC && USB_XHCI_MTK && RTC_DRV_AT91SAM9 && LPC18XX_DMAMUX && VIDEO_OMAP4 && COMMON_CLK_GEMINI && COMMON_CLK_ASPEED && COMMON_CLK_NXP && COMMON_CLK_OXNAS && COMMON_CLK_BOSTON && QCOM_ADSP_PIL && QCOM_Q6V5_PIL && QCOM_GSBI && ATMEL_EBI && ST_IRQCHIP && RESET_IMX7 && PHY_HI6220_USB && PHY_RALINK_USB && PHY_ROCKCHIP_PCIE && PHY_DA8XX_USB) selects MFD_SYSCON which has unmet direct dependencies (HAS_IOMEM) warning: (PINCTRL_AT91 && PINCTRL_AT91PIO4 && PINCTRL_OXNAS && PINCTRL_PISTACHIO && PINCTRL_PIC32 && PINCTRL_MESON && PINCTRL_NOMADIK && PINCTRL_MTK && PINCTRL_MT7622 && GPIO_TB10X) selects OF_GPIO which has unmet direct dependencies (GPIOLIB && OF && HAS_IOMEM) warning: (FAULT_INJECTION_STACKTRACE_FILTER && LATENCYTOP && LOCKDEP) selects FRAME_POINTER which has unmet direct dependencies (DEBUG_KERNEL && (CRIS || M68K || FRV || UML || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS) will be turned into: $ make ARCH=score allyesconfig scripts/kconfig/conf --allyesconfig Kconfig WARNING: unmet direct dependencies detected for MFD_SYSCON Depends on [n]: HAS_IOMEM [=n] Selected by [y]: - PINCTRL_STM32 [=y] && PINCTRL [=y] && (ARCH_STM32 || COMPILE_TEST [=y]) && OF [=y] - RTC_DRV_AT91SAM9 [=y] && RTC_CLASS [=y] && (ARCH_AT91 || COMPILE_TEST [=y]) - RESET_IMX7 [=y] && RESET_CONTROLLER [=y] - PHY_HI6220_USB [=y] && (ARCH_HISI && ARM64 || COMPILE_TEST [=y]) - PHY_RALINK_USB [=y] && (RALINK || COMPILE_TEST [=y]) - PHY_ROCKCHIP_PCIE [=y] && (ARCH_ROCKCHIP && OF [=y] || COMPILE_TEST [=y]) WARNING: unmet direct dependencies detected for OF_GPIO Depends on [n]: GPIOLIB [=y] && OF [=y] && HAS_IOMEM [=n] Selected by [y]: - PINCTRL_MTK [=y] && PINCTRL [=y] && (ARCH_MEDIATEK || COMPILE_TEST [=y]) && OF [=y] - PINCTRL_MT7622 [=y] && PINCTRL [=y] && (ARCH_MEDIATEK || COMPILE_TEST [=y]) && OF [=y] && (ARM64 || COMPILE_TEST [=y]) WARNING: unmet direct dependencies detected for FRAME_POINTER Depends on [n]: DEBUG_KERNEL [=y] && (CRIS || M68K || FRV || UML || SUPERH || BLACKFIN || MN10300 || METAG) || ARCH_WANT_FRAME_POINTERS [=n] Selected by [y]: - LATENCYTOP [=y] && DEBUG_KERNEL [=y] && STACKTRACE_SUPPORT [=y] && PROC_FS [=y] && !MIPS && !PPC && !S390 && !MICROBLAZE && !ARM_UNWIND && !ARC && !X86 Signed-off-by: Masahiro Yamada Reviewed-by: Petr Vorel --- scripts/kconfig/expr.c | 42 ---------------------------------------- scripts/kconfig/expr.h | 1 - scripts/kconfig/symbol.c | 35 +++++++++++++++++++++------------ 3 files changed, 23 insertions(+), 55 deletions(-) diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 49376e12fa30..e1a39e90841d 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1137,48 +1137,6 @@ static int expr_compare_type(enum expr_type t1, enum expr_type t2) return 0; } -static inline struct expr * -expr_get_leftmost_symbol(const struct expr *e) -{ - - if (e == NULL) - return NULL; - - while (e->type != E_SYMBOL) - e = e->left.expr; - - return expr_copy(e); -} - -/* - * Given expression `e1' and `e2', returns the leaf of the longest - * sub-expression of `e1' not containing 'e2. - */ -struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2) -{ - struct expr *ret; - - switch (e1->type) { - case E_OR: - return expr_alloc_and( - expr_simplify_unmet_dep(e1->left.expr, e2), - expr_simplify_unmet_dep(e1->right.expr, e2)); - case E_AND: { - struct expr *e; - e = expr_alloc_and(expr_copy(e1), expr_copy(e2)); - e = expr_eliminate_dups(e); - ret = (!expr_eq(e, e1)) ? e1 : NULL; - expr_free(e); - break; - } - default: - ret = e1; - break; - } - - return expr_get_leftmost_symbol(ret); -} - void expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), void *data, int prevtoken) diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index 8dbf2a4cdae1..94a383b21df6 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -305,7 +305,6 @@ struct expr *expr_transform(struct expr *e); int expr_contains_symbol(struct expr *dep, struct symbol *sym); bool expr_depends_symbol(struct expr *dep, struct symbol *sym); struct expr *expr_trans_compare(struct expr *e, enum expr_type type, struct symbol *sym); -struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2); void expr_fprint(struct expr *e, FILE *out); struct gstr; /* forward */ diff --git a/scripts/kconfig/symbol.c b/scripts/kconfig/symbol.c index 6acf536c9726..f0b2e3b3102d 100644 --- a/scripts/kconfig/symbol.c +++ b/scripts/kconfig/symbol.c @@ -333,6 +333,27 @@ static struct symbol *sym_calc_choice(struct symbol *sym) return def_sym; } +static void sym_warn_unmet_dep(struct symbol *sym) +{ + struct gstr gs = str_new(); + + str_printf(&gs, + "\nWARNING: unmet direct dependencies detected for %s\n", + sym->name); + str_printf(&gs, + " Depends on [%c]: ", + sym->dir_dep.tri == mod ? 'm' : 'n'); + expr_gstr_print(sym->dir_dep.expr, &gs); + str_printf(&gs, "\n"); + + expr_gstr_print_revdep(sym->rev_dep.expr, &gs, yes, + " Selected by [y]:\n"); + expr_gstr_print_revdep(sym->rev_dep.expr, &gs, mod, + " Selected by [m]:\n"); + + fputs(str_get(&gs), stderr); +} + void sym_calc_value(struct symbol *sym) { struct symbol_value newval, oldval; @@ -414,18 +435,8 @@ void sym_calc_value(struct symbol *sym) } } calc_newval: - if (sym->dir_dep.tri < sym->rev_dep.tri) { - struct expr *e; - e = expr_simplify_unmet_dep(sym->rev_dep.expr, - sym->dir_dep.expr); - fprintf(stderr, "warning: ("); - expr_fprint(e, stderr); - fprintf(stderr, ") selects %s which has unmet direct dependencies (", - sym->name); - expr_fprint(sym->dir_dep.expr, stderr); - fprintf(stderr, ")\n"); - expr_free(e); - } + if (sym->dir_dep.tri < sym->rev_dep.tri) + sym_warn_unmet_dep(sym); newval.tri = EXPR_OR(newval.tri, sym->rev_dep.tri); } if (newval.tri == mod && From 26561514cc9defed09a043dfaedc900274b76ff2 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Thu, 15 Mar 2018 15:25:27 +0900 Subject: [PATCH 27/30] kconfig: do not include both curses.h and ncurses.h for nconfig nconf.h includes and "ncurses.h", but it does not need to include both. Generally, it should fall back to curses.h only when ncurses.h is not found. But, looks like it has never happened; these includes have been here for many years since commit 692d97c380c6 ("kconfig: new configuration interface (nconfig)"), and nobody has complained about hard-coding of ncurses.h . Let's simply drop the curses.h inclusion. I replaced "ncurses.h" with since it is not a local file. Signed-off-by: Masahiro Yamada --- scripts/kconfig/nconf.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/scripts/kconfig/nconf.h b/scripts/kconfig/nconf.h index 0d5261705ef5..9f6f21d3b0d4 100644 --- a/scripts/kconfig/nconf.h +++ b/scripts/kconfig/nconf.h @@ -15,7 +15,7 @@ #include #include #include -#include +#include #include #include #include @@ -24,8 +24,6 @@ #include #include -#include "ncurses.h" - #define max(a, b) ({\ typeof(a) _a = a;\ typeof(b) _b = b;\ From 32a94b8b0c3e5ae575919850c5e49e936b704d45 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 23 Mar 2018 02:00:12 +0900 Subject: [PATCH 28/30] kconfig: remove duplicated file name and lineno of recursive inclusion As in the unit test, the error message for the recursive inclusion looks like this: Kconfig.inc1:4: recursive inclusion detected. Inclusion path: current file : 'Kconfig.inc1' included from: 'Kconfig.inc3:1' included from: 'Kconfig.inc2:3' included from: 'Kconfig.inc1:4' The 'Kconfig.inc1:4' is duplicated in the first and last lines. Also, the single quotes do not help readability. Change the message like follows: Recursive inclusion detected. Inclusion path: current file : Kconfig.inc1 included from: Kconfig.inc3:1 included from: Kconfig.inc2:3 included from: Kconfig.inc1:4 Signed-off-by: Masahiro Yamada --- .../kconfig/tests/err_recursive_inc/expected_stderr | 11 ++++++----- scripts/kconfig/zconf.l | 9 ++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/scripts/kconfig/tests/err_recursive_inc/expected_stderr b/scripts/kconfig/tests/err_recursive_inc/expected_stderr index a15dbedd253a..6b582eee2176 100644 --- a/scripts/kconfig/tests/err_recursive_inc/expected_stderr +++ b/scripts/kconfig/tests/err_recursive_inc/expected_stderr @@ -1,5 +1,6 @@ -Kconfig.inc1:4: recursive inclusion detected. Inclusion path: - current file : 'Kconfig.inc1' - included from: 'Kconfig.inc3:1' - included from: 'Kconfig.inc2:3' - included from: 'Kconfig.inc1:4' +Recursive inclusion detected. +Inclusion path: + current file : Kconfig.inc1 + included from: Kconfig.inc3:1 + included from: Kconfig.inc2:3 + included from: Kconfig.inc1:4 diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l index 88b650eb9cc9..6f139d2dc65a 100644 --- a/scripts/kconfig/zconf.l +++ b/scripts/kconfig/zconf.l @@ -328,14 +328,13 @@ void zconf_nextfile(const char *name) for (iter = current_file->parent; iter; iter = iter->parent ) { if (!strcmp(current_file->name,iter->name) ) { fprintf(stderr, - "%s:%d: recursive inclusion detected. " - "Inclusion path:\n current file : '%s'\n", - zconf_curname(), zconf_lineno(), - zconf_curname()); + "Recursive inclusion detected.\n" + "Inclusion path:\n" + " current file : %s\n", zconf_curname()); iter = current_file; do { iter = iter->parent; - fprintf(stderr, " included from: '%s:%d'\n", + fprintf(stderr, " included from: %s:%d\n", iter->name, iter->lineno - 1); } while (strcmp(iter->name, current_file->name)); exit(1); From 379a8eb8eb1a55344e1cf976fa589a12c68b60a7 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 23 Mar 2018 02:00:13 +0900 Subject: [PATCH 29/30] kconfig: detect recursive inclusion earlier Currently, the recursive inclusion is not detected when the offending file is about to be included; it is detected the offending file is about to include the *next* file. This is because the detection loop does not involve the file being included. Do this check against the file that is about to be included so that the recursive inclusion is detected before unneeded parsing happens. Signed-off-by: Masahiro Yamada --- scripts/kconfig/zconf.l | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l index 6f139d2dc65a..29b5d338d6bc 100644 --- a/scripts/kconfig/zconf.l +++ b/scripts/kconfig/zconf.l @@ -325,23 +325,25 @@ void zconf_nextfile(const char *name) buf->parent = current_buf; current_buf = buf; - for (iter = current_file->parent; iter; iter = iter->parent ) { - if (!strcmp(current_file->name,iter->name) ) { + file->parent = current_file; + + for (iter = current_file; iter; iter = iter->parent) { + if (!strcmp(iter->name, file->name)) { fprintf(stderr, "Recursive inclusion detected.\n" "Inclusion path:\n" - " current file : %s\n", zconf_curname()); - iter = current_file; + " current file : %s\n", file->name); + iter = file; do { iter = iter->parent; fprintf(stderr, " included from: %s:%d\n", iter->name, iter->lineno - 1); - } while (strcmp(iter->name, current_file->name)); + } while (strcmp(iter->name, file->name)); exit(1); } } + file->lineno = 1; - file->parent = current_file; current_file = file; } From 18492685e479fd4d8e1dca836f57c11b6800f083 Mon Sep 17 00:00:00 2001 From: Masahiro Yamada Date: Fri, 23 Mar 2018 02:00:14 +0900 Subject: [PATCH 30/30] kconfig: use yylineno option instead of manual lineno increments Tracking the line number by hand is error-prone since you need to increment it in every \n matching pattern. If '%option yylineno' is set, flex defines 'yylineno' to contain the current line number and automatically updates it each time it reads a \n character. This is much more convenient although the lexer does not initializes yylineno, so you need to set it to 1 each time you start reading a new file, and restore it you go back to the previous file. I tested this with DEBUG_PARSE, and confirmed the same dump message was produced. I removed the perf-report option. Otherwise, I see the following message: %option yylineno entails a performance penalty ONLY on rules that can match newline characters Signed-off-by: Masahiro Yamada --- scripts/kconfig/lkc.h | 1 + scripts/kconfig/zconf.l | 20 +++++++++----------- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/scripts/kconfig/lkc.h b/scripts/kconfig/lkc.h index 2d5ec2d0e952..f4394af6e4b8 100644 --- a/scripts/kconfig/lkc.h +++ b/scripts/kconfig/lkc.h @@ -68,6 +68,7 @@ struct kconf_id { enum symbol_type stype; }; +extern int yylineno; void zconfdump(FILE *out); void zconf_starthelp(void); FILE *zconf_fopen(const char *name); diff --git a/scripts/kconfig/zconf.l b/scripts/kconfig/zconf.l index 29b5d338d6bc..045093d827e1 100644 --- a/scripts/kconfig/zconf.l +++ b/scripts/kconfig/zconf.l @@ -1,5 +1,5 @@ %option nostdinit noyywrap never-interactive full ecs -%option 8bit nodefault perf-report perf-report +%option 8bit nodefault yylineno %option noinput %x COMMAND HELP STRING PARAM %{ @@ -83,7 +83,6 @@ n [A-Za-z0-9_-] [ \t]*#.*\n | [ \t]*\n { - current_file->lineno++; return T_EOL; } [ \t]*#.* @@ -104,7 +103,7 @@ n [A-Za-z0-9_-] const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); BEGIN(PARAM); current_pos.file = current_file; - current_pos.lineno = current_file->lineno; + current_pos.lineno = yylineno; if (id && id->flags & TF_COMMAND) { yylval.id = id; return id->token; @@ -116,7 +115,6 @@ n [A-Za-z0-9_-] . warn_ignored_character(*yytext); \n { BEGIN(INITIAL); - current_file->lineno++; return T_EOL; } } @@ -138,7 +136,7 @@ n [A-Za-z0-9_-] new_string(); BEGIN(STRING); } - \n BEGIN(INITIAL); current_file->lineno++; return T_EOL; + \n BEGIN(INITIAL); return T_EOL; ({n}|[/.])+ { const struct kconf_id *id = kconf_id_lookup(yytext, yyleng); if (id && id->flags & TF_PARAM) { @@ -150,7 +148,7 @@ n [A-Za-z0-9_-] return T_WORD; } #.* /* comment */ - \\\n current_file->lineno++; + \\\n ; [[:blank:]]+ . warn_ignored_character(*yytext); <> { @@ -187,7 +185,6 @@ n [A-Za-z0-9_-] fprintf(stderr, "%s:%d:warning: multi-line strings not supported\n", zconf_curname(), zconf_lineno()); - current_file->lineno++; BEGIN(INITIAL); return T_EOL; } @@ -220,12 +217,10 @@ n [A-Za-z0-9_-] } } [ \t]*\n/[^ \t\n] { - current_file->lineno++; zconf_endhelp(); return T_HELPTEXT; } [ \t]*\n { - current_file->lineno++; append_string("\n", 1); } [^ \t\n].* { @@ -304,7 +299,7 @@ void zconf_initscan(const char *name) memset(current_buf, 0, sizeof(*current_buf)); current_file = file_lookup(name); - current_file->lineno = 1; + yylineno = 1; } void zconf_nextfile(const char *name) @@ -325,6 +320,7 @@ void zconf_nextfile(const char *name) buf->parent = current_buf; current_buf = buf; + current_file->lineno = yylineno; file->parent = current_file; for (iter = current_file; iter; iter = iter->parent) { @@ -343,7 +339,7 @@ void zconf_nextfile(const char *name) } } - file->lineno = 1; + yylineno = 1; current_file = file; } @@ -352,6 +348,8 @@ static void zconf_endfile(void) struct buffer *parent; current_file = current_file->parent; + if (current_file) + yylineno = current_file->lineno; parent = current_buf->parent; if (parent) {