Update build_unloaded_load()

1 files changed, 46 insertions, 233 deletions

diff --git a/mod/mod-ci-github.cxx b/mod/mod-ci-github.cxx
index b289671..fb4b29d 100644
--- a/mod/mod-ci-github.cxx
+++ b/mod/mod-ci-github.cxx
@@ -389,10 +389,9 @@ namespace brep
     }
   }
 
-  // Let's capitalize the synthetic check run names to make them easier to
-  // distinguish from the regular ones.
+  // Let's capitalize the synthetic conclusion check run name to make it
+  // easier to distinguish from the regular ones.
   //
-  static string merge_check_run_name ("MERGE-COMMIT");
   static string conclusion_check_run_name ("CONCLUSION");
 
   // Return the colored circle corresponding to a result_status.
@@ -531,6 +530,7 @@ namespace brep
                      iat->expires_at,
                      cs.installation.id,
                      move (cs.repository.node_id),
+                     move (cs.repository.clone_url),
                      kind, false /* pre_check */, re_requested,
                      move (check_sha),
                      move (cs.check_suite.head_sha) /* report_sha */);
@@ -777,12 +777,12 @@ namespace brep
                      iat->expires_at,
                      pr.installation.id,
                      move (pr.repository.node_id),
+                     move (pr.repository.clone_url),
                      pr.pull_request.node_id,
                      kind, true /* pre_check */, false /* re_request */,
                      move (check_sha),
                      move (pr.pull_request.head_sha) /* report_sha */,
-                     pr.pull_request.number,
-                     move (pr.repository.clone_url));
+                     pr.pull_request.number);
 
     // Create an unloaded CI tenant for the pre-check phase (during which we
     // wait for the PR's merge commit and behindness to become available).
@@ -1018,40 +1018,6 @@ namespace brep
     if (iat == nullptr)
       return nullptr; // Try again on the next call.
 
-    auto make_iat_updater = [&new_iat, &error] ()
-    {
-      function<optional<string> (const tenant_service&)> r;
-
-      if (new_iat)
-      {
-        r = [&error,
-             iat = move (new_iat)] (const tenant_service& ts)
-          -> optional<string>
-        {
-          // NOTE: this lambda may be called repeatedly (e.g., due to
-          // transaction being aborted) and so should not move out of its
-          // captures.
-
-          service_data sd;
-          try
-          {
-            sd = service_data (*ts.data);
-          }
-          catch (const invalid_argument& e)
-          {
-            error << "failed to parse service data: " << e;
-            return nullopt;
-          }
-
-          sd.installation_access = *iat;
-
-          return sd.json ();
-        };
-      }
-
-      return r;
-    };
-
     // Create a synthetic check run with an in-progress state. Return the
     // check run on success or nullopt on failure.
     //
@@ -1113,223 +1079,73 @@ namespace brep
         return nullopt;
     };
 
-    // Synthetic merge check run node ID. Empty until created on the first
-    // call or retrieved from service data on subsequent calls.
-    //
-    string merge_node_id;
+    // Note that there is a window between receipt of a check_suite or
+    // pull_request event and the first bot/worker asking for a task, which
+    // could be substantial. We could probably (also) try to (re)create the
+    // conclusion checkrun in the webhook handler. @@ Maybe/later.
 
-    // True if this is the first call (or the merge commit couldn't be created
-    // on the first call, in which case we just re-try by treating it as a
-    // first call).
+    // (Re)create the synthetic conclusion check run first in order to convert
+    // a potentially completed check suite to building as early as possible.
     //
-    bool first (!sd.merge_node_id);
+    string conclusion_node_id; // Conclusion check run node ID.
 
