diff options
| -rw-r--r-- | libbuild2/adhoc-rule-buildscript.cxx | 12 | ||||
| -rw-r--r-- | libbuild2/adhoc-rule-regex-pattern.cxx | 14 | ||||
| -rw-r--r-- | libbuild2/algorithm.cxx | 51 | ||||
| -rw-r--r-- | libbuild2/build/script/parser.cxx | 2 | ||||
| -rw-r--r-- | libbuild2/dyndep.cxx | 63 | ||||
| -rw-r--r-- | libbuild2/dyndep.hxx | 9 | ||||
| -rw-r--r-- | libbuild2/operation.cxx | 21 | ||||
| -rw-r--r-- | libbuild2/target-type.hxx | 3 | ||||
| -rw-r--r-- | libbuild2/target.cxx | 6 |
9 files changed, 89 insertions, 92 deletions
diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index cd760b4..cf45699 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -295,8 +295,7 @@ namespace build2 { tracer trace ("adhoc_buildscript_rule::apply"); - // Handle matching explicit group members (see adhoc_rule::match() for - // background). + // Handle matching group members (see adhoc_rule::match() for background). // if (const group* g = t.group != nullptr ? t.group->is_a<group> () : nullptr) { @@ -681,11 +680,10 @@ namespace build2 if (g != nullptr) { pair<const build2::file&, bool> r ( - dyndep::inject_group_member ( - what, - a, bs, *g, - move (f), - map_ext, def_tt, filter, true /* skip_match */)); + dyndep::inject_group_member (what, + a, bs, *g, + move (f), + map_ext, def_tt, filter)); 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 3f207dc..257952f 100644 --- a/libbuild2/adhoc-rule-regex-pattern.cxx +++ b/libbuild2/adhoc-rule-regex-pattern.cxx @@ -370,7 +370,8 @@ namespace build2 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. + // that we likely need to insert the target, set its group, etc. In a + // sense, they are rule-static, but group-dynamic. // // Note: a custom version of the dyndep_rule::inject_group_member() // logic. @@ -386,10 +387,6 @@ namespace build2 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; @@ -400,13 +397,6 @@ namespace build2 if (find (ms.begin (), ms.end (), &t) != ms.end ()) continue; - // Check if we already belong to this group. Note that this not a - // mere optimization since we may be in the member->group->member - // chain and trying to lock the member the second time would - // deadlock (this can be triggered, for example, by dist, which sort - // of depends on such members directly @@ maybe this should be fixed - // there?). - // if (t.group != g) // Note: atomic. { // We can only update the group under lock. diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index cf972e7..581da2e 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -521,42 +521,45 @@ namespace build2 return dynamic_cast<const fallback_rule*> (&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<group> () : nullptr) + if (const target* g = t.group) { - 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 this is a group with dynamic members, then match it with the + // group's rule automatically. See dyndep_rule::inject_group_member() + // for background. // - if (g->matched (a, memory_order_acquire)) + if ((g->type ().flags & target_type::flag::dyn_members) == + target_type::flag::dyn_members) { - const rule_match* r (g->state[a].rule); - - if (r != nullptr && adhoc_rule_match (*r)) + if (g->matched (a, memory_order_acquire)) + { + const rule_match* r (g->state[a].rule); + assert (r != nullptr); // Shouldn't happen with dyn_members. return r; + } - // Fall through: if some rule matched the group then it must also deal - // with the members. + // Assume static member and fall through. } - else + + // 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. + // + // Note that such a group is dyn_members so we would have tried the + // "already matched" case above. + // + if (g->is_a<group> ()) { // 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<group&> (*g), skip, true /* try_match */, &me)) + a, const_cast<target&> (*g), skip, true /* try_match */, &me)) return r; // Fall through to normal match of the member. diff --git a/libbuild2/build/script/parser.cxx b/libbuild2/build/script/parser.cxx index 85fe429..7d8a881 100644 --- a/libbuild2/build/script/parser.cxx +++ b/libbuild2/build/script/parser.cxx @@ -2968,7 +2968,7 @@ namespace build2 what_tgt, a, bs, *g, f, // Can't move since need to return dyn_targets. - map_ext, *def_tt, filter, true /* skip_match */)); + map_ext, *def_tt, filter)); // Note: no target_decl shenanigans since reset the members on // each update. diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index 5ac9648..9c0f8a8 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -831,12 +831,21 @@ 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<dyndep_rule::group_filter_func>& fl, - bool skip_match) + const function<dyndep_rule::group_filter_func>& fl) { // NOTE: see adhoc_rule_regex_pattern::apply_group_members() for a variant // of the same code. + // Note that we used to directly match such a member with group_recipe. + // But that messes up our dependency counts since we don't really know + // whether someone will execute such a member. + // + // So instead we now just link the member up to the group and rely on the + // special semantics in match_rule() for groups with the dyn_members flag. + // + assert ((g.type ().flags & target_type::flag::dyn_members) == + target_type::flag::dyn_members); + // We expect that nobody else can insert these members (seems reasonable // seeing that their names are dynamically discovered). // @@ -851,21 +860,15 @@ namespace build2 const file& t (l.first.as<file> ()); // Note: non-const only if have lock. - bool locked (l.second); - if (locked) + // 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 (l.second) { l.first.group = &g; l.second.unlock (); - - // 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<const file&, bool> (t, true); - } + t.path (move (f)); + return pair<const file&, bool> (t, true); } else { @@ -877,9 +880,9 @@ namespace build2 // optimization since we may be in the member->group->member chain and // trying to lock the member the second time would deadlock (this can be // triggered, for example, by dist, which sort of depends on such members - // directly @@ maybe this should be fixed there?). + // directly... which was not quite correct and is now fixed). // - if (t.group == &g && skip_match) // Note: group query is atomic. + if (t.group == &g) // Note: atomic. t.path (move (f)); else { @@ -895,22 +898,13 @@ namespace build2 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; - } + 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<const file&, bool> (t, true); @@ -927,8 +921,7 @@ namespace build2 return inject_group_member_impl (a, bs, g, move (f), move (n).string (), move (e), tt, - nullptr /* filter */, - false).first; + nullptr /* filter */).first; } static const target_type& @@ -974,8 +967,7 @@ namespace build2 path f, const function<map_extension_func>& map_ext, const target_type& fallback, - const function<group_filter_func>& filter, - bool skip_match) + const function<group_filter_func>& filter) { path n (f.leaf ()); string e (n.extension ()); @@ -989,8 +981,7 @@ namespace build2 return inject_group_member_impl (a, bs, g, move (f), move (n).string (), move (e), tt, - filter, - skip_match); + filter); } pair<const file&, bool> dyndep_rule:: diff --git a/libbuild2/dyndep.hxx b/libbuild2/dyndep.hxx index 07511ee..ee9c1dd 100644 --- a/libbuild2/dyndep.hxx +++ b/libbuild2/dyndep.hxx @@ -232,7 +232,8 @@ namespace build2 // set its path, and match it with group_recipe. // // The file path must be absolute and normalized. Note that this function - // assumes that this member can only be matched via this group. + // assumes that this member can only be matched via this group. The group + // type must have the target_type::flag::dyn_members flag. // // Note: we can split this function into {enter,match}_group_member() // if necessary. @@ -259,9 +260,6 @@ namespace build2 // 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&); static pair<const file&, bool> @@ -270,8 +268,7 @@ namespace build2 path, const function<map_extension_func>&, const target_type& fallback, - const function<group_filter_func>& = nullptr, - bool skip_match = false); + const function<group_filter_func>& = nullptr); // Find or insert a target file path as a target, make it a member of the diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index f199827..7355f38 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -774,6 +774,21 @@ namespace build2 if (ctx.dependency_count.load (memory_order_relaxed) != 0) { + auto dependents = [base = ctx.count_base ()] (action a, + const target& t) + { + const target::opstate& s (t.state[a]); + + // Only consider targets that have been matched for this operation + // (since matching is what causes the dependents count reset). + // + size_t c (s.task_count.load (memory_order_relaxed) - base); + + return (c >= target::offset_applied + ? s.dependents.load (memory_order_relaxed) + : 0); + }; + diag_record dr; dr << info << "detected unexecuted matched targets:"; @@ -781,14 +796,12 @@ namespace build2 { const target& t (*pt); - if (size_t n = t[a].dependents.load (memory_order_relaxed)) + if (size_t n = dependents (a, t)) dr << text << t << ' ' << n; if (a.outer ()) { - action ia (a.inner_action ()); - - if (size_t n = t[ia].dependents.load (memory_order_relaxed)) + if (size_t n = dependents (a.inner_action (), t)) dr << text << t << ' ' << n; } } diff --git a/libbuild2/target-type.hxx b/libbuild2/target-type.hxx index 735532b..a0fc5a2 100644 --- a/libbuild2/target-type.hxx +++ b/libbuild2/target-type.hxx @@ -109,7 +109,8 @@ namespace build2 none = 0, group = 0x01, // A (non-adhoc) group. see_through = group | 0x02, // A group with "see through" semantics. - member_hint = group | 0x04 // Untyped rule hint applies to members. + member_hint = group | 0x04, // Untyped rule hint applies to members. + dyn_members = group | 0x08 // A group with dynamic members. }; flag flags; diff --git a/libbuild2/target.cxx b/libbuild2/target.cxx index c1002b9..6b2c702 100644 --- a/libbuild2/target.cxx +++ b/libbuild2/target.cxx @@ -1263,7 +1263,11 @@ namespace build2 nullptr, nullptr, &target_search, - target_type::flag::group + // + // Note that the dyn_members semantics is used not only to handle + // depdb-dyndep --dyn-target, but also pattern rule-static members. + // + target_type::flag::group | target_type::flag::dyn_members }; // alias |