Generalize to adhoc_rule/adhoc_script_rule

6 files changed, 180 insertions, 118 deletions

diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx
index b4ff22a..ceb18e4 100644
--- a/libbuild2/algorithm.cxx
+++ b/libbuild2/algorithm.cxx
@@ -331,8 +331,31 @@ namespace build2
             dr << info << "while matching ad hoc recipe to " << diag_do (a, t);
         });
 
-      if (adhoc_rule::instance.match (a, t, string () /* hint */))
-        return &adhoc_rule::match_instance;
+      // @@ TODO:
+      //
+      // If action is Y-for-X, how would we distinguish between X and Y-for-X?
+      // See match_rule() for the hairy details. We could start with
+      // supporting just the inner case. Or we could try to just match an
+      // inner rule by default? I think need a clear use-case to see what's
+      // the correct semantics.
+
+      auto b (t.adhoc_recipes.begin ()), e (t.adhoc_recipes.end ());
+      auto i (find_if (b, e,
+                       [a, &t] (const adhoc_recipe& r)
+                       {
+                         return r.action == a &&
+                           r.rule->match (a, t, string () /* hint */, nullopt);
+                       }));
+
+      if (i == e)
+        i = find_if (b, e,
+                     [a, &t] (const adhoc_recipe& r)
+                     {
+                       return r.action != a &&
+                         r.rule->match (a, t, string () /* hint */, r.action);
+                     });
+      if (i != e)
+        return &i->rule->rule_match;
     }
 
     // If this is an outer operation (Y-for-X), then we look for rules
diff --git a/libbuild2/dump.cxx b/libbuild2/dump.cxx
index 6eb8018..9f60900 100644
--- a/libbuild2/dump.cxx
+++ b/libbuild2/dump.cxx
@@ -3,6 +3,7 @@
 
 #include <libbuild2/dump.hxx>
 
+#include <libbuild2/rule.hxx>
 #include <libbuild2/scope.hxx>
 #include <libbuild2/target.hxx>
 #include <libbuild2/context.hxx>
@@ -351,30 +352,8 @@ namespace build2
     {
       for (const adhoc_recipe r: t.adhoc_recipes)
       {
-        // @@ TODO: indentation is multi-line recipes is off (would need to
-        //          insert indentation after every newline).
-        //
         os << endl;
-
-        // Do we need the header?
-        //
-        if (r.diag)
-        {
-          os << ind << '%';
-
-          if (r.diag)
-          {
-            os << " [";
-            os << "diag="; to_stream (os, name (*r.diag), true /*quote*/, '@');
-            os << ']';
-          }
-
-          os << endl;
-        }
-
-        os << ind << string (r.braces, '{') << endl
-           << ind << r.script
-           << ind << string (r.braces, '}');
+        r.rule->dump (os, ind); // @@ TODO: pass action(s).
       }
 
       used = true;
diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx
index aa4ada9..54c2065 100644
--- a/libbuild2/parser.cxx
+++ b/libbuild2/parser.cxx
@@ -9,6 +9,7 @@
 #include <libbutl/filesystem.mxx>   // path_search
 #include <libbutl/path-pattern.mxx>
 
+#include <libbuild2/rule.hxx>
 #include <libbuild2/dump.hxx>
 #include <libbuild2/scope.hxx>
 #include <libbuild2/module.hxx>
@@ -1086,13 +1087,18 @@ namespace build2
         fail (t) << "unterminated recipe block" <<
           info (st) << "recipe block starts here" << endf;
 
+      // @@ TODO: we need to reuse the same rules for all the targets! Kill
+      //    me now.
+      //
+      shared_ptr<adhoc_rule> ar (
+        new adhoc_script_rule (move (t.value),
+                               move (diag),
+                               get_location (st),
+                               st.value.size ()));
+
       action a (perform_id, update_id);
 
-      target_->adhoc_recipes.emplace_back (a,
-                                           move (t.value),
-                                           move (diag),
-                                           get_location (st),
-                                           st.value.size ());
+      target_->adhoc_recipes.push_back (adhoc_recipe {a, move (ar)});
 
       next (t, tt);
       assert (tt == type::multi_rcbrace);
diff --git a/libbuild2/recipe.hxx b/libbuild2/recipe.hxx
index 4c903b5..efd184a 100644
--- a/libbuild2/recipe.hxx
+++ b/libbuild2/recipe.hxx
@@ -51,29 +51,17 @@ namespace build2
 
   // Ad hoc recipe.
   //
+  // A recipe is a fragment of a rule so we handle ad hoc recipies by
+  // "completing" them to rules.
+  //
+  class adhoc_rule;
+
   struct adhoc_recipe
   {
-    using action_type = build2::action;
-    using location_type = build2::location;
-
-    action_type      action;
-    string           script;
-    optional<string> diag;   // Command name for low-verbosity diagnostics.
-
-    // Diagnostics-related information.
+    // @@ TODO: maybe we should have a small vector of actions (for dump).
     //
-    path_name_value file;    // Buildfile of recipe.
-    location_type location;  // Buildfile location of recipe.
-    size_t braces;           // Number of braces in multi-brace tokens.
-
-    adhoc_recipe (action_type a,
-                  string s,
-                  optional<string> d,
-                  const location_type& l, size_t b)
-        : action (a),
-          script (move (s)),
-          diag (move (d)),
-          file (l.file), location (file, l.line, l.column), braces (b) {}
+    build2::action         action;
+    shared_ptr<adhoc_rule> rule;
   };
 }
 
diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx
index f3b9253..d645ec3 100644
--- a/libbuild2/rule.cxx
+++ b/libbuild2/rule.cxx
@@ -98,11 +98,6 @@ namespace build2
   recipe file_rule::
   apply (action a, target& t) const
   {
-    /*
-      @@ outer
-      return noop_recipe;
-    */
-
     // Update triggers the update of this target's prerequisites so it would
     // seem natural that we should also trigger their cleanup. However, this
     // possibility is rather theoretical so until we see a real use-case for
@@ -309,42 +304,72 @@ namespace build2
 
   // adhoc_rule
   //
-  static inline const adhoc_recipe*
-  find_recipe (action a, target& t)
+  bool adhoc_rule::
+  match (action a, target& t, const string& h, optional<action> fallback) const
   {
-    auto i (find_if (t.adhoc_recipes.begin (),
-                     t.adhoc_recipes.end (),
-                     [a] (const adhoc_recipe& r) {return r.action == a;}));
-
-    return i != t.adhoc_recipes.end () ? &*i : nullptr;
+    return !fallback && match (a, t, h);
   }
 
   bool adhoc_rule::
-  match (action a, target& t, const string&) const
+  match (action, target&, const string&) const
+  {
+    return true;
+  }
+
+  // adhoc_script_rule
+  //
+  void adhoc_script_rule::
+  dump (ostream& os, const string& ind) const
   {
-    // TODO:
+    // @@ TODO: indentation is multi-line recipes is off (would need to insert
+    //          indentation after every newline). Maybe if we pre-parse them?
     //
-    // @@ If action is Y-for-X, how would we distinguish between X and
-    //    Y-for-X? See match_rule() for the hairy details. We could start with
-    //    supporting just the inner case. Or we could try to just match an
-    //    inner rule by default? I think need a clear use-case to see what's
-    //    the correct semantics.
 
-    if (find_recipe (a, t))
-      return true;
+    // Do we need the header?
+    //
+    if (diag)
+    {
+      os << ind << '%';
+
+      if (diag)
+      {
+        os << " [";
+        os << "diag="; to_stream (os, name (*diag), true /* quote */, '@');
+        os << ']';
+      }
+
+      os << endl;
+    }
+
+    os << ind << string (braces, '{') << endl
+       << ind << script
+       << ind << string (braces, '}');
+  }
 
-    // If this is clean for a file target and we have a recipe for update,
-    // then we will supply the standard clean.
+  bool adhoc_script_rule::
+  match (action a, target& t, const string&, optional<action> fb) const
+  {
+    if (!fb)
+      ;
+    // If this is clean for a file target and we are supplying the update,
+    // then we will also supply the standard clean.
     //
-    if (a == perform_clean_id &&
-        t.is_a<file> ()       &&
-        find_recipe (action (perform_id, update_id), t))
-      return true;
+    else if (a   == perform_clean_id  &&
+             *fb == perform_update_id &&
+             t.is_a<file> ())
+      ;
+    else
+      return false;
 
-    return false;
+    // It's unfortunate we have to resort to this but we need to remember this
+    // in apply().
+    //
+    t.data (fb.has_value ());
+
+    return true;
   }
 
-  recipe adhoc_rule::
+  recipe adhoc_script_rule::
   apply (action a, target& t) const
   {
     // Derive file names for the target and its ad hoc group members, if any.
@@ -366,6 +391,11 @@ namespace build2
     //
     match_prerequisite_members (a, t);
 
+    // See if we are providing the standard clean as a fallback.
+    //
+    if (t.data<bool> ())
+      return &perform_clean_depdb;
+
     // For update inject dependency on the tool target(s).
     //
     // @@ We could see that it's a target and do it but not sure if we should
@@ -376,33 +406,24 @@ namespace build2
     // if (a == perform_update_id)
     //  inject (a, t, tgt);
 
-    if (const adhoc_recipe* ar = find_recipe (a, t))
+    if (a == perform_update_id && t.is_a<file> ())
     {
-      if (a == perform_update_id && t.is_a<file> ())
+      return [this] (action a, const target& t)
       {
-        return [ar] (action a, const target& t)
-        {
-          return perform_update_file (a, t, *ar);
-        };
-      }
-      else
-      {
-        return [ar] (action a, const target& t)
-        {
-          return default_action (a, t, *ar);
-        };
-      }
+        return perform_update_file (a, t);
+      };
     }
     else
     {
-      // Otherwise this should be the standard clean.
-      //
-      return &perform_clean_depdb;
+      return [this] (action a, const target& t)
+      {
+        return default_action (a, t);
+      };
     }
   }
 
