cmCTestRunTest: Implement the ENVIRONMENT test property with EnvDiff too

Going through the same internal API for both `ENVIRONMENT` and
`ENVIRONMENT_MODIFICATION` properties will make it easier to implement
checkpointing for `MYVAR=reset:` more efficiently if the need ever
presents itself.  It also makes the two-stage nature of the environment
mutation clearer in the code itself.

3 files changed, 34 insertions, 13 deletions

diff --git a/Source/CTest/cmCTestRunTest.cxx b/Source/CTest/cmCTestRunTest.cxx
index 11be132..5efe69f 100644
--- a/Source/CTest/cmCTestRunTest.cxx
+++ b/Source/CTest/cmCTestRunTest.cxx
@@ -784,16 +784,15 @@ bool cmCTestRunTest::ForkProcess(
   this->TestProcess->SetTimeout(timeout);
 
   cmSystemTools::SaveRestoreEnvironment sre;
-
   std::ostringstream envMeasurement;
+
+  // We split processing ENVIRONMENT and ENVIRONMENT_MODIFICATION into two
+  // phases to ensure that MYVAR=reset: in the latter phase resets to the
+  // former phase's settings, rather than to the original environment.
   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 in `EnvDiff::ParseOperation`.
-    cmSystemTools::AppendEnv(*environment);
-    for (auto const& var : *environment) {
-      envMeasurement << var << std::endl;
-    }
+    cmSystemTools::EnvDiff diff;
+    diff.AppendEnv(*environment);
+    diff.ApplyToCurrentEnv(&envMeasurement);
   }
 
   if (environment_modification && !environment_modification->empty()) {
diff --git a/Source/cmSystemTools.cxx b/Source/cmSystemTools.cxx
index c35c1e7..2c32f09 100644
--- a/Source/cmSystemTools.cxx
+++ b/Source/cmSystemTools.cxx
@@ -1564,6 +1564,24 @@ void cmSystemTools::AppendEnv(std::vector<std::string> const& env)
   }
 }
 
+void cmSystemTools::EnvDiff::AppendEnv(std::vector<std::string> const& env)
+{
+  for (std::string const& eit : env) {
+    this->PutEnv(eit);
+  }
+}
+
+void cmSystemTools::EnvDiff::PutEnv(const std::string& env)
+{
+  auto const eq_loc = env.find('=');
+  if (eq_loc != std::string::npos) {
+    std::string name = env.substr(0, eq_loc);
+    diff[name] = env.substr(eq_loc + 1);
+  } else {
+    diff[env] = {};
+  }
+}
+
 bool cmSystemTools::EnvDiff::ParseOperation(const std::string& envmod)
 {
   char path_sep = GetSystemPathlistSeparator();
@@ -1575,11 +1593,6 @@ bool cmSystemTools::EnvDiff::ParseOperation(const std::string& envmod)
     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;
diff --git a/Source/cmSystemTools.h b/Source/cmSystemTools.h
index 3c5c28c..e8c04b1 100644
--- a/Source/cmSystemTools.h
+++ b/Source/cmSystemTools.h
@@ -388,6 +388,15 @@ public:
   class EnvDiff
   {
   public:
+    /** Append multiple variables to the current environment diff */
+    void AppendEnv(std::vector<std::string> const& env);
+
+    /**
+     * Add a single variable (or remove if no = sign) to the current
+     * environment diff.
+     */
+    void PutEnv(const std::string& env);
+
     /**
      * Apply an ENVIRONMENT_MODIFICATION operation to this diff. Returns
      * false and issues an error on parse failure.