diff options
| -rw-r--r-- | bbot/agent/agent.cxx | 23 | ||||
| -rw-r--r-- | bbot/worker/worker.cxx | 183 | ||||
| -rw-r--r-- | doc/manual.cli | 79 | ||||
| -rw-r--r-- | tests/integration/testscript | 7 |
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" != "") |