-  target_state adhoc_rule::
-  perform_update_file (action a, const target& xt, const adhoc_recipe& ar)
+  target_state adhoc_script_rule::
+  perform_update_file (action a, const target& xt) const
   {
     tracer trace ("adhoc_rule::perform_update_file");
 
@@ -445,7 +466,7 @@ namespace build2
       //          It feels like we need a special execute mode that instead
       //          of executing hashes the commands.
       //
-      if (dd.expect (sha256 (ar.script).string ()) != nullptr)
+      if (dd.expect (sha256 (script).string ()) != nullptr)
         l4 ([&]{trace << "recipe change forcing update of " << t;});
     }
 
@@ -467,7 +488,7 @@ namespace build2
 
       //print_process (args);
 
-      text << trim (string (ar.script));
+      text << trim (string (script));
     }
     else if (verb)
     {
@@ -483,7 +504,7 @@ namespace build2
       //   (including tools)?
       //
 
-      text << (ar.diag ? ar.diag->c_str () : "adhoc") << ' ' << t;
+      text << (diag ? diag->c_str () : "adhoc") << ' ' << t;
     }
 
     if (!t.ctx.dry_run)
@@ -498,8 +519,8 @@ namespace build2
     return target_state::changed;
   }
 
-  target_state adhoc_rule::
-  default_action (action a, const target& t, const adhoc_recipe& ar)
+  target_state adhoc_script_rule::
+  default_action (action a, const target& t) const
   {
     tracer trace ("adhoc_rule::default_action");
 
@@ -511,13 +532,13 @@ namespace build2
 
       //print_process (args);
 
-      text << trim (string (ar.script));
+      text << trim (string (script));
     }
     else if (verb)
     {
       // @@ TODO: as above
 
-      text << (ar.diag ? ar.diag->c_str () : "adhoc") << ' ' << t;
+      text << (diag ? diag->c_str () : "adhoc") << ' ' << t;
     }
 
     if (!t.ctx.dry_run)
@@ -528,7 +549,4 @@ namespace build2
 
     return target_state::changed;
   }
-
-  const adhoc_rule adhoc_rule::instance;
-  const rule_match adhoc_rule::match_instance {"adhoc", adhoc_rule::instance};
 }
diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx
index 669f7db..39f89fa 100644
--- a/libbuild2/rule.hxx
+++ b/libbuild2/rule.hxx
@@ -109,28 +109,76 @@ namespace build2
     static const noop_rule instance;
   };
 
-  // Ad hoc recipe rule.
+  // Ad hoc rule.
+  //
+  class LIBBUILD2_SYMEXPORT adhoc_rule: rule
+  {
+  public:
+    using location_type = build2::location;
+
+    // Diagnostics-related information.
+    //
+    path_name_value buildfile; // Buildfile of recipe.
+    location_type   location;  // Buildfile location of recipe.
+    size_t          braces;    // Number of braces in multi-brace tokens.
+
+    build2::rule_match rule_match;
+
+    adhoc_rule (const location_type& l, size_t b)
+      : buildfile (l.file), location (buildfile, l.line, l.column), braces (b),
+        rule_match ("adhoc", *this) {}
+
+  public:
+    // Some of the operations come in compensating pairs, sush as update and
+    // clean, install and uninstall. An ad hoc rule implementation may choose
+    // to provide a fallback implementation of a compensating operation if it
+    // is providing the other half (passed in the fallback argument).
+    //
+    // The default implementation calls rule::match() if fallback is absent
+    // and returns false if fallback is present. So an implementation that
+    // doesn't care about this semantics can implement the straight rule
+    // interface.
+    //
+    virtual bool
+    match (action, target&, const string&, optional<action> fallback) const;
+
+    virtual bool
+    match (action, target&, const string&) const override;
+
+    virtual void
+    dump (ostream&, const string& indentation) const = 0;
+  };
+
+  // Ad hoc script rule.
   //
   // Note: should not be used directly (i.e., registered).
   //
-  class LIBBUILD2_SYMEXPORT adhoc_rule: public rule
+  class LIBBUILD2_SYMEXPORT adhoc_script_rule: public adhoc_rule
   {
   public:
     virtual bool
-    match (action, target&, const string&) const override;
+    match (action, target&, const string&, optional<action>) const override;
 
     virtual recipe
     apply (action, target&) const override;
 
-    static target_state
-    perform_update_file (action, const target&, const adhoc_recipe&);
+    target_state
+    perform_update_file (action, const target&) const;
 
-    static target_state
-    default_action (action, const target&, const adhoc_recipe&);
+    target_state
+    default_action (action, const target&) const;
 
-    adhoc_rule () {}
-    static const adhoc_rule instance;
-    static const rule_match match_instance;
+    virtual void
+    dump (ostream&, const string&) const override;
+
+    adhoc_script_rule (string s,
+                       optional<string> d,
+                       const location_type& l, size_t b)
+        : adhoc_rule (l, b), script (move (s)), diag (move (d)) {}
+
+  public:
+    string           script;
+    optional<string> diag;   // Command name for low-verbosity diagnostics.
   };
 }