aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--bbot/agent/agent.cxx23
-rw-r--r--bbot/worker/worker.cxx183
-rw-r--r--doc/manual.cli79
-rw-r--r--tests/integration/testscript7
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" != "")