Allow to enforce rebuild for building state

7 files changed, 113 insertions, 53 deletions

diff --git a/libbrep/build.cxx b/libbrep/build.cxx
index ece2fd7..2391165 100644
--- a/libbrep/build.cxx
+++ b/libbrep/build.cxx
@@ -30,6 +30,30 @@ namespace brep
     else throw invalid_argument ("invalid build state '" + s + "'");
   }
 
+  // force_state
+  //
+  string
+  to_string (force_state s)
+  {
+    switch (s)
+    {
+    case force_state::unforced: return "unforced";
+    case force_state::forcing:  return "forcing";
+    case force_state::forced:   return "forced";
+    }
+
+    return string (); // Should never reach.
+  }
+
+  force_state
+  to_force_state (const string& s)
+  {
+         if (s == "unforced") return force_state::unforced;
+    else if (s == "forcing")  return force_state::forcing;
+    else if (s == "forced")   return force_state::forced;
+    else throw invalid_argument ("invalid force state '" + s + "'");
+  }
+
   // build
   //
   build::
@@ -46,7 +70,7 @@ namespace brep
         toolchain_version (move (tvr)),
         state (build_state::building),
         timestamp (timestamp_type::clock::now ()),
-        forced (false),
+        force (force_state::unforced),
         machine (move (mnm)),
         machine_summary (move (msm)),
         target (move (trg))
diff --git a/libbrep/build.hxx b/libbrep/build.hxx
index 9258bb3..afa96ed 100644
--- a/libbrep/build.hxx
+++ b/libbrep/build.hxx
@@ -82,6 +82,28 @@ namespace brep
     to(to_string (?))                         \
     from(brep::to_build_state (?))
 
+  // force_state
+  //
+  enum class force_state: std::uint8_t
+  {
+    unforced,
+    forcing,  // Rebuild is forced while being in the building state.
+    forced    // Rebuild is forced while being in the built state.
+  };
+
+  string
+  to_string (force_state);
+
+  force_state
+  to_force_state (const string&); // May throw invalid_argument.
+
+  inline ostream&
+  operator<< (ostream& os, force_state s) {return os << to_string (s);}
+
+  #pragma db map type(force_state) as(string) \
+    to(to_string (?))                         \
+    from(brep::to_force_state (?))
+
   // result_status
   //
   using bbot::result_status;
@@ -118,7 +140,7 @@ namespace brep
     using timestamp_type = brep::timestamp;
 
     // Create the build object with the building state, non-existent status,
