Add a --runtime-config option for SAVE_CONFIG

2 files changed, 33 insertions, 79 deletions

diff --git a/klippy/configfile.py b/klippy/configfile.py
index e8ede0c3..b0c0c4ba 100644
--- a/klippy/configfile.py
+++ b/klippy/configfile.py
@@ -3,7 +3,7 @@
 # Copyright (C) 2016-2024  Kevin O'Connor <kevin@koconnor.net>
 #
 # This file may be distributed under the terms of the GNU GPLv3 license.
-import os, glob, re, time, logging, configparser, io
+import os, glob, re, time, logging, configparser, io, shutil
 
 error = configparser.Error
 
@@ -326,12 +326,6 @@ class ConfigFileReader:
 # Config auto save helper
 ######################################################################
 
-AUTOSAVE_HEADER = """
-#*# <---------------------- SAVE_CONFIG ---------------------->
-#*# DO NOT EDIT THIS BLOCK OR BELOW. The contents are auto-generated.
-#*#
-"""
-
 
 class ConfigAutoSave:
     def __init__(self, printer):
@@ -344,34 +338,6 @@ class ConfigAutoSave:
             "SAVE_CONFIG", self.cmd_SAVE_CONFIG, desc=self.cmd_SAVE_CONFIG_help
         )
 
-    def _find_autosave_data(self, data):
-        regular_data = data
-        autosave_data = ""
-        pos = data.find(AUTOSAVE_HEADER)
-        if pos >= 0:
-            regular_data = data[:pos]
-            autosave_data = data[pos + len(AUTOSAVE_HEADER) :].strip()
-        # Check for errors and strip line prefixes
-        if "\n#*# " in regular_data or autosave_data.find(AUTOSAVE_HEADER) >= 0:
-            logging.warning(
-                "Can't read autosave from config file" " - autosave state corrupted"
-            )
-            return data, ""
-        out = [""]
-        for line in autosave_data.split("\n"):
-            if (
-                not line.startswith("#*#")
-                or (len(line) >= 4 and not line.startswith("#*# "))
-            ) and autosave_data:
-                logging.warning(
-                    "Can't read autosave from config file"
-                    " - modifications after header"
-                )
-                return data, ""
-            out.append(line[4:])
-        out.append("")
-        return regular_data, "\n".join(out)
-
     comment_r = re.compile("[#;].*$")
     value_r = re.compile("[^A-Za-z0-9_].*$")
 
@@ -399,16 +365,21 @@ class ConfigAutoSave:
         return "\n".join(lines)
 
     def load_main_config(self):
-        filename = self.printer.get_start_args()["config_file"]
+        regular_filename = self.printer.get_start_args()["config_file"]
+        autosave_filename = self.printer.get_start_args()["runtime_config"]
         cfgrdr = ConfigFileReader()
-        data = cfgrdr.read_config_file(filename)
-        regular_data, autosave_data = self._find_autosave_data(data)
+        regular_data = cfgrdr.read_config_file(regular_filename)
+        # TODO: TOCTOU - future refactor
+        if os.path.exists(autosave_filename):
+            autosave_data = cfgrdr.read_config_file(autosave_filename)
+        else:
+            autosave_data = ""
         regular_fileconfig = cfgrdr.build_fileconfig_with_includes(
-            regular_data, filename
+            regular_data, regular_filename
         )
         autosave_data = self._strip_duplicates(autosave_data, regular_fileconfig)
-        self.fileconfig = cfgrdr.build_fileconfig(autosave_data, filename)
-        cfgrdr.append_fileconfig(regular_fileconfig, autosave_data, "*AUTOSAVE*")
+        self.fileconfig = cfgrdr.build_fileconfig(autosave_data, autosave_filename)
+        cfgrdr.append_fileconfig(regular_fileconfig, autosave_data, autosave_filename)
         return regular_fileconfig, self.fileconfig
 
     def get_status(self, eventtime):
@@ -465,51 +436,27 @@ class ConfigAutoSave:
             return
         # Create string containing autosave data
         cfgrdr = ConfigFileReader()
