From 734bf117f0eb596fc40579810613a6e13c43d3c4 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 21 Aug 2024 09:03:42 +0200 Subject: Add context-wide pre/post operation callbacks --- build2/b.cxx | 27 ++++-- libbuild2/context.hxx | 45 +++++++++- libbuild2/dist/operation.cxx | 192 +++++++++++++++++++++++-------------------- libbuild2/module.cxx | 14 +++- libbuild2/operation.cxx | 135 ++++++++++++++++++++++++------ libbuild2/operation.hxx | 12 ++- libbuild2/scope.hxx | 10 ++- 7 files changed, 307 insertions(+), 128 deletions(-) diff --git a/build2/b.cxx b/build2/b.cxx index 6f1452f..0b4ec3a 100644 --- a/build2/b.cxx +++ b/build2/b.cxx @@ -1559,8 +1559,13 @@ main (int argc, char* argv[]) if (dump_match_pre) dump (ctx, a); - if (mif->execute != nullptr && !ctx.match_only) - mif->execute (mparams, a, tgs, diag, true /* progress */); + if (mif->execute != nullptr) + { + if (!ctx.match_only) + mif->execute (mparams, a, tgs, diag, true /* progress */); + else if (mif->execute == &perform_execute) + perform_post_operation_callbacks (ctx, a, tgs, false /*failed*/); + } } if (pre_oif->operation_post != nullptr) @@ -1603,8 +1608,13 @@ main (int argc, char* argv[]) if (dump_match) dump (ctx, a); - if (mif->execute != nullptr && !ctx.match_only) - mif->execute (mparams, a, tgs, diag, true /* progress */); + if (mif->execute != nullptr) + { + if (!ctx.match_only) + mif->execute (mparams, a, tgs, diag, true /* progress */); + else if (mif->execute == &perform_execute) + perform_post_operation_callbacks (ctx, a, tgs, false /*failed*/); + } } if (oif->operation_post != nullptr) @@ -1647,8 +1657,13 @@ main (int argc, char* argv[]) if (dump_match_post) dump (ctx, a); - if (mif->execute != nullptr && !ctx.match_only) - mif->execute (mparams, a, tgs, diag, true /* progress */); + if (mif->execute != nullptr) + { + if (!ctx.match_only) + mif->execute (mparams, a, tgs, diag, true /* progress */); + else if (mif->execute == &perform_execute) + perform_post_operation_callbacks (ctx, a, tgs, false /*failed*/); + } } if (post_oif->operation_post != nullptr) diff --git a/libbuild2/context.hxx b/libbuild2/context.hxx index 828c41e..db126bc 100644 --- a/libbuild2/context.hxx +++ b/libbuild2/context.hxx @@ -351,10 +351,48 @@ namespace build2 (current_mname.empty () && current_oname == mo)); }; + // Operation callbacks. + // + // An entity (module, core) can register a function that will be called + // when an action is executed on a set of targets. The pre callback is + // called before any recipes for the action are matched and the post -- + // after all have been executed. The post callback is called even if + // execution has failed. + // + // The callback should only be registered during the load phase. Note + // that it's registered for the inner action, meaning that it will be + // called for any outer action (which is discernible from the first + // argument of the callback). Note also that meta-operations other than + // perform never actually execute any recipes and it probably only makes + // sense to register these callbacks for the perform_* actions. + // + // Note that the callbacks will also be called when building a build + // system module or an ad hoc C++ recipe. See create_module_context() for + // details. + // + // See also scope::operation_callback. + // + struct operation_callback + { + using pre_callback = + void (context&, action, const action_targets&); + + using post_callback = + void (context&, action, const action_targets&, bool failed); + + function pre; + function post; + }; + + using operation_callback_map = multimap; + + operation_callback_map operation_callbacks; + // Meta/operation-specific context-global auxiliary data storage. // - // Note: cleared by current_[meta_]operation() below. Normally set by - // meta/operation-specific callbacks from [mate_]operation_info. + // Normally set by meta/operation-specific callbacks from + // [mata_]operation_info. The operation data is cleared by + // current_operation() below. // // Note also: watch out for MT-safety in the data itself. // @@ -759,6 +797,9 @@ namespace build2 // Set current meta-operation and operation. // + // Note that the context instance is not to be re-used between different + // meta-operations. + // void current_meta_operation (const meta_operation_info&); diff --git a/libbuild2/dist/operation.cxx b/libbuild2/dist/operation.cxx index 6dc830b..5b00980 100644 --- a/libbuild2/dist/operation.cxx +++ b/libbuild2/dist/operation.cxx @@ -282,120 +282,134 @@ namespace build2 const location loc (pn); // Dummy location. action_targets ts {tgt}; - auto process_postponed = [&ctx, &mod] () { - if (!mod.postponed.list.empty ()) + auto mog = make_guard ([&ctx] () {ctx.match_only = nullopt;}); + ctx.match_only = match_only_level::all; + + auto process_postponed = [&ctx, &mod, &ts] (action a) { - // Re-grab the phase lock similar to perform_match(). - // - phase_lock l (ctx, run_phase::match); + if (!mod.postponed.list.empty ()) + { + auto eg ( + make_exception_guard ( + [&ctx, a, &ts] () + { + perform_post_operation_callbacks ( + ctx, a, ts, true /* failed */); + })); - // Note that we don't need to bother with the mutex since we do - // all of this serially. But we can end up with new elements at - // the end. - // - // Strictly speaking, to handle this correctly we would need to do - // multiple passes over this list and only give up when we cannot - // make any progress since earlier entries that we cannot resolve - // could be "fixed" by later entries. But this feels far-fetched - // and so let's wait for a real example before complicating this. - // - for (auto i (mod.postponed.list.begin ()); - i != mod.postponed.list.end (); - ++i) - rule::match_postponed (*i); - } - }; + // Re-grab the phase lock similar to perform_match(). + // + phase_lock l (ctx, run_phase::match); - auto mog = make_guard ([&ctx] () {ctx.match_only = nullopt;}); - ctx.match_only = match_only_level::all; + // Note that we don't need to bother with the mutex since we do + // all of this serially. But we can end up with new elements at + // the end. + // + // Strictly speaking, to handle this correctly we would need to + // do multiple passes over this list and only give up when we + // cannot make any progress since earlier entries that we cannot + // resolve could be "fixed" by later entries. But this feels + // far-fetched and so let's wait for a real example before + // complicating this. + // + for (auto i (mod.postponed.list.begin ()); + i != mod.postponed.list.end (); + ++i) + rule::match_postponed (*i); + } + }; - const operations& ops (rs.root_extra->operations); - for (operations::size_type id (default_id + 1); // Skip default_id. - id < ops.size (); - ++id) - { - if (const operation_info* oif = ops[id]) + const operations& ops (rs.root_extra->operations); + for (operations::size_type id (default_id + 1); // Skip default_id. + id < ops.size (); + ++id) { - // Skip aliases (e.g., update-for-install). In fact, one can argue - // the default update should be sufficient since it is assumed to - // update all prerequisites and we no longer support ad hoc stuff - // like test.input. Though here we are using the dist - // meta-operation, not perform. - // - if (oif->id != id) - continue; - - // Use standard (perform) match. - // - if (auto pp = oif->pre_operation) + if (const operation_info* oif = ops[id]) { - if (operation_id pid = pp (ctx, {}, dist_id, loc)) + // Skip aliases (e.g., update-for-install). In fact, one can + // argue the default update should be sufficient since it is + // assumed to update all prerequisites and we no longer support + // ad hoc stuff like test.input. Though here we are using the + // dist meta-operation, not perform. + // + if (oif->id != id) + continue; + + // Use standard (perform) match. + // + if (auto pp = oif->pre_operation) { - const operation_info* poif (ops[pid]); - ctx.current_operation (*poif, oif, false /* diag_noise */); + if (operation_id pid = pp (ctx, {}, dist_id, loc)) + { + const operation_info* poif (ops[pid]); + ctx.current_operation (*poif, oif, false /* diag_noise */); - if (oif->operation_pre != nullptr) - oif->operation_pre (ctx, {}, false /* inner */, loc); + if (oif->operation_pre != nullptr) + oif->operation_pre (ctx, {}, false /* inner */, loc); - if (poif->operation_pre != nullptr) - poif->operation_pre (ctx, {}, true /* inner */, loc); + if (poif->operation_pre != nullptr) + poif->operation_pre (ctx, {}, true /* inner */, loc); - action a (dist_id, poif->id, oif->id); - mod.postponed.list.clear (); - perform_match ({}, a, ts, - 1 /* diag (failures only) */, - false /* progress */); - process_postponed (); + action a (dist_id, poif->id, oif->id); + mod.postponed.list.clear (); + perform_match ({}, a, ts, + 1 /* diag (failures only) */, + false /* progress */); + process_postponed (a); + perform_post_operation_callbacks (ctx, a, ts, false /*failed*/); - if (poif->operation_post != nullptr) - poif->operation_post (ctx, {}, true /* inner */); + if (poif->operation_post != nullptr) + poif->operation_post (ctx, {}, true /* inner */); - if (oif->operation_post != nullptr) - oif->operation_post (ctx, {}, false /* inner */); + if (oif->operation_post != nullptr) + oif->operation_post (ctx, {}, false /* inner */); + } } - } - ctx.current_operation (*oif, nullptr, false /* diag_noise */); + ctx.current_operation (*oif, nullptr, false /* diag_noise */); - if (oif->operation_pre != nullptr) - oif->operation_pre (ctx, {}, true /* inner */, loc); + if (oif->operation_pre != nullptr) + oif->operation_pre (ctx, {}, true /* inner */, loc); - action a (dist_id, oif->id); - mod.postponed.list.clear (); - perform_match ({}, a, ts, - 1 /* diag (failures only) */, - false /* progress */); - process_postponed (); + action a (dist_id, oif->id); + mod.postponed.list.clear (); + perform_match ({}, a, ts, + 1 /* diag (failures only) */, + false /* progress */); + process_postponed (a); + perform_post_operation_callbacks (ctx, a, ts, false /*failed*/); - if (oif->operation_post != nullptr) - oif->operation_post (ctx, {}, true /* inner */); + if (oif->operation_post != nullptr) + oif->operation_post (ctx, {}, true /* inner */); - if (auto po = oif->post_operation) - { - if (operation_id pid = po (ctx, {}, dist_id)) + if (auto po = oif->post_operation) { - const operation_info* poif (ops[pid]); - ctx.current_operation (*poif, oif, false /* diag_noise */); + if (operation_id pid = po (ctx, {}, dist_id)) + { + const operation_info* poif (ops[pid]); + ctx.current_operation (*poif, oif, false /* diag_noise */); - if (oif->operation_pre != nullptr) - oif->operation_pre (ctx, {}, false /* inner */, loc); + if (oif->operation_pre != nullptr) + oif->operation_pre (ctx, {}, false /* inner */, loc); - if (poif->operation_pre != nullptr) - poif->operation_pre (ctx, {}, true /* inner */, loc); + if (poif->operation_pre != nullptr) + poif->operation_pre (ctx, {}, true /* inner */, loc); - action a (dist_id, poif->id, oif->id); - mod.postponed.list.clear (); - perform_match ({}, a, ts, - 1 /* diag (failures only) */, - false /* progress */); - process_postponed (); + action a (dist_id, poif->id, oif->id); + mod.postponed.list.clear (); + perform_match ({}, a, ts, + 1 /* diag (failures only) */, + false /* progress */); + process_postponed (a); + perform_post_operation_callbacks (ctx, a, ts, false /*failed*/); - if (poif->operation_post != nullptr) - poif->operation_post (ctx, {}, true /* inner */); + if (poif->operation_post != nullptr) + poif->operation_post (ctx, {}, true /* inner */); - if (oif->operation_post != nullptr) - oif->operation_post (ctx, {}, false /* inner */); + if (oif->operation_post != nullptr) + oif->operation_post (ctx, {}, false /* inner */); + } } } } diff --git a/libbuild2/module.cxx b/libbuild2/module.cxx index 1aaa38d..520b993 100644 --- a/libbuild2/module.cxx +++ b/libbuild2/module.cxx @@ -96,18 +96,28 @@ namespace build2 nullopt)); /* module_context */ // We use the same context for building any nested modules that might be - // required while building modules. + // required while building modules. Note: this is also used to detect + // module building context. @@ Maybe we should invent special build.mode? // context& mctx (*(ctx.module_context = ctx.module_context_storage->get ())); mctx.module_context = &mctx; + // Copy over any operation callbacks. If a callback implementation does + // not wish to see module context's calls, it can filter them out based on + // the passed context. + // + // Note also that only the callbacks registered before we need to build + // the first module will be in effect. Probably good enough for now. + // + mctx.operation_callbacks = ctx.operation_callbacks; + // Setup the context to perform update. In a sense we have a long-running // perform meta-operation batch (indefinite, in fact, since we never call // the meta-operation's *_post() callbacks) in which we periodically // execute update operations. // // Note that we perform each build in a separate update operation. Failed - // that, if the same target is update twice (which may happen with ad hoc + // that, if the same target is updated twice (which may happen with ad hoc // recipes) we will see the old state. // if (mo_perform.meta_operation_pre != nullptr) diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index d7b5b92..38e5bb4 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -335,6 +335,16 @@ namespace build2 }); } + // Call the pre operation callbacks. + // + // See a comment in perform_execute() for why we are doing it here + // (short answer: phase switches). + // + auto cs (ctx.operation_callbacks.equal_range (a)); + for (auto i (cs.first); i != cs.second; ++i) + if (const auto& f = i->second.pre) + f (ctx, a, ts); + // Start asynchronous matching of prerequisites keeping track of how // many we have started. Wait with unlocked phase to allow phase // switching. @@ -437,7 +447,11 @@ namespace build2 diag_progress.clear (); } - // We are now running serially. Re-examine targets that we have matched. + // We are now running serially. + // + + // Re-examine targets that we have matched and determine whether we have + // failed. // for (size_t j (0); j != n; ++j) { @@ -463,11 +477,8 @@ namespace build2 case target_state::postponed: { // We bailed before matching it (leave state in action_target as - // unknown). + // unknown for the structured result printing). // - if (verb != 0 && diag >= 1) - info << "not " << diag_did (a, t); - break; } case target_state::unknown: @@ -480,9 +491,6 @@ namespace build2 { // Things didn't go well for this target. // - if (verb != 0 && diag >= 1) - info << "failed to " << diag_do (a, t); - at.state = s; fail = true; break; @@ -492,6 +500,36 @@ namespace build2 } } + // Call the post operation callbacks if perform_execute() won't be + // called. + // + if (fail) + perform_post_operation_callbacks (ctx, a, ts, fail); + + // Re-examine targets that we have matched and print diagnostics. + // + if (verb != 0 && diag >= 1) + { + for (size_t j (0); j != n; ++j) + { + action_target& at (ts[j]); + const target& t (at.as ()); + + if (at.state == target_state::failed) + { + // Things didn't go well for this target. + // + info << "failed to " << diag_do (a, t); + } + else if (j >= i || t.matched_state (a) == target_state::postponed) + { + // We bailed before matching it. + // + info << "not " << diag_did (a, t); + } + } + } + if (fail) throw failed (); @@ -622,8 +660,8 @@ namespace build2 switch (ctx.current_inner_oif->concurrency) { case 0: sched_tune = tune_guard (*ctx.sched, 1); break; // Run serially. - case 1: break; // Run as is. - default: assert (false); // Not supported. + case 1: break; // Run as is. + default: assert (false); // Not supported. } // Set the dry-run flag. @@ -675,6 +713,13 @@ namespace build2 } } + // Note that while this would seem like the natural place to call the + // pre operation callbacks, it is actually too late since during match + // we may switch to the execute phase and execute some recipes (think + // building a tool to generate some code). So we have to do this in + // perform_match() and then carefully make sure the post callbacks are + // called for all the exit paths (match failed, match_only, etc). + // In the 'last' execution mode run post hoc first. // if (ctx.current_mode == execution_mode::last) @@ -723,9 +768,44 @@ namespace build2 // We are now running serially. // - // Clear the dry-run flag. + // Re-examine all the targets and determine whether we have failed. // - ctx.dry_run = false; + for (action_target& at: ts) + { + const target& t (at.as ()); + + // Similar to match we cannot attribute post hoc failures to specific + // targets so it seems the best we can do is just fail them all. + // + if (!posthoc_fail) + { + // Note that here we call executed_state() directly instead of + // execute_complete() since we know there is no need to wait. + // + at.state = t.executed_state (a, false /* fail */); + } + else + at.state = /*t.state[a].state =*/ target_state::failed; + + switch (at.state) + { + case target_state::unknown: + case target_state::unchanged: + case target_state::changed: + break; + case target_state::failed: + { + fail = true; + break; + } + default: + assert (false); + } + } + + // Call the post operation callbacks. + // + perform_post_operation_callbacks (ctx, a, ts, fail); // Clear the progress if present. // @@ -735,6 +815,10 @@ namespace build2 diag_progress.clear (); } + // Clear the dry-run flag. + // + ctx.dry_run = false; + // Restore original scheduler settings. } @@ -759,19 +843,6 @@ namespace build2 { const target& t (at.as ()); - // Similar to match we cannot attribute post hoc failures to specific - // targets so it seems the best we can do is just fail them all. - // - if (!posthoc_fail) - { - // Note that here we call executed_state() directly instead of - // execute_complete() since we know there is no need to wait. - // - at.state = t.executed_state (a, false /* fail */); - } - else - at.state = /*t.state[a].state =*/ target_state::failed; - switch (at.state) { case target_state::unknown: @@ -806,7 +877,6 @@ namespace build2 if (verb != 0 && diag >= 1) info << "failed to " << diag_do (a, t); - fail = true; break; } default: @@ -1004,6 +1074,19 @@ namespace build2 #endif } + void + perform_post_operation_callbacks (context& ctx, + action a, + const action_targets& ts, + bool failed) + { + auto cs (ctx.operation_callbacks.equal_range (a)); + + for (auto i (cs.first); i != cs.second; ++i) + if (const auto& f = i->second.post) + f (ctx, a, ts, failed); + } + const meta_operation_info mo_perform { perform_id, "perform", diff --git a/libbuild2/operation.hxx b/libbuild2/operation.hxx index e8ff38a..7d11161 100644 --- a/libbuild2/operation.hxx +++ b/libbuild2/operation.hxx @@ -177,9 +177,19 @@ namespace build2 // diagnostics (unless quiet). // LIBBUILD2_SYMEXPORT void - perform_execute (const values&, action, const action_targets&, + perform_execute (const values&, action, action_targets&, uint16_t diag, bool prog); + // Call the context-wide post operation callbacks. Should be called after + // perfrom_match() if perform_execute() will not be called. Note that + // perform_match() handles its own failures but not the match_only case. + // + LIBBUILD2_SYMEXPORT void + perform_post_operation_callbacks (context&, + action, + const action_targets&, + bool failed); + LIBBUILD2_SYMEXPORT extern const meta_operation_info mo_noop; LIBBUILD2_SYMEXPORT extern const meta_operation_info mo_perform; LIBBUILD2_SYMEXPORT extern const meta_operation_info mo_info; diff --git a/libbuild2/scope.hxx b/libbuild2/scope.hxx index ece78b7..d792ed2 100644 --- a/libbuild2/scope.hxx +++ b/libbuild2/scope.hxx @@ -459,14 +459,20 @@ namespace build2 // when an action is executed on the dir{} target that corresponds to this // scope. The pre callback is called just before the recipe and the post // -- immediately after. The callbacks are only called if the recipe - // (including noop recipe) is executed for the corresponding target. The - // callbacks should only be registered during the load phase. + // (including noop recipe) is executed for the corresponding target. + // + // The callback should only be registered during the load phase. Note that + // it's registered for the inner action, meaning that it will be called + // for any outer action (which is discernible from the first argument of + // the callback). // // It only makes sense for callbacks to return target_state changed or // unchanged and to throw failed in case of an error. These pre/post // states will be merged with the recipe state and become the target // state. See execute_recipe() for details. // + // See also context::operation_callback. + // public: struct operation_callback { -- cgit v1.1