kconfiglib: Save previous configuration to .config.old

Update Kconfiglib (and menuconfig, just to sync) to upstream revision
094f4a9622046, to add the commit below.

  Save existing configuration to .<filename>.old in write_config()

  Add a default-True 'save_old' flag to write_config(). If 'save_old' is
  True and an existing configuration file is being overwritten, a copy
  of the old configuration file is saved to .<filename>.old (e.g.
  .config.old) in the same directory.

  Errors are ignored, as the old configuration would usually just be a
  nice-to-have, and not essential.

  The same functionality could be added for minimal configuration files
  and headers, but it's probably most useful for configuration files.

Other changes:

  - Parsing performance is improved a bit

  - scripts/kconfig/kconfig.py now prints the path to the merged
    configuration in zephyr/.config, to make it a bit easier to
    discover.

Fixes: #2907

Signed-off-by: Ulf Magnusson <Ulf.Magnusson@nordicsemi.no>

doc/application/application.rst

@@ -268,6 +268,12 @@ described above.)
 * :file:`.config`, which contains the configuration settings
   used to build the application.
 
+  .. note::
+
+     The previous version of :file:`.config` is saved to :file:`.config.old`
+     whenever the configuration is updated. This is for convenience, as
+     comparing the old and new versions can be handy.
+
 * Various object files (:file:`.o` files and :file:`.a` files) containing
   compiled kernel and application code.
 

scripts/kconfig/kconfig.py

@@ -90,6 +90,7 @@ def main():
 
     # Write the merged configuration and the C header
     kconf.write_config(args.dotconfig)
+    print("Configuration written to '{}'".format(args.dotconfig))
     kconf.write_autoconf(args.autoconf)
 
 

scripts/kconfig/kconfiglib.py

@@ -1021,7 +1021,7 @@ class Kconfig(object):
 
         return None
 
-    def load_config(self, filename, replace=True):
+    def load_config(self, filename=None, replace=True, verbose=True):
         """
         Loads symbol values from a file in the .config format. Equivalent to
         calling Symbol.set_value() to set each of the values.
@@ -1039,13 +1039,61 @@ class Kconfig(object):
         caught as OSError on Python 3.
 
         filename:
-          The file to load. Respects $srctree if set (see the class
+          Path to load configuration from (a string). Respects $srctree if set
-          documentation).
+          (see the class documentation).
+
+          If 'filename' is None, the configuration file to load (if any) is
+          calculated automatically, giving the behavior you'd usually want:
+
+            1. If the KCONFIG_CONFIG environment variable is set, it gives the
+               path to the configuration file to load. Otherwise, ".config" is
+               used. See standard_config_filename().
+
+            2. If the path from (1.) doesn't exist, the configuration file
+               given by kconf.defconfig_filename is loaded instead, which is
+               derived from the 'option defconfig_list' symbol.
+
+            3. If (1.) and (2.) fail to find a configuration file to load, no
+               configuration file is loaded, and symbols retain their current
+               values (e.g., their default values). This is not an error.
+
+           See the return value as well.
 
         replace (default: True):
-          True if all existing user values should be cleared before loading the
+          If True, all existing user values will be cleared before loading the
           .config. Pass False to merge configurations.
+
+        verbose (default: True):
+          If True and filename is None (automatically infer configuration
+          file), a message will be printed to stdout telling which file got
+          loaded (or that no file got loaded). This is meant to reduce
+          boilerplate in tools.
+
+        Returns True if an existing configuration was loaded (that didn't come
+        from the 'option defconfig_list' symbol), and False otherwise. This is
+        mostly useful in conjunction with filename=None, as True will always be
+        returned otherwise.
         """
+        loaded_existing = True
+        if filename is None:
+            filename = standard_config_filename()
+            if os.path.exists(filename):
+                if verbose:
+                    print("Using existing configuration '{}' as base"
+                          .format(filename))
+            else:
+                filename = self.defconfig_filename
+                if filename is None:
+                    if verbose:
+                        print("Using default symbol values as base")
+                    return False
+
+                if verbose:
+                    print("Using default configuration found in '{}' as "
+                          "base".format(filename))
+
+                loaded_existing = False
+
         # Disable the warning about assigning to symbols without prompts. This
         # is normal and expected within a .config file.
         self._warn_for_no_prompt = False
