From 56d79a62e64180f639dad02f0887fef5d57bb096 Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Thu, 25 May 2023 12:34:08 +0200 Subject: Explicit group: pattern members --- libbuild2/adhoc-rule-buildscript.cxx | 57 +++++++++++---------- libbuild2/adhoc-rule-regex-pattern.cxx | 93 ++++++++++++++++++++++++++++++---- libbuild2/adhoc-rule-regex-pattern.hxx | 2 +- libbuild2/build/script/parser.cxx | 2 +- libbuild2/dyndep.cxx | 71 ++++++++++++++++++-------- libbuild2/dyndep.hxx | 8 ++- libbuild2/parser.cxx | 30 ++++++----- libbuild2/rule.hxx | 5 +- 8 files changed, 197 insertions(+), 71 deletions(-) diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index 7c987f1..cd760b4 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -326,22 +326,11 @@ namespace build2 group* g (t.is_a ()); // Explicit group. - // Inject pattern's ad hoc group members, if any. + // Inject pattern's ad hoc group members, if any (explicit group members + // are injected after reset below). // - if (pattern != nullptr) - { - // @@ TODO: expl: pattern: if first must be file, we should probably add - // them after the group's static_members. Suppress duplicates that - // are already in group::static_members. - - pattern->apply_adhoc_members (a, t, bs, me); - - // A pattern rule that matches an explicit group should not inject any - // ad hoc members. - // - if (g != nullptr) - assert (t.adhoc_member == nullptr); - } + if (g == nullptr && pattern != nullptr) + pattern->apply_group_members (a, t, bs, me); // Derive file names for the target and its static/ad hoc group members, // if any. @@ -357,27 +346,37 @@ namespace build2 // is a file. // for (const target& m: g->static_members) - { - if (auto* p = m.is_a ()) - p->derive_path (); - g->members.push_back (&m); - } g->members_static = g->members.size (); + if (pattern != nullptr) + { + pattern->apply_group_members (a, *g, bs, me); + g->members_static = g->members.size (); + } + if (g->members_static == 0) { if (!script.depdb_dyndep_dyn_target) fail << "group " << *g << " has no static or dynamic members"; } - else if (!g->members.front ()->is_a ()) + else { - // We use the first static member to derive depdb path, get mtime, - // etc. So it must be file-based. + 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"; + } + + // Derive paths for all the static members. // - fail << "first static member " << g->members.front () << " of group " - << *g << " is not a file"; + for (const target* m: g->members) + if (auto* p = m->is_a ()) + p->derive_path (); } } else @@ -404,6 +403,12 @@ namespace build2 g->members.push_back (&m); g->members_static = g->members.size (); + + if (pattern != nullptr) + { + pattern->apply_group_members (a, *g, bs, me); + g->members_static = g->members.size (); + } } } @@ -680,7 +685,7 @@ namespace build2 what, a, bs, *g, move (f), - map_ext, def_tt, filter)); + map_ext, def_tt, filter, true /* skip_match */)); if (r.second) g->members.push_back (&r.first); diff --git a/libbuild2/adhoc-rule-regex-pattern.cxx b/libbuild2/adhoc-rule-regex-pattern.cxx index 354b859..0f2ecc3 100644 --- a/libbuild2/adhoc-rule-regex-pattern.cxx +++ b/libbuild2/adhoc-rule-regex-pattern.cxx @@ -319,8 +319,14 @@ namespace build2 } void adhoc_rule_regex_pattern:: - apply_adhoc_members (action a, target& t, const scope&, match_extra&) const + apply_group_members (action a, target& t, const scope& bs, + match_extra&) const { + if (targets_.size () == 1) // The group/primary target is always present. + return; + + group* g (t.is_a ()); + const auto& mr (t.data (a)); for (auto i (targets_.begin () + 1); i != targets_.end (); ++i) @@ -348,14 +354,83 @@ namespace build2 d.normalize (); } - // @@ TODO: currently this uses type as the ad hoc member identity. - // - add_adhoc_member ( - t, - e.type, - move (d), - dir_path () /* out */, - substitute (t, mr, e.name.value, "ad hoc target group member")); + string n (substitute ( + t, + mr, + e.name.value, + (g != nullptr + ? "explicit target group member" + : "ad hoc target group member"))); + + // @@ TODO: what if name contains extension? Shouldn't we call + // split_name()? + + if (g != nullptr) + { + auto& ms (g->members); + + // These are conceptually static but they behave more like dynamic in + // that we likely need to insert the target, set its group, etc. + // + // Note: a custom version of the dyndep_rule::inject_group_member() + // logic. + // + auto l (search_new_locked ( + bs.ctx, + e.type, + move (d), + dir_path (), // Always in out. + move (n), + nullptr /* ext */, + &bs)); + + const target& t (l.first); // Note: non-const only if have lock. + + // Note: we don't need to match the group recipe directy due to the + // special ad hoc recipe/rule semantics for explicit group members + // in match_rule(). + // + if (l.second) + { + l.first.group = g; + l.second.unlock (); + } + else + { + if (find (ms.begin (), ms.end (), &t) != ms.end ()) + continue; + + // We can only update the group under lock. + // + target_lock tl (lock (a, t)); + + if (!tl) + fail << "group " << *g << " member " << t << " is already matched" << + info << "static group members specified by pattern rules cannot " + << "be used as prerequisites directly, only via group"; + + if (t.group == nullptr) + tl.target->group = g; + else if (t.group != g) + { + fail << "group " << *g << " member " << t + << " is already member of group " << *t.group; + } + } + + ms.push_back (&t); + } + else + { + // @@ TODO: currently this uses type as the ad hoc member identity. + // + add_adhoc_member ( + t, + e.type, + move (d), + dir_path (), // Always in out. + move (n)); + } } } diff --git a/libbuild2/adhoc-rule-regex-pattern.hxx b/libbuild2/adhoc-rule-regex-pattern.hxx index eb75ea0..9cb7874 100644 --- a/libbuild2/adhoc-rule-regex-pattern.hxx +++ b/libbuild2/adhoc-rule-regex-pattern.hxx @@ -35,7 +35,7 @@ namespace build2 match (action, const target&, const string&, match_extra&) const override; virtual void - apply_adhoc_members (action, target&, + apply_group_members (action, target&, const scope&, match_extra&) const override; diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index d61b7d7..3b24802 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -2928,7 +2928,7 @@ namespace build2 what_tgt, a, bs, *g, f, // Can't move since need to return dyn_targets. - map_ext, *def_tt, filter)); + map_ext, *def_tt, filter, true /* skip_match */)); // Note: no target_decl shenanigans since reset the members on // each update. diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index d34834e..8781812 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -831,10 +831,11 @@ namespace build2 inject_group_member_impl (action a, const scope& bs, mtime_target& g, path f, string n, string e, const target_type& tt, - const function& fl) + const function& fl, + bool skip_match) { - // Assume nobody else can insert these members (seems reasonable seeing - // that their names are dynamically discovered). + // We expect that nobody else can insert these members (seems reasonable + // seeing that their names are dynamically discovered). // auto l (search_new_locked ( bs.ctx, @@ -847,29 +848,56 @@ namespace build2 const file& t (l.first.as ()); // Note: non-const only if have lock. - if (fl != nullptr && !fl (g, t)) - return pair (t, false); - - if (l.second) + bool locked (l.second); + if (locked) { l.first.group = &g; l.second.unlock (); - t.path (move (f)); // Only do this once. + + // We don't need to match the group recipe directy from ad hoc + // recipes/rules due to the special semantics for explicit group members + // in match_rule(). This is what skip_match is for. + // + if (skip_match) + { + t.path (move (f)); + return pair (t, true); + } } else - // Must have been already done (e.g., on previous operation in a - // batch). - // - assert (t.group == &g); + { + if (fl != nullptr && !fl (g, t)) + return pair (t, false); + } - // This shouldn't fail since we are the only ones that should be matching - // this target. + // This shouldn't normally fail since we are the only ones that should + // know about this target (otherwise why is it dynamicaly discovered). + // However, nothing prevents the user from depending on such a target, + // however misguided. // target_lock tl (lock (a, t)); - assert (tl); - match_inc_dependents (a, g); - match_recipe (tl, group_recipe); + if (!tl) + fail << "group " << g << " member " << t << " is already matched" << + info << "dynamically extracted group members cannot be used as " + << "prerequisites directly, only via group"; + + if (!locked) + { + if (t.group == nullptr) + tl.target->group = &g; + else if (t.group != &g) + fail << "group " << g << " member " << t + << " is already member of group " << *t.group; + } + + t.path (move (f)); + + if (!skip_match) + { + match_inc_dependents (a, g); + match_recipe (tl, group_recipe); + } return pair (t, true); } @@ -885,7 +913,8 @@ namespace build2 return inject_group_member_impl (a, bs, g, move (f), move (n).string (), move (e), tt, - nullptr /* filter */).first; + nullptr /* filter */, + false).first; } static const target_type& @@ -931,7 +960,8 @@ namespace build2 path f, const function& map_ext, const target_type& fallback, - const function& filter) + const function& filter, + bool skip_match) { path n (f.leaf ()); string e (n.extension ()); @@ -945,7 +975,8 @@ namespace build2 return inject_group_member_impl (a, bs, g, move (f), move (n).string (), move (e), tt, - filter); + filter, + skip_match); } pair dyndep_rule:: diff --git a/libbuild2/dyndep.hxx b/libbuild2/dyndep.hxx index 0463215..07511ee 100644 --- a/libbuild2/dyndep.hxx +++ b/libbuild2/dyndep.hxx @@ -256,6 +256,11 @@ namespace build2 // // If specified, the group_filter function is called on the target before // making it a group member, skipping it if this function returns false. + // Note that the filter is skipped if the target is newly inserted (the + // filter is meant to be used to skip duplicates). + // + // Note that skip_match is an implementation detail and should not be + // used. // using group_filter_func = bool (mtime_target& g, const file&); @@ -265,7 +270,8 @@ namespace build2 path, const function&, const target_type& fallback, - const function& = nullptr); + const function& = nullptr, + bool skip_match = false); // Find or insert a target file path as a target, make it a member of the diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 692e284..1b8e2d3 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -1263,22 +1263,17 @@ namespace build2 check_pattern (n, nloc); - // If we have group members, make sure it's for an ad hoc group. A - // rule for an explicit group that wishes to match based on some of - // its members feels far fetched. + // If we have group members, verify all the members are patterns or + // substitutions (ad hoc) or subsitutions (explicit) and of the + // correct pattern type. A rule for an explicit group that wishes to + // match based on some of its members feels far fetched. // - // @@ TODO: expl: pattern: this can be used to inject static members - // (which otherwise would be tedious to repeat). + // For explicit groups the use-case is to inject static members + // which could otherwise be tedious to specify for each group. // const location& mloc (gns.empty () ? location () : gns[0].member_loc); - - if (!gns.empty () && gns[0].expl) - fail (mloc) << "explicit group members in ad hoc pattern rule"; - - // Then verify all the ad hoc members are patterns or substitutions - // and of the correct type. - // names ns (gns.empty () ? names () : move (gns[0].ns)); + bool expl (gns.empty () ? false : gns[0].expl); for (name& n: ns) { @@ -1289,7 +1284,12 @@ namespace build2 } if (*n.pattern != pattern_type::regex_substitution) + { + if (expl) + fail (mloc) << "explicit group member pattern " << n; + check_pattern (n, mloc); + } } // The same for prerequisites except here we can have non-patterns. @@ -1351,6 +1351,12 @@ namespace build2 if (ttype == nullptr) fail (nloc) << "unknown target type " << n.type; + if (!gns.empty ()) + { + if (ttype->is_a () != expl) + fail (nloc) << "group type and target type mismatch"; + } + unique_ptr rp; switch (pt) { diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx index b89821b..4f77432 100644 --- a/libbuild2/rule.hxx +++ b/libbuild2/rule.hxx @@ -333,8 +333,11 @@ namespace build2 virtual bool match (action, const target&, const string&, match_extra&) const = 0; + // Append additional group members. Note that this function should handle + // both ad hoc and explicit groups. + // virtual void - apply_adhoc_members (action, target&, + apply_group_members (action, target&, const scope& base, match_extra&) const = 0; -- cgit v1.1