diff options
| -rw-r--r-- | libbuild2/adhoc-rule-buildscript.cxx | 4 | ||||
| -rw-r--r-- | libbuild2/adhoc-rule-cxx.cxx | 12 | ||||
| -rw-r--r-- | libbuild2/adhoc-rule-cxx.hxx | 3 | ||||
| -rw-r--r-- | libbuild2/algorithm.cxx | 322 | ||||
| -rw-r--r-- | libbuild2/algorithm.hxx | 88 | ||||
| -rw-r--r-- | libbuild2/algorithm.ixx | 112 | ||||
| -rw-r--r-- | libbuild2/dyndep.cxx | 6 | ||||
| -rw-r--r-- | libbuild2/file.cxx | 2 | ||||
| -rw-r--r-- | libbuild2/operation.cxx | 11 | ||||
| -rw-r--r-- | libbuild2/parser.cxx | 3 | ||||
| -rw-r--r-- | libbuild2/rule.cxx | 13 | ||||
| -rw-r--r-- | libbuild2/rule.hxx | 11 | ||||
| -rw-r--r-- | libbuild2/scheduler.hxx | 8 | ||||
| -rw-r--r-- | libbuild2/scheduler.txx | 4 | ||||
| -rw-r--r-- | libbuild2/target.hxx | 107 | ||||
| -rw-r--r-- | libbuild2/target.ixx | 4 |
16 files changed, 537 insertions, 173 deletions
diff --git a/libbuild2/adhoc-rule-buildscript.cxx b/libbuild2/adhoc-rule-buildscript.cxx index 3c42e66..0263ddd 100644 --- a/libbuild2/adhoc-rule-buildscript.cxx +++ b/libbuild2/adhoc-rule-buildscript.cxx @@ -301,7 +301,9 @@ namespace build2 // if (const group* g = t.group != nullptr ? t.group->is_a<group> () : nullptr) { - match_sync (a, *g); + // Note: this looks very similar to how we handle ad hoc group members. + // + match_sync (a, *g, 0 /* options */); return group_recipe; // Execute the group's recipe. } diff --git a/libbuild2/adhoc-rule-cxx.cxx b/libbuild2/adhoc-rule-cxx.cxx index e3dfe92..637f9af 100644 --- a/libbuild2/adhoc-rule-cxx.cxx +++ b/libbuild2/adhoc-rule-cxx.cxx @@ -691,10 +691,20 @@ namespace build2 ? t.group->is_a<group> () : nullptr)) { - match_sync (a, *g); + // @@ Hm, this looks very similar to how we handle ad hoc group members. + // Shouldn't impl be given a chance to translate options or some + // such? + // + match_sync (a, *g, 0 /* options */); return group_recipe; // Execute the group's recipe. } return impl.load (memory_order_relaxed)->apply (a, t, me); } + + void adhoc_cxx_rule:: + reapply (action a, target& t, match_extra& me) const + { + return impl.load (memory_order_relaxed)->reapply (a, t, me); + } } diff --git a/libbuild2/adhoc-rule-cxx.hxx b/libbuild2/adhoc-rule-cxx.hxx index b563881..89bc05f 100644 --- a/libbuild2/adhoc-rule-cxx.hxx +++ b/libbuild2/adhoc-rule-cxx.hxx @@ -70,6 +70,9 @@ namespace build2 virtual recipe apply (action, target&, match_extra&) const override; + virtual void + reapply (action, target&, match_extra&) const override; + adhoc_cxx_rule (string, const location&, size_t, uint64_t ver, optional<string> sep); diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index 01d5016..bc4b835 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -232,8 +232,14 @@ namespace build2 // If the work_queue is absent, then we don't wait. // + // While already applied or executed targets are normally not locked, if + // options contain any bits that are not already in cur_options, then the + // target is locked even in these states. + // target_lock - lock_impl (action a, const target& ct, optional<scheduler::work_queue> wq) + lock_impl (action a, const target& ct, + optional<scheduler::work_queue> wq, + uint64_t options) { context& ctx (ct.ctx); @@ -248,7 +254,8 @@ namespace build2 size_t appl (b + target::offset_applied); size_t busy (b + target::offset_busy); - atomic_count& task_count (ct[a].task_count); + const target::opstate& cs (ct[a]); + atomic_count& task_count (cs.task_count); while (!task_count.compare_exchange_strong ( e, @@ -281,9 +288,10 @@ namespace build2 e = ctx.sched->wait (busy - 1, task_count, u, *wq); } - // We don't lock already applied or executed targets. + // We don't lock already applied or executed targets unless there + // are new options. // - if (e >= appl) + if (e >= appl && (cs.match_extra.cur_options & options) == options) return target_lock {a, nullptr, e - b, false}; } @@ -304,12 +312,7 @@ namespace build2 offset = target::offset_touched; } else - { offset = e - b; - assert (offset == target::offset_touched || - offset == target::offset_tried || - offset == target::offset_matched); - } return target_lock {a, &t, offset, first}; } @@ -448,7 +451,7 @@ namespace build2 auto match = [a, &t, &me] (const adhoc_rule& r, bool fallback) -> bool { - me.init (fallback); + me.reinit (fallback); if (auto* f = (a.outer () ? t.ctx.current_outer_oif @@ -550,10 +553,11 @@ 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_extra* pme) + match_rule_impl (action a, target& t, + uint64_t options, + const rule* skip, + bool try_match, + match_extra* pme) { using fallback_rule = adhoc_rule_pattern::fallback_rule; @@ -567,6 +571,12 @@ namespace build2 return dynamic_cast<const fallback_rule*> (&r.second.get ()); }; + // Note: we copy the options value to me.new_options after successfully + // matching the rule to make sure rule::match() implementations don't rely + // on it. + // + match_extra& me (pme == nullptr ? t[a].match_extra : *pme); + if (const target* g = t.group) { // If this is a group with dynamic members, then match it with the @@ -580,6 +590,8 @@ namespace build2 { const rule_match* r (g->state[a].rule); assert (r != nullptr); // Shouldn't happen with dyn_members. + + me.new_options = options; return r; } @@ -587,8 +599,8 @@ namespace build2 } // 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 + // 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 @@ -603,10 +615,16 @@ namespace build2 // 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<target&> (*g), skip, true /* try_match */, &me)) + match_extra gme (false /* locked */); + if (const rule_match* r = match_rule_impl (a, const_cast<target&> (*g), + 0 /* options */, + skip, + true /* try_match */, + &gme)) + { + me.new_options = options; return r; + } // Fall through to normal match of the member. } @@ -620,8 +638,6 @@ namespace build2 if (const scope* rs = bs.root_scope ()) penv = auto_project_env (*rs); - match_extra& me (pme == nullptr ? t[a].match_extra : *pme); - // First check for an ad hoc recipe. // // Note that a fallback recipe is preferred over a non-fallback rule. @@ -629,7 +645,10 @@ namespace build2 if (!t.adhoc_recipes.empty ()) { if (const rule_match* r = match_adhoc_recipe (a, t, me)) + { + me.new_options = options; return r; + } } // If this is an outer operation (Y-for-X), then we look for rules @@ -744,8 +763,9 @@ namespace build2 } } - // Skip non-ad hoc rules if the target is not locked (see - // above). + // Skip non-ad hoc rules if the target is not locked (see above; + // note that in this case match_extra is a temporary which we + // can reinit). // if (!me.locked && !adhoc_rule_match (*r)) continue; @@ -756,7 +776,7 @@ namespace build2 if (&ru == skip) continue; - me.init (oi == 0 /* fallback */); + me.reinit (oi == 0 /* fallback */); { auto df = make_diag_frame ( [a, &t, &n](const diag_record& dr) @@ -825,7 +845,10 @@ namespace build2 } if (!ambig) + { + me.new_options = options; return r; + } else dr << info << "use rule hint to disambiguate this match"; } @@ -938,10 +961,39 @@ namespace build2 recipe re (ar != nullptr ? f (*ar, a, t, me) : ru.apply (a, t, me)); - me.free (); + me.free (); // Note: cur_options are still in use. return re; } + static void + reapply_impl (action a, + target& t, + const pair<const string, reference_wrapper<const rule>>& m) + { + const scope& bs (t.base_scope ()); + + // Reapply rules in project environment. + // + auto_project_env penv; + if (const scope* rs = bs.root_scope ()) + penv = auto_project_env (*rs); + + const rule& ru (m.second); + match_extra& me (t[a].match_extra); + + auto df = make_diag_frame ( + [a, &t, &m](const diag_record& dr) + { + if (verb != 0) + dr << info << "while reapplying rule " << m.first << " to " + << diag_do (a, t); + }); + + // Note: for now no adhoc_reapply(). + // + ru.reapply (a, t, me); + } + // If anything goes wrong, set target state to failed and return false. // // Note: must be called while holding target_lock. @@ -1029,10 +1081,24 @@ namespace build2 // the first half of the result. // static pair<bool, target_state> - match_impl (target_lock& l, - bool step = false, - bool try_match = false) + match_impl_impl (target_lock& l, + uint64_t options, + bool step = false, + bool try_match = false) { + // With regards to options, the semantics that we need to achieve for each + // target::offeset_*: + // + // tried -- nothing to do (no match) + // touched -- set to new_options + // matched -- add to new_options + // applied -- reapply if any new options + // executed -- check and fail if any new options + // busy -- postpone until *_complete() call + // + // Note that if options is 0 (see resolve_{members,group}_impl()), then + // all this can be skipped. + assert (l.target != nullptr); action a (l.action); @@ -1045,10 +1111,6 @@ namespace build2 // if (t.adhoc_group_member ()) { - assert (!step); - - const target& g (*t.group); - // It feels natural to "convert" this call to the one for the group, // including the try_match part. Semantically, we want to achieve the // following: @@ -1056,6 +1118,29 @@ namespace build2 // [try_]match (a, g); // match_recipe (l, group_recipe); // + // Currently, ad hoc group members cannot have options. An alternative + // semantics could be to call the goup's rule to translate member + // options to group options and then (re)match the group with that. + // The implementation of this semantics could look like this: + // + // 1. Lock the group. + // 2. If not already offset_matched, do one step to get the rule. + // 3. Call the rule to translate options. + // 4. Continue matching the group passing the translated options. + // 5. Keep track of member options in member's cur_options to handle + // member rematches (if already offset_{applied,executed}). + // + // Note: see also similar semantics but for explicit groups in + // adhoc-rule-*.cxx. + + assert (!step && options == match_extra::all_options); + + const target& g (*t.group); + + // What should we do with options? After some rumination it fells most + // natural to treat options for the group and for its ad hoc member as + // the same entity ... or not. + // auto df = make_diag_frame ( [a, &t](const diag_record& dr) { @@ -1063,14 +1148,22 @@ namespace build2 dr << info << "while matching group rule to " << diag_do (a, t); }); - pair<bool, target_state> r (match_impl (a, g, 0, nullptr, try_match)); + pair<bool, target_state> r ( + match_impl (a, g, 0 /* options */, 0, nullptr, try_match)); if (r.first) { if (r.second != target_state::failed) { + // Note: in particular, passing all_options makes sure we will + // never re-lock this member if already applied/executed. + // match_inc_dependents (a, g); - match_recipe (l, group_recipe); + match_recipe (l, group_recipe, match_extra::all_options); + + // Note: no need to call match_posthoc() since an ad hoc member + // has no own prerequisites and the group's ones will be matched + // by the group. } } else @@ -1103,7 +1196,8 @@ namespace build2 // clear_target (a, t); - const rule_match* r (match_rule (a, t, nullptr, try_match)); + const rule_match* r ( + match_rule_impl (a, t, options, nullptr, try_match)); assert (l.offset != target::offset_tried); // Should have failed. @@ -1125,25 +1219,67 @@ namespace build2 // Fall through. case target::offset_matched: { + // Add any new options. + // + s.match_extra.new_options |= options; + // Apply. // set_recipe (l, apply_impl (a, t, *s.rule)); l.offset = target::offset_applied; + + if (t.has_group_prerequisites ()) // Ok since already matched. + { + if (!match_posthoc (a, t)) + s.state = target_state::failed; + } + + break; + } + case target::offset_applied: + { + // Reapply if any new options. + // + match_extra& me (s.match_extra); + me.new_options = options & ~me.cur_options; // Clear existing. + assert (me.new_options != 0); // Otherwise should not have locked. + + // Feels like this can only be a logic bug since to end up with a + // subset of options requires a rule (see match_extra for details). + // + assert (s.rule != nullptr); + + reapply_impl (a, t, *s.rule); break; } + case target::offset_executed: + { + // Diagnose new options after execute. + // + match_extra& me (s.match_extra); + assert ((me.cur_options & options) != options); // Otherwise no lock. + + fail << "change of match options after " << diag_do (a, t) + << " has been executed" << + info << "executed options 0x" << hex << me.cur_options << + info << "requested options 0x" << hex << options << endf; + } default: assert (false); } } catch (const failed&) { + s.state = target_state::failed; + l.offset = target::offset_applied; + } + + if (s.state == target_state::failed) + { // As a sanity measure clear the target data since it can be incomplete // or invalid (mark()/unmark() should give you some ideas). // clear_target (a, t); - - s.state = target_state::failed; - l.offset = target::offset_applied; } return make_pair (true, s.state); @@ -1153,10 +1289,9 @@ namespace build2 // the first half of the result. // pair<bool, target_state> - match_impl (action a, - const target& ct, - size_t start_count, - atomic_count* task_count, + match_impl (action a, const target& ct, + uint64_t options, + size_t start_count, atomic_count* task_count, bool try_match) { // If we are blocking then work our own queue one task at a time. The @@ -1178,30 +1313,16 @@ namespace build2 ct, task_count == nullptr ? optional<scheduler::work_queue> (scheduler::work_none) - : nullopt)); + : nullopt, + options)); if (l.target != nullptr) { - assert (l.offset < target::offset_applied); // Shouldn't lock otherwise. - if (try_match && l.offset == target::offset_tried) return make_pair (false, target_state::unknown); if (task_count == nullptr) - { - pair<bool, target_state> r (match_impl (l, false /*step*/, try_match)); - - if (r.first && - r.second != target_state::failed && - l.offset == target::offset_applied && - ct.has_group_prerequisites ()) // Already matched. - { - if (!match_posthoc (a, *l.target)) - r.second = target_state::failed; - } - - return r; - } + return match_impl_impl (l, options, false /* step */, try_match); // Pass "disassembled" lock since the scheduler queue doesn't support // task destruction. @@ -1211,12 +1332,18 @@ namespace build2 // Also pass our diagnostics and lock stacks (this is safe since we // expect the caller to wait for completion before unwinding its stack). // + // Note: pack captures and arguments a bit to reduce the storage space + // requrements. + // + bool first (ld.first); + if (ct.ctx.sched->async ( start_count, *task_count, - [a, try_match] (const diag_frame* ds, - const target_lock* ls, - target& t, size_t offset, bool first) + [a, try_match, first] (const diag_frame* ds, + const target_lock* ls, + target& t, size_t offset, + uint64_t options) { // Switch to caller's diag and lock stacks. // @@ -1230,24 +1357,15 @@ namespace build2 // Note: target_lock must be unlocked within the match phase. // target_lock l {a, &t, offset, first}; // Reassemble. - - pair<bool, target_state> r ( - match_impl (l, false /* step */, try_match)); - - if (r.first && - r.second != target_state::failed && - l.offset == target::offset_applied && - t.has_group_prerequisites ()) // Already matched. - match_posthoc (a, t); + match_impl_impl (l, options, false /* step */, try_match); } } catch (const failed&) {} // Phase lock failure. }, diag_frame::stack (), target_lock::stack (), - ref (*ld.target), - ld.offset, - ld.first)) + ref (*ld.target), ld.offset, + options)) return make_pair (true, target_state::postponed); // Queued. // Matched synchronously, fall through. @@ -1266,16 +1384,28 @@ namespace build2 } void - match_only_sync (action a, const target& t) + match_only_sync (action a, const target& t, uint64_t options) { assert (t.ctx.phase == run_phase::match); - target_lock l (lock_impl (a, t, scheduler::work_none)); + target_lock l (lock_impl (a, t, scheduler::work_none, options)); - if (l.target != nullptr && l.offset < target::offset_matched) + if (l.target != nullptr) { - if (match_impl (l, true /* step */).second == target_state::failed) - throw failed (); + if (l.offset != target::offset_matched) + { + if (match_impl_impl (l, + options, + true /* step */).second == target_state::failed) + throw failed (); + } + else + { + // If the target is already matched, then we need to add any new + // options but not call apply() (thus cannot use match_impl_impl()). + // + (*l.target)[a].match_extra.new_options |= options; + } } } @@ -1299,11 +1429,11 @@ namespace build2 { // Match (locked). // - if (match_impl (l, true /* step */).second == target_state::failed) + if (match_impl_impl (l, + 0 /* options */, + true /* step */).second == target_state::failed) throw failed (); - // Note: only matched so no call to match_posthoc(). - if ((r = g.group_members (a)).members != nullptr) break; @@ -1314,14 +1444,8 @@ namespace build2 { // Apply (locked). // - pair<bool, target_state> s (match_impl (l, true /* step */)); - - if (s.second != target_state::failed && - g.has_group_prerequisites ()) // Already matched. - { - if (!match_posthoc (a, *l.target)) - s.second = target_state::failed; - } + pair<bool, target_state> s ( + match_impl_impl (l, 0 /* options */, true /* step */)); if (s.second == target_state::failed) throw failed (); @@ -1435,21 +1559,15 @@ namespace build2 // Note: lock is a reference to avoid the stacking overhead. // void - resolve_group_impl (action a, const target& t, target_lock&& l) + resolve_group_impl (target_lock&& l) { - assert (a.inner ()); + assert (l.action.inner ()); pair<bool, target_state> r ( - match_impl (l, true /* step */, true /* try_match */)); - - if (r.first && - r.second != target_state::failed && - l.offset == target::offset_applied && - t.has_group_prerequisites ()) // Already matched. - { - if (!match_posthoc (a, *l.target)) - r.second = target_state::failed; - } + match_impl_impl (l, + 0 /* options */, + true /* step */, + true /* try_match */)); l.unlock (); diff --git a/libbuild2/algorithm.hxx b/libbuild2/algorithm.hxx index 983e7f5..19f7db2 100644 --- a/libbuild2/algorithm.hxx +++ b/libbuild2/algorithm.hxx @@ -363,25 +363,34 @@ namespace build2 // to be unchanged after match. If it is unmatch::safe, then unmatch the // target if it is safe (this includes unchanged or if we know that someone // else will execute this target). Return true in first half of the pair if - // unmatch succeeded. Always throw if failed. + // unmatch succeeded. Always throw if failed. Note that unmatching may not + // play well with options -- if unmatch succeeds, the options that have been + // passed to match will not be cleared. // enum class unmatch {none, unchanged, safe}; target_state - match_sync (action, const target&, bool fail = true); + match_sync (action, const target&, + uint64_t options = match_extra::all_options, + bool fail = true); pair<bool, target_state> - try_match_sync (action, const target&, bool fail = true); + try_match_sync (action, const target&, + uint64_t options = match_extra::all_options, + bool fail = true); pair<bool, target_state> - match_sync (action, const target&, unmatch); + match_sync (action, const target&, + unmatch, + uint64_t options = match_extra::all_options); // As above but only match the target (unless already matched) without // applying the match (which is normally done with match_sync()). You will // most likely regret using this function. // LIBBUILD2_SYMEXPORT void - match_only_sync (action, const target&); + match_only_sync (action, const target&, + uint64_t options = match_extra::all_options); // Start asynchronous match. Return target_state::postponed if the // asynchronous operation has been started and target_state::busy if the @@ -393,16 +402,23 @@ namespace build2 // failed. Otherwise, throw the failed exception if keep_going is false and // return target_state::failed otherwise. // + // Note: same options must be passed to match_async() and match_complete(). + // target_state match_async (action, const target&, size_t start_count, atomic_count& task_count, + uint64_t options = match_extra::all_options, bool fail = true); target_state - match_complete (action, const target&, bool fail = true); + match_complete (action, const target&, + uint64_t options = match_extra::all_options, + bool fail = true); pair<bool, target_state> - match_complete (action, const target&, unmatch); + match_complete (action, const target&, + unmatch, + uint64_t options = match_extra::all_options); // As above but without incrementing the target's dependents count. Should // be executed with execute_direct_*(). @@ -410,22 +426,34 @@ namespace build2 // For async, call match_async() followed by match_direct_complete(). // target_state - match_direct_sync (action, const target&, bool fail = true); + match_direct_sync (action, const target&, + uint64_t options = match_extra::all_options, + bool fail = true); target_state - match_direct_complete (action, const target&, bool fail = true); + match_direct_complete (action, const target&, + uint64_t options = match_extra::all_options, + bool fail = true); // Apply the specified recipe directly and without incrementing the // dependency counts. The target must be locked. // + // Note that there will be no way to rematch on options change (since there + // is no rule), so passing anything other than all_options is most likely a + // bad idea. + // void - match_recipe (target_lock&, recipe); + match_recipe (target_lock&, + recipe, + uint64_t options = match_extra::all_options); // Match (but do not apply) the specified rule directly and without // incrementing the dependency counts. The target must be locked. // void - match_rule (target_lock&, const rule_match&); + match_rule (target_lock&, + const rule_match&, + uint64_t options = match_extra::all_options); // Match a "delegate rule" from withing another rules' apply() function // avoiding recursive matches (thus the third argument). Unless try_match is @@ -434,7 +462,10 @@ namespace build2 // See also the companion execute_delegate(). // recipe - match_delegate (action, target&, const rule&, bool try_match = false); + match_delegate (action, target&, + const rule&, + uint64_t options = match_extra::all_options, + bool try_match = false); // Incrementing the dependency counts of the specified target. // @@ -446,10 +477,39 @@ namespace build2 // and inner_recipe. // target_state - match_inner (action, const target&); + match_inner (action, const target&, + uint64_t options = match_extra::all_options); pair<bool, target_state> - match_inner (action, const target&, unmatch); + match_inner (action, const target&, + unmatch, + uint64_t options = match_extra::all_options); + + // Re-match with new options a target that has already been matched with one + // of the match_*() functions. Note that natually you cannot rematch a + // target that you have unmatched. + // + // Note also that there is no way to check if the rematch is unnecessary + // (i.e., because the target is already matched with this option) because + // that would require MT-safety considerations (since there could be a + // concurrent rematch). Instead, you should rematch unconditionally and if + // the option is already present, it will be a cheap noop. + // + target_state + rematch_sync (action, const target&, + uint64_t options, + bool fail = true); + + target_state + rematch_async (action, const target&, + size_t start_count, atomic_count& task_count, + uint64_t options, + bool fail = true); + + target_state + rematch_complete (action, const target&, + uint64_t options, + bool fail = true); // The standard prerequisite search and match implementations. They call // search() (unless a custom is provided) and then match() (unless custom diff --git a/libbuild2/algorithm.ixx b/libbuild2/algorithm.ixx index 6d83984..09fc6d9 100644 --- a/libbuild2/algorithm.ixx +++ b/libbuild2/algorithm.ixx @@ -214,7 +214,9 @@ namespace build2 } LIBBUILD2_SYMEXPORT target_lock - lock_impl (action, const target&, optional<scheduler::work_queue>); + lock_impl (action, const target&, + optional<scheduler::work_queue>, + uint64_t = 0); LIBBUILD2_SYMEXPORT void unlock_impl (action, target&, size_t); @@ -392,16 +394,18 @@ namespace build2 } LIBBUILD2_SYMEXPORT const rule_match* - match_rule (action, target&, - const rule* skip, - bool try_match = false, - match_extra* = nullptr); + match_rule_impl (action, target&, + uint64_t options, + const rule* skip, + bool try_match = false, + match_extra* = nullptr); LIBBUILD2_SYMEXPORT recipe apply_impl (action, target&, const rule_match&); LIBBUILD2_SYMEXPORT pair<bool, target_state> match_impl (action, const target&, + uint64_t options, size_t, atomic_count*, bool try_match = false); @@ -413,11 +417,11 @@ namespace build2 } inline target_state - match_sync (action a, const target& t, bool fail) + match_sync (action a, const target& t, uint64_t options, bool fail) { assert (t.ctx.phase == run_phase::match); - target_state r (match_impl (a, t, 0, nullptr).second); + target_state r (match_impl (a, t, options, 0, nullptr).second); if (r != target_state::failed) match_inc_dependents (a, t); @@ -428,12 +432,12 @@ namespace build2 } inline pair<bool, target_state> - try_match_sync (action a, const target& t, bool fail) + try_match_sync (action a, const target& t, uint64_t options, bool fail) { assert (t.ctx.phase == run_phase::match); pair<bool, target_state> r ( - match_impl (a, t, 0, nullptr, true /* try_match */)); + match_impl (a, t, options, 0, nullptr, true /* try_match */)); if (r.first) { @@ -447,11 +451,11 @@ namespace build2 } inline pair<bool, target_state> - match_sync (action a, const target& t, unmatch um) + match_sync (action a, const target& t, unmatch um, uint64_t options) { assert (t.ctx.phase == run_phase::match); - target_state s (match_impl (a, t, 0, nullptr).second); + target_state s (match_impl (a, t, options, 0, nullptr).second); if (s == target_state::failed) throw failed (); @@ -492,12 +496,13 @@ namespace build2 inline target_state match_async (action a, const target& t, size_t sc, atomic_count& tc, + uint64_t options, bool fail) { context& ctx (t.ctx); assert (ctx.phase == run_phase::match); - target_state r (match_impl (a, t, sc, &tc).second); + target_state r (match_impl (a, t, options, sc, &tc).second); if (r == target_state::failed && fail && !ctx.keep_going) throw failed (); @@ -506,23 +511,23 @@ namespace build2 } inline target_state - match_complete (action a, const target& t, bool fail) + match_complete (action a, const target& t, uint64_t options, bool fail) { - return match_sync (a, t, fail); + return match_sync (a, t, options, fail); } inline pair<bool, target_state> - match_complete (action a, const target& t, unmatch um) + match_complete (action a, const target& t, unmatch um, uint64_t options) { - return match_sync (a, t, um); + return match_sync (a, t, um, options); } inline target_state - match_direct_sync (action a, const target& t, bool fail) + match_direct_sync (action a, const target& t, uint64_t options, bool fail) { assert (t.ctx.phase == run_phase::match); - target_state r (match_impl (a, t, 0, nullptr).second); + target_state r (match_impl (a, t, options, 0, nullptr).second); if (r == target_state::failed && fail) throw failed (); @@ -531,12 +536,14 @@ namespace build2 } inline target_state - match_direct_complete (action a, const target& t, bool fail) + match_direct_complete (action a, const target& t, + uint64_t options, + bool fail) { - return match_direct_sync (a, t, fail); + return match_direct_sync (a, t, options, fail); } - // Clear rule match-specific target data. + // Clear rule match-specific target data (except match_extra). // inline void clear_target (action a, target& t) @@ -605,12 +612,16 @@ namespace build2 } inline void - match_recipe (target_lock& l, recipe r) + match_recipe (target_lock& l, recipe r, uint64_t options) { assert (l.target != nullptr && - l.offset != target::offset_matched && + l.offset < target::offset_matched && l.target->ctx.phase == run_phase::match); + match_extra& me ((*l.target)[l.action].match_extra); + + me.reinit (false /* fallback */); + me.cur_options = options; // Already applied, so cur_, not new_options. clear_target (l.action, *l.target); set_rule (l, nullptr); // No rule. set_recipe (l, move (r)); @@ -618,47 +629,82 @@ namespace build2 } inline void - match_rule (target_lock& l, const rule_match& r) + match_rule (target_lock& l, const rule_match& r, uint64_t options) { assert (l.target != nullptr && - l.offset != target::offset_matched && + l.offset < target::offset_matched && l.target->ctx.phase == run_phase::match); + match_extra& me ((*l.target)[l.action].match_extra); + + me.reinit (false /* fallback */); + me.new_options = options; clear_target (l.action, *l.target); set_rule (l, &r); l.offset = target::offset_matched; } inline recipe - match_delegate (action a, target& t, const rule& dr, bool try_match) + match_delegate (action a, target& t, + const rule& dr, + uint64_t options, + bool try_match) { assert (t.ctx.phase == run_phase::match); // Note: we don't touch any of the t[a] state since that was/will be set // for the delegating rule. // - const rule_match* r (match_rule (a, t, &dr, try_match)); + const rule_match* r (match_rule_impl (a, t, options, &dr, try_match)); return r != nullptr ? apply_impl (a, t, *r) : empty_recipe; } inline target_state - match_inner (action a, const target& t) + match_inner (action a, const target& t, uint64_t options) { // In a sense this is like any other dependency. // assert (a.outer ()); - return match_sync (a.inner_action (), t); + return match_sync (a.inner_action (), t, options); } inline pair<bool, target_state> - match_inner (action a, const target& t, unmatch um) + match_inner (action a, const target& t, unmatch um, uint64_t options) { assert (a.outer ()); - return match_sync (a.inner_action (), t, um); + return match_sync (a.inner_action (), t, um, options); + } + + // Note: rematch is basically normal match but without the counts increment, + // so we just delegate to match_direct_*(). + // + inline target_state + rematch_sync (action a, const target& t, + uint64_t options, + bool fail) + { + return match_direct_sync (a, t, options, fail); + } + + inline target_state + rematch_async (action a, const target& t, + size_t start_count, atomic_count& task_count, + uint64_t options, + bool fail) + { + return match_async (a, t, start_count, task_count, options, fail); + } + + inline target_state + rematch_complete (action a, const target& t, + uint64_t options, + bool fail) + { + return match_direct_complete (a, t, options, fail); } LIBBUILD2_SYMEXPORT void - resolve_group_impl (action, const target&, target_lock&&); + resolve_group_impl (target_lock&&); inline const target* resolve_group (action a, const target& t) @@ -678,7 +724,7 @@ namespace build2 // then unlock and return. // if (t.group == nullptr && l.offset < target::offset_tried) - resolve_group_impl (a, t, move (l)); + resolve_group_impl (move (l)); break; } diff --git a/libbuild2/dyndep.cxx b/libbuild2/dyndep.cxx index e6d0643..c6294bb 100644 --- a/libbuild2/dyndep.cxx +++ b/libbuild2/dyndep.cxx @@ -863,7 +863,8 @@ namespace build2 // 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. + // special semantics in match_rule_impl() for groups with the dyn_members + // flag. // assert ((g.type ().flags & target_type::flag::dyn_members) == target_type::flag::dyn_members); @@ -884,7 +885,8 @@ namespace build2 // 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. + // in match_rule_impl(). This is what skip_match is for. + // if (l.second) { l.first.group = &g; diff --git a/libbuild2/file.cxx b/libbuild2/file.cxx index 2a09aea..1e6cb4c 100644 --- a/libbuild2/file.cxx +++ b/libbuild2/file.cxx @@ -3021,7 +3021,7 @@ namespace build2 { assert (pk.scope != nullptr); - // Note: similar to/inspired by match_rule(). + // Note: similar to/inspired by match_rule_impl(). // // Search scopes outwards, stopping at the project root. // diff --git a/libbuild2/operation.cxx b/libbuild2/operation.cxx index 4af03fe..7b6dc3c 100644 --- a/libbuild2/operation.cxx +++ b/libbuild2/operation.cxx @@ -330,7 +330,10 @@ namespace build2 const target& t (ts[i].as<target> ()); l5 ([&]{trace << diag_doing (a, t);}); - target_state s (match_async (a, t, 0, task_count, false)); + target_state s (match_async (a, t, + 0, task_count, + match_extra::all_options, + false /* fail */)); // Bail out if the target has failed and we weren't instructed to // keep going. @@ -382,7 +385,9 @@ namespace build2 // for (const target* pt: p.prerequisite_targets) { - target_state s (match_direct_sync (a, *pt, false /* fail */)); + target_state s (match_direct_sync (a, *pt, + match_extra::all_options, + false /* fail */)); if (s == target_state::failed) { @@ -419,7 +424,7 @@ namespace build2 target_state s; if (j < i) { - s = match_complete (a, t, false); + s = match_complete (a, t, match_extra::all_options, false /* fail */); if (posthoc_fail) s = /*t.state[a].state =*/ target_state::failed; diff --git a/libbuild2/parser.cxx b/libbuild2/parser.cxx index 55c5c6c..3b78072 100644 --- a/libbuild2/parser.cxx +++ b/libbuild2/parser.cxx @@ -2180,7 +2180,8 @@ namespace build2 fail (l) << "project " << *root_ << " does not support " << "operation " << ctx->operation_table[oi]; - // Note: for now always inner (see match_rule() for details). + // Note: for now always inner (see match_rule_impl() for + // details). // action a (mi, oi); diff --git a/libbuild2/rule.cxx b/libbuild2/rule.cxx index 13dd467..04d6b38 100644 --- a/libbuild2/rule.cxx +++ b/libbuild2/rule.cxx @@ -15,13 +15,22 @@ using namespace butl; namespace build2 { - // rule (vtable) + // rule // rule:: ~rule () { } + void rule:: + reapply (action, target&, match_extra&) const + { + // Unless the rule overrode cur_options, this function should never get + // called. And if it did, then it should override this function. + // + assert (false); + } + const target* rule:: import (const prerequisite_key&, const optional<string>&, @@ -37,7 +46,7 @@ namespace build2 sub_match (const string& n, operation_id o, action a, target& t, match_extra& me) const { - // First check for an ad hoc recipe (see match_rule() for details). + // First check for an ad hoc recipe (see match_rule_impl() for details). // if (!t.adhoc_recipes.empty ()) { diff --git a/libbuild2/rule.hxx b/libbuild2/rule.hxx index dea9c9a..acd22fe 100644 --- a/libbuild2/rule.hxx +++ b/libbuild2/rule.hxx @@ -34,6 +34,10 @@ namespace build2 // implementations. It is also a way for us to later pass more information // without breaking source compatibility. // + // A rule may support match options and if such a rule is rematched with + // different options, then reapply() is called. See + // match_extra::{cur,new}_options for background and details. + // struct match_extra; class LIBBUILD2_SYMEXPORT rule @@ -45,6 +49,9 @@ namespace build2 virtual recipe apply (action, target&, match_extra&) const = 0; + virtual void + reapply (action, target&, match_extra&) const; + rule () = default; virtual @@ -265,8 +272,8 @@ namespace build2 // 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 + // called on the group while apply() on the member (see match_rule_impl() + // 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 diff --git a/libbuild2/scheduler.hxx b/libbuild2/scheduler.hxx index dcddfcc..d3b8ceb 100644 --- a/libbuild2/scheduler.hxx +++ b/libbuild2/scheduler.hxx @@ -555,8 +555,8 @@ namespace build2 atomic_count* task_count; size_t start_count; - func_type func; args_type args; + func_type func; template <size_t... i> void @@ -685,7 +685,11 @@ namespace build2 // struct task_data { - alignas (std::max_align_t) unsigned char data[sizeof (void*) * 8]; + static const size_t data_size = (sizeof (void*) == 4 + ? sizeof (void*) * 16 + : sizeof (void*) * 8); + + alignas (std::max_align_t) unsigned char data[data_size]; void (*thunk) (scheduler&, lock&, void*); }; diff --git a/libbuild2/scheduler.txx b/libbuild2/scheduler.txx index 5c6b339..460c4d4 100644 --- a/libbuild2/scheduler.txx +++ b/libbuild2/scheduler.txx @@ -64,8 +64,8 @@ namespace build2 new (&td->data) task { &task_count, start_count, - decay_copy (forward<F> (f)), - typename task::args_type (decay_copy (forward<A> (a))...)}; + typename task::args_type (decay_copy (forward<A> (a))...), + decay_copy (forward<F> (f))}; td->thunk = &task_thunk<F, A...>; diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 1178371..f537d59 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -178,7 +178,96 @@ 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()). + bool fallback; // True if matching a fallback rule (see match_rule_impl()). + + // When matching a rule, the caller may wish to request a subset of the + // full functionality of performing the operation on the target. This is + // achieved with match options. + // + // Since the match caller normally has no control over which rule will be + // matched, the options are not specific to a particular rule. Rather, + // options are defined for performing a specific operation on a specific + // target type and would normally be part of the target type semantics. + // To put it another way, when a rule matches a target of certain type for + // certain operation, there is an expectation of certain semantics, some + // parts of which could be made optional. + // + // As a concrete example, consider installing libs{}, which traditionally + // has two parts: runtime (normally just the versioned shared library) and + // build-time (non-versioned symlinks, pkg-config files, headers, etc). + // The option to install only the runtime files is part of the bin::libs{} + // semantics, not of, say, cc::install_rule. + // + // The match options are specified as a uint64_t mask, which means there + // can be a maximum of 64 options per operation/target type. Options are + // opt-out rather than opt-in. That is, by default, all the options are + // enabled unless the match caller explicitly opted out of some + // functionality. Even if the caller opted out, there is no guarantee that + // the matching rule will honor this request (for example, because it is a + // user-provided ad hoc recipe). To put it another way, support for + // options is a quality of implementation matter. + // + // From the rule implementation's point view, match options are handled as + // follows: On initial match()/apply(), cur_options is initialized to ~0 + // (all options enabled) and the matching rule is expected to override it + // with new_options in apply() (note that match() should no base any + // decisions on new_options since they may change between match() and + // apply()). This way a rule that does not support any match options does + // not need to do anything. Subsequent match calls may add new options + // which causes a rematch that manifests in the rule's reapply() call. In + // reapply(), cur_options are the currently enabled options and + // new_options are the newly requested options. Here the rule is expected + // to factor new_options to cur_options as appropriate. Note also that on + // rematch, if current options already include new options, then no call + // to reapply() is made. This, in particular, means that a rule that does + // not adjust cur_options in match() will never get a reapply() call + // (because all the options are enabled from the start). If a rematch is + // triggered after the rule has already been executed, an error is issued. + // This means that match options are not usable for operation/target types + // that could plausibly be executed during match. In particular, using + // match options for update and clean operations is a bad idea (update of + // pretty much any target can happen during match as a result of a tool + // update while clean might have to be performed during match to provide + // the mirror semantics). + // + // Note also that with rematches the assumption that in the match phase + // after matching the target we can MT-safely examine its state (such as + // its prerequisite_targets) no longer holds since such state could be + // modified during a rematch. As a result, if the target type specifies + // options for a certain operation, then you should not rely on this + // assumption for targets of this type during this operation. + // + // A rule that supports match options must also be prepared to handle the + // apply() call with new_options set to 0, for example, by using a + // minimally supported set of options instead. While 0 usually won't be + // passed by the match caller, this value is passed in the following + // circumstances: + // + // - match to resolve group (resolve_group()) + // - match to resolve members (resolve_members()) + // - match of ad hoc group via one of its ad hoc members + // + // When it comes to match options specified for group members, the + // semantics differs between explicit and ad hoc groups. For explicit + // groups, the standard semantics described above applies and the group's + // reapply() function will be called both for the group itself as well as + // for its members and its the responsibility of the rule to decide what + // to do with the two sets of options (e.g., factor member's options into + // group's options, etc). For ad hoc groups, members are not matched to a + // rule but to the group_recipe directly (so there cannot be a call to + // reapply()). Currently, ad hoc group members cannot have options (more + // precisely, their options should always be ~0). An alternative semantics + // where the group rule is called to translate member options to group + // options may be implemented in the future (see match_impl_impl() for + // details). + // + // Note: match options are currently not exposed in Buildscript ad hoc + // recipes/rules (but are in C++). + // + uint64_t cur_options; + uint64_t new_options; + + static constexpr uint64_t all_options = ~uint64_t (0); // Auxiliary data storage. // @@ -245,14 +334,16 @@ namespace build2 // Implementation details. // - // NOTE: see match_rule() in algorithms.cxx if changing anything here. + // NOTE: see match_rule_impl() in algorithms.cxx if changing anything here. // public: explicit - match_extra (bool l = true, bool f = false): locked (l), fallback (f) {} + match_extra (bool l = true, bool f = false) + : locked (l), fallback (f), + cur_options (all_options), new_options (0) {} void - init (bool fallback); + reinit (bool fallback); // Force freeing of the dynamically-allocated memory. // @@ -764,7 +855,11 @@ namespace build2 // mutable atomic_count dependents {0}; - // Match state storage between the match() and apply() calls. + // Match state storage between the match() and apply() calls with only + // the *_options members extended to reapply(). + // + // Note: in reality, cur_options are used beyong (re)apply() as an + // implementation detail. // build2::match_extra match_extra; @@ -2190,7 +2285,7 @@ namespace build2 // 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). + // recipe/rule (see match_rule_impl() in algorithms.cxx for details). // class LIBBUILD2_SYMEXPORT group: public mtime_target { diff --git a/libbuild2/target.ixx b/libbuild2/target.ixx index f0d5cea..03cf444 100644 --- a/libbuild2/target.ixx +++ b/libbuild2/target.ixx @@ -136,10 +136,12 @@ namespace build2 // match_extra // inline void match_extra:: - init (bool f) + reinit (bool f) { clear_data (); fallback = f; + cur_options = all_options; + new_options = 0; } inline void match_extra:: |