remove workaround for "changed" status handling

2 files changed, 27 insertions, 37 deletions

diff --git a/src/cstore/cstore.cpp b/src/cstore/cstore.cpp
index 0634988..f146cb0 100644
--- a/src/cstore/cstore.cpp
+++ b/src/cstore/cstore.cpp
@@ -1763,6 +1763,23 @@ Cstore::unmarkCfgPathDeactivated(const vector<string>& path_comps)
  * note: if a node is changed, all of its ancestors are also considered
  *       changed.
  * return true if successful. otherwise return false.
+ *
+ * the original backend implementation only uses the "changed" marker at
+ * "root" to indicate whether the whole config has changed. for the rest
+ * of the config hierarchy, the original implementation treated all nodes
+ * that are present in the unionfs "changes only" directory as "changed".
+ *
+ * this worked until the introduction of "deactivate". since deactivated
+ * nodes are also present in the "changes only" directory, the backend
+ * treat them as "changed". on the other hand, deleted nodes don't appear
+ * in "changes only", so they are _not_ treated as "changed". this creates
+ * problems in various parts of the backend.
+ *
+ * the new CLI backend/library marks all changed nodes explicitly, and the
+ * "changed" status depends on such markers. note that the actual marking
+ * is done by the low-level implementation, so it does not have to be done
+ * as a "file marker" as long as the low-level implementation can correctly
+ * answer the "changed" query for a given path.
  */
 bool
 Cstore::markCfgPathChanged(const vector<string>& path_comps)
diff --git a/src/cstore/unionfs/cstore-unionfs.cpp b/src/cstore/unionfs/cstore-unionfs.cpp
index 7f14483..46611ed 100644
--- a/src/cstore/unionfs/cstore-unionfs.cpp
+++ b/src/cstore/unionfs/cstore-unionfs.cpp
@@ -780,33 +780,16 @@ UnionfsCstore::unmark_changed_with_descendants()
 bool
 UnionfsCstore::mark_changed()
 {
-  if (!mutable_cfg_path.has_parent_path()) {
-    /* at root, mark changed. root marker is needed by the original
-     * implementation as an indication of whether the whole config
-     * has changed.
-     */
-    b_fs::path marker = get_work_path() / C_MARKER_CHANGED;
-    if (b_fs_exists(marker)) {
-      // already marked. treat as success.
-      return true;
-    }
-    if (!create_file(marker.file_string())) {
-      output_internal("failed to mark changed [%s]\n",
-                      get_work_path().file_string().c_str());
-      return false;
-    }
+  b_fs::path marker = get_work_path() / C_MARKER_CHANGED;
+  if (b_fs_exists(marker)) {
+    // already marked. treat as success.
     return true;
   }
-
-  /* XXX not at root => nop for now.
-   *     we should be marking changed here. however, as commit is still
-   *     using its own unionfs implementation, it will not understand the
-   *     markers and therefore will not perform the necessary cleanup when
-   *     it's done.
-   *
-   *     for now, don't mark anything besides root. the query function
-   *     will use unionfs-specific implementation (changes-only dir).
-   */
+  if (!create_file(marker.file_string())) {
+    output_internal("failed to mark changed [%s]\n",
+                    get_work_path().file_string().c_str());
+    return false;
+  }
   return true;
 }
 
@@ -883,18 +866,8 @@ UnionfsCstore::get_comment(string& comment, bool active_cfg)
 bool
 UnionfsCstore::marked_changed()
 {
-  /* this function is only called by cfgPathChanged() in base class.
-   *
-   * XXX currently just use the changes_only dir for this query.
-   *     see explanation in mark_changed().
-   *
-   *     this implementation relies on the fact that cfgPathChanged()
-   *     includes deleted/added nodes (including deactivated/activated
-   *     nodes since it's NOT deactivate-aware). if that is not the case,
-   *     result will be different between deleted nodes (NOT IN
-   *     changes_only) and deactivated nodes (IN changes_only).
-   */
-  return b_fs_exists(get_change_path());
+  b_fs::path marker = get_work_path() / C_MARKER_CHANGED;
+  return b_fs_exists(marker);
 }
 
 /* XXX currently "committed marking" is done inside commit.