scripts: kconfig.py: Detect all assignments to promptless symbols

Assigning to promptless symbols has no effect.

Previously, the only check was for whether the value assigned to a
symbol matched its final value. This misses cases where a promptless
symbol is assigned to and just happens to get the assigned-to value as
its final value.

Instead, detect whether configuration files are being merged (by
checking if zephyr/.config already exists), and explicitly check for
assignments to promptless symbols in that case. We can't do it when
zephyr/.config already exists (and is being loaded), because it includes
values for promptless symbols as well.

With the no-prompt check moved out, also use a more specific message for
it, and remove stuff related to prompts elsewhere. Shorten messages a
bit at the same time, and add two warn() and err() helpers.

Fixes: zephyrproject-rtos#20697

Signed-off-by: Ulf Magnusson <[email protected]>

scripts/kconfig/kconfig.py

@@ -45,6 +45,16 @@ def main():
         # replace=False creates a merged configuration
         print(kconf.load_config(config, replace=False))
 
+    if not os.path.exists(args.autoconf):
+        # If zephyr/.config does not exist, it means we just merged
+        # configuration fragments. Check that there were no assignments to
+        # promptless symbols in them. Such assignments have no effect.
+        #
+        # This won't work if zephyr/.config already exists (which means it's
+        # being loaded), because zephyr/.config is a full configuration file
+        # that includes values for promptless symbols.
+        verify_no_promptless_assign(kconf)
+
     # Print warnings for symbols whose actual value doesn't match the assigned
     # value
     for sym in kconf.unique_defined_syms:
@@ -97,26 +107,17 @@ def main():
     write_kconfig_filenames(kconf.kconfig_filenames, kconf.srctree, args.sources)
 
 
-# Message printed when a promptless symbol is assigned (and doesn't get the
-# assigned value)
-PROMPTLESS_HINT = """
-This symbol has no prompt, meaning assignments in configuration files have no
-effect on it. It can only be set indirectly, via Kconfig defaults (e.g. in a
-Kconfig.defconfig file) or through being 'select'ed or 'imply'd (note: try to
-avoid Kconfig 'select's except for trivial promptless "helper" symbols without
-dependencies, as it ignores dependencies and forces symbols on)."""
+def verify_no_promptless_assign(kconf):
+    # Checks that no promptless symbols are assigned
 
-# Message about where to look up symbol information
-SYM_INFO_HINT = """
-You can check symbol information (including dependencies) in the 'menuconfig'
-interface (see the Application Development Primer section of the manual), or in
-the Kconfig reference at
-http://docs.zephyrproject.org/latest/reference/kconfig/CONFIG_{}.html (which is
-updated regularly from the master branch). See the 'Setting configuration
-values' section of the Board Porting Guide as well."""
+    for sym in kconf.unique_defined_syms:
+        if sym.user_value is not None and promptless(sym):
+            err(("""\
+{0.name_and_loc} is assigned in a configuration file, but is not
+directly user-configurable (has no prompt). It gets its value indirectly from
+other symbols. \
+""" + SYM_INFO_HINT).format(sym))
 
-PROMPTLESS_HINT_EXTRA = """
-It covers Kconfig.defconfig files."""
 
 def verify_assigned_sym_value(sym):
     # Verifies that the value assigned to 'sym' "took" (matches the value the
@@ -130,17 +131,10 @@ def verify_assigned_sym_value(sym):
         user_value = sym.user_value
 
     if user_value != sym.str_value:
-        msg = "warning: {} was assigned the value '{}' but got the " \
-              "value '{}'." \
-              .format(sym.name_and_loc, user_value, sym.str_value)
-
-        if promptless(sym): msg += PROMPTLESS_HINT
-        msg += SYM_INFO_HINT.format(sym.name)
-        if promptless(sym): msg += PROMPTLESS_HINT_EXTRA
-
-        # Use a large fill() width to try to avoid linebreaks in the symbol
-        # reference link
-        print("\n" + textwrap.fill(msg, 100), file=sys.stderr)
+        warn(("""\
+{0.name_and_loc} was assigned the value '{1}' but got the value
+'{0.str_value}'. Check its dependencies. \
+""" + SYM_INFO_HINT).format(sym, user_value))
 
 
 def verify_assigned_choice_value(choice):
@@ -157,14 +151,22 @@ def verify_assigned_choice_value(choice):
     # y ended up as n, and print a spurious warning.
 
     if choice.user_selection is not choice.selection:
-        msg = "warning: the choice symbol {} was selected (set =y), but {} " \
-              "ended up as the choice selection. {}" \
-              .format(choice.user_selection.name_and_loc,
-                      choice.selection.name_and_loc if choice.selection
-                          else "no symbol",
-                      SYM_INFO_HINT.format(choice.user_selection.name))
+        warn(("""\
+the choice symbol {0.name_and_loc} was selected (set =y), but {1} ended up as
+the choice selection. \
+""" + SYM_INFO_HINT).format(
+            choice.user_selection,
+            choice.selection.name_and_loc if choice.selection else "no symbol"))
+
 
-        print("\n" + textwrap.fill(msg, 100), file=sys.stderr)
+# Hint on where to find symbol information. Expects the first argument of
+# format() to be the symbol.
+SYM_INFO_HINT = """\
+See http://docs.zephyrproject.org/latest/reference/kconfig/CONFIG_{0.name}.html
+and/or look up {0.name} in the menuconfig/guiconfig interface. The Application
+Development Primer, Setting Configuration Values, and Kconfig - Tips and Best
+Practices sections of the manual might be helpful too.\
+"""
 
 
 def promptless(sym):
@@ -186,7 +188,8 @@ def write_kconfig_filenames(paths, root_path, output_file_path):
     # to ensure that different representations of the same path does not end
     # up with two entries, as that could cause the build system to fail.
 
-    paths_uniq = sorted({os.path.realpath(os.path.join(root_path, path)) for path in paths})
+    paths_uniq = sorted({os.path.realpath(os.path.join(root_path, path))
+                         for path in paths})
 
     with open(output_file_path, 'w') as out:
         for path in paths_uniq:
@@ -209,5 +212,17 @@ def parse_args():
     return parser.parse_args()
 
 
+def warn(msg):
+    # Use a large fill() width to try to avoid linebreaks in the symbol
+    # reference link. Add some extra newlines to set the message off from
+    # surrounding text (this usually gets printed as part of spammy CMake
+    # output).
+    print("\nwarning: " + textwrap.fill(msg, 100) + "\n", file=sys.stderr)
+
+
+def err(msg):
+    sys.exit("\nerror: " + textwrap.fill(msg, 100) + "\n")
+
+
 if __name__ == "__main__":
     main()