@@ -1058,6 +1106,8 @@ class Kconfig(object):
         finally:
             self._warn_for_no_prompt = True
 
+        return loaded_existing
+
     def _load_config(self, filename, replace):
         with self._open_config(filename) as f:
             if replace:
@@ -1235,7 +1285,7 @@ class Kconfig(object):
                                 .format(self.config_prefix, sym.name,
                                         escape(val)))
 
-                    elif sym.orig_type in _INT_HEX:
+                    else:  # sym.orig_type in _INT_HEX:
                         if sym.orig_type is HEX and \
                            not val.startswith(("0x", "0X")):
                             val = "0x" + val
@@ -1243,13 +1293,9 @@ class Kconfig(object):
                         f.write("#define {}{} {}\n"
                                 .format(self.config_prefix, sym.name, val))
 
-                    else:
+    def write_config(self, filename=None,
-                        _internal_error("Internal error while creating C "
+                     header="# Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)\n",
-                                        'header: unknown type "{}".'
+                     save_old=True, verbose=True):
-                                        .format(sym.orig_type))
-
-    def write_config(self, filename,
-                     header="# Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)\n"):
         r"""
         Writes out symbol values in the .config format. The format matches the
         C implementation, including ordering.
@@ -1262,14 +1308,40 @@ class Kconfig(object):
         See the 'Intro to symbol values' section in the module docstring to
         understand which symbols get written out.
 
-        filename:
+        filename (default: None):
-          Self-explanatory.
+          Filename to save configuration to (a string).
+
+          If None, the filename in the the environment variable KCONFIG_CONFIG
+          is used if set, and ".config" otherwise. See
+          standard_config_filename().
 
         header (default: "# Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)\n"):
           Text that will be inserted verbatim at the beginning of the file. You
           would usually want each line to start with '#' to make it a comment,
           and include a final terminating newline.
+
+        save_old (default: True):
+          If True and <filename> already exists, a copy of it will be saved to
+          .<filename>.old in the same directory before the new configuration is
+          written. The leading dot is added only if the filename doesn't
+          already start with a dot.
+
+          Errors are silently ignored if .<filename>.old cannot be written
+          (e.g. due to being a directory).
+
+        verbose (default: True):
+          If True and filename is None (automatically infer configuration
+          file), a message will be printed to stdout telling which file got
+          written. This is meant to reduce boilerplate in tools.
         """
+        if filename is None:
+            filename = standard_config_filename()
+        else:
+            verbose = False
+
+        if save_old:
+            _save_old(filename)
+
         with self._open(filename, "w") as f:
             f.write(header)
 
@@ -1285,6 +1357,9 @@ class Kconfig(object):
 
                     f.write("\n#\n# {}\n#\n".format(node.prompt[0]))
 
+        if verbose:
+            print("Configuration written to '{}'".format(filename))
+
     def write_min_config(self, filename,
                          header="# Generated by Kconfiglib (https://github.com/ulfalizer/Kconfiglib)\n"):
         """
@@ -1435,15 +1510,7 @@ class Kconfig(object):
                 continue
 
             # 'sym' has a new value. Flag it.
-
+            _touch_dep_file(sym.name)
-            sym_path = sym.name.lower().replace("_", os.sep) + ".h"
-            sym_path_dir = os.path.dirname(sym_path)
-            if sym_path_dir and not os.path.exists(sym_path_dir):
-                os.makedirs(sym_path_dir, 0o755)
-
-            # A kind of truncating touch, mirroring the C tools
-            os.close(os.open(
-                sym_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o644))
 
         # Remember the current values as the "new old" values.
         #
@@ -1501,6 +1568,10 @@ class Kconfig(object):
                         val = unescape(match.group(1))
 
                     self.syms[name]._old_val = val
+                else:
+                    # Flag that the symbol no longer exists, in
+                    # case something still depends on it
+                    _touch_dep_file(name)
 
     def node_iter(self, unique_syms=False):
         """
@@ -1830,24 +1901,23 @@ class Kconfig(object):
         return True
 
     def _line_after_help(self, line):
