Add support for interrupt build result status

2 files changed, 152 insertions, 113 deletions

diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx
index 6f40a0b..99bdc8d 100644
--- a/mod/mod-build-result.cxx
+++ b/mod/mod-build-result.cxx
@@ -435,126 +435,164 @@ handle (request& rq, response&)
 
       if (auth)
       {
-        // Verify the result status/checksums.
+        // If the build is interrupted, then revert it to the original built
+        // state if this is a rebuild and delete it from the database
+        // otherwise.
         //
-        // Specifically, if the result status is skip, then it can only be in
-        // response to the soft rebuild task (all checksums are present in the
-        // build object) and the result checksums must match the build object
-        // checksums. On verification failure respond with the bad request
-        // HTTP code (400).
-        //
-        if (rqm.result.status == result_status::skip)
+        if (rqm.result.status == result_status::interrupt)
         {
-          if (!b->agent_checksum  ||
-              !b->worker_checksum ||
-              !b->dependency_checksum)
-            throw invalid_request (400, "unexpected skip result status");
-
-          // Can only be absent for initial build, in which case the checksums
-          // are also absent and we would end up with the above 400 response.
-          //
-          assert (b->status);
-
-          // Verify that the result checksum matches the build checksum and
-          // throw invalid_request(400) if that's not the case.
-          //
-          auto verify = [] (const string& build_checksum,
-                            const optional<string>& result_checksum,
-                            const char* what)
+          if (b->status) // Is this a rebuild?
           {
-            if (!result_checksum)
-              throw invalid_request (
-                400,
-                string (what) +
-                " checksum is expected for skip result status");
-
-            if (*result_checksum != build_checksum)
-              throw invalid_request (
-                400,
-                string (what) + " checksum '" + build_checksum  +
-                "' is expected instead of '" + *result_checksum +
-                "' for skip result status");
-          };
-
-          verify (*b->agent_checksum, rqm.agent_checksum, "agent");
-
-          verify (*b->worker_checksum,
-                  rqm.result.worker_checksum,
-                  "worker");
-
-          verify (*b->dependency_checksum,
-                  rqm.result.dependency_checksum,
-                  "dependency");
-        }
+            b->state = build_state::built;
 
-        unforced = b->force == force_state::unforced;
+            // Keep the force rebuild indication. Note that the forcing state
+            // is only valid for the building state.
+            //
+            if (b->force == force_state::forcing)
+              b->force = force_state::forced;
 
-        // Don't send email to the build-email address for the
-        // success-to-success status change, unless the build was forced.
-        //
-        build_notify = !(rqm.result.status == result_status::success &&
-                         b->status                                   &&
-                         *b->status == rqm.result.status             &&
-                         unforced);
+            // Cleanup the interactive build login information.
+            //
+            b->interactive = nullopt;
 
-        b->state  = build_state::built;
-        b->force  = force_state::unforced;
+            // Cleanup the authentication data.
+            //
+            b->agent_fingerprint = nullopt;
+            b->agent_challenge = nullopt;
 
-        // Cleanup the interactive build login information.
-        //
-        b->interactive = nullopt;
+            // Note that we are unable to restore the pre-rebuild timestamp
+            // since it has been overwritten when the build task was issued.
+            // That, however, feels ok and we just keep it unchanged.
 
-        // Cleanup the authentication data.
-        //
-        b->agent_fingerprint = nullopt;
-        b->agent_challenge = nullopt;
+            build_db_->update (b);
+          }
+          else
+            build_db_->erase (b);
+        }
+        else
+        {
+          // Verify the result status/checksums.
+          //
+          // Specifically, if the result status is skip, then it can only be
+          // in response to the soft rebuild task (all checksums are present
+          // in the build object) and the result checksums must match the
+          // build object checksums. On verification failure respond with the
+          // bad request HTTP code (400).
+          //
+          if (rqm.result.status == result_status::skip)
+          {
+            if (!b->agent_checksum  ||
+                !b->worker_checksum ||
+                !b->dependency_checksum)
+              throw invalid_request (400, "unexpected skip result status");
+
+            // Can only be absent for initial build, in which case the
+            // checksums are also absent and we would end up with the above
+            // 400 response.
+            //
+            assert (b->status);
+
+            // Verify that the result checksum matches the build checksum and
+            // throw invalid_request(400) if that's not the case.
+            //
+            auto verify = [] (const string& build_checksum,
+                              const optional<string>& result_checksum,
+                              const char* what)
+            {
+              if (!result_checksum)
+                throw invalid_request (
+                  400,
+                  string (what) +
+                  " checksum is expected for skip result status");
+
+              if (*result_checksum != build_checksum)
+                throw invalid_request (
+                  400,
+                  string (what) + " checksum '" + build_checksum  +
+                  "' is expected instead of '" + *result_checksum +
+                  "' for skip result status");
+            };
+
+            verify (*b->agent_checksum, rqm.agent_checksum, "agent");
+
+            verify (*b->worker_checksum,
+                    rqm.result.worker_checksum,
+                    "worker");
+
+            verify (*b->dependency_checksum,
+                    rqm.result.dependency_checksum,
+                    "dependency");
+          }
 
-        b->timestamp = system_clock::now ();
-        b->soft_timestamp = b->timestamp;
+          unforced = b->force == force_state::unforced;
 
-        // If the result status is other than skip, then save the status,
-        // results, and checksums and update the hard timestamp.
-        //
-        if (rqm.result.status != result_status::skip)
-        {
-          b->status = rqm.result.status;
-          b->hard_timestamp = b->soft_timestamp;
+          // Don't send email to the build-email address for the
+          // success-to-success status change, unless the build was forced.
+          //
+          build_notify = !(rqm.result.status == result_status::success &&
+                           b->status                                   &&
+                           *b->status == rqm.result.status             &&
+                           unforced);
+
+          b->state  = build_state::built;
+          b->force  = force_state::unforced;
 
-          // Mark the section as loaded, so results are updated.
+          // Cleanup the interactive build login information.
           //
-          b->results_section.load ();
-          b->results = move (rqm.result.results);
+          b->interactive = nullopt;
 
-          // Save the checksums.
+          // Cleanup the authentication data.
           //
-          b->agent_checksum      = move (rqm.agent_checksum);
-          b->worker_checksum     = move (rqm.result.worker_checksum);
-          b->dependency_checksum = move (rqm.result.dependency_checksum);
-        }
+          b->agent_fingerprint = nullopt;
+          b->agent_challenge = nullopt;
 
-        build_db_->update (b);
+          b->timestamp = system_clock::now ();
+          b->soft_timestamp = b->timestamp;
 
-        // Don't send the build notification email if the task result is
-        // `skip`, the configuration is hidden, or is now excluded by the
-        // package.
-        //
-        if (rqm.result.status != result_status::skip && belongs (*tc, "all"))
-        {
-          shared_ptr<build_package> p (
-            build_db_->load<build_package> (b->id.package));
+          // If the result status is other than skip, then save the status,
+          // results, and checksums and update the hard timestamp.
+          //
+          if (rqm.result.status != result_status::skip)
+          {
+            b->status = rqm.result.status;
+            b->hard_timestamp = b->soft_timestamp;
+
+            // Mark the section as loaded, so results are updated.
+            //
+            b->results_section.load ();
+            b->results = move (rqm.result.results);
+
+            // Save the checksums.
+            //
+            b->agent_checksum      = move (rqm.agent_checksum);
+            b->worker_checksum     = move (rqm.result.worker_checksum);
+            b->dependency_checksum = move (rqm.result.dependency_checksum);
+          }
 
-          // The package configuration should be present (see mod-builds.cxx
-          // for details) but if it is not, let's log the warning.
+          build_db_->update (b);
+
+          // Don't send the build notification email if the task result is
+          // `skip`, the configuration is hidden, or is now excluded by the
+          // package.
           //
-          if (const build_package_config* pc = find (b->package_config_name,
-                                                     p->configs))
+          if (rqm.result.status != result_status::skip && belongs (*tc, "all"))
           {
-            if (!exclude (*pc, p->builds, p->constraints, *tc))
-              bld = move (b);
+            shared_ptr<build_package> p (
+              build_db_->load<build_package> (b->id.package));
+
+            // The package configuration should be present (see mod-builds.cxx
+            // for details) but if it is not, let's log the warning.
+            //
+            if (const build_package_config* pc = find (b->package_config_name,
+                                                       p->configs))
+            {
+              if (!exclude (*pc, p->builds, p->constraints, *tc))
+                bld = move (b);
+            }
+            else
+              warn << "cannot find configuration '" << b->package_config_name
+                   << "' for package " << p->id.name << '/' << p->version;
           }
-          else
-            warn << "cannot find configuration '" << b->package_config_name
-                 << "' for package " << p->id.name << '/' << p->version;
         }
       }
     }
diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx
index bd5034b..9ce2520 100644
--- a/mod/mod-build-task.cxx
+++ b/mod/mod-build-task.cxx
@@ -720,25 +720,19 @@ handle (request& rq, response& rs)
         b.hard_timestamp <= hard_rebuild_expiration;
       };
 
-      // Convert a build to the hard rebuild, resetting the agent checksum and
-      // dropping the previous build task result.
+      // Convert a build to the hard rebuild, resetting the agent checksum.
       //
       // Note that since the checksums are hierarchical, the agent checksum
       // reset will trigger resets of the "subordinate" checksums up to the
       // dependency checksum and so the package will be rebuilt.
       //
-      // Also note that there is no sense to keep the build task result since we
-      // don't accept the skip result for the hard rebuild task. We, however,
-      // keep the status intact (see below for the reasoning).
+      // Also note that we keep the previous build task result and status
+      // intact since we may still need to revert the build into the built
+      // state if the task execution is interrupted.
       //
       auto convert_to_hard = [] (const shared_ptr<build>& b)
       {
         b->agent_checksum = nullopt;
-
-        // Mark the section as loaded, so results are updated.
-        //
-        b->results_section.load ();
-        b->results.clear ();
       };
 
       // Return SHA256 checksum of the controller logic and the configuration
@@ -1051,8 +1045,9 @@ handle (request& rq, response& rs)
                   // Note that in both cases we keep the status intact to be
                   // able to compare it with the final one in the result
                   // request handling in order to decide if to send the
-                  // email. The same is true for the forced flag (in the sense
-                  // that we don't set the force state to unforced).
+                  // notification email or to revert it to the built state if
+                  // interrupted. The same is true for the forced flag (in
+                  // the sense that we don't set the force state to unforced).
                   //
                   assert (b->state == build_state::building);
 
@@ -1139,7 +1134,13 @@ handle (request& rq, response& rs)
           if (x->status != y->status)
             return x->status > y->status;     // Larger status goes first.
 
-          return x->timestamp < y->timestamp; // Older goes first.
+          // Older build completion goes first.
+          //
+          // Note that a completed build can have the state change timestamp
+          // (timestamp member) newer than the completion timestamp
+          // (soft_timestamp member) if the build was interrupted.
+          //
+          return x->soft_timestamp < y->soft_timestamp;
         };
 
         sort (rebuilds.begin (), rebuilds.end (), cmp);