Merge pull request #6072 from nickanderson/CFE-3866/master

nickanderson · web-flow · commit e27bdf8dff76 · 2026-04-17T08:47:41.000-05:00
CFE-3866: Allowed select_region to converge across passes
diff --git a/cf-agent/files_edit.h b/cf-agent/files_edit.h
@@ -47,12 +47,16 @@ typedef struct
     char *changes_filename;
     Item *file_start;
     int num_edits;
+    int pass;  // Current convergence pass (1 to CF_DONEPASSES-1)
 #ifdef HAVE_LIBXML2
     xmlDocPtr xmldoc;
 #endif
     NewLineMode new_line_mode;
 } EditContext;
 
+// Check if we're on the final convergence pass where errors should be reported
+#define EDIT_CONTEXT_IS_FINAL_PASS(ec) ((ec)->pass >= CF_DONEPASSES - 1)
+
 // filename must not be freed until FinishEditContext.
 EditContext *NewEditContext(char *filename, const Attributes *a);
 void FinishEditContext(EvalContext *ctx, EditContext *ec,
diff --git a/cf-agent/files_editline.c b/cf-agent/files_editline.c
@@ -96,6 +96,7 @@ static bool SanityCheckDeletions(const Attributes *a, const Promise *pp);
 static bool SelectLine(EvalContext *ctx, const char *line, const Attributes *a);
 static bool NotAnchored(char *s);
 static bool SelectRegion(EvalContext *ctx, Item *start, Item **begin_ptr, Item **end_ptr, const Attributes *a, EditContext *edcontext);
+static PromiseResult HandleSelectRegionFailure(EvalContext *ctx, const Promise *pp, const Attributes *a, EditContext *edcontext, const char *operation_type);
 static bool MultiLineString(char *s);
 static bool InsertFileAtLocation(EvalContext *ctx, Item **start, Item *begin_ptr, Item *end_ptr, Item *location, Item *prev, const Attributes *a, const Promise *pp, EditContext *edcontext, PromiseResult *result);
 
@@ -131,6 +132,7 @@ bool ScheduleEditLineOperations(EvalContext *ctx, const Bundle *bp, const Attrib
 
     for (pass = 1; pass < CF_DONEPASSES; pass++)
     {
+        edcontext->pass = pass;  // Track current pass for convergence
         for (type = 0; EDITLINETYPESEQUENCE[type] != NULL; type++)
         {
             const BundleSection *sp = BundleGetSection(bp, EDITLINETYPESEQUENCE[type]);
@@ -390,22 +392,7 @@ static PromiseResult VerifyLineDeletions(EvalContext *ctx, const Promise *pp, Ed
     }
     else if (!SelectRegion(ctx, *start, &begin_ptr, &end_ptr, &a, edcontext))
     {
-        if (a.region.include_end || a.region.include_start)
-        {
-            cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, &a,
-                 "The promised line deletion '%s' could not select an edit region in '%s'"
-                 " (this is a good thing, as policy suggests deleting the markers)",
-                 pp->promiser, edcontext->filename);
-        }
-        else
-        {
-            cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, &a,
-                 "The promised line deletion '%s' could not select an edit region in '%s'"
-                 " (but the delimiters were expected in the file)",
-                 pp->promiser, edcontext->filename);
-        }
-        result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED);
-        return result;
+        return HandleSelectRegionFailure(ctx, pp, &a, edcontext, "line deletion");
     }
     if (!end_ptr && a.region.select_end && !a.region.select_end_match_eof)
     {
@@ -502,11 +489,7 @@ static PromiseResult VerifyColumnEdits(EvalContext *ctx, const Promise *pp, Edit
     }
     else if (!SelectRegion(ctx, *start, &begin_ptr, &end_ptr, &a, edcontext))
     {
-        cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, &a,
-             "The promised column edit '%s' could not select an edit region in '%s'",
-             pp->promiser, edcontext->filename);
-        result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED);
-        return result;
+        return HandleSelectRegionFailure(ctx, pp, &a, edcontext, "column edit");
     }
 
 /* locate and split line */
@@ -581,11 +564,7 @@ static PromiseResult VerifyPatterns(EvalContext *ctx, const Promise *pp, EditCon
     }
     else if (!SelectRegion(ctx, *start, &begin_ptr, &end_ptr, &a, edcontext))
     {
-        cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, &a,
-             "The promised pattern replace '%s' could not select an edit region in '%s'",
-             pp->promiser, edcontext->filename);
-        result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED);
-        return result;
+        return HandleSelectRegionFailure(ctx, pp, &a, edcontext, "pattern replace");
     }
 
     snprintf(lockname, CF_BUFSIZE - 1, "replace-%s-%s", pp->promiser, edcontext->filename);
@@ -774,11 +753,7 @@ static PromiseResult VerifyLineInsertions(EvalContext *ctx, const Promise *pp, E
     }
     else if (!SelectRegion(ctx, *start, &begin_ptr, &end_ptr, &a, edcontext))
     {
-        cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, &a,
-             "The promised line insertion '%s' could not select an edit region in '%s'",
-             pp->promiser, edcontext->filename);
-        result = PromiseResultUpdate(result, PROMISE_RESULT_INTERRUPTED);
-        return result;
+        return HandleSelectRegionFailure(ctx, pp, &a, edcontext, "line insertion");
     }
 
     if (!end_ptr && a.region.select_end && !a.region.select_end_match_eof)