-        # Tokenizes the line after a help text. This case is special in that
+        # Tokenizes a line after a help text. This case is special in that the
-        # the line has already been fetched (to discover that it isn't part of
+        # line has already been fetched (to discover that it isn't part of the
-        # the help text).
+        # help text).
         #
         # An earlier version used a _saved_line variable instead that was
         # checked in _next_line(). This special-casing gets rid of it and makes
         # _reuse_tokens alone sufficient to handle unget.
 
-        if line:
+        # Handle line joining
-            # Handle line joining
+        while line.endswith("\\\n"):
-            while line.endswith("\\\n"):
+            line = line[:-2] + self._file.readline()
-                line = line[:-2] + self._file.readline()
+            self._linenr += 1
-                self._linenr += 1
 
-            self._line = line
+        self._line = line
 
-            self._tokens = self._tokenize(line)
+        self._tokens = self._tokenize(line)
-            self._reuse_tokens = True
+        self._reuse_tokens = True
 
 
     #
@@ -2867,65 +2937,68 @@ class Kconfig(object):
         node.prompt = (prompt, self._parse_cond())
 
     def _parse_help(self, node):
-        # Find first non-blank (not all-space) line and get its indentation
-
         if node.help is not None:
-            self._warn(_name_and_loc(node.item) +
+            self._warn(_name_and_loc(node.item) + " defined with more than "
-                       " defined with more than one help text -- only the "
+                       "one help text -- only the last one will be used")
-                       "last one will be used")
 
-        # Small optimization. This code is pretty hot.
+        # Micro-optimization. This code is pretty hot.
         readline = self._file.readline
 
+        # Find first non-blank (not all-space) line and get its
+        # indentation
+
         while 1:
             line = readline()
             self._linenr += 1
-            if not line or not line.isspace():
+            if not line:
+                self._empty_help(node, line)
+                return
+            if not line.isspace():
                 break
 
-        if not line:
+        len_ = len  # Micro-optimization
-            self._warn(_name_and_loc(node.item) +
-                       " has 'help' but empty help text")
 
-            node.help = ""
+        # Use a separate 'expline' variable here and below to avoid stomping on
-            return
+        # any tabs people might've put deliberately into the first line after
-
+        # the help text
-        indent = _indentation(line)
+        expline = line.expandtabs()
+        indent = len_(expline) - len_(expline.lstrip())
         if not indent:
-            # If the first non-empty lines has zero indent, there is no help
+            self._empty_help(node, line)
-            # text
-            self._warn(_name_and_loc(node.item) +
-                       " has 'help' but empty help text")
-
-            node.help = ""
-            self._line_after_help(line)
             return
 
-        # The help text goes on till the first non-empty line with less indent
+        # The help text goes on till the first non-blank line with less indent
         # than the first line
 
-        help_lines = []
+        # Add the first line
-        # Small optimizations
+        lines = [expline[indent:]]
-        add_help_line = help_lines.append
+        add_line = lines.append  # Micro-optimization
-        indentation = _indentation
-
-        while line and (line.isspace() or indentation(line) >= indent):
-            # De-indent 'line' by 'indent' spaces and rstrip() it to remove any
-            # newlines (which gets rid of other trailing whitespace too, but
-            # that's fine).
-            #
-            # This prepares help text lines in a speedy way: The [indent:]
-            # might already remove trailing newlines for lines shorter than
-            # indent (e.g. empty lines). The rstrip() makes it consistent,
-            # meaning we can join the lines with "\n" later.
-            add_help_line(line.expandtabs()[indent:].rstrip())
 
+        while 1:
             line = readline()
+            if not line:
+                break
 
-        self._linenr += len(help_lines)
+            if line.isspace():
+                # No need to preserve the exact whitespace in these
+                add_line("\n")
+            else:
+                expline = line.expandtabs()
+                if len_(expline) - len_(expline.lstrip()) < indent:
+                    break
+                add_line(expline[indent:])
 