+        # TODO: This will no longer comment out duplicate sections in the main
+        # config. Which is fine because they're ignored when the main config is
+        # read. But maybe there should be a warning either here or at load as
+        # there's no feedback mechanism anymore.
         autosave_data = cfgrdr.build_config_string(self.fileconfig)
-        lines = [("#*# " + l).strip() for l in autosave_data.split("\n")]
-        lines.insert(0, "\n" + AUTOSAVE_HEADER.rstrip())
-        lines.append("")
-        autosave_data = "\n".join(lines)
-        # Read in and validate current config file
-        cfgname = self.printer.get_start_args()["config_file"]
-        try:
-            data = cfgrdr.read_config_file(cfgname)
-        except error as e:
-            msg = "Unable to read existing config on SAVE_CONFIG"
-            logging.exception(msg)
-            raise gcmd.error(msg)
-        regular_data, old_autosave_data = self._find_autosave_data(data)
-        regular_data = self._strip_duplicates(regular_data, self.fileconfig)
-        data = regular_data.rstrip() + autosave_data
-        new_regular_data, new_autosave_data = self._find_autosave_data(data)
-        if not new_autosave_data:
-            raise gcmd.error(
-                "Existing config autosave is corrupted." " Can't complete SAVE_CONFIG"
-            )
-        try:
-            regular_fileconfig = cfgrdr.build_fileconfig_with_includes(
-                new_regular_data, cfgname
-            )
-        except error as e:
-            msg = "Unable to parse existing config on SAVE_CONFIG"
-            logging.exception(msg)
-            raise gcmd.error(msg)
-        self._disallow_include_conflicts(regular_fileconfig)
+        autosave_filename = self.printer.get_start_args()["runtime_config"]
         # Determine filenames
         datestr = time.strftime("-%Y%m%d_%H%M%S")
-        backup_name = cfgname + datestr
-        temp_name = cfgname + "_autosave"
-        if cfgname.endswith(".cfg"):
-            backup_name = cfgname[:-4] + datestr + ".cfg"
-            temp_name = cfgname[:-4] + "_autosave.cfg"
+        backup_name = autosave_filename + datestr
+        temp_name = autosave_filename + "_autosave"
+        if autosave_filename.endswith(".cfg"):
+            backup_name = autosave_filename[:-4] + datestr + ".cfg"
+            temp_name = autosave_filename[:-4] + "_autosave.cfg"
         # Create new config file with temporary name and swap with main config
-        logging.info("SAVE_CONFIG to '%s' (backup in '%s')", cfgname, backup_name)
+        logging.info("SAVE_CONFIG to '%s' (backup in '%s')", autosave_filename, backup_name)
         try:
             f = open(temp_name, "w")
-            f.write(data)
+            f.write(autosave_data)
             f.close()
-            os.rename(cfgname, backup_name)
-            os.rename(temp_name, cfgname)
+            shutil.copy2(autosave_filename, backup_name)
+            os.rename(temp_name, autosave_filename)
         except:
             msg = "Unable to write config file during SAVE_CONFIG"
             logging.exception(msg)
diff --git a/klippy/klippy.py b/klippy/klippy.py
index 757a2efc..83a962fa 100644
--- a/klippy/klippy.py
+++ b/klippy/klippy.py
@@ -308,6 +308,12 @@ def main():
         help="api server unix domain socket filename",
     )
     opts.add_option(
+        "-R",
+        "--runtime-config",
+        default="/var/run/klipper/run.cfg",
+        help="path to where SAVE_CONFIG should store the runtime configuration",
+    )
+    opts.add_option(
         "-l", "--logfile", dest="logfile", help="write log to file instead of stderr"
     )
     opts.add_option(
@@ -392,6 +398,7 @@ def main():
     extra_git_desc += "\nTracked URL: %s" % (git_info["url"])
     start_args["software_version"] = git_vers
     start_args["cpu_info"] = util.get_cpu_info()
+    start_args["runtime_config"] = options.runtime_config
     if bglogger is not None:
         versions = "\n".join(
             [