Add support for soft and hard rebuilds

4 files changed, 263 insertions, 29 deletions

diff --git a/bbot/agent/agent.cxx b/bbot/agent/agent.cxx
index cfff5e4..cfd1e7d 100644
--- a/bbot/agent/agent.cxx
+++ b/bbot/agent/agent.cxx
@@ -803,7 +803,9 @@ try
     tm.name,
     tm.version,
     result_status::abort,
-    operation_results {}};
+    operation_results {},
+    nullopt /* worker_checksum */,
+    nullopt /* dependency_checksum */};
 
   if (ops.fake_build ())
     return r;
@@ -1087,6 +1089,8 @@ handle_signal (int sig)
   }
 }
 
+static const string agent_checksum ("1"); // Logic version.
+
 int
 main (int argc, char* argv[])
 try
@@ -1296,6 +1300,8 @@ try
   // 4. If a build task is returned, do it, upload the result, and go to #1
   //    (immediately).
   //
+  // NOTE: consider updating agent_checksum if making any logic changes.
+  //
   auto rand_sleep = [g = std::mt19937 (std::random_device {} ())] () mutable
   {
     return std::uniform_int_distribution<unsigned int> (50, 60) (g);
@@ -1365,6 +1371,7 @@ try
         "fake-session", // Dummy session.
         nullopt,        // No challenge.
         url,            // Empty result URL.
+        agent_checksum,
         move (t)};
 
       url = "http://example.org";
@@ -1515,6 +1522,15 @@ try
     if (!ops.trust ().empty ())
       t.trust = ops.trust ();
 
+    // Reset the worker checksum if the task's agent checksum doesn't match
+    // the current one.
+    //
+    // Note that since the checksums are hierarchical, such reset will trigger
+    // resets of the "subordinate" checksums (dependency checksum, etc).
+    //
+    if (!tr.agent_checksum || *tr.agent_checksum != agent_checksum)
+      t.worker_checksum = nullopt;
+
     const dir_path& d (); // The -<toolchain> directory.
     const bootstrapped_machine_manifest& m ();
 
@@ -1562,7 +1578,10 @@ try
 
     // Upload the result.
     //
