From 9a0f07035b34a356ce9b5601d71d388595762184 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Fri, 8 May 2020 15:43:13 +0200 Subject: Generalize to adhoc_rule/adhoc_script_rule --- libbuild2/algorithm.cxx | 27 +++++++++- libbuild2/dump.cxx | 25 +-------- libbuild2/parser.cxx | 16 ++++-- libbuild2/recipe.hxx | 28 +++------- libbuild2/rule.cxx | 134 +++++++++++++++++++++++++++--------------------- libbuild2/rule.hxx | 68 ++++++++++++++++++++---- 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 +#include #include #include #include @@ -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 // path_search #include +#include #include #include #include @@ -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 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 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 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 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 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 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 () && - find_recipe (action (perform_id, update_id), t)) - return true; + else if (a == perform_clean_id && + *fb == perform_update_id && + t.is_a ()) + ; + 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 ()) + 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 ()) { - if (a == perform_update_id && t.is_a ()) + 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 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) 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 d, + const location_type& l, size_t b) + : adhoc_rule (l, b), script (move (s)), diag (move (d)) {} + + public: + string script; + optional diag; // Command name for low-verbosity diagnostics. }; } -- cgit v1.1