cmSystemTools: Add EnvDiff class to hold ENVIRONMENT_MODIFICATION logic

Prepare to re-use this logic when enhancing `cmake -E env`.

Source/CTest/cmCTestRunTest.cxx

@@ -8,16 +8,12 @@
 #include <cstdint>
 #include <cstdio>
 #include <cstring>
-#include <functional>
 #include <iomanip>
 #include <ratio>
 #include <sstream>
 #include <utility>
 
 #include <cm/memory>
-#include <cm/optional>
-#include <cm/string_view>
-#include <cmext/string_view>
 
 #include "cmsys/RegularExpression.hxx"
 
@@ -793,7 +789,7 @@ bool cmCTestRunTest::ForkProcess(
   if (environment && !environment->empty()) {
     // Environment modification works on the assumption that the environment is
     // actually modified here. If another strategy is used, there will need to
-    // be updates below in `apply_diff`.
+    // be updates in `EnvDiff::ParseOperation`.
     cmSystemTools::AppendEnv(*environment);
     for (auto const& var : *environment) {
       envMeasurement << var << std::endl;
@@ -801,129 +797,18 @@ bool cmCTestRunTest::ForkProcess(
   }
 
   if (environment_modification && !environment_modification->empty()) {
-    std::map<std::string, cm::optional<std::string>> env_application;
-
-    char path_sep = cmSystemTools::GetSystemPathlistSeparator();
-
-    auto apply_diff =
-      [&env_application](const std::string& name,
-                         std::function<void(std::string&)> const& apply) {
-        cm::optional<std::string> old_value = env_application[name];
-        std::string output;
-        if (old_value) {
-          output = *old_value;
-        } else {
-          // This only works because the environment is actually modified above
-          // (`AppendEnv`). If CTest ever just creates an environment block
-          // directly, that block will need to be queried for the subprocess'
-          // value instead.
-          const char* curval = cmSystemTools::GetEnv(name);
-          if (curval) {
-            output = curval;
-          }
-        }
-        apply(output);
-        env_application[name] = output;
-      };
-
-    bool err_occurred = false;
+    cmSystemTools::EnvDiff diff;
+    bool env_ok = true;
 
     for (auto const& envmod : *environment_modification) {
-      // Split on `=`
-      auto const eq_loc = envmod.find_first_of('=');
-      if (eq_loc == std::string::npos) {
-        cmCTestLog(this->CTest, ERROR_MESSAGE,
-                   "Error: Missing `=` after the variable name in: "
-                     << envmod << std::endl);
-        err_occurred = true;
-        continue;
-      }
-      auto const name = envmod.substr(0, eq_loc);
-
-      // Split value on `:`
-      auto const op_value_start = eq_loc + 1;
-      auto const colon_loc = envmod.find_first_of(':', op_value_start);
-      if (colon_loc == std::string::npos) {
-        cmCTestLog(this->CTest, ERROR_MESSAGE,
-                   "Error: Missing `:` after the operation in: " << envmod
-                                                                 << std::endl);
-        err_occurred = true;
-        continue;
-      }
-      auto const op =
-        envmod.substr(op_value_start, colon_loc - op_value_start);
-
-      auto const value_start = colon_loc + 1;
-      auto const value = envmod.substr(value_start);
-
-      // Determine what to do with the operation.
-      if (op == "reset"_s) {
-        auto entry = env_application.find(name);
-        if (entry != env_application.end()) {
-          env_application.erase(entry);
-        }
-      } else if (op == "set"_s) {
-        env_application[name] = value;
-      } else if (op == "unset"_s) {
-        env_application[name] = {};
-      } else if (op == "string_append"_s) {
-        apply_diff(name, [&value](std::string& output) { output += value; });
-      } else if (op == "string_prepend"_s) {
-        apply_diff(name,
-                   [&value](std::string& output) { output.insert(0, value); });
-      } else if (op == "path_list_append"_s) {
-        apply_diff(name, [&value, path_sep](std::string& output) {
-          if (!output.empty()) {
-            output += path_sep;
-          }
-          output += value;
-        });
-      } else if (op == "path_list_prepend"_s) {
-        apply_diff(name, [&value, path_sep](std::string& output) {
-          if (!output.empty()) {
-            output.insert(output.begin(), path_sep);
-          }
-          output.insert(0, value);
-        });
-      } else if (op == "cmake_list_append"_s) {
-        apply_diff(name, [&value](std::string& output) {
-          if (!output.empty()) {
-            output += ';';
-          }
-          output += value;
-        });
-      } else if (op == "cmake_list_prepend"_s) {
-        apply_diff(name, [&value](std::string& output) {
-          if (!output.empty()) {
-            output.insert(output.begin(), ';');
-          }
-          output.insert(0, value);
-        });
-      } else {
-        cmCTestLog(this->CTest, ERROR_MESSAGE,
-                   "Error: Unrecognized environment manipulation argument: "
-                     << op << std::endl);
-        err_occurred = true;
-        continue;
-      }
+      env_ok &= diff.ParseOperation(envmod);
     }
 
-    if (err_occurred) {
+    if (!env_ok) {
       return false;
     }
 
-    for (auto const& env_apply : env_application) {
-      if (env_apply.second) {
-        auto const env_update =
-          cmStrCat(env_apply.first, '=', *env_apply.second);
-        cmSystemTools::PutEnv(env_update);
-        envMeasurement << env_update << std::endl;
-      } else {
-        cmSystemTools::UnsetEnv(env_apply.first.c_str());
-        // Signify that this variable is being actively unset
-        envMeasurement << "#" << env_apply.first << "=" << std::endl;
-      }
-    }
+    diff.ApplyToCurrentEnv(&envMeasurement);
   }
 
   if (this->UseAllocatedResources) {

Source/cmSystemTools.cxx

@@ -16,6 +16,7 @@
 
 #include <cm/optional>
 #include <cmext/algorithm>
+#include <cmext/string_view>
 
 #include <cm3p/uv.h>
 
@@ -1563,6 +1564,126 @@ void cmSystemTools::AppendEnv(std::vector<std::string> const& env)
   }
 }
 
+bool cmSystemTools::EnvDiff::ParseOperation(const std::string& envmod)
+{
+  char path_sep = GetSystemPathlistSeparator();
+
+  auto apply_diff = [this](const std::string& name,
+                           std::function<void(std::string&)> const& apply) {
+    cm::optional<std::string> old_value = diff[name];
+    std::string output;
+    if (old_value) {
+      output = *old_value;
+    } else {
+      // This only works because the environment is actually modified when
+      // processing the ENVIRONMENT property in CTest and cmake -E env
+      // (`AppendEnv`). If either one ever just creates an environment block
+      // directly, that block will need to be queried for the subprocess'
+      // value instead.
+      const char* curval = cmSystemTools::GetEnv(name);
+      if (curval) {
+        output = curval;
+      }
+    }
+    apply(output);
+    diff[name] = output;
+  };
+
+  // Split on `=`
+  auto const eq_loc = envmod.find_first_of('=');
+  if (eq_loc == std::string::npos) {
+    cmSystemTools::Error(cmStrCat(
+      "Error: Missing `=` after the variable name in: ", envmod, '\n'));
+    return false;
+  }
+
+  auto const name = envmod.substr(0, eq_loc);
+
+  // Split value on `:`
+  auto const op_value_start = eq_loc + 1;
+  auto const colon_loc = envmod.find_first_of(':', op_value_start);
+  if (colon_loc == std::string::npos) {
+    cmSystemTools::Error(
+      cmStrCat("Error: Missing `:` after the operation in: ", envmod, '\n'));
+    return false;
+  }
+  auto const op = envmod.substr(op_value_start, colon_loc - op_value_start);
+
+  auto const value_start = colon_loc + 1;
+  auto const value = envmod.substr(value_start);
+
+  // Determine what to do with the operation.
+  if (op == "reset"_s) {
+    auto entry = diff.find(name);
+    if (entry != diff.end()) {
+      diff.erase(entry);
+    }
+  } else if (op == "set"_s) {
+    diff[name] = value;
+  } else if (op == "unset"_s) {
+    diff[name] = {};
+  } else if (op == "string_append"_s) {
+    apply_diff(name, [&value](std::string& output) { output += value; });
+  } else if (op == "string_prepend"_s) {
+    apply_diff(name,
+               [&value](std::string& output) { output.insert(0, value); });
+  } else if (op == "path_list_append"_s) {
+    apply_diff(name, [&value, path_sep](std::string& output) {
+      if (!output.empty()) {
+        output += path_sep;
+      }
+      output += value;
+    });
+  } else if (op == "path_list_prepend"_s) {
+    apply_diff(name, [&value, path_sep](std::string& output) {
+      if (!output.empty()) {
+        output.insert(output.begin(), path_sep);
+      }
+      output.insert(0, value);
+    });
+  } else if (op == "cmake_list_append"_s) {
+    apply_diff(name, [&value](std::string& output) {
+      if (!output.empty()) {
+        output += ';';
+      }
+      output += value;
+    });
+  } else if (op == "cmake_list_prepend"_s) {
+    apply_diff(name, [&value](std::string& output) {
+      if (!output.empty()) {
+        output.insert(output.begin(), ';');
+      }
+      output.insert(0, value);
+    });
+  } else {
+    cmSystemTools::Error(cmStrCat(
+      "Error: Unrecognized environment manipulation argument: ", op, '\n'));
+    return false;
+  }
+
+  return true;
+}
+
+void cmSystemTools::EnvDiff::ApplyToCurrentEnv(std::ostringstream* measurement)
+{
+  for (auto const& env_apply : diff) {
+    if (env_apply.second) {
+      auto const env_update =
+        cmStrCat(env_apply.first, '=', *env_apply.second);
+      cmSystemTools::PutEnv(env_update);
+      if (measurement) {
+        *measurement << env_update << std::endl;
+      }
+    } else {
+      cmSystemTools::UnsetEnv(env_apply.first.c_str());
+      if (measurement) {
+        // Signify that this variable is being actively unset
+        *measurement << '#' << env_apply.first << "=\n";
+      }
+    }
+  }
+}
+
 cmSystemTools::SaveRestoreEnvironment::SaveRestoreEnvironment()
 {
   this->Env = cmSystemTools::GetEnvironmentVariables();

Source/cmSystemTools.h

@@ -6,9 +6,12 @@
 
 #include <cstddef>
 #include <functional>
+#include <map>
+#include <sstream>
 #include <string>
 #include <vector>
 
+#include <cm/optional>
 #include <cm/string_view>
 
 #include "cmsys/Process.h"
@@ -377,6 +380,30 @@ public:
   /** Append multiple variables to the current environment. */
   static void AppendEnv(std::vector<std::string> const& env);
 
+  /**
+   * Helper class to represent an environment diff directly. This is to avoid
+   * repeated in-place environment modification (i.e. via setenv/putenv), which
+   * could be slow.
+   */
+  class EnvDiff
+  {
+  public:
+    /**
+     * Apply an ENVIRONMENT_MODIFICATION operation to this diff. Returns
+     * false and issues an error on parse failure.
+     */
+    bool ParseOperation(const std::string& envmod);
+
+    /**
+     * Apply this diff to the actual environment, optionally writing out the
+     * modifications to a CTest-compatible measurement stream.
+     */
+    void ApplyToCurrentEnv(std::ostringstream* measurement = nullptr);
+
+  private:
+    std::map<std::string, cm::optional<std::string>> diff;
+  };
+
   /** Helper class to save and restore the environment.
       Instantiate this class as an automatic variable on
       the stack. Its constructor saves a copy of the current