-        node.help = "\n".join(help_lines).rstrip()
+        self._linenr += len_(lines)
-        self._line_after_help(line)
+        node.help = "".join(lines).rstrip()
+        if line:
+            self._line_after_help(line)
+
+    def _empty_help(self, node, line):
+        self._warn(_name_and_loc(node.item) +
+                   " has 'help' but empty help text")
+        node.help = ""
+        if line:
+            self._line_after_help(line)
 
     def _parse_expr(self, transform_m):
         # Parses an expression from the tokens in Kconfig._tokens using a
@@ -3332,9 +3405,7 @@ class Kconfig(object):
                                        "default value for string symbol "
                                        + _name_and_loc(sym))
 
-                    elif sym.orig_type in _INT_HEX and \
+                    elif not num_ok(default, sym.orig_type):  # INT/HEX
-                         not num_ok(default, sym.orig_type):
-
                         self._warn("the {0} symbol {1} has a non-{0} default {2}"
                                    .format(TYPE_TO_STR[sym.orig_type],
                                            _name_and_loc(sym),
@@ -3913,11 +3984,11 @@ class Symbol(object):
                 # Used to implement the warning below
                 has_default = False
 
-                for val_sym, cond in self.defaults:
+                for sym, cond in self.defaults:
                     if expr_value(cond):
                         has_default = self._write_to_conf = True
 
-                        val = val_sym.str_value
+                        val = sym.str_value
 
                         if _is_base_n(val, base):
                             val_num = int(val, base)
@@ -3958,9 +4029,9 @@ class Symbol(object):
                 val = self.user_value
             else:
                 # Otherwise, look at defaults
-                for val_sym, cond in self.defaults:
+                for sym, cond in self.defaults:
                     if expr_value(cond):
-                        val = val_sym.str_value
+                        val = sym.str_value
                         self._write_to_conf = True
                         break
 
@@ -4097,12 +4168,9 @@ class Symbol(object):
             return "{}{}={}\n" \
                    .format(self.kconfig.config_prefix, self.name, val)
 
-        if self.orig_type is STRING:
+        # sym.orig_type is STRING
-            return '{}{}="{}"\n' \
+        return '{}{}="{}"\n' \
-                   .format(self.kconfig.config_prefix, self.name, escape(val))
+               .format(self.kconfig.config_prefix, self.name, escape(val))
-
-        _internal_error("Internal error while creating .config: unknown "
-                        'type "{}".'.format(self.orig_type))
 
     def set_value(self, value):
         """
@@ -5182,16 +5250,9 @@ class MenuNode(object):
         elif self.item is MENU:
             fields.append("menu node for menu")
 
-        elif self.item is COMMENT:
+        else:  # self.item is COMMENT
             fields.append("menu node for comment")
 
-        elif not self.item:
-            fields.append("menu node for if (should not appear in the final "
-                          " tree)")
-
-        else:
-            _internal_error("unable to determine type in MenuNode.__repr__()")
-
         if self.prompt:
             fields.append('prompt "{}" (visibility {})'
                           .format(self.prompt[0],
@@ -5390,7 +5451,7 @@ class KconfigError(Exception):
 KconfigSyntaxError = KconfigError  # Backwards compatibility
 
 class InternalError(Exception):
-    "Exception raised for internal errors"
+    "Never raised. Kept around for backwards compatibility."
 
 # Workaround:
 #
@@ -5439,35 +5500,33 @@ def expr_value(expr):
     if expr[0] is NOT:
         return 2 - expr_value(expr[1])
 
-    if expr[0] in _RELATIONS:
+    # Relation
-        # Implements <, <=, >, >= comparisons as well. These were added to
+    #
-        # kconfig in 31847b67 (kconfig: allow use of relations other than
+    # Implements <, <=, >, >= comparisons as well. These were added to
-        # (in)equality).
+    # kconfig in 31847b67 (kconfig: allow use of relations other than
+    # (in)equality).
 
-        rel, v1, v2 = expr
+    rel, v1, v2 = expr
 
-        # If both operands are strings...
+    # If both operands are strings...
-        if v1.orig_type is STRING and v2.orig_type is STRING:
+    if v1.orig_type is STRING and v2.orig_type is STRING:
-            # ...then compare them lexicographically
+        # ...then compare them lexicographically
+        comp = _strcmp(v1.str_value, v2.str_value)
+    else:
+        # Otherwise, try to compare them as numbers
+        try:
+            comp = _sym_to_num(v1) - _sym_to_num(v2)
+        except ValueError:
+            # Fall back on a lexicographic comparison if the operands don't
+            # parse as numbers
             comp = _strcmp(v1.str_value, v2.str_value)
-        else:
-            # Otherwise, try to compare them as numbers
-            try:
-                comp = _sym_to_num(v1) - _sym_to_num(v2)
-            except ValueError:
-                # Fall back on a lexicographic comparison if the operands don't
-                # parse as numbers
-                comp = _strcmp(v1.str_value, v2.str_value)
 
-        if rel is EQUAL:         return 2*(comp == 0)
+    if rel is EQUAL:      return 2*(comp == 0)
-        if rel is UNEQUAL:       return 2*(comp != 0)
+    if rel is UNEQUAL:    return 2*(comp != 0)
-        if rel is LESS:          return 2*(comp < 0)
+    if rel is LESS:       return 2*(comp < 0)
-        if rel is LESS_EQUAL:    return 2*(comp <= 0)
+    if rel is LESS_EQUAL: return 2*(comp <= 0)
-        if rel is GREATER:       return 2*(comp > 0)
+    if rel is GREATER:    return 2*(comp > 0)
-        if rel is GREATER_EQUAL: return 2*(comp >= 0)
+    return 2*(comp >= 0)  # rel is GREATER_EQUAL
-
-    _internal_error("Internal error while evaluating expression: "
-                    "unknown operation {}.".format(expr[0]))
 
 def standard_sc_expr_str(sc):
     """
@@ -5637,6 +5696,9 @@ def standard_config_filename():
     """
     Helper for tools. Returns the value of KCONFIG_CONFIG (which specifies the
     .config file to load/save) if it is set, and ".config" otherwise.
+
+    Note: Calling load_config() with filename=None might give the behavior you
+    want, without having to use this function.
     """
     return os.environ.get("KCONFIG_CONFIG", ".config")
 
@@ -5753,13 +5815,6 @@ def _parenthesize(expr, type_, sc_expr_str_fn):
         return "({})".format(expr_str(expr, sc_expr_str_fn))
     return expr_str(expr, sc_expr_str_fn)
 
-def _indentation(line):
-    # Returns the length of the line's leading whitespace, treating tab stops
-    # as being spaced 8 characters apart.
-
-    line = line.expandtabs()
-    return len(line) - len(line.lstrip())
-
 def _ordered_unique(lst):
     # Returns 'lst' with any duplicates removed, preserving order. This hacky
     # version seems to be a common idiom. It relies on short-circuit evaluation
@@ -5791,12 +5846,43 @@ def _sym_to_num(sym):
     return sym.tri_value if sym.orig_type in _BOOL_TRISTATE else \
            int(sym.str_value, _TYPE_TO_BASE[sym.orig_type])
 
-def _internal_error(msg):
+def _touch_dep_file(sym_name):
-    raise InternalError(
+    # If sym_name is MY_SYM_NAME, touches my/sym/name.h. See the sync_deps()
-        msg +
+    # docstring.
-        "\nSorry! You may want to send an email to ulfalizer a.t Google's "
+
-        "email service to tell me about this. Include the message above and "
+    sym_path = sym_name.lower().replace("_", os.sep) + ".h"
-        "the stack trace and describe what you were doing.")
+    sym_path_dir = os.path.dirname(sym_path)
+    if sym_path_dir and not os.path.exists(sym_path_dir):
+        os.makedirs(sym_path_dir, 0o755)
+
+    # A kind of truncating touch, mirroring the C tools
+    os.close(os.open(
+        sym_path, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o644))
+
+def _save_old(path):
+    # See write_config()
+
+    dirname, basename = os.path.split(path)
+    backup = os.path.join(dirname,
+                          basename + ".old" if basename.startswith(".") else
+                              "." + basename + ".old")
+
+    # os.replace() would be nice here, but it's Python 3 (3.3+) only
+    try:
+        # Use copyfile() if 'path' is a symlink. The intention is probably to
+        # overwrite the target in that case.
+        if os.name == "posix" and not os.path.islink(path):
+            # Will remove .<filename>.old if it already exists on POSIX
+            # systems
+            os.rename(path, backup)
+        else:
+            import shutil
+            shutil.copyfile(path, backup)
+    except:
+        # Ignore errors from 'filename' missing as well as other errors. The
+        # backup file is more of a nice-to-have, and not worth erroring out
+        # over e.g. if .<filename>.old happens to be a directory.
+        pass
 
 def _decoding_error(e, filename, macro_linenr=None):
     # Gives the filename and context for UnicodeDecodeError's, which are a pain

scripts/kconfig/menuconfig.py

@@ -608,39 +608,20 @@ def menuconfig(kconf):
       Kconfig instance to be configured
     """
     global _kconf
-    global _config_filename
     global _show_all
     global _conf_changed
 
     _kconf = kconf
 
-    _config_filename = standard_config_filename()
+    # Always prompt for save if the configuration file doesn't exist
-
+    _conf_changed = not kconf.load_config()
-    if os.path.exists(_config_filename):
-        _conf_changed = False
-        print("Using existing configuration '{}' as base"
-              .format(_config_filename))
-        _kconf.load_config(_config_filename)
-
-    else:
-        # Always prompt for save if the .config doesn't exist
-        _conf_changed = True
-
-        if kconf.defconfig_filename is not None:
-            print("Using default configuration found in '{}' as base"
-                  .format(kconf.defconfig_filename))
-            _kconf.load_config(kconf.defconfig_filename)
-
-        else:
-            print("Using default symbol values as base")
-
 
     # Any visible items in the top menu?
     _show_all = False
-    if not _shown_nodes(_kconf.top_node):
+    if not _shown_nodes(kconf.top_node):
         # Nothing visible. Start in show-all mode and try again.
         _show_all = True
-        if not _shown_nodes(_kconf.top_node):
+        if not _shown_nodes(kconf.top_node):
             # Give up. The implementation relies on always having a selected
             # node.
             print("Empty configuration -- nothing to configure.\n"
@@ -649,7 +630,7 @@ def menuconfig(kconf):
 
     # Disable warnings. They get mangled in curses mode, and we deal with
     # errors ourselves.
-    _kconf.disable_warnings()
+    kconf.disable_warnings()
 
     # Make curses use the locale settings specified in the environment
     locale.setlocale(locale.LC_ALL, "")
@@ -795,8 +776,7 @@ def _menuconfig(stdscr):
             _set_sel_node_tri_val(2)
 
         elif c in (curses.KEY_LEFT, curses.KEY_BACKSPACE, _ERASE_CHAR,
-                   "\x1B",  # \x1B = ESC
+                   "\x1B", "h", "H"):  # \x1B = ESC
-                   "h", "H"):
 
             if c == "\x1B" and _cur_menu is _kconf.top_node:
                 res = _quit_dialog()
@@ -822,7 +802,7 @@ def _menuconfig(stdscr):
                 _conf_changed = False
 
         elif c in ("s", "S"):
-            if _save_dialog(_kconf.write_config, _config_filename,
+            if _save_dialog(_kconf.write_config, standard_config_filename(),
                             "configuration"):
 
                 _conf_changed = False
@@ -870,11 +850,11 @@ def _quit_dialog():
             return None
 
         if c == "y":
-            if _try_save(_kconf.write_config, _config_filename,
+            if _try_save(_kconf.write_config, standard_config_filename(),
                          "configuration"):
 
                 return "Configuration saved to '{}'" \
-                       .format(_config_filename)
+                       .format(standard_config_filename())
 
         elif c == "n":
             return "Configuration was not saved"
@@ -1421,6 +1401,9 @@ def _shown_nodes(menu):
         #
         # Note: Named choices are pretty broken in the C tools, and this is
         # super obscure, so you probably won't find much that relies on this.
+        # This whole 'if' could be deleted if you don't care about defining
+        # choices in multiple locations to add symbols (which will still work,
+        # just with things being displayed in a way that might be unexpected).
 
         # Do some additional work to avoid listing choice symbols twice if all
         # or part of the choice is copied in multiple locations (e.g. by