Add context-wide pre/post operation callbacks

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_callback> pre;
+      function<post_callback> post;
+    };
+
+    using operation_callback_map = multimap<action_id, operation_callback>;
+
+    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<target> ());
+
+          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<target> ());
+
+        // 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<target> ());
 
-      // 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
     {