-    // the timestamp set to now and the forced flag set to false.
+    // the timestamp set to now and the force state set to unforced.
     //
     build (string package_name, version package_version,
            string configuration,
@@ -140,9 +162,7 @@ namespace brep
     //
     timestamp_type timestamp;
 
-    // True if the package rebuild has been forced.
-    //
-    bool forced;
+    force_state force;
 
     // Must present for the built state, may present for the building state.
     //
diff --git a/libbrep/build.xml b/libbrep/build.xml
index 72cbd5f..7466c97 100644
--- a/libbrep/build.xml
+++ b/libbrep/build.xml
@@ -18,7 +18,7 @@
       <column name="toolchain_version_release" type="TEXT" null="true"/>
       <column name="state" type="TEXT" null="false"/>
       <column name="timestamp" type="BIGINT" null="false"/>
-      <column name="forced" type="BOOLEAN" null="false"/>
+      <column name="force" type="TEXT" null="false"/>
       <column name="status" type="TEXT" null="true"/>
       <column name="machine" type="TEXT" null="true"/>
       <column name="machine_summary" type="TEXT" null="true"/>
diff --git a/mod/mod-build-force.cxx b/mod/mod-build-force.cxx
index 9389566..c0c3d2d 100644
--- a/mod/mod-build-force.cxx
+++ b/mod/mod-build-force.cxx
@@ -169,12 +169,17 @@ handle (request& rq, response& rs)
     // Respond with 409 (conflict) if the package configuration is in
     // inappropriate state for being rebuilt.
     //
-    else if (b->state != build_state::built)
+    else if (b->state != build_state::built &&
+             b->state != build_state::building)
       throw invalid_request (409, "state is " + to_string (b->state));
 
-    if (!b->forced)
+    force_state force (b->state == build_state::built
+                       ? force_state::forced
+                       : force_state::forcing);
+
+    if (b->force != force)
     {
-      b->forced = true;
+      b->force = force;
       build_db_->update (b);
 
       l1 ([&]{trace << "force rebuild for "
diff --git a/mod/mod-build-result.cxx b/mod/mod-build-result.cxx
index d78c45f..f2b4ffd 100644
--- a/mod/mod-build-result.cxx
+++ b/mod/mod-build-result.cxx
@@ -248,13 +248,14 @@ handle (request& rq, response&)
       // build was forced.
       //
       notify = !(rqm.result.status == result_status::success &&
-                 b->status && *b->status == rqm.result.status && !b->forced);
+                 b->status && *b->status == rqm.result.status &&
+                 b->force == force_state::unforced);
 
       prev_status = move (b->status);
 
-      b->state = build_state::built;
+      b->state  = build_state::built;
       b->status = rqm.result.status;
-      b->forced = false;
+      b->force  = force_state::unforced;
 
       // Mark the section as loaded, so results are updated.
       //
diff --git a/mod/mod-build-task.cxx b/mod/mod-build-task.cxx
index 911fb9b..e47ae60 100644
--- a/mod/mod-build-task.cxx
+++ b/mod/mod-build-task.cxx
@@ -149,7 +149,7 @@ handle (request& rq, response& rs)
   // collect the list of the built package configurations which it's time to
   // rebuilt. So if no unbuilt package is found, we will pickup one to
   // rebuild. The rebuild preference is given in the following order: the
-  // greater force flag, the greater overall status, the lower timestamp.
+  // greater force state, the greater overall status, the lower timestamp.
   //
   if (!cfg_machines.empty ())
   {
@@ -212,15 +212,18 @@ handle (request& rq, response& rs)
         expiration (timeout).time_since_epoch ()).count ();
     };
 
-    uint64_t build_expiration_ns (
+    uint64_t normal_result_expiration_ns (
       expiration_ns (options_->build_result_timeout ()));
 
-    timestamp forced_rebuild_expiration (
-      expiration (options_->build_forced_rebuild_timeout ()));
+    uint64_t forced_result_expiration_ns (
+      expiration_ns (options_->build_forced_rebuild_timeout ()));
 
     timestamp normal_rebuild_expiration (
       expiration (options_->build_normal_rebuild_timeout ()));
 
+    timestamp forced_rebuild_expiration (
+      expiration (options_->build_forced_rebuild_timeout ()));
+
     // Convert butl::standard_version type to brep::version.
     //
     brep::version toolchain_version (tqm.toolchain_version.string ());
@@ -309,7 +312,10 @@ handle (request& rq, response& rs)
 
       (bld_query::state == "built" ||
        (bld_query::state == "building" &&
-        bld_query::timestamp > build_expiration_ns)));
+        ((bld_query::force == "forcing" &&
+          bld_query::timestamp > forced_result_expiration_ns) ||
+         (bld_query::force != "forcing" && // Unforced or forced.
+          bld_query::timestamp > normal_result_expiration_ns)))));
 
     connection_ptr bld_conn (build_db_->connection ());
 
@@ -370,11 +376,15 @@ handle (request& rq, response& rs)
             assert (j != configs.end ());
             configs.erase (j);
 
-            if (i->state == build_state::built &&
-                i->timestamp <= (i->forced
-                                 ? forced_rebuild_expiration
-                                 : normal_rebuild_expiration))
-              rebuilds.emplace_back (i.load ());
+            if (i->state == build_state::built)
+            {
+              assert (i->force != force_state::forcing);
+
+              if (i->timestamp <= (i->force == force_state::forced
+                                   ? forced_rebuild_expiration
+                                   : normal_rebuild_expiration))
+                rebuilds.emplace_back (i.load ());
+            }
           }
 
           if (!configs.empty ())
