From 57e2aaa28e19da1cd4e51982f46ef2630739ff8a Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 26 May 2020 13:10:37 +0200 Subject: Wrap up $<, $> variables and related ($target.path(), hashing, cleanup) --- libbuild2/algorithm.cxx | 5 +- libbuild2/build/script/lexer.cxx | 9 +- .../build/script/parser+variable.test.testscript | 2 +- libbuild2/build/script/parser.cxx | 9 +- libbuild2/build/script/parser.hxx | 3 +- libbuild2/build/script/parser.test.cxx | 4 +- libbuild2/build/script/runner.cxx | 19 ++++ libbuild2/build/script/script.cxx | 42 +++++--- libbuild2/build/script/script.hxx | 19 ++-- libbuild2/functions-name.cxx | 108 ++++++++++++++++----- libbuild2/rule.cxx | 82 ++++++++++++---- libbuild2/script/script.cxx | 2 +- libbuild2/target-key.hxx | 12 ++- libbuild2/target.cxx | 10 +- libbuild2/target.hxx | 3 + libbuild2/target.ixx | 6 ++ libbuild2/test/script/script.hxx | 13 +-- 17 files changed, 258 insertions(+), 90 deletions(-) diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index ceb18e4..11f2a56 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -1015,8 +1015,11 @@ namespace build2 if (r != nullptr) { + // Make it ad hoc so that it doesn't end up in prerequisite_targets + // after execution. + // match (a, *r); - t.prerequisite_targets[a].emplace_back (r); + t.prerequisite_targets[a].emplace_back (r, include_type::adhoc); } return r; diff --git a/libbuild2/build/script/lexer.cxx b/libbuild2/build/script/lexer.cxx index 716d898..f5a7333 100644 --- a/libbuild2/build/script/lexer.cxx +++ b/libbuild2/build/script/lexer.cxx @@ -242,14 +242,19 @@ namespace build2 { lexer_mode m (st.mode); - // Customized implementation that handles special variable names ($>). + // Customized implementation that handles special variable names ($>, + // $<). + // + // @@ TODO: $(<), $(>): feels like this will have to somehow be + // handled at the top-level lexer level. Maybe provide a + // string of one-char special variable names as state::data? // if (m != lexer_mode::variable) return base_lexer::word (st, sep); xchar c (peek ()); - if (c != '>') + if (c != '>' && c != '<') return base_lexer::word (st, sep); get (); diff --git a/libbuild2/build/script/parser+variable.test.testscript b/libbuild2/build/script/parser+variable.test.testscript index 2d9bb1e..5040e66 100644 --- a/libbuild2/build/script/parser+variable.test.testscript +++ b/libbuild2/build/script/parser+variable.test.testscript @@ -13,7 +13,7 @@ EOO : primary-target : $* <>EOO -echo $> +echo $name($>) EOI echo driver EOO diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index 60000c2..b170088 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -248,7 +248,8 @@ namespace build2 // void parser:: - execute (const script& s, environment& e, runner& r) + execute (const scope& rs, const scope& bs, + environment& e, const script& s, runner& r) { path_ = nullptr; // Set by replays. @@ -256,6 +257,12 @@ namespace build2 set_lexer (nullptr); + // The script shouldn't be able to modify the scopes. + // + root_ = const_cast (&rs); + scope_ = const_cast (&bs); + pbase_ = scope_->src_path_; + script_ = const_cast (&s); runner_ = &r; environment_ = &e; diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx index 7183509..4c16087 100644 --- a/libbuild2/build/script/parser.hxx +++ b/libbuild2/build/script/parser.hxx @@ -68,7 +68,8 @@ namespace build2 // public: void - execute (const script&, environment&, runner&); + execute (const scope& root, const scope& base, + environment&, const script&, runner&); protected: void diff --git a/libbuild2/build/script/parser.test.cxx b/libbuild2/build/script/parser.test.cxx index b179884..42681e3 100644 --- a/libbuild2/build/script/parser.test.cxx +++ b/libbuild2/build/script/parser.test.cxx @@ -166,9 +166,9 @@ namespace build2 { case mode::run: { - environment e (tt); + environment e (perform_update_id, tt); print_runner r (print_line); - p.execute (s, e, r); + p.execute (ctx.global_scope, ctx.global_scope, e, s, r); break; } case mode::dump: diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index c4a93fd..559a6cd 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -5,6 +5,7 @@ #include +#include #include using namespace butl; @@ -75,6 +76,24 @@ namespace build2 void default_runner:: leave (environment& env, const location& ll) { + // Drop cleanups of target paths. + // + for (auto i (env.cleanups.begin ()); i != env.cleanups.end (); ) + { + const target* m (&env.target); + for (; m != nullptr; m = m->adhoc_member) + { + if (const path_target* pm = m->is_a ()) + if (i->path == pm->path ()) + break; + } + + if (m != nullptr) + i = env.cleanups.erase (i); + else + ++i; + } + clean (env, ll); // Note that since the temporary directory may only contain special diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index b43203c..c258c4c 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -20,30 +20,46 @@ namespace build2 static const string wd_name ("current directory"); environment:: - environment (const target& pt) + environment (action a, const target_type& t) : build2::script::environment ( - pt.ctx, - cast (pt.ctx.global_scope["build.host"]), + t.ctx, + cast (t.ctx.global_scope["build.host"]), work, wd_name, temp_dir.path, false /* temp_dir_keep */, redirect (redirect_type::none), redirect (redirect_type::merge, 2), redirect (redirect_type::pass)), - primary_target (pt), + target (t), vars (context, false /* global */) { - // Set the $> variable. + // Set special variables. // { - //@@ TODO + // $> // - value& v (assign (var_pool.insert (">"))); + names ns; + for (const target_type* m (&t); m != nullptr; m = m->adhoc_member) + m->as_name (ns); - if (auto* t = pt.is_a ()) - v = t->path ().string (); - else - //fail << "target " << pt << " is not path-based"; - v = pt.name; //@@ TMP + assign (var_pool.insert (">")) = move (ns); + } + + { + // $< + // + // Note that at this stage (after execute_prerequisites()) ad hoc + // prerequisites are no longer in prerequisite_targets which means + // they won't end up in $< either. While at first thought ad hoc + // prerequisites in ad hoc recipes don't seem to make much sense, + // they could be handy to exclude certain preresquisites from $< + // while still treating them as such. + // + names ns; + for (const target_type* pt: t.prerequisite_targets[a]) + if (pt != nullptr) + pt->as_name (ns); + + assign (var_pool.insert ("<")) = move (ns); } } @@ -109,7 +125,7 @@ namespace build2 if (pvar == nullptr) return lookup_type (); - return primary_target[*pvar]; + return target[*pvar]; } value& environment:: diff --git a/libbuild2/build/script/script.hxx b/libbuild2/build/script/script.hxx index ec603a7..2139f28 100644 --- a/libbuild2/build/script/script.hxx +++ b/libbuild2/build/script/script.hxx @@ -62,7 +62,9 @@ namespace build2 class environment: public build2::script::environment { public: - environment (const target& primary_target); + using target_type = build2::target; + + environment (action, const target_type&); environment (environment&&) = delete; environment (const environment&) = delete; @@ -70,7 +72,7 @@ namespace build2 environment& operator= (const environment&) = delete; public: - const target& primary_target; + const target_type& target; // Script-local variable pool. // @@ -121,17 +123,18 @@ namespace build2 lookup_in_buildfile (const string&) const; // Return a value suitable for assignment. If the variable does not - // exist in this environment's map, then a new one with the NULL value - // is added and returned. Otherwise the existing value is returned. + // exist in this environment's variable map, then a new one with the + // NULL value is added and returned. Otherwise the existing value is + // returned. // value& assign (const variable& var) {return vars.assign (var);} // Return a value suitable for append/prepend. If the variable does - // not exist in this environment's map, then outer scopes are searched - // for the same variable. If found then a new variable with the found - // value is added to the environment and returned. Otherwise this - // function proceeds as assign() above. + // not exist in this environment's variable map, then outer scopes are + // searched for the same variable. If found then a new variable with + // the found value is added to the environment and returned. Otherwise + // this function proceeds as assign() above. // value& append (const variable&); diff --git a/libbuild2/functions-name.cxx b/libbuild2/functions-name.cxx index 283b1a6..70659ee 100644 --- a/libbuild2/functions-name.cxx +++ b/libbuild2/functions-name.cxx @@ -4,6 +4,7 @@ #include #include #include +#include using namespace std; @@ -14,7 +15,7 @@ namespace build2 // out of scope). See scope::find_target_type() for details. // static pair> - to_target (const scope* s, name&& n) + to_target_name (const scope* s, name&& n) { optional e; @@ -31,74 +32,129 @@ namespace build2 return make_pair (move (n), move (e)); } + static const target& + to_target (const scope& s, name&& n, name&& o) + { + if (const target* r = search_existing (n, s, o.dir)) + return *r; + + fail << "target " + << (n.pair ? names {move (n), move (o)} : names {move (n)}) + << " not found" << endf; + } + void name_functions (function_map& m) { - function_family f (m, "name"); - // These functions treat a name as a target/prerequisite name. // // While on one hand it feels like calling them target.name(), etc., would // have been more appropriate, on the other hand they can also be called // on prerequisite names. They also won't always return the same result as // if we were interrogating an actual target (e.g., the directory may be - // relative). + // relative). Plus we now have functions that can only be called on + // targets (see below). // - f["name"] = [](const scope* s, name n) + function_family fn (m, "name"); + + fn["name"] = [](const scope* s, name n) { - return to_target (s, move (n)).first.value; + return to_target_name (s, move (n)).first.value; }; - f["name"] = [](const scope* s, names ns) + fn["name"] = [](const scope* s, names ns) { - return to_target (s, convert (move (ns))).first.value; + return to_target_name (s, convert (move (ns))).first.value; }; // Note: returns NULL if extension is unspecified (default) and empty if // specified as no extension. // - f["extension"] = [](const scope* s, name n) + fn["extension"] = [](const scope* s, name n) { - return to_target (s, move (n)).second; + return to_target_name (s, move (n)).second; }; - f["extension"] = [](const scope* s, names ns) + fn["extension"] = [](const scope* s, names ns) { - return to_target (s, convert (move (ns))).second; + return to_target_name (s, convert (move (ns))).second; }; - f["directory"] = [](const scope* s, name n) + fn["directory"] = [](const scope* s, name n) { - return to_target (s, move (n)).first.dir; + return to_target_name (s, move (n)).first.dir; }; - f["directory"] = [](const scope* s, names ns) + fn["directory"] = [](const scope* s, names ns) { - return to_target (s, convert (move (ns))).first.dir; + return to_target_name (s, convert (move (ns))).first.dir; }; - f["target_type"] = [](const scope* s, name n) + fn["target_type"] = [](const scope* s, name n) { - return to_target (s, move (n)).first.type; + return to_target_name (s, move (n)).first.type; }; - f["target_type"] = [](const scope* s, names ns) + fn["target_type"] = [](const scope* s, names ns) { - return to_target (s, convert (move (ns))).first.type; + return to_target_name (s, convert (move (ns))).first.type; }; // Note: returns NULL if no project specified. // - f["project"] = [](const scope* s, name n) + fn["project"] = [](const scope* s, name n) { - return to_target (s, move (n)).first.proj; + return to_target_name (s, move (n)).first.proj; }; - f["project"] = [](const scope* s, names ns) + fn["project"] = [](const scope* s, names ns) { - return to_target (s, convert (move (ns))).first.proj; + return to_target_name (s, convert (move (ns))).first.proj; + }; + + // Functions that can be called only on real targets. + // + function_family ft (m, "target"); + + fn["path"] = [](const scope* s, names ns) + { + if (s == nullptr) + fail << "target.path() called out of scope" << endf; + + // Most of the time we will have a single target so optimize for that. + // + small_vector r; + + for (auto i (ns.begin ()); i != ns.end (); ++i) + { + name& n (*i), o; + const target& t (to_target (*s, move (n), move (n.pair ? *++i : o))); + + if (const auto* pt = t.is_a ()) + { + const path& p (pt->path ()); + + if (&p != &empty_path) + r.push_back (p); + else + fail << "target " << t << " path is not assigned"; + } + else + fail << "target " << t << " is not path-based"; + } + + // We want the result to be path if we were given a single target and + // paths if multiple (or zero). The problem is, we cannot distinguish it + // based on the argument type (e.g., name vs names) since passing an + // out-qualified single target requires two names. + // + if (r.size () == 1) + return value (move (r[0])); + + return value (paths (make_move_iterator (r.begin ()), + make_move_iterator (r.end ()))); }; // Name-specific overloads from builtins. // - function_family b (m, "builtin"); + function_family fb (m, "builtin"); - b[".concat"] = [](dir_path d, name n) + fb[".concat"] = [](dir_path d, name n) { d /= n.dir; n.dir = move (d); diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index b9441e0..eabe578 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -480,8 +480,8 @@ namespace build2 bool update (!ps); - // We use depdb to track changes to the script itself, input file names, - // tools, etc. + // We use depdb to track changes to the script itself, input/output file + // names, tools, etc. // depdb dd (tp + ".d"); { @@ -508,12 +508,18 @@ namespace build2 // - We may "overhash" by including variables that are actually // script-local. // + // - There are functions like $install.resolve() with result based on + // external (to the script) information. + // if (dd.expect (checksum) != nullptr) l4 ([&]{trace << "recipe text change forcing update of " << t;}); // For each variable hash its name, undefined/null/non-null indicator, // and the value if non-null. // + // Note that this excludes the special $< and $> variables which we + // handle below. + // { sha256 cs; names storage; @@ -539,13 +545,50 @@ namespace build2 } } - //@@ TODO (later): hash special variables values (targets, - // prerequisites). - if (dd.expect (cs.string ()) != nullptr) l4 ([&]{trace << "recipe variable change forcing update of " << t;}); } + // Target and prerequisite sets ($> and $<). + // + // How should we hash them? We could hash them as target names (i.e., + // the same as the $>/< content) or as paths (only for path-based + // targets). While names feel more general, they are also more expensive + // to compute. And for path-based targets, path is generally a good + // proxy for the target name. Since the bulk of the ad hoc recipes will + // presumably be operating exclusively on path-based targets, let's do + // it both ways. + // + { + auto hash = [ns = names ()] (sha256& cs, const target& t) mutable + { + if (const path_target* pt = t.is_a ()) + cs.append (pt->path ().string ()); + else + { + ns.clear (); + t.as_name (ns); + for (const name& n: ns) + to_checksum (cs, n); + } + }; + + sha256 tcs; + for (const target* m (&t); m != nullptr; m = m->adhoc_member) + hash (tcs, *m); + + if (dd.expect (tcs.string ()) != nullptr) + l4 ([&]{trace << "target set change forcing update of " << t;}); + + sha256 pcs; + for (const target* pt: t.prerequisite_targets[a]) + if (pt != nullptr) + hash (pcs, *pt); + + if (dd.expect (pcs.string ()) != nullptr) + l4 ([&]{trace << "prerequisite set change forcing update of " << t;}); + } + // Then the tools checksums. // // @@ TODO: obtain checksums of all the targets used as commands in @@ -567,13 +610,7 @@ namespace build2 if (!update) return *ps; - if (verb >= 2) - { - //@@ TODO - - //print_process (args); - } - else if (verb) + if (verb == 1) { // @@ TODO: // @@ -590,12 +627,17 @@ namespace build2 text << (diag ? diag->c_str () : "adhoc") << ' ' << t; } - if (!ctx.dry_run) + if (!ctx.dry_run || verb >= 2) { - // @@ TODO - // - touch (ctx, tp, true, verb_never); - dd.check_mtime (tp); + const scope& bs (t.base_scope ()); + + build::script::environment e (a, t); + build::script::parser p (ctx); + build::script::default_runner r; + p.execute (*bs.root_scope (), bs, e, script, r); + + if (!ctx.dry_run) + dd.check_mtime (tp); } t.mtime (system_clock::now ()); @@ -620,10 +662,12 @@ namespace build2 if (!ctx.dry_run || verb >= 2) { + const scope& bs (t.base_scope ()); + + build::script::environment e (a, t); build::script::parser p (ctx); - build::script::environment e (t); build::script::default_runner r; - p.execute (script, e, r); + p.execute (*bs.root_scope (), bs, e, script, r); } return target_state::changed; diff --git a/libbuild2/script/script.cxx b/libbuild2/script/script.cxx index 5a2eda3..9e8780e 100644 --- a/libbuild2/script/script.cxx +++ b/libbuild2/script/script.cxx @@ -633,7 +633,7 @@ namespace build2 const path& p (c.path); - if (sandbox_dir.empty () || !p.sub (sandbox_dir)) + if (!sandbox_dir.empty () && !p.sub (sandbox_dir)) { if (implicit) return; diff --git a/libbuild2/target-key.hxx b/libbuild2/target-key.hxx index 0096d46..62bcc25 100644 --- a/libbuild2/target-key.hxx +++ b/libbuild2/target-key.hxx @@ -32,10 +32,18 @@ namespace build2 bool is_a () const {return type->is_a ();} bool is_a (const target_type& tt) const {return type->is_a (tt);} - // Return the target name or a pair of names if out-qualified. + // Append/return the target name or a pair of names if out-qualified. // + void + as_name (names&) const; + names - as_name () const; + as_name () const + { + names r; + as_name (r); + return r; + } }; inline bool diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index dfd0075..b9cfea7 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -44,11 +44,9 @@ namespace build2 // target_key // - names target_key:: - as_name () const + void target_key:: + as_name (names& r) const { - names r; - string v (*name); target::combine_name (v, ext, false /* @@ TODO: what to do? */); @@ -56,11 +54,9 @@ namespace build2 if (!out->empty ()) { - r.front ().pair = '@'; + r.back ().pair = '@'; r.push_back (build2::name (*out)); } - - return r; } // target_state diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 932aa22..72b7acc 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -264,6 +264,9 @@ namespace build2 names as_name () const; + void + as_name (names&) const; + // Scoping. // public: diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx index 36ca3c8..611e562 100644 --- a/libbuild2/target.ixx +++ b/libbuild2/target.ixx @@ -36,6 +36,12 @@ namespace build2 return key ().as_name (); } + inline void target:: + as_name (names& r) const + { + return key ().as_name (r); + } + inline auto target:: prerequisites () const -> const prerequisites_type& { diff --git a/libbuild2/test/script/script.hxx b/libbuild2/test/script/script.hxx index 163ad65..93846df 100644 --- a/libbuild2/test/script/script.hxx +++ b/libbuild2/test/script/script.hxx @@ -115,17 +115,18 @@ namespace build2 lookup_in_buildfile (const string&, bool target_only = true) const; // Return a value suitable for assignment. If the variable does not - // exist in this scope's map, then a new one with the NULL value is - // added and returned. Otherwise the existing value is returned. + // exist in this scope's variable map, then a new one with the NULL + // value is added and returned. Otherwise the existing value is + // returned. // value& assign (const variable& var) {return vars.assign (var);} // Return a value suitable for append/prepend. If the variable does - // not exist in this scope's map, then outer scopes are searched for - // the same variable. If found then a new variable with the found - // value is added to this scope and returned. Otherwise this function - // proceeds as assign() above. + // not exist in this scope's variable map, then outer scopes are + // searched for the same variable. If found then a new variable with + // the found value is added to this scope and returned. Otherwise this + // function proceeds as assign() above. // value& append (const variable&); -- cgit v1.1