-    // If this is the first call, (re)create the synthetic merge check run as
-    // soon as possible to make sure the previous check suite, if any, is no
-    // longer completed.
-    //
-    // Note that there is still a window between receipt of the pull_request
-    // event and the first bot/worker asking for a task, which could be
-    // substantial. We could probably (also) try to (re)create the merge
-    // checkrun in the webhook. @@ Maybe/later.
-    //
-    if (first)
+    if (auto cr = create_synthetic_cr (conclusion_check_run_name))
     {
-      if (auto cr = create_synthetic_cr (merge_check_run_name))
-      {
-        l3 ([&]{trace << "created check_run { " << *cr << " }";});
+      l3 ([&]{trace << "created check_run { " << *cr << " }";});
 
-        merge_node_id = move (*cr->node_id);
-      }
-      else
-        return make_iat_updater (); // Try again on the next call.
+      conclusion_node_id = move (*cr->node_id);
     }
-    else
-      merge_node_id = *sd.merge_node_id;
 
-    // @@@ TMP: move/remove.
-    //
-#if 0
-    // Start/check PR mergeability.
+    // Load the CI tenant if the conclusion check run was created.
     //
-    optional<gq_pr_pre_check> mc ( // Merge commit.
-      gq_pull_request_pre_check_info (error, iat->token, *sd.pr_node_id));
-
-    if (!mc || mc->merge_commit_sha.empty ())
+    if (!conclusion_node_id.empty ())
     {
-      if (!mc) // No merge commit yet.
-      {
-        // If this is a subsequent notification and there is no merge commit,
-        // then there is nothing to do.
-        //
-        if (!first)
-          return make_iat_updater ();
+      string ru; // Repository URL.
 
-        // Fall through to update service data.
-      }
-      else // Not mergeable.
+      // CI the merge test commit for remote PRs and the head commit for
+      // everything else (branch push or local PRs).
+      //
+      if (sd.kind == service_data::remote)
       {
-        // If the commit is not mergeable, cancel the CI request and fail the
-        // merge check run.
+        // E.g. #pull/28/merge@1b6c9a361086ed93e6f1e67189e82d52de91c49b
         //
-        // Note that it feels like in this case we don't really need to create a
-        // failed synthetic conclusion check run since the PR cannot be merged
-        // anyway.
-
-        if (auto cr = update_synthetic_cr (
-              merge_node_id,
-              merge_check_run_name,
-              result_status::error,
-              "GitHub is unable to create test merge commit"))
-        {
-          l3 ([&]{trace << "updated check_run { " << *cr << " }";});
-
-          // Cancel the CI request.
-          //
-          // Ignore failure because this CI request may have been cancelled
-          // elsewhere due to an update to the PR base or head branches.
-          //
-          if (!cancel (error, warn, verb_ ? &trace : nullptr,
-                       *build_db_, ts.type, ts.id))
-          {
-            l3 ([&]{trace << "CI for PR " << *sd.pr_node_id
-                          << " already cancelled";});
-          }
-
-          return nullptr; // No need to update service data in this case.
-        }
-        else
-        {
-          // Don't cancel the CI request if the merge check run update failed
-          // so that we can try again on the next call.
+        ru = sd.repository_clone_url + "#pull/" + to_string (*sd.pr_number) +
+             "/merge@" + sd.check_sha;
+      }
+      else
+        ru = sd.repository_clone_url + '#' + sd.check_sha;
 
-          if (!first)
-            return make_iat_updater ();
+      repository_location rl (move (ru), repository_type::git);
 
-          // Fall through to update service data.
-        }
-      }
+      optional<start_result> r (load (error, warn, verb_ ? &trace : nullptr,
+                                      *build_db_, move (ts), move (rl)));
 