@@ -404,31 +414,29 @@ handle (request& rq, response& rs)
             else
             {
               // The package configuration can be in the building or unbuilt
-              // state, so the forced flag is false and the status is absent,
-              // unless in the building state (in which case they may or may
-              // not be set/exist), and there are no results.
+              // state, and there are no results.
               //
-              // Note that in the building state the status can be present if
-              // the rebuild task have been issued. 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
-              // notification email. The same is true for the forced flag. We
-              // just assert that if the force flag is set, then the status
-              // exists.
+              // 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 notification email.
+              // The same is true for the forced flag (in the sense that we
+              // don't set the force state to unforced).
               //
               // Load the section to assert the above statement.
               //
               build_db_->load (*b, b->results_section);
 
-              assert (b->state != build_state::built &&
+              assert (b->state != build_state::built && b->results.empty ());
 
-                      ((!b->forced && !b->status) ||
-                       (b->state == build_state::building &&
-                        (!b->forced || b->status))) &&
+              b->state = build_state::building;
 
-                      b->results.empty ());
+              // Switch the force state not to reissue the task after the
+              // forced rebuild timeout. Note that the result handler will
+              // still recognize that the rebuild was forced.
+              //
+              if (b->force == force_state::forcing)
+                b->force = force_state::forced;
 
-              b->state = build_state::building;
               b->toolchain_name = move (tqm.toolchain_name);
               b->machine = mh.name;
               b->machine_summary = move (mh.summary);
@@ -476,15 +484,15 @@ handle (request& rq, response& rs)
       // Sort the package configuration rebuild list with the following sort
       // priority:
       //
-      // 1: forced flag
+      // 1: force state
       // 2: overall status
       // 3: timestamp (less is preferred)
       //
       auto cmp = [] (const shared_ptr<build>& x,
                      const shared_ptr<build>& y) -> bool
       {
-        if (x->forced != y->forced)
-          return x->forced > y->forced;     // Forced goes first.
+        if (x->force != y->force)
+          return x->force > y->force;       // Forced goes first.
 
         assert (x->status && y->status);    // Both built.
 
@@ -502,7 +510,7 @@ handle (request& rq, response& rs)
       // anymore (as we have committed the database transactions that were
       // used to collect this data) so we recheck. If we find one that matches
       // then put it into the building state, refresh the timestamp and
-      // update. Note that we don't amend the status and the force flag to
+      // update. Note that we don't amend the status and the force state to
       // have them available in the result request handling (see above).
       //
       for (auto& b: rebuilds)
@@ -514,7 +522,7 @@ handle (request& rq, response& rs)
           b = build_db_->find<build> (b->id);
 
           if (b != nullptr && b->state == build_state::built &&
-              b->timestamp <= (b->forced
+              b->timestamp <= (b->force == force_state::forced
                                ? forced_rebuild_expiration
                                : normal_rebuild_expiration))
           {
diff --git a/mod/mod-builds.cxx b/mod/mod-builds.cxx
index c831359..e62e1e5 100644
--- a/mod/mod-builds.cxx
+++ b/mod/mod-builds.cxx
@@ -162,7 +162,7 @@ build_query (const C& configs, const brep::params::builds& params)
     if (rs != "*")
     {
       if (rs == "pending")
-        q = q && query::forced;
+        q = q && query::force != "unforced";
       else if (rs == "building")
         q = q && query::state == "building";
       else
@@ -382,7 +382,7 @@ handle (request& rq, response& rs)
       <<         SPAN(CLASS="value");
 
     if (b.state == build_state::building)
-      s << "building";
+      s << "building | ";
     else
     {
       assert (b.state == build_state::built);
@@ -423,16 +423,18 @@ handle (request& rq, response& rs)
           << ~A
           << " | ";
       }
-
-      if (b.forced)
-        s << "pending";
-      else
-        s << A
-          <<   HREF << force_rebuild_url (host, root, b) << ~HREF
-          <<   "rebuild"
-          << ~A;
     }
 
+    if (b.force == (b.state == build_state::building
+                    ? force_state::forcing
+                    : force_state::forced))
+      s << "pending";
+    else
+      s << A
+        <<   HREF << force_rebuild_url (host, root, b) << ~HREF
+        <<   "rebuild"
+        << ~A;
+
     s <<         ~SPAN
       <<       ~TD
       <<     ~TR