-    result_request_manifest rq {tr.session, move (challenge), move (r)};
+    result_request_manifest rq {tr.session,
+                                move (challenge),
+                                agent_checksum,
+                                move (r)};
     {
       const string& u (*tr.result_url);
 
diff --git a/bbot/worker/worker.cxx b/bbot/worker/worker.cxx
index 250d0d1..4688373 100644
--- a/bbot/worker/worker.cxx
+++ b/bbot/worker/worker.cxx
@@ -311,6 +311,10 @@ using regexes = vector<regex>;
 // expressions and return the warning result status (instead of success) in
 // case of a match. Save the executed command into last_cmd.
 //
+// Redirect stdout to stderr if the out argument is NULL. Otherwise, save the
+// process output into the referenced variable. Note: currently assumes that
+// the output will always fit into the pipe buffer.
+//
 // If bkp_step is present and is equal to the command step, then prior to
 // running this command ask the user if to continue or abort the task
 // execution. If bkp_status is present, then ask for that if the command
@@ -323,7 +327,7 @@ template <typename... A>
 static result_status
 run_cmd (step_id step,
          tracer& t,
-         string& log, const regexes& warn_detect,
+         string& log, optional<string>* out, const regexes& warn_detect,
          const string& name,
          const optional<step_id>& bkp_step,
          const optional<result_status>& bkp_status,
@@ -438,22 +442,28 @@ run_cmd (step_id step,
     {
       try
       {
-        fdpipe pipe (fdopen_pipe ()); // Text mode seems appropriate.
+        // Redirect stdout to stderr, if the caller is not interested in it.
+        //
+        // Text mode seems appropriate.
+        //
+        fdpipe out_pipe (out != nullptr ? fdopen_pipe () : fdpipe ());
+        fdpipe err_pipe (fdopen_pipe ());
 
         process pr (
           process_start_callback (cmdc,
                                   fdopen_null (), // Never reads from stdin.
-                                  2,              // 1>&2
-                                  pipe,
+                                  out != nullptr ? out_pipe.out.get () : 2,
+                                  err_pipe,
                                   pe,
                                   forward<A> (a)...));
 
-        pipe.out.close ();
+        out_pipe.out.close ();
+        err_pipe.out.close ();
 
         {
           // Skip on exception.
           //
-          ifdstream is (move (pipe.in), fdstream_mode::skip);
+          ifdstream is (move (err_pipe.in), fdstream_mode::skip);
 
           for (string l; is.peek () != ifdstream::traits_type::eof (); )
           {
@@ -494,6 +504,16 @@ run_cmd (step_id step,
           r = e.normal () ? result_status::error : result_status::abnormal;
         }
 
+        // Only read the buffered output if the process terminated normally.
+        //
+        if (out != nullptr && pr.exit->normal ())
+        {
+          // Note: shouldn't throw since the output is buffered.
+          //
+          ifdstream is (move (out_pipe.in));
+          *out = is.read_text ();
+        }
+
         last_cmd = move (next_cmd);
 
         if (bkp_status && r >= *bkp_status)
@@ -530,7 +550,7 @@ static result_status
 run_bpkg (step_id step,
           const V& envvars,
           tracer& t,
-          string& log, const regexes& warn_detect,
+          string& log, optional<string>* out, const regexes& warn_detect,
           const optional<step_id>& bkp_step,
           const optional<result_status>& bkp_status,
           string& last_cmd,
@@ -539,13 +559,54 @@ run_bpkg (step_id step,
 {
   return run_cmd (step,
                   t,
-                  log, warn_detect,
+                  log, out, warn_detect,
                   "bpkg " + cmd,
                   bkp_step, bkp_status, last_cmd,
                   process_env ("bpkg", envvars),
                   verbosity, cmd, forward<A> (a)...);
 }
 
+template <typename V, typename... A>
+static result_status
+run_bpkg (step_id step,
+          const V& envvars,
+          tracer& t,
+          string& log, const regexes& warn_detect,
+          const optional<step_id>& bkp_step,
+          const optional<result_status>& bkp_status,
+          string& last_cmd,
+          const char* verbosity,
+          const string& cmd, A&&... a)
+{
+  return run_bpkg (step,
+                   envvars,
+                   t,
+                   log, nullptr /* out */, warn_detect,
+                   bkp_step, bkp_status, last_cmd,
+                   verbosity, cmd, forward<A> (a)...);
+}
+
+template <typename... A>
+static result_status
+run_bpkg (step_id step,
+          tracer& t,
+          string& log, optional<string>* out, const regexes& warn_detect,
+          const optional<step_id>& bkp_step,
+          const optional<result_status>& bkp_status,
+          string& last_cmd,
+          const char* verbosity,
+          const string& cmd, A&&... a)
+{
+  const char* const* envvars (nullptr);
+
+  return run_bpkg (step,
+                   envvars,
+                   t,
+                   log, out, warn_detect,
+                   bkp_step, bkp_status, last_cmd,
+                   verbosity, cmd, forward<A> (a)...);
+}
+
 template <typename... A>
 static result_status
 run_bpkg (step_id step,
@@ -590,7 +651,7 @@ run_b (step_id step,
 
   return run_cmd (step,
                   t,
-                  log, warn_detect,
+                  log, nullptr /* out */, warn_detect,
                   name,
                   bkp_step, bkp_status, last_cmd,
                   process_env ("b", envvars),
@@ -611,7 +672,7 @@ run_b (step_id step,
 {
   return run_cmd (step,
                   t,
-                  log, warn_detect,
+                  log, nullptr /* out */, warn_detect,
                   "b " + buildspec,
                   bkp_step, bkp_status, last_cmd,
                   process_env ("b", envvars),
@@ -723,6 +784,8 @@ upload_manifest (tracer& trace,
   }
 }
 
+static const string worker_checksum ("1"); // Logic version.
+
 static int bbot::
 build (size_t argc, const char* argv[])
 {
@@ -743,6 +806,8 @@ build (size_t argc, const char* argv[])
   //
   // 3. Upload the result manifest.
   //
+  // NOTE: consider updating agent_checksum if making any logic changes.
+  //
   // Note also that we are being "watched" by the startup version of us which
   // will upload an appropriate result in case we exit with an error. So here
   // for abnormal situations (like a failure to parse the manifest), we just
@@ -751,11 +816,19 @@ build (size_t argc, const char* argv[])
   task_manifest tm (
     parse_manifest<task_manifest> (path ("task.manifest"), "task"));
 
+  // Reset the dependency checksum if the task's worker checksum doesn't match
+  // the current one.
+  //
+  if (!tm.worker_checksum || *tm.worker_checksum != worker_checksum)
+    tm.dependency_checksum = nullopt;
+
   result_manifest rm {
     tm.name,
     tm.version,
     result_status::success,
-    operation_results {}
+    operation_results {},
+    worker_checksum,
+    nullopt /* dependency_checksum */
   };
 
   auto add_result = [&rm] (string o) -> operation_result&
@@ -779,12 +852,14 @@ build (size_t argc, const char* argv[])
 
   for (;;) // The "breakout" loop.
   {
-    auto abort_operation = [&trace] (operation_result& r, const string& e)
+    auto fail_operation = [&trace] (operation_result& r,
+                                    const string& e,
+                                    result_status s)
     {
       l3 ([&]{trace << e;});
 
       r.log    += "error: " + e + '\n';
-      r.status  = result_status::abort;
+      r.status  = s;
     };
 
     // Regular expressions that detect different forms of build2 toolchain
@@ -826,8 +901,9 @@ build (size_t argc, const char* argv[])
 
       if (!bkp_step && !bkp_status)
       {
-        abort_operation (add_result ("configure"),
-                         "invalid interactive build breakpoint '" + b + "'");
+        fail_operation (add_result ("configure"),
+                        "invalid interactive build breakpoint '" + b + "'",
+                        result_status::abort);
 
         break;
       }
@@ -1213,9 +1289,10 @@ build (size_t argc, const char* argv[])
     //
     if (target_pkg && has_buildtime_tests)
     {
-      abort_operation (
+      fail_operation (
         add_result ("configure"),
-        "build-time tests in package not marked with `requires: host`");
+        "build-time tests in package not marked with `requires: host`",
+        result_status::abort);
 
       break;
     }
@@ -1755,22 +1832,22 @@ build (size_t argc, const char* argv[])
 
       // Finally, configure all the packages.
       //
-      // Note that in the future we can run this command with the
-      // --rebuild-checksum option first to obtain the dependency tree
-      // checksum and bail out if it didn't change since the previous build.
-      //
       {
         step_id b (step_id::bpkg_configure_build);
         step_id s (step_id::bpkg_global_configure_build);
 
+        optional<string> dependency_checksum;
+
         r.status |= run_bpkg (
           b,
-          trace, r.log, wre,
+          trace, r.log, &dependency_checksum, wre,
           bkp_step, bkp_status, last_cmd,
           "-v",
           "build",
           "--configure-only",
           "--checkout-root", dist_root,
+          "--rebuild-checksum",
+          tm.dependency_checksum ? *tm.dependency_checksum : "",
           "--yes",
           "-d", root_conf,
           step_args (env_args,    s),
@@ -1778,6 +1855,59 @@ build (size_t argc, const char* argv[])
           "--",
           pkg_args);
 
+        // The dependency checksum is tricky, here are the possibilities:
+        //
+        // - absent: bpkg terminated abnormally (or was not executed due to
+        //           a breakpoint) -- nothing to do here.
+        //
+        // - empty: bpkg terminated normally with error before calculating the
+        //          checksum -- nothing to do here either.
+        //
+        // - one line: bpkg checksum that we want.
+        //
+        // - many lines: someone else (e.g., buildfile) printed to stdout,
+        //               which we consider an error.
+        //
+        if (dependency_checksum && !dependency_checksum->empty ())
+        {
+          string& s (*dependency_checksum);
+
+          // Make sure that the output contains a single line, and bail out
+          // with the error status if that's not the case.
+          //
+          if (s.find ('\n') == s.size () - 1)
+          {
+            s.pop_back ();
+
+            // If the dependency checksum didn't change, then save it to the
+            // result manifest, clean the logs and bail out with the skip
+            // result status.
+            //
+            if (tm.dependency_checksum && *tm.dependency_checksum == s)
+            {
+              l3 ([&]{trace << "skip";});
+
+              rm.status = result_status::skip;
+              rm.dependency_checksum = move (s);
+              rm.results.clear ();
+              break;
+            }
+
+            // Save the (new) dependency checksum to the result manifest.
+            //
+            // Also note that we save the checksum if bpkg failed after the
+            // checksum was printed. As a result, we won't be rebuilding the
+            // package until the error is fixed (in a package or worker) and
+            // the checksum changes, which feels like a proper behavior.
+            //
+            rm.dependency_checksum = move (s);
+          }
+          else
+            fail_operation (r,
+                            "unexpected bpkg output:\n'" + s + "'",
+                            result_status::error);
+        }
+
         if (!r.status)
           break;
       }
@@ -2756,7 +2886,7 @@ build (size_t argc, const char* argv[])
     if (!error)
     {
       r.status |= run_cmd (step_id::end,
-                           trace, r.log, regexes (),
+                           trace, r.log, nullptr /* out */, regexes (),
                            "" /* name */,
                            bkp_step, bkp_status, last_cmd,
                            process_env ());
@@ -2805,7 +2935,8 @@ build (size_t argc, const char* argv[])
     }
   }
   else
-    assert (rm.status == result_status::abort);
+    assert (rm.status == result_status::abort ||
+            rm.status == result_status::skip);
 
   if (!rwd.empty ())
     change_wd (trace, nullptr /* log */, rwd);
@@ -2999,7 +3130,9 @@ startup ()
       tm.name.empty ()    ? bpkg::package_name ("unknown") : tm.name,
       tm.version.empty () ? bpkg::version ("0")            : tm.version,
       result_status::abnormal,
-      operation_results {}
+      operation_results {},
+      worker_checksum,
+      nullopt /* dependency_checksum */
     };
 
     try
diff --git a/doc/manual.cli b/doc/manual.cli
index 93ba6f0..38b52e0 100644
--- a/doc/manual.cli
+++ b/doc/manual.cli
@@ -198,10 +198,13 @@ summary: Windows 10 build 1607 with VC 14 update 3
 id: <machine-id>
 \
 
-The uniquely machine version/revision/build identifies. For virtual machines
+The unique machine version/revision/build identifier. For virtual machines
 this can be the disk image checksum. For a container this can be UUID that is
 re-generated every time a container filesystem is altered.
 
+Note that we assume that a different machine identifier is assigned on any
+change that may affect the build result.
+
 
 \h2#arch-machine-header-name|\c{name}|
 
@@ -334,6 +337,7 @@ repository-url: <repository-url>
 [tests]: <dependency-package>
 [examples]: <dependency-package>
 [benchmarks]: <dependency-package>
+[dependency-checksum]: <checksum>
 
 machine: <machine-name>
 target: <target-triplet>
@@ -342,6 +346,7 @@ target: <target-triplet>
 [host]: true|false
 [warning-regex]: <warning-regex>
 [interactive]: <breakpoint>
+[worker-checksum]: <checksum>
 \
 
 
@@ -382,6 +387,7 @@ The repository type (see \c{repository-url} for details). Alternatively, the
 repository type can be specified as part of the URL scheme. See
 \l{bpkg-repository-types(1)} for details.
 
+
 \h2#arch-task-trust|\c{trust}|
 
 \
@@ -413,6 +419,16 @@ excluded from building due to their \c{builds}, \c{build-include}, and
 \c{build-exclude} manifest values.
 
 
+\h2#arch-task-dependency-checksum|\c{dependency-checksum}|
+
+\
+[dependency-checksum]: <checksum>
+\
+
+The package dependency checksum received as a part of the previous build task
+result (see \l{#arch-result Result Manifest}).
+
+
 \h2#arch-task-machine|\c{machine}|
 
 \
@@ -532,6 +548,16 @@ special \c{error} or \c{warning} value. See \l{#arch-worker Worker Logic} for
 details.
 
 
+\h2#arch-task-worker-checksum|\c{worker-checksum}|
+
+\
+[worker-checksum]: <checksum>
+\
+
+The worker checksum received as a part of the previous build task result (see
+\l{#arch-result Result Manifest}).
+
+
 \h#arch-result|Result Manifest|
 
 The result manifest describes a build result. The manifest synopsis is
@@ -556,6 +582,9 @@ status:                  <status>
 [install-log]:        <text>
 [test-installed-log]: <text>
 [uninstall-log]:      <text>
+
+[worker-checksum]:     <checksum>
+[dependency-checksum]: <checksum>
 \
 
 
@@ -586,6 +615,7 @@ status: <status>
 The overall (cumulative) build result status. Valid values are:
 
 \
+skip       # Package update and subsequent operations were skipped.
 success    # All operations completed successfully.
 warning    # One or more operations completed with warnings.
 error      # One or more operations completed with errors.
@@ -606,6 +636,12 @@ abnormally, for example, due to the package manager or build system crash.
 Note that the overall \c{status} value should appear before any per-operation
 \c{*-status} values.
 
+The \c{skip} status indicates that the received from the controller build task
+checksums have not changed and the task execution has therefore been skipped
+under the assumtion that it would have produced the same result. See
+\c{agent-checksum}, \c{worker-checksum}, and \c{dependency-checksum} for
+details.
+
 
 \h2#arch-result-x-status|\c{*-status}|
 
@@ -640,6 +676,26 @@ the list of supported operation names refer to the \c{*-status} value
 description.
 
 
+\h2#arch-result-dependency-checksum|\c{dependency-checksum}|
+
+\
+[dependency-checksum]: <checksum>
+\
+
+The package dependency checksum obtained as a byproduct of the package
+configuration operation. See \l{bpkg-pkg-build(1)} command's
+\c{--rebuild-checksum} option for details.
+
+
+\h2#arch-result-worker-checksum|\c{worker-checksum}|
+
+\
+[worker-checksum]: <checksum>
+\
+
+The version of the worker logic used to perform the package build task.
+
+
 \h#arch-task-req|Task Request Manifest|
 
 An agent (or controller acting as an agent) sends a task request to its
@@ -734,6 +790,7 @@ subsequent sections.
 session: <id>
 [challenge]: <text>
 [result-url]: <url>
+[agent-checksum]: <checksum>
 \
 
 
@@ -771,6 +828,16 @@ private key and then \c{base64}-encoding the result.
 The URL to POST (upload) the result request to.
 
 
+\h2#arch-task-res-agent-checksum|\c{agent-checksum}|
+
+\
+[agent-checksum]: <checksum>
+\
+
+The agent checksum received as a part of the previous build task result
+request (see \l{#arch-result-req Result Request Manifest}).
+
+
 \h#arch-result-req|Result Request Manifest|
 
 On completion of a task an agent (or controller acting as an agent) sends the
@@ -784,6 +851,7 @@ description of each value in subsequent sections.
 \
 session: <id>
 [challenge]: <text>
+[agent-checksum]: <checksum>
 \
 
 
@@ -807,6 +875,15 @@ response. It must be present only if the \c{challenge} value was present in
 the task response.
 
 
+\h2#arch-result-req-agent-checksum|\c{agent-checksum}|
+
+\
+[agent-checksum]: <checksum>
+\
+
+The version of the agent logic used to perform the package build task.
+
+
 \h#arch-worker|Worker Logic|
 
 The \c{bbot} worker builds each package in a \i{build environment} that is
diff --git a/tests/integration/testscript b/tests/integration/testscript
index d59eec2..446d32b 100644
--- a/tests/integration/testscript
+++ b/tests/integration/testscript
@@ -55,6 +55,7 @@ ver = 1.0.0+7
 rep_url = https://stage.build2.org/1
 rep_type = pkg
 rfp = yes
+#dependency_checksum = 'dependency-checksum: e6f10587696020674c260669f4e7000a0139df72467bff9770aea2f2b8b57ba0'
 
 #\
 pkg = hello
@@ -91,7 +92,7 @@ host='host: true'
 #
 #\
 pkg = libbuild2-kconfig
-ver = 0.1.0-a.0.20210825082040.000d8026a71f
+ver = 0.1.0-a.0.20210928065354.40a5c6beeb5c
 rep_url = "https://github.com/build2/libbuild2-kconfig.git#master"
 rep_type = git
 #ver = 0.1.0-a.0.20200910053253.a71aa3f3938b
@@ -102,6 +103,7 @@ requires='requires: bootstrap'
 tests="tests: * libbuild2-kconfig-tests == $ver
 examples: * kconfig-hello == $ver"
 host='host: true'
+#dependency_checksum = 'dependency-checksum: 72ae02bed9a05aaf022147297a99b84d63b712e15d05cc073551da39003e87e8'
 #\
 
 #\
@@ -149,6 +151,7 @@ requires='requires: host'
 tests="tests: * xsd-tests == $ver
 examples: * xsd-examples == $ver"
 host='host: true'
+#dependency_checksum = 'dependency-checksum: 4c51751ab1872fb208cbd84b09799708296988d773d201156e3be6136c3246b7'
 #\
 
 #\
@@ -197,6 +200,8 @@ bpkg.test-separate-installed.create:\"config.cc.loptions=-L'$~/install/lib'\""
   config: $config
   $interactive
   $host
+  worker-checksum: 1
+  $dependency_checksum
   EOI
 
 +if ("$environment" != "")