-      // This is a first notification, so record the merge check run in the
-      // service data.
-      //
-      return [&error,
-              iat = move (new_iat),
-              mni = move (merge_node_id)] (const tenant_service& ts)
-        -> optional<string>
+      if (!r || r->status != 200)
       {
-        // NOTE: this lambda may be called repeatedly (e.g., due to
-        // transaction being aborted) and so should not move out of its
-        // captures.
-
-        service_data sd;
-        try
-        {
-          sd = service_data (*ts.data);
-        }
-        catch (const invalid_argument& e)
+        if (auto cr = update_synthetic_cr (conclusion_node_id,
+                                           conclusion_check_run_name,
+                                           result_status::error,
+                                           to_check_run_summary (r)))
         {
-          error << "failed to parse service data: " << e;
-          return nullopt;
+          l3 ([&]{trace << "updated check_run { " << *cr << " }";});
         }
-
-        if (iat)
-          sd.installation_access = *iat;
-
-        sd.merge_node_id = mni;
-
-        return sd.json ();
-      };
-    }
-#endif
-
-    // If we are here, then it means we have a merge commit that we can load.
-    //
-    // Note that this can still be the first call (first=true).
-    //
-
-    // As a first step, (re)create the synthetic conclusion check run and then
-    // change the merge check run state to success. Do it in this order so
-    // that the check suite does not become completed.
-
-    // Synthetic conclusion check run node ID. Empty until created on the
-    // "second" call or retrieved from service data on subsequent calls.
-    //
-    string conclusion_node_id;
-
-    // True if this is the first call after the merge commit became available,
-    // which we will call the "second" call (or we couldn't create the
-    // conclusion check run on the first such call, in which case we just
-    // re-try by treating it as a "second" call).
-    //
-    bool second (!sd.conclusion_node_id);
-
-    if (second)
-    {
-      if (auto cr = create_synthetic_cr (conclusion_check_run_name))
-      {
-        l3 ([&]{trace << "created check_run { " << *cr << " }";});
-
-        conclusion_node_id = move (*cr->node_id);
-      }
-    }
-    else
-      conclusion_node_id = *sd.conclusion_node_id;
-
-    if (!conclusion_node_id.empty ()) // Conclusion check run was created.
-    {
-      // Update merge check run to successful.
-      //
-      if (auto cr = update_synthetic_cr (merge_node_id,
-                                         merge_check_run_name,
-                                         result_status::success,
-                                         "GitHub created test merge commit"))
-      {
-        l3 ([&]{trace << "updated check_run { " << *cr << " }";});
-
-        // Load the CI request.
-        //
-        // Example repository URL fragment:
-        //
-        //   #pull/28/merge@1b6c9a361086ed93e6f1e67189e82d52de91c49b
-        //
-        repository_location rl (*sd.pr_repository_clone_url + "#pull/" +
-                                to_string (*sd.pr_number) + "/merge@" +
-                                sd.check_sha, // @@ TODO Not for local PRs
-                                repository_type::git);
-
-        optional<start_result> r (load (error, warn, verb_ ? &trace : nullptr,
-                                        *build_db_, move (ts), rl));
-
-        if (!r || r->status != 200)
+        else
         {
-          if (auto cr = update_synthetic_cr (conclusion_node_id,
-                                             conclusion_check_run_name,
-                                             result_status::error,
-                                             to_check_run_summary (r)))
-          {
-            l3 ([&]{trace << "updated check_run { " << *cr << " }";});
-          }
-          else
-          {
-            // Nothing really we can do in this case since we will not receive
-            // any further notifications.
-          }
+          // Nothing really we can do in this case since we will not receive
+          // any further notifications. Log the error as a last resort.
 
-          return nullptr; // No need to update service data in this case.
+          error << "failed to load CI tenant " << ts.id;
         }
-      }
-      else
-      {
-        // Don't load the CI request if the merge check run update failed so
-        // that we can try again on the next call.
+
+        return nullptr; // No need to update service data in this case.
       }
     }
+    else if (!new_iat)
+      return nullptr; // Nothing to save (but retry on next call).
 
     return [&error,
             iat = move (new_iat),
-            mni = (first ? move (merge_node_id) : string ()),
-            cni = (second ? move (conclusion_node_id) : string ())]
+            cni = move (conclusion_node_id)]
       (const tenant_service& ts) -> optional<string>
     {
       // NOTE: this lambda may be called repeatedly (e.g., due to
@@ -1350,9 +1166,6 @@ namespace brep
       if (iat)
         sd.installation_access = *iat;
 
-      if (!mni.empty ())
-        sd.merge_node_id = mni;
-
       if (!cni.empty ())
         sd.conclusion_node_id = cni;