From e05f7c7383cc48823bd408c0bc5187191a9a1c48 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Tue, 23 May 2023 09:23:16 +0200 Subject: Explicit group: static members --- libbuild2/adhoc-rule-buildscript.cxx | 369 ++++++++++++++++++++++++++------- libbuild2/adhoc-rule-buildscript.hxx | 12 +- libbuild2/adhoc-rule-cxx.cxx | 17 +- libbuild2/adhoc-rule-cxx.hxx | 3 + libbuild2/adhoc-rule-regex-pattern.cxx | 23 +- libbuild2/adhoc-rule-regex-pattern.hxx | 2 +- libbuild2/algorithm.cxx | 81 +++++++- libbuild2/algorithm.ixx | 5 +- libbuild2/build/script/parser.cxx | 20 +- libbuild2/build/script/parser.hxx | 18 +- libbuild2/build/script/runner.cxx | 27 ++- libbuild2/build/script/script.cxx | 24 ++- libbuild2/parser.cxx | 3 + libbuild2/rule.cxx | 7 +- libbuild2/rule.hxx | 14 +- libbuild2/target.cxx | 38 ++++ libbuild2/target.hxx | 36 +++- libbuild2/target.ixx | 4 +- 18 files changed, 580 insertions(+), 123 deletions(-) diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index f3e3560..62f3594 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -211,11 +211,13 @@ namespace build2 bool adhoc_buildscript_rule:: reverse_fallback (action a, const target_type& tt) const { - // We can provide clean for a file target if we are providing update. + // We can provide clean for a file or group target if we are providing + // update. // - return a == perform_clean_id && tt.is_a () && - find (actions.begin (), actions.end (), - perform_update_id) != actions.end (); + return (a == perform_clean_id && + (tt.is_a () || tt.is_a ()) && + find (actions.begin (), actions.end (), + perform_update_id) != actions.end ()); } struct adhoc_buildscript_rule::match_data @@ -256,22 +258,27 @@ namespace build2 }; bool adhoc_buildscript_rule:: - match (action a, target& t, const string& h, match_extra& me) const + match (action a, target& xt, const string& h, match_extra& me) const { + const target& t (xt); // See adhoc_rule::match(). + // We pre-parsed the script with the assumption it will be used on a - // non/file-based target. Note that this should not be possible with - // patterns. + // non/file-based (or file group-based) target. Note that this should not + // be possible with patterns. // if (pattern == nullptr) { - if ((t.is_a () != nullptr) != ttype->is_a ()) - { + // Let's not allow mixing file/group. + // + if ((t.is_a () != nullptr) == ttype->is_a () || + (t.is_a () != nullptr) == ttype->is_a ()) + ; + else fail (loc) << "incompatible target types used with shared recipe" << - info << "all targets must be file-based or non-file-based"; - } + info << "all targets must be file- or file group-based or non"; } - return adhoc_rule::match (a, t, h, me); + return adhoc_rule::match (a, xt, h, me); } recipe adhoc_buildscript_rule:: @@ -282,17 +289,27 @@ namespace build2 recipe adhoc_buildscript_rule:: apply (action a, - target& xt, + target& t, match_extra& me, - const optional& d) const + const optional& deadline) const { tracer trace ("adhoc_buildscript_rule::apply"); + // Handle matching explicit group members (see adhoc_rule::match() for + // background). + // + if (const group* g = t.group != nullptr ? t.group->is_a () : nullptr) + { + match_sync (a, *g); + return group_recipe; // Execute the group's recipe. + } + // We don't support deadlines for any of these cases (see below). // - if (d && (a.outer () || - me.fallback || - (a == perform_update_id && xt.is_a ()))) + if (deadline && (a.outer () || + me.fallback || + (a == perform_update_id && + (t.is_a () || t.is_a ())))) return empty_recipe; // If this is an outer operation (e.g., update-for-test), then delegate to @@ -300,26 +317,80 @@ namespace build2 // if (a.outer ()) { - match_inner (a, xt); + match_inner (a, t); return execute_inner; } - context& ctx (xt.ctx); - const scope& bs (xt.base_scope ()); + context& ctx (t.ctx); + const scope& bs (t.base_scope ()); + + group* g (t.is_a ()); // Explicit group. // Inject pattern's ad hoc group members, if any. // if (pattern != nullptr) - pattern->apply_adhoc_members (a, xt, bs, me); + { + pattern->apply_adhoc_members (a, t, bs, me); - // Derive file names for the target and its ad hoc group members, if any. + // A pattern rule that matches an explicit group should not inject any + // ad hoc members. + // + if (g != nullptr) + assert (t.adhoc_member == nullptr); + } + + // Derive file names for the target and its static/ad hoc group members, + // if any. // if (a == perform_update_id || a == perform_clean_id) { - for (target* m (&xt); m != nullptr; m = m->adhoc_member) + if (g != nullptr) + { + g->reset_members (a); // See group::group_members() for background. + + for (const target& m: g->static_members) + { + if (auto* p = m.is_a ()) + p->derive_path (); + + g->members.push_back (&m); + } + + if (g->members.empty ()) + { + if (!script.depdb_dyndep_dyn_target) // @@ TODO: expl: first must file + fail << "group " << *g << " has no static or dynamic members"; + } + else if (!g->members.front ()->is_a ()) + { + // We use the first static member to derive depdb path, get mtime, + // etc. So it must be file-based. + // + fail << "first static member " << g->members.front () << " of group " + << *g << " is not a file"; + } + } + else + { + for (target* m (&t); m != nullptr; m = m->adhoc_member) + { + if (auto* p = m->is_a ()) + p->derive_path (); + } + } + } + else if (g != nullptr) + { + // This could be, for example, configure/dist update which could need a + // "representative sample" of members (in order to be able to match the + // rules). So add static members if there aren't any. + // + if (g->group_members (a).members == nullptr) // Note: not g->member. { - if (auto* p = m->is_a ()) - p->derive_path (); + g->reset_members (a); + + for (const target& m: g->static_members) + g->members.push_back (&m); } } @@ -332,22 +403,22 @@ namespace build2 // prerequisites injected by the pattern. So we have to handle this ad hoc // below. // - const fsdir* dir (inject_fsdir (a, xt, false /* prereq */)); + const fsdir* dir (inject_fsdir (a, t, false /* prereq */)); // Match prerequisites. // // This is essentially match_prerequisite_members() but with support // for update=unmatch|match. // - auto& pts (xt.prerequisite_targets[a]); + auto& pts (t.prerequisite_targets[a]); { // Re-create the clean semantics as in match_prerequisite_members(). // - bool clean (a.operation () == clean_id && !xt.is_a ()); + bool clean (a.operation () == clean_id && !t.is_a ()); // Add target's prerequisites. // - for (prerequisite_member p: group_prerequisite_members (a, xt)) + for (prerequisite_member p: group_prerequisite_members (a, t)) { // Note that we have to recognize update=unmatch|match for *(update), // not just perform(update). But only actually do anything about it @@ -355,7 +426,7 @@ namespace build2 // lookup l; // The `update` variable value, if any. include_type pi ( - include (a, xt, p, a.operation () == update_id ? &l : nullptr)); + include (a, t, p, a.operation () == update_id ? &l : nullptr)); // Use prerequisite_target::include to signal update during match or // unmatch. @@ -387,7 +458,7 @@ namespace build2 if (!pi) continue; - const target& pt (p.search (xt)); + const target& pt (p.search (t)); if (&pt == dir) // Don't add injected fsdir{} twice. continue; @@ -406,19 +477,19 @@ namespace build2 // Inject pattern's prerequisites, if any. // if (pattern != nullptr) - pattern->apply_prerequisites (a, xt, bs, me); + pattern->apply_prerequisites (a, t, bs, me); // Start asynchronous matching of prerequisites. Wait with unlocked // phase to allow phase switching. // - wait_guard wg (ctx, ctx.count_busy (), xt[a].task_count, true); + wait_guard wg (ctx, ctx.count_busy (), t[a].task_count, true); for (const prerequisite_target& pt: pts) { if (pt.target == dir) // Don't match injected fsdir{} twice. continue; - match_async (a, *pt.target, ctx.count_busy (), xt[a].task_count); + match_async (a, *pt.target, ctx.count_busy (), t[a].task_count); } wg.wait (); @@ -525,9 +596,9 @@ namespace build2 { // For depdb-dyndep --dyn-target use depdb to clean dynamic targets. // - if (script.depdb_dyndep && script.depdb_dyndep_dyn_target) + if (script.depdb_dyndep_dyn_target) { - file& t (xt.as ()); + file& ft (t.as ()); // @@ TODO: expl // Note that only removing the relevant filesystem entries is not // enough: we actually have to populate the group with members since @@ -543,8 +614,13 @@ namespace build2 return dyndep::map_extension (bs, n, e, nullptr); }); - for (path& f: read_dyn_targets (t.path () + ".d")) + // @@ TODO: expl + // + // - depdb name? + + for (path& f: read_dyn_targets (ft.path () + ".d")) { + // Note that this logic should be consistent with what we have in // exec_depdb_dyndep(). // @@ -567,13 +643,13 @@ namespace build2 // type mapping in order to resolve ambiguities. // dyndep::inject_adhoc_group_member ("file", - a, bs, t, + a, bs, ft, move (f), map_ext, file::static_type); } } - return perform_clean_file; + return g == nullptr ? perform_clean_file : perform_clean_group; } // If we have any update during match prerequisites, now is the time to @@ -586,17 +662,17 @@ namespace build2 // prerequisite_target::data. // if (a == perform_update_id) - update_during_match_prerequisites (trace, a, xt); + update_during_match_prerequisites (trace, a, t); - // See if this is not update or not on a file-based target. + // See if this is not update or not on a file/group-based target. // - if (a != perform_update_id || !xt.is_a ()) + if (a != perform_update_id || !(g != nullptr || t.is_a ())) { // Make sure we get small object optimization. // - if (d) + if (deadline) { - return [dv = *d, this] (action a, const target& t) + return [dv = *deadline, this] (action a, const target& t) { return default_action (a, t, dv); }; @@ -616,14 +692,14 @@ namespace build2 { return [this] (action a, const target& t) { - return perform_update_file (a, t); + return perform_update_file_or_group (a, t); }; } - // This is a perform update on a file target with extraction of dynamic - // dependency information either in the depdb preamble (depdb-dyndep - // without --byproduct) or as a byproduct of the recipe body execution - // (depdb-dyndep with --byproduct). + // This is a perform update on a file or group target with extraction of + // dynamic dependency information either in the depdb preamble + // (depdb-dyndep without --byproduct) or as a byproduct of the recipe body + // execution (depdb-dyndep with --byproduct). // // For the former case, we may need to add additional prerequisites (or // even target group members). We also have to save any such additional @@ -641,9 +717,6 @@ namespace build2 // example and all this logic is based on the prior work in the cc module // where you can often find more detailed rationale for some of the steps // performed (like the fsdir update below). - // - file& t (xt.as ()); - const path& tp (t.path ()); // Re-acquire fsdir{} specified by the user, similar to inject_fsdir() // (which we have disabled; see above). @@ -689,6 +762,11 @@ namespace build2 } } + assert (g != nullptr); // @@ TODO: expl + + file& ft (t.as ()); + const path& tp (ft.path ()); + // Note that while it's tempting to turn match_data* into recipes, some of // their members are not movable. And in the end we will have the same // result: one dynamic memory allocation. @@ -821,8 +899,10 @@ namespace build2 update = true; else { - if ((mt = t.mtime ()) == timestamp_unknown) - t.mtime (mt = mtime (tp)); // Cache. + // @@ TODO: expl mtime + + if ((mt = ft.mtime ()) == timestamp_unknown) + ft.mtime (mt = mtime (tp)); // Cache. update = dd.mtime > mt; } @@ -854,7 +934,7 @@ namespace build2 { build::script::parser p (ctx); mdb->byp = p.execute_depdb_preamble_dyndep_byproduct ( - a, bs, t, + a, bs, ft, // @@ TODO: expl env, script, run, dd, update, mt); } @@ -978,8 +1058,8 @@ namespace build2 else { // Run the second half of the preamble (depdb-dyndep commands) to update - // our prerequisite targets and extract dynamic dependencies (targets and - // prerequisites). + // our prerequisite targets and extract dynamic dependencies (targets + // and prerequisites). // // Note that this should be the last update to depdb (the invalidation // order semantics). @@ -987,7 +1067,7 @@ namespace build2 md->deferred_failure = false; { build::script::parser p (ctx); - p.execute_depdb_preamble_dyndep (a, bs, t, + p.execute_depdb_preamble_dyndep (a, bs, ft, // @@ TODO: expl env, script, run, dd, md->dyn_targets, @@ -1341,21 +1421,25 @@ namespace build2 } target_state adhoc_buildscript_rule:: - perform_update_file (action a, const target& xt) const + perform_update_file_or_group (action a, const target& t) const { - tracer trace ("adhoc_buildscript_rule::perform_update_file"); + tracer trace ("adhoc_buildscript_rule::perform_update_file_or_group"); - context& ctx (xt.ctx); + context& ctx (t.ctx); + const scope& bs (t.base_scope ()); - const file& t (xt.as ()); - const path& tp (t.path ()); + // For a group we use the first (static) member to derive depdb path, as a + // source of mtime, etc. + // + const group* g (t.is_a ()); - const scope& bs (t.base_scope ()); + const file& ft ((g == nullptr ? t : *g->members.front ()).as ()); + const path& tp (ft.path ()); // Update prerequisites and determine if any of them render this target // out-of-date. // - timestamp mt (t.load_mtime ()); + timestamp mt (g == nullptr ? ft.load_mtime () : g->load_mtime (tp)); // This is essentially ps=execute_prerequisites(a, t, mt) which we // cannot use because we need to see ad hoc prerequisites. @@ -1449,8 +1533,18 @@ namespace build2 // { sha256 tcs; - for (const target* m (&t); m != nullptr; m = m->adhoc_member) - hash_target (tcs, *m, storage); + if (g == nullptr) + { + for (const target* m (&t); m != nullptr; m = m->adhoc_member) + hash_target (tcs, *m, storage); + } + else + { + // Feels like there is not much sense in hashing the group itself. + // + for (const target* m: g->members) + hash_target (tcs, *m, storage); + } if (dd.expect (tcs.string ()) != nullptr) l4 ([&]{trace << "target set change forcing update of " << t;}); @@ -1534,7 +1628,11 @@ namespace build2 { // Prepare to execute the script diag preamble and/or body. // - if ((r = execute_update_file (bs, a, t, env, run))) + r = g == nullptr + ? execute_update_file (bs, a, ft, env, run) + : execute_update_group (bs, a, *g, env, run); + + if (r) { if (!ctx.dry_run) dd.check_mtime (tp); @@ -1544,7 +1642,8 @@ namespace build2 if (r || depdb_preamble) run.leave (env, script.end_loc); - t.mtime (system_clock::now ()); + const auto& m (g == nullptr ? static_cast (ft) : *g); + m.mtime (system_clock::now ()); return target_state::changed; } @@ -1653,6 +1752,8 @@ namespace build2 build::script::default_runner& run, bool deferred_failure) const { + // NOTE: similar to execute_update_group() below. + // context& ctx (t.ctx); const scope& rs (*bs.root_scope ()); @@ -1787,6 +1888,115 @@ namespace build2 return exec_diag || exec_body; } + bool adhoc_buildscript_rule:: + execute_update_group (const scope& bs, + action a, const group& g, + build::script::environment& env, + build::script::default_runner& run, + bool deferred_failure) const + { + // Note: similar to execute_update_file() above (see there for comments). + // + context& ctx (g.ctx); + + const scope& rs (*bs.root_scope ()); + + build::script::parser p (ctx); + + bool exec_body (!ctx.dry_run || verb >= 2); + bool exec_diag (!script.diag_preamble.empty () && (exec_body || verb == 1)); + bool exec_depdb (!script.depdb_preamble.empty ()); + + if (script.diag_name) + { + if (verb == 1) + { + const file* pt (nullptr); + for (const prerequisite_target& p: g.prerequisite_targets[a]) + { + if (p.target != nullptr && !p.adhoc ()) + { + pt = p.target->is_a (); + break; + } + } + + if (pt != nullptr) + print_diag (script.diag_name->c_str (), *pt, g); + else + print_diag (script.diag_name->c_str (), g); + } + } + else if (exec_diag) + { + if (script.diag_preamble_temp_dir && !script.depdb_preamble_temp_dir) + env.set_temp_dir_variable (); + + pair diag ( + p.execute_diag_preamble (rs, bs, + env, script, run, + verb == 1 /* diag */, + !exec_depdb /* enter */, + false /* leave */)); + if (verb == 1) + print_custom_diag (bs, move (diag.first), diag.second); + } + + if (exec_body) + { + // On failure remove the target files that may potentially exist but + // be invalid. + // + small_vector rms; + + if (!ctx.dry_run) + { + for (const target* m: g.members) + { + if (auto* f = m->is_a ()) + rms.emplace_back (f->path ()); + } + } + + if (script.body_temp_dir && + !script.depdb_preamble_temp_dir && + !script.diag_preamble_temp_dir) + env.set_temp_dir_variable (); + + p.execute_body (rs, bs, + env, script, run, + !exec_depdb && !exec_diag /* enter */, + false /* leave */); + + if (!ctx.dry_run) + { + if (deferred_failure) + fail << "expected error exit status from recipe body"; + +#ifndef _WIN32 + auto chmod = [] (const path& p) + { + path_perms (p, + (path_perms (p) | + permissions::xu | + permissions::xg | + permissions::xo)); + }; + + for (const target* m: g.members) + { + if (auto* p = m->is_a ()) + chmod (p->path ()); + } +#endif + for (auto& rm: rms) + rm.cancel (); + } + } + + return exec_diag || exec_body; + } + target_state adhoc_buildscript_rule:: perform_clean_file (action a, const target& t) { @@ -1802,7 +2012,7 @@ namespace build2 // Finally, we print the entire ad hoc group at verbosity level 1, similar // to the default update diagnostics. // - // @@ TODO: .t may also be a temporary directory. + // @@ TODO: .t may also be a temporary directory (and below). // return perform_clean_extra (a, t.as (), @@ -1812,6 +2022,25 @@ namespace build2 } target_state adhoc_buildscript_rule:: + perform_clean_group (action a, const target& xt) + { + const group& g (xt.as ()); + + path d, t; + if (!g.static_members.empty ()) + { + const path& p (g.static_members.front ().get ().as ().path ()); + d = p + ".d"; + t = p + ".t"; + } + else + assert (false); // @@ TODO: expl + + return perform_clean_group_extra (a, g, {d.string ().c_str (), + t.string ().c_str ()}); + } + + target_state adhoc_buildscript_rule:: default_action (action a, const target& t, const optional& deadline) const diff --git a/libbuild2/adhoc-rule-buildscript.hxx b/libbuild2/adhoc-rule-buildscript.hxx index 02939c1..13b272f 100644 --- a/libbuild2/adhoc-rule-buildscript.hxx +++ b/libbuild2/adhoc-rule-buildscript.hxx @@ -36,7 +36,7 @@ namespace build2 const optional&) const override; target_state - perform_update_file (action, const target&) const; + perform_update_file_or_group (action, const target&) const; struct match_data; struct match_data_byproduct; @@ -58,9 +58,19 @@ namespace build2 build::script::default_runner&, bool deferred_failure = false) const; + bool + execute_update_group (const scope&, + action a, const group&, + build::script::environment&, + build::script::default_runner&, + bool deferred_failure = false) const; + static target_state perform_clean_file (action, const target&); + static target_state + perform_clean_group (action, const target&); + target_state default_action (action, const target&, const optional&) const; diff --git a/libbuild2/adhoc-rule-cxx.cxx b/libbuild2/adhoc-rule-cxx.cxx index db5c5ab..ad19481 100644 --- a/libbuild2/adhoc-rule-cxx.cxx +++ b/libbuild2/adhoc-rule-cxx.cxx @@ -95,8 +95,10 @@ namespace build2 load_module_library (const path& lib, const string& sym, string& err); bool adhoc_cxx_rule:: - match (action a, target& t, const string& hint, match_extra& me) const + match (action a, target& xt, const string& hint, match_extra& me) const { + const target& t (xt); // See adhoc_rule::match() for background. + if (pattern != nullptr && !pattern->match (a, t, hint, me)) return false; @@ -674,13 +676,24 @@ namespace build2 } } - return impl->match (a, t, hint, me); + return impl->match (a, xt, hint, me); } #endif // BUILD2_BOOTSTRAP || LIBBUILD2_STATIC_BUILD recipe adhoc_cxx_rule:: apply (action a, target& t, match_extra& me) const { + // Handle matching explicit group member (see adhoc_rule::match() for + // background). + // + if (const group* g = (t.group != nullptr + ? t.group->is_a () + : nullptr)) + { + match_sync (a, *g); + return group_recipe; // Execute the group's recipe. + } + return impl.load (memory_order_relaxed)->apply (a, t, me); } } diff --git a/libbuild2/adhoc-rule-cxx.hxx b/libbuild2/adhoc-rule-cxx.hxx index 9a17447..b563881 100644 --- a/libbuild2/adhoc-rule-cxx.hxx +++ b/libbuild2/adhoc-rule-cxx.hxx @@ -52,6 +52,9 @@ namespace build2 // Return true by default. // + // Note: must treat target as const (unless known to match a non-group). + // See adhoc_rule::match() for background. + // virtual bool match (action, target&) const override; }; diff --git a/libbuild2/adhoc-rule-regex-pattern.cxx b/libbuild2/adhoc-rule-regex-pattern.cxx index 59a63bc..354b859 100644 --- a/libbuild2/adhoc-rule-regex-pattern.cxx +++ b/libbuild2/adhoc-rule-regex-pattern.cxx @@ -126,10 +126,13 @@ namespace build2 } bool adhoc_rule_regex_pattern:: - match (action a, target& t, const string&, match_extra& me) const + match (action a, const target& t, const string&, match_extra& me) const { tracer trace ("adhoc_rule_regex_pattern::match"); + // Note: target may not be locked in which case we should not modify + // target or match_extra (see adhoc_rule::match() for background). + // The plan is as follows: First check the "type signature" of the target // and its prerequisites (the primary target type has already been matched // by the rule matching machinery). If there is a match, then concatenate @@ -161,12 +164,17 @@ namespace build2 // implementation. Except we support the unmatch and match values in // the update variable. // + // Note: assuming group prerequisites are immutable (not locked). + // for (prerequisite_member p: group_prerequisite_members (a, t)) { // Note that here we don't validate the update operation override // value (since we may not match). Instead the rule does this in // apply(). // + // Note: assuming include()'s use of target only relied on immutable + // data (not locked). + // lookup l; if (include (a, t, p, a.operation () == update_id ? &l : nullptr) == include_type::normal && p.is_a (tt)) @@ -205,10 +213,13 @@ namespace build2 // So the plan is to store the string in match_extra::data() and // regex_match_results (which we can move) in the auxiliary data storage. // + // Note: only cache if locked. + // static_assert (sizeof (string) <= match_extra::data_size, "match data too large"); - string& ns (me.data (string ())); + string tmp; + string& ns (me.locked ? me.data (string ()) : tmp); auto append_name = [&ns, first = true, @@ -226,10 +237,12 @@ namespace build2 // Primary target (always a pattern). // auto te (targets_.end ()), ti (targets_.begin ()); - append_name (t.key (), *ti); + append_name (t.key (), *ti); // Immutable (not locked). // Match ad hoc group members. // + // Note: shouldn't be in effect for an explicit group (not locked). + // while ((ti = find_if (ti + 1, te, pattern)) != te) { const target* at (find_adhoc_member (t, ti->type)); @@ -279,7 +292,9 @@ namespace build2 return false; } - t.data (a, move (mr)); + if (me.locked) + t.data (a, move (mr)); + return true; } diff --git a/libbuild2/adhoc-rule-regex-pattern.hxx b/libbuild2/adhoc-rule-regex-pattern.hxx index 597f30d..eb75ea0 100644 --- a/libbuild2/adhoc-rule-regex-pattern.hxx +++ b/libbuild2/adhoc-rule-regex-pattern.hxx @@ -32,7 +32,7 @@ namespace build2 names&&, const location&); virtual bool - match (action, target&, const string&, match_extra&) const override; + match (action, const target&, const string&, match_extra&) const override; virtual void apply_adhoc_members (action, target&, diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 4489e2b..966c7a8 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -504,8 +504,65 @@ namespace build2 // Return the matching rule or NULL if no match and try_match is true. // const rule_match* - match_rule (action a, target& t, const rule* skip, bool try_match) + match_rule (action a, target& t, + const rule* skip, + bool try_match, + match_extra* pme) { + using fallback_rule = adhoc_rule_pattern::fallback_rule; + + auto adhoc_rule_match = [] (const rule_match& r) + { + return dynamic_cast (&r.second.get ()); + }; + + auto fallback_rule_match = [] (const rule_match& r) + { + return dynamic_cast (&r.second.get ()); + }; + + // If this is a member of group-based target, then first try to find a + // matching ad hoc recipe/rule by matching (to an ad hoc recipe/rule) the + // group but applying to the member. See adhoc_rule::match() for + // background, including for why const_cast should be safe. + // + // To put it another way, if a group is matched by an ad hoc recipe/rule, + // then we want all the member to be matched to the same recipe/rule. + // + if (const group* g = t.group != nullptr ? t.group->is_a () : nullptr) + { + assert (pme == nullptr); + + // As an optimization, check if the group is already matched for this + // action. Note that this can become important when matching adhoc regex + // rules since we can potentially call match() for many members. This is + // probably ok for static members (of which we don't expect more than a + // handful) but can become an issue for dynamic members. + // + if (g->matched (a, memory_order_acquire)) + { + const rule_match* r (g->state[a].rule); + + if (r != nullptr && adhoc_rule_match (*r)) + return r; + + // Fall through: if some rule matched the group then it must also deal + // with the members. + } + else + { + // We cannot init match_extra from the target if it's unlocked so use + // a temporary (it shouldn't be modified if unlocked). + // + match_extra me (false /* locked */); + if (const rule_match* r = match_rule ( + a, const_cast (*g), skip, true /* try_match */, &me)) + return r; + + // Fall through to normal match of the member. + } + } + const scope& bs (t.base_scope ()); // Match rules in project environment. @@ -514,7 +571,7 @@ namespace build2 if (const scope* rs = bs.root_scope ()) penv = auto_project_env (*rs); - match_extra& me (t[a].match_extra); + match_extra& me (pme == nullptr ? t[a].match_extra : *pme); // First check for an ad hoc recipe. // @@ -619,8 +676,6 @@ namespace build2 // reverse_fallback() rather than it returning (a list) of // reverse actions, which would be necessary to register them. // - using fallback_rule = adhoc_rule_pattern::fallback_rule; - auto find_fallback = [mo, o, tt] (const fallback_rule& fr) -> const rule_match* { @@ -633,14 +688,19 @@ namespace build2 if (oi == 0) { - if (auto* fr = - dynamic_cast (&r->second.get ())) + if (const fallback_rule* fr = fallback_rule_match (*r)) { if ((r = find_fallback (*fr)) == nullptr) continue; } } + // Skip non-ad hoc rules if the target is not locked (see + // above). + // + if (!me.locked && !adhoc_rule_match (*r)) + continue; + const string& n (r->first); const rule& ru (r->second); @@ -672,14 +732,16 @@ namespace build2 if (oi == 0) { - if (auto* fr = - dynamic_cast (&r1->second.get ())) + if (const fallback_rule* fr = fallback_rule_match (*r1)) { if ((r1 = find_fallback (*fr)) == nullptr) continue; } } + if (!me.locked && !adhoc_rule_match (*r1)) + continue; + const string& n1 (r1->first); const rule& ru1 (r1->second); @@ -698,8 +760,7 @@ namespace build2 // // @@ Can't we temporarily swap things out in target? // - match_extra me1; - me1.init (oi == 0); + match_extra me1 (me.locked, oi == 0 /* fallback */); if (!ru1.match (a, t, *hint, me1)) continue; } diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx index 6029290..d2cd018 100644 --- a/libbuild2/algorithm.ixx +++ b/libbuild2/algorithm.ixx @@ -392,7 +392,10 @@ namespace build2 } LIBBUILD2_SYMEXPORT const rule_match* - match_rule (action, target&, const rule* skip, bool try_match = false); + match_rule (action, target&, + const rule* skip, + bool try_match = false, + match_extra* = nullptr); LIBBUILD2_SYMEXPORT recipe apply_impl (action, target&, const rule_match&); diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index e7268f9..c71f218 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -61,7 +61,7 @@ namespace build2 pbase_ = scope_->src_path_; - file_based_ = tt.is_a (); + file_based_ = tt.is_a () || tt.is_a (); perform_update_ = find (as.begin (), as.end (), perform_update_id) != as.end (); @@ -748,8 +748,8 @@ namespace build2 } if (!file_based_) - fail (l) << "'depdb' builtin can only be used for file-based " - << "targets"; + fail (l) << "'depdb' builtin can only be used for file- or " + << "file group-based targets"; if (!diag_preamble_.empty ()) fail (diag_loc ()) << "'diag' builtin call before 'depdb' call" << @@ -1287,7 +1287,7 @@ namespace build2 } void parser:: - exec_depdb_preamble (action a, const scope& bs, const file& t, + exec_depdb_preamble (action a, const scope& bs, const target& t, environment& e, const script& s, runner& r, lines_iterator begin, lines_iterator end, depdb& dd, @@ -1314,7 +1314,7 @@ namespace build2 action a; const scope& bs; - const file& t; + const target& t; environment& env; const script& scr; @@ -1360,7 +1360,7 @@ namespace build2 // exec_depdb_dyndep (t, tt, li, ll, - data.a, data.bs, const_cast (data.t), + data.a, data.bs, const_cast (data.t), data.dd, *data.dyn_targets, *data.update, @@ -1645,7 +1645,7 @@ namespace build2 void parser:: exec_depdb_dyndep (token& lt, build2::script::token_type& ltt, size_t li, const location& ll, - action a, const scope& bs, file& t, + action a, const scope& bs, target& t, depdb& dd, paths& dyn_targets, bool& update, @@ -2455,6 +2455,8 @@ namespace build2 // if (ops.drop_cycles ()) { + // @@ TODO: expl + for (const target* m (&t); m != nullptr; m = m->adhoc_member) { if (ft == m) @@ -2874,7 +2876,7 @@ namespace build2 // that there are only real targets to start with. // optional> dts; - for (const target* m (&t); m != nullptr; m = m->adhoc_member) + for (const target* m (&t); m != nullptr; m = m->adhoc_member) // @@ TODO: expl { if (m->decl != target_decl::real) dts = vector (); @@ -2914,7 +2916,7 @@ namespace build2 // if (dts) { - for (target* p (&t); p->adhoc_member != nullptr; ) + for (target* p (&t); p->adhoc_member != nullptr; ) // @@ TODO: expl { target* m (p->adhoc_member); diff --git a/libbuild2/build/script/parser.hxx b/libbuild2/build/script/parser.hxx index b615a90..856ad64 100644 --- a/libbuild2/build/script/parser.hxx +++ b/libbuild2/build/script/parser.hxx @@ -103,8 +103,10 @@ namespace build2 // runner's enter() function is called before the first preamble/body // command execution and leave() -- after the last command. // + // Note: target must be file or group. + // void - execute_depdb_preamble (action a, const scope& base, const file& t, + execute_depdb_preamble (action a, const scope& base, const target& t, environment& e, const script& s, runner& r, depdb& dd) { @@ -219,8 +221,10 @@ namespace build2 names exec_special (token&, build2::script::token_type&, bool skip_first); + // Note: target must be file or group. + // void - exec_depdb_preamble (action, const scope& base, const file&, + exec_depdb_preamble (action, const scope& base, const target&, environment&, const script&, runner&, lines_iterator begin, lines_iterator end, depdb&, @@ -230,10 +234,12 @@ namespace build2 bool* deferred_failure = nullptr, dyndep_byproduct* = nullptr); + // Note: target must be file or group. + // void exec_depdb_dyndep (token&, build2::script::token_type&, size_t line_index, const location&, - action, const scope& base, file&, + action, const scope& base, target&, depdb&, paths& dyn_targets, bool& update, @@ -276,9 +282,9 @@ namespace build2 script* script_; const small_vector* actions_; // Non-NULL during pre-parse. - // True if this script is for file-based targets and performing update - // is one of the actions, respectively. Only set for the pre-parse - // mode. + // True if this script is for file- or file group-based targets and + // performing update is one of the actions, respectively. Only set for + // the pre-parse mode. // bool file_based_; bool perform_update_; diff --git a/libbuild2/build/script/runner.cxx b/libbuild2/build/script/runner.cxx index c52ef66..e08ebbf 100644 --- a/libbuild2/build/script/runner.cxx +++ b/libbuild2/build/script/runner.cxx @@ -28,12 +28,29 @@ namespace build2 // for (auto i (env.cleanups.begin ()); i != env.cleanups.end (); ) { - const target* m (&env.target); - for (; m != nullptr; m = m->adhoc_member) + const target* m (nullptr); + if (const group* g = env.target.is_a ()) { - if (const path_target* pm = m->is_a ()) - if (i->path == pm->path ()) - break; + for (const target* gm: g->members) + { + if (const path_target* pm = gm->is_a ()) + { + if (i->path == pm->path ()) + { + m = gm; + break; + } + } + } + } + else + { + for (m = &env.target; m != nullptr; m = m->adhoc_member) + { + if (const path_target* pm = m->is_a ()) + if (i->path == pm->path ()) + break; + } } if (m != nullptr) diff --git a/libbuild2/build/script/script.cxx b/libbuild2/build/script/script.cxx index 9d9b5a8..0f31e7f 100644 --- a/libbuild2/build/script/script.cxx +++ b/libbuild2/build/script/script.cxx @@ -58,11 +58,27 @@ namespace build2 { // $> // + // What should it contain for an explicit group? While it may seem + // that just the members should be enough (and analogous to the ad + // hoc case), this won't let us get the group name for diagnostics. + // So the group name followed by all the members seems like the + // logical choice. + // names ns; - for (const target_type* m (&target); - m != nullptr; - m = m->adhoc_member) - m->as_name (ns); + + if (const group* g = target.is_a ()) + { + g->as_name (ns); + for (const target_type* m: g->members) + m->as_name (ns); + } + else + { + for (const target_type* m (&target); + m != nullptr; + m = m->adhoc_member) + m->as_name (ns); + } assign (var_ts) = move (ns); } diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index a00f6a5..45c56af 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -1267,6 +1267,9 @@ namespace build2 // rule for an explicit group that wishes to match based on some of // its members feels far fetched. // + // @@ TODO: expl: this can be used to inject static members (which + // otherwise would be tedious to repeat). + // const location& mloc (gns.empty () ? location () : gns[0].member_loc); if (!gns.empty () && gns[0].expl) diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index 0b401c2..097e15a 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -44,7 +44,7 @@ namespace build2 // Use scratch match_extra since if there is no recipe, then we don't // want to keep any changes and if there is, then we want it discarded. // - match_extra s; + match_extra s (true /* locked */); // Not called from adhoc_rule::match(). if (match_adhoc_recipe (action (a.meta_operation (), o), t, s) != nullptr) return false; } @@ -73,7 +73,7 @@ namespace build2 { if (!t.adhoc_recipes.empty ()) { - match_extra s; + match_extra s (true /* locked */); // Not called from adhoc_rule::match(). if (match_adhoc_recipe (action (a.meta_operation (), o), t, s) != nullptr) return false; } @@ -403,8 +403,9 @@ namespace build2 } bool adhoc_rule:: - match (action a, target& t, const string& h, match_extra& me) const + match (action a, target& xt, const string& h, match_extra& me) const { + const target& t (xt); return pattern == nullptr || pattern->match (a, t, h, me); } diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx index 913c597..b89821b 100644 --- a/libbuild2/rule.hxx +++ b/libbuild2/rule.hxx @@ -252,6 +252,16 @@ namespace build2 // The default implementation forwards to the pattern's match() if there // is a pattern and returns true otherwise. // + // Note also that in case of a member of a group-based target, match() is + // called on the group while apply() on the member (see match_rule() in + // algorithms.cxx for details). This means that match() may be called + // without having the target locked and as a result match() should (unless + // known to only match a non-group) treat the target as const and only + // rely on immutable information (type, name, etc) since the group could + // be matched concurrenly. This case can be detected by examining + // match_extra::locked (see adhoc_rule_regex_pattern::match() for a + // use-case). + // virtual bool match (action, target&, const string&, match_extra&) const override; @@ -318,8 +328,10 @@ namespace build2 ~adhoc_rule_pattern (); public: + // Note: the adhoc_rule::match() restrictions apply here as well. + // virtual bool - match (action, target&, const string&, match_extra&) const = 0; + match (action, const target&, const string&, match_extra&) const = 0; virtual void apply_adhoc_members (action, target&, diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index a9ae47a..c1002b9 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -1217,6 +1217,42 @@ namespace build2 target_type::flag::none }; + // group + // + group_view group:: + group_members (action a) const + { + if (members_on == 0) // Not yet discovered. + return group_view {nullptr, 0}; + + // Members discovered during anything other than perform_update are only + // good for that operation. For example, we only return the static members + // ("representative sample") for perform_configure. + // + // We also re-discover the members on each update and clean not to + // overcomplicate the already twisted adhoc_buildscript_rule::apply() + // logic. + // + if (members_on != ctx.current_on) + { + if (members_action != perform_update_id || + a == perform_update_id || + a == perform_clean_id) + return group_view {nullptr, 0}; + } + + // Note that we may have no members (e.g., perform_configure and there are + // no static members). However, whether std::vector returns a non-NULL + // pointer in this case is undefined. + // + size_t n (members.size ()); + return group_view { + n != 0 + ? members.data () + : reinterpret_cast (this), + n}; + } + const target_type group::static_type { "group", @@ -1230,6 +1266,8 @@ namespace build2 target_type::flag::group }; + // alias + // static const target* alias_search (const target& t, const prerequisite_key& pk) { diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 01e82bb..428b4a0 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -170,6 +170,7 @@ namespace build2 // struct match_extra { + bool locked; // Normally true (see adhoc_rule::match() for background). bool fallback; // True if matching a fallback rule (see match_rule()). // Auxiliary data storage. @@ -237,7 +238,12 @@ namespace build2 // Implementation details. // + // NOTE: see match_rule() in algorithms.cxx if changing anything here. + // public: + explicit + match_extra (bool l = true, bool f = false): locked (l), fallback (f) {} + void init (bool fallback); @@ -852,8 +858,11 @@ namespace build2 // Return true if the target has been matched for the specified action. // This function can only be called during the match or execute phases. // + // If you need to observe something in the matched target (e.g., the + // matched rule), use memory_order_acquire. + // bool - matched (action) const; + matched (action, memory_order mo = memory_order_relaxed) const; // This function can only be called during match if we have observed // (synchronization-wise) that this target has been matched (i.e., the @@ -2160,15 +2169,34 @@ namespace build2 // Note that it is not see-through but a derived group can be made see- // through via the [see_through] attribute. // - // Note that normally you wouldn't use it as a base for a custom group - // defined in C++, instead deriving from mtime_target directly and using a - // custom members layout more appropriate for the group's semantics. + // Note also that you shouldn't use it as a base for a custom group defined + // in C++, instead deriving from mtime_target directly and using a custom + // members layout more appropriate for the group's semantics. To put it + // another way, a group-based target should only be matched by an ad hoc + // recipe/rule (see match_rule() in algorithms.cxx for details). // class LIBBUILD2_SYMEXPORT group: public mtime_target { public: vector> static_members; + // Note: we expect no NULL entries in members. + // + vector members; // Layout compatible with group_view. + action members_action; // Action on which members were resolved. + size_t members_on = 0; // Operation number on which members were resolved. + + void + reset_members (action a) + { + members.clear (); + members_action = a; + members_on = ctx.current_on; + } + + virtual group_view + group_members (action) const override; + group (context& c, dir_path d, dir_path o, string n) : mtime_target (c, move (d), move (o), move (n)) { diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx index 899e829..3f005c3 100644 --- a/libbuild2/target.ixx +++ b/libbuild2/target.ixx @@ -235,13 +235,13 @@ namespace build2 } inline bool target:: - matched (action a) const + matched (action a, memory_order mo) const { assert (ctx.phase == run_phase::match || ctx.phase == run_phase::execute); const opstate& s (state[a]); - size_t c (s.task_count.load (memory_order_relaxed) - ctx.count_base ()); + size_t c (s.task_count.load (mo) - ctx.count_base ()); if (ctx.phase == run_phase::match) { -- cgit v1.1