From 3cdc5d8ceafeddf04aa8dea2ac76c954d3de116e Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Sun, 22 Oct 2023 11:48:02 +0200 Subject: WIP: update documentation, deal with ad hoc group members --- libbuild2/algorithm.cxx | 34 +++++++++------- libbuild2/target.hxx | 100 ++++++++++++++++++++++++++++++++++++------------ 2 files changed, 97 insertions(+), 37 deletions(-) diff --git a/libbuild2/algorithm.cxx b/libbuild2/algorithm.cxx index bd89fc5..5b543cf 100644 --- a/libbuild2/algorithm.cxx +++ b/libbuild2/algorithm.cxx @@ -1111,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: @@ -1122,16 +1118,26 @@ 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}). + + 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. // - // @@ But we cannot reapply them if options change since there is - // no rule! Feels easiest to just assume no options for now? Or - // member options are group options (won't then still need - // reapply() support via member). Need to keep KISS for now so - // that don't need to deal with _applied/_executed! - // auto df = make_diag_frame ( [a, &t](const diag_record& dr) { @@ -1139,15 +1145,17 @@ namespace build2 dr << info << "while matching group rule to " << diag_do (a, t); }); - // @@ TODO: unclear what to do about options here. - // @@ TODO: this could also be _applied/_executed, theoretically. - pair r (match_impl (a, g, 0, nullptr, try_match)); if (r.first) { if (r.second != target_state::failed) { + // Note: in particular, this makes sure we will never re-lock this + // member if already applied/executed. + // + s.match_extra.cur_options = match_extra::all_options; + match_inc_dependents (a, g); match_recipe (l, group_recipe); diff --git a/libbuild2/target.hxx b/libbuild2/target.hxx index 32f717f..fc733b6 100644 --- a/libbuild2/target.hxx +++ b/libbuild2/target.hxx @@ -180,30 +180,82 @@ namespace build2 bool locked; // Normally true (see adhoc_rule::match() for background). bool fallback; // True if matching a fallback rule (see match_rule()). - // Match options. - // - // 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. On rematch in the reapply() call, 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). - // - // Note: options are currently not supported in ad hoc recipes/rules. - // - // @@ We could use 0 new_options (which otherwise don't make sense) for - // match for the purpose of resolving members? - // - // @@ TODO: clear already enabled options from new_options on rematch. - // - // @@ TODO doc + // 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 expectaion 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 mirro semantics). + // + // 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; -- cgit v1.1