ulfalizer / Kconfiglib

A flexible Python 2/3 Kconfig implementation and library
ISC License
456 stars 161 forks source link

menuconfig won't write new config symbol to .config if just quit the menuconfig dialog. #63

Closed swordligit closed 5 years ago

swordligit commented 5 years ago

Test steps: 1, there is an old .config 2,change some Kconfig and add a new config symbol(default maybe yes or no) 3, open menuconfig, do nothing, press 'ESC' to quit the dialog, menuconfig will not let you select "save or no", just quit anyway. 4,menuconfig left the .config to original one, even it find the new config symbol.

This is not work the same way like kernel. With this case, the kernel will find the changes and override the old one with new symbol.

But if you do anything in the dialog, e.g. toggle the new symbol on/off, menuconfig will find this change and override the old one(if you select "save" option).

ulfalizer commented 5 years ago

Ohh... right, this is an oversight. Will fix it when I get back in a week or so.

Should check a combination of Symbol.config_string (to see if the symbol would get written), Symbol.user_value (the value from the .config file, if any), and Symbol.tri/str_value (the actual value of the symbol), to see if some symbol is missing from .config or has changed value.

Thanks for the report!

ulfalizer commented 5 years ago

Here's a patch that seems to work. Just going to do some more testing on it before pushing it out. My setup is a bit awkward at the moment. :)

diff --git a/kconfiglib.py b/kconfiglib.py
index 30b7507..8f86ece 100644
--- a/kconfiglib.py
+++ b/kconfiglib.py
@@ -700,6 +700,10 @@ class Kconfig(object):
       enabled will get added to Kconfig.warnings. See the various
       Kconfig.enable/disable_*_warnings() functions.

+    missing_syms:
+      A list of (name, value) tuples for all assignments to undefined symbols
+      within the most recently loaded .config file(s). See load_config().
+
     srctree:
       The value of the $srctree environment variable when the configuration was
       loaded, or the empty string if $srctree wasn't set. This gives nice
@@ -747,6 +751,7 @@ class Kconfig(object):
         "m",
         "mainmenu_text",
         "menus",
+        "missing_syms",
         "modules",
         "n",
         "named_choices",
@@ -863,6 +868,8 @@ class Kconfig(object):
         self.const_syms = {}
         self.defined_syms = []

+        self.missing_syms = []
+
         self.named_choices = {}
         self.choices = []

@@ -1032,10 +1039,15 @@ class Kconfig(object):
         "# CONFIG_FOO is not set" within a .config file sets the user value of
         FOO to n. The C tools work the same way.

-        The Symbol.user_value attribute can be inspected afterwards to see what
-        value the symbol was assigned in the .config file (if any). The user
-        value might differ from Symbol.str/tri_value if there are unsatisfied
-        dependencies.
+        For each symbol, the Symbol.user_value attribute holds the value the
+        symbol was assigned in the .config file (if any). The user value might
+        differ from Symbol.str/tri_value if there are unsatisfied dependencies.
+
+        After calling load_config(), the Kconfig.missing_syms attribute holds a
+        list of all assignments to undefined symbols within the loaded .config
+        file. Kconfig.missing_syms is cleared if load_config() is called with
+        replace=True (the default), and appended to otherwise. See the
+        documentation for Kconfig.missing_syms as well.

         Raises (possibly a subclass of) IOError on IO errors ('errno',
         'strerror', and 'filename' are available). Note that IOError can be
@@ -1115,6 +1127,8 @@ class Kconfig(object):
     def _load_config(self, filename, replace):
         with self._open_config(filename) as f:
             if replace:
+                self.missing_syms = []
+
                 # If we're replacing the configuration, keep track of which
                 # symbols and choices got set so that we can unset the rest
                 # later. This avoids invalidating everything and is faster.
@@ -1140,14 +1154,12 @@ class Kconfig(object):
                 if match:
                     name, val = match.groups()
                     if name not in syms:
-                        self._warn_undef_assign_load(name, val, filename,
-                                                     linenr)
+                        self._undef_assign(name, val, filename, linenr)
                         continue

                     sym = syms[name]
                     if not sym.nodes:
-                        self._warn_undef_assign_load(name, val, filename,
-                                                     linenr)
+                        self._undef_assign(name, val, filename, linenr)
                         continue

                     if sym.orig_type in _BOOL_TRISTATE:
@@ -1209,8 +1221,7 @@ class Kconfig(object):

                     name = match.group(1)
                     if name not in syms:
-                        self._warn_undef_assign_load(name, "n", filename,
-                                                     linenr)
+                        self._undef_assign(name, "n", filename, linenr)
                         continue

                     sym = syms[name]
@@ -1251,6 +1262,16 @@ class Kconfig(object):
                 if not choice._was_set:
                     choice.unset_value()

+    def _undef_assign(self, name, val, filename, linenr):
+        # Called for assignments to undefined symbols during .config loading
+
+        self.missing_syms.append((name, val))
+
+        if self._warn_for_undef_assign:
+            self._warn(
+                "attempt to assign the value '{}' to the undefined symbol {}"
+                .format(val, name), filename, linenr)
+
     def write_autoconf(self, filename,
                        header="/* Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib) */\n"):
         r"""
@@ -3617,19 +3638,6 @@ class Kconfig(object):
             if self._warn_to_stderr:
                 sys.stderr.write(msg + "\n")

-    def _warn_undef_assign(self, msg, filename, linenr):
-        # See the class documentation
-
-        if self._warn_for_undef_assign:
-            self._warn(msg, filename, linenr)
-
-    def _warn_undef_assign_load(self, name, val, filename, linenr):
-        # Special version for load_config()
-
-        self._warn_undef_assign(
-            'attempt to assign the value "{}" to the undefined symbol {}'
-            .format(val, name), filename, linenr)
-
     def _warn_override(self, msg, filename, linenr):
         # See the class documentation

diff --git a/menuconfig.py b/menuconfig.py
index 4f3bef8..ec155b6 100755
--- a/menuconfig.py
+++ b/menuconfig.py
@@ -192,7 +192,7 @@ import sys
 import textwrap

 from kconfiglib import Symbol, Choice, MENU, COMMENT, MenuNode, \
-                       BOOL, STRING, INT, HEX, UNKNOWN, \
+                       BOOL, TRISTATE, STRING, INT, HEX, UNKNOWN, \
                        AND, OR, \
                        expr_str, expr_value, split_expr, \
                        standard_sc_expr_str, \
@@ -631,8 +631,7 @@ def menuconfig(kconf):

     _kconf = kconf

-    # Always prompt for save if the configuration file doesn't exist
-    _conf_changed = not kconf.load_config()
+    _conf_changed = _load_config()

     # Any visible items in the top menu?
     _show_all = False
@@ -678,6 +677,38 @@ def menuconfig(kconf):
     # curses has been de-initialized.
     print(curses.wrapper(_menuconfig))

+def _load_config():
+    # Loads any existing configuration file on startup. See the
+    # Kconfig.load_config() docstring.
+    #
+    # Returns True if no .config exists or if the existing .config is outdated.
+    # We always want to prompt for save in those cases.
+
+    if not _kconf.load_config():
+        # No existing .config
+        return True
+
+    if _kconf.missing_syms:
+        # .config has assignments to undefined symbols, which will get removed
+        # when saving
+        return True
+
+    for sym in _kconf.unique_defined_syms:
+        if sym.user_value is None:
+            if sym.config_string:
+                # Unwritten symbol
+                return True
+        elif sym.type in (BOOL, TRISTATE):
+            if sym.tri_value != sym.user_value:
+                # Written bool/tristate symbol, new value
+                return True
+        elif sym.str_value != sym.user_value:
+            # Written string/int/hex symbol, new value
+            return True
+
+    # No need to prompt for save
+    return False
+
 # Global variables used below:
 #
 #   _stdscr:
@@ -811,7 +842,7 @@ def _menuconfig(stdscr):
                     continue

             if _load_dialog():
-                _conf_changed = False
+                _conf_changed = True

         elif c in ("s", "S"):
             if _save_dialog(_kconf.write_config, standard_config_filename(),
swordligit commented 5 years ago

Oh, So productive, I have test the patch, it is ok, Thank you!

ulfalizer commented 5 years ago

No problem! Pushed out in a release now as well.