@@ -939,6 +914,60 @@ If no such region matches, begin_ptr and end_ptr should point to NULL
 
 /*****************************************************************************/
 
+static PromiseResult HandleSelectRegionFailure(EvalContext *ctx, const Promise *pp, 
+                                                const Attributes *a, EditContext *edcontext,
+                                                const char *operation_type)
+/*
+  Common error handling for SelectRegion failures across multiple edit operations.
+  Returns appropriate PromiseResult based on whether we're in final pass or not.
+  Special handling for line deletions where missing region markers may be intended.
+*/
+{
+    assert(pp != NULL);
+    assert(a != NULL);
+    assert(edcontext != NULL);
+    assert(operation_type != NULL);
+
+    if (!EDIT_CONTEXT_IS_FINAL_PASS(edcontext))
+    {
+        int remaining_passes = (CF_DONEPASSES - 1) - edcontext->pass;
+        Log(LOG_LEVEL_VERBOSE,
+            "The promised %s '%s' could not select edit region in '%s' (pass %d/%d, %d more %s to try)",
+            operation_type, pp->promiser, edcontext->filename, 
+            edcontext->pass, CF_DONEPASSES - 1, remaining_passes,
+            remaining_passes == 1 ? "pass" : "passes");
+        return PROMISE_RESULT_NOOP;  // Allow retry in next pass
+    }
+
+    // Special case for line deletions: missing markers might be intentional
+    if (StringEqual(operation_type, "line deletion"))
+    {
+        if (a->region.include_end || a->region.include_start)
+        {
+            cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a,
+                 "The promised %s '%s' could not select an edit region in '%s' after %d passes"
+                 " (this is a good thing, as policy suggests deleting the markers)",
+                 operation_type, pp->promiser, edcontext->filename, CF_DONEPASSES - 1);
+        }
+        else
+        {
+            cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a,
+                 "The promised %s '%s' could not select an edit region in '%s' after %d passes"
+                 " (but the delimiters were expected in the file)",
+                 operation_type, pp->promiser, edcontext->filename, CF_DONEPASSES - 1);
+        }
+        return PROMISE_RESULT_INTERRUPTED;
+    }
+
+    // Standard error for final pass
+    cfPS(ctx, LOG_LEVEL_ERR, PROMISE_RESULT_INTERRUPTED, pp, a,
+         "The promised %s '%s' could not select an edit region in '%s' after %d passes",
+         operation_type, pp->promiser, edcontext->filename, CF_DONEPASSES - 1);
+    return PROMISE_RESULT_INTERRUPTED;
+}
+
+/*****************************************************************************/
+
 static int MatchRegion(EvalContext *ctx, const char *chunk, const Item *begin, const Item *end, bool regex)
 /*
   Match a region in between the selection delimiters. It is
diff --git a/tests/acceptance/31_tickets/CFE-3866/test.cf b/tests/acceptance/31_tickets/CFE-3866/test.cf
@@ -0,0 +1,93 @@
+body file control
+{
+        inputs => {
+                    "../../default.cf.sub",
+        };
+}
+
+bundle agent __main__
+{
+  methods:
+        "bundlesequence"  usebundle => default("$(this.promise_filename)");
+}
+
+bundle agent init
+{
+  files:
+      "$(G.testfile)"
+        delete => init_delete;
+}
+
+body delete init_delete
+{
+      dirlinks => "delete";
+      rmdirs   => "true";
+}
+
+bundle agent test
+{
+  meta:
+      "description"
+        string => "Test that select_region converges across multiple edit_line passes when the region is created in an earlier pass",
+        meta => { "CFE-3866" };
+
+  files:
+      "$(G.testfile)"
+        create => "true",
+        edit_line => insert_section_then_add_content;
+}
+
+bundle edit_line insert_section_then_add_content
+{
+  # First promise: Create the section header
+  insert_lines:
+      "[section]"
+        location => start;
+
+  # Second promise: Add content within the section using select_region
+  # This should converge even though the section didn't exist when
+  # select_region was first evaluated (pass 1)
+
+  insert_lines:
+      "key=value"
+        location => append,
+        select_region => section_region;
+}
+
+body location append
+{
+      before_after => "after";
+}
+
+body select_region section_region
+{
+      select_start => "^\[section\]";
+      include_start_delimiter => "true";
+      select_end => "^\[.*\]";
+      select_end_match_eof => "true";
+}
+
+bundle agent check
+{
+  vars:
+      "expected"
+        string => "[section]
+key=value
+";
+
+      "actual"
+        string => readfile("$(G.testfile)", "1000");
+
+  classes:
+      "ok" expression => strcmp("$(expected)", "$(actual)");
+
+  reports:
+    DEBUG::
+      "Expected: '$(expected)'";
+      "Actual: '$(actual)'";
+
+    ok::
+      "$(this.promise_filename) Pass";
+    !ok::
+      "$(this.promise_filename) FAIL";
+}
