From 378b2598a305d4e332e52460ca89dd867546a58b Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 7 Feb 2018 10:00:46 +0200 Subject: Initial work for default update outer operation While update still uses the old "all update rules update all their prerequisites" assumption, test and install have been fixed not to rely on this. --- build2/algorithm.cxx | 36 ++++++++++----------- build2/algorithm.hxx | 43 +++++++++++++++++++++---- build2/algorithm.ixx | 58 ++++++++++++++++++++++++++++------ build2/bin/init.cxx | 2 ++ build2/bin/rule.cxx | 2 ++ build2/cc/link-rule.cxx | 33 ++++++++++++++++++-- build2/cli/init.cxx | 2 ++ build2/cli/rule.cxx | 6 ++-- build2/config/init.cxx | 22 ++++++++----- build2/context.cxx | 3 +- build2/dist/init.cxx | 2 +- build2/operation.hxx | 6 ++++ build2/rule.cxx | 25 +++++++++++++-- build2/rule.hxx | 26 ++++++---------- build2/test/rule.cxx | 83 +++++++++++++++++++++++++++++++++---------------- build2/test/rule.hxx | 2 +- 16 files changed, 257 insertions(+), 94 deletions(-) diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx index 4e76e4a..58ddb24 100644 --- a/build2/algorithm.cxx +++ b/build2/algorithm.cxx @@ -1039,24 +1039,24 @@ namespace build2 template target_state - straight_execute_members (action a, const target& t, T ts[], size_t n) + straight_execute_members (action a, atomic_count& tc, + T ts[], size_t n, size_t p) { target_state r (target_state::unchanged); // Start asynchronous execution of prerequisites. // - wait_guard wg (target::count_busy (), t[a].task_count); + wait_guard wg (target::count_busy (), tc); - for (size_t i (0); i != n; ++i) + n += p; + for (size_t i (p); i != n; ++i) { const target*& mt (ts[i]); if (mt == nullptr) // Skipped. continue; - target_state s ( - execute_async ( - a, *mt, target::count_busy (), t[a].task_count)); + target_state s (execute_async (a, *mt, target::count_busy (), tc)); if (s == target_state::postponed) { @@ -1071,7 +1071,7 @@ namespace build2 // or executed and synchronized (and we have blanked out all the postponed // ones). // - for (size_t i (0); i != n; ++i) + for (size_t i (p); i != n; ++i) { if (ts[i] == nullptr) continue; @@ -1092,24 +1092,24 @@ namespace build2 template target_state - reverse_execute_members (action a, const target& t, T ts[], size_t n) + reverse_execute_members (action a, atomic_count& tc, + T ts[], size_t n, size_t p) { // Pretty much as straight_execute_members() but in reverse order. // target_state r (target_state::unchanged); - wait_guard wg (target::count_busy (), t[a].task_count); + wait_guard wg (target::count_busy (), tc); - for (size_t i (n); i != 0; ) + n = p - n; + for (size_t i (p); i != n; ) { const target*& mt (ts[--i]); if (mt == nullptr) continue; - target_state s ( - execute_async ( - a, *mt, target::count_busy (), t[a].task_count)); + target_state s (execute_async (a, *mt, target::count_busy (), tc)); if (s == target_state::postponed) { @@ -1120,7 +1120,7 @@ namespace build2 wg.wait (); - for (size_t i (n); i != 0; ) + for (size_t i (p); i != n; ) { if (ts[--i] == nullptr) continue; @@ -1141,19 +1141,19 @@ namespace build2 // template target_state straight_execute_members ( - action, const target&, const target*[], size_t); + action, atomic_count&, const target*[], size_t, size_t); template target_state reverse_execute_members ( - action, const target&, const target*[], size_t); + action, atomic_count&, const target*[], size_t, size_t); template target_state straight_execute_members ( - action, const target&, prerequisite_target[], size_t); + action, atomic_count&, prerequisite_target[], size_t, size_t); template target_state reverse_execute_members ( - action, const target&, prerequisite_target[], size_t); + action, atomic_count&, prerequisite_target[], size_t, size_t); pair, const target*> execute_prerequisites (const target_type* tt, diff --git a/build2/algorithm.hxx b/build2/algorithm.hxx index 8978eba..5287f23 100644 --- a/build2/algorithm.hxx +++ b/build2/algorithm.hxx @@ -389,10 +389,12 @@ namespace build2 // changed and target_state::unchanged otherwise. If a prerequisite's // execution is postponed, then set its pointer in prerequisite_targets to // NULL (since its state cannot be queried MT-safely). If count is not 0, - // then only the first count prerequisites are executed. + // then only the first count prerequisites are executed beginning from + // start. // target_state - straight_execute_prerequisites (action, const target&, size_t count = 0); + straight_execute_prerequisites (action, const target&, + size_t count = 0, size_t start = 0); // As above but iterates over the prerequisites in reverse. // @@ -404,6 +406,19 @@ namespace build2 target_state execute_prerequisites (action, const target&, size_t count = 0); + // As above but execute prerequisites for the inner action (that have + // been matched with match_inner()). + // + target_state + straight_execute_prerequisites_inner (action, const target&, + size_t count = 0, size_t start = 0); + + target_state + reverse_execute_prerequisites_inner (action, const target&, size_t count = 0); + + target_state + execute_prerequisites_inner (action, const target&, size_t count = 0); + // A version of the above that also determines whether the action needs to // be executed on the target based on the passed timestamp and filter. // @@ -469,11 +484,27 @@ namespace build2 // template target_state - straight_execute_members (action, const target&, T[], size_t); + straight_execute_members (action, atomic_count&, T[], size_t, size_t); template target_state - reverse_execute_members (action, const target&, T[], size_t); + reverse_execute_members (action, atomic_count&, T[], size_t, size_t); + + template + inline target_state + straight_execute_members (action a, const target& t, + T ts[], size_t c, size_t s) + { + return straight_execute_members (a, t[a].task_count, ts, c, s); + } + + template + inline target_state + reverse_execute_members (action a, const target& t, + T ts[], size_t c, size_t s) + { + return reverse_execute_members (a, t[a].task_count, ts, c, s); + } // Call straight or reverse depending on the current mode. // @@ -484,14 +515,14 @@ namespace build2 inline target_state straight_execute_members (action a, const target& t, const target* (&ts)[N]) { - return straight_execute_members (a, t, ts, N); + return straight_execute_members (a, t, ts, N, 0); } template inline target_state reverse_execute_members (action a, const target& t, const target* (&ts)[N]) { - return reverse_execute_members (a, t, ts, N); + return reverse_execute_members (a, t, ts, N, N); } template diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx index 7cad7ff..f0330f6 100644 --- a/build2/algorithm.ixx +++ b/build2/algorithm.ixx @@ -349,14 +349,14 @@ namespace build2 // In a sense this is like any other dependency. // assert (a.outer ()); - return match (action (a.meta_operation (), a.operation ()), t); + return match (a.inner_action (), t); } inline bool match_inner (action a, const target& t, unmatch um) { assert (a.outer ()); - return match (action (a.meta_operation (), a.operation ()), t, um); + return match (a.inner_action (), t, um); } group_view @@ -368,7 +368,7 @@ namespace build2 group_view r; if (a.outer ()) - a = action (a.meta_operation (), a.operation ()); + a = a.inner_action (); // We can be called during execute though everything should have been // already resolved. @@ -480,21 +480,28 @@ namespace build2 execute_inner (action a, const target& t) { assert (a.outer ()); - return execute_wait (action (a.meta_operation (), a.operation ()), t); + return execute_wait (a.inner_action (), t); } inline target_state - straight_execute_prerequisites (action a, const target& t, size_t c) + straight_execute_prerequisites (action a, const target& t, + size_t c, size_t s) { auto& p (t.prerequisite_targets[a]); - return straight_execute_members (a, t, p.data (), c == 0 ? p.size () : c); + return straight_execute_members (a, t, + p.data (), + c == 0 ? p.size () - s: c, + s); } inline target_state reverse_execute_prerequisites (action a, const target& t, size_t c) { auto& p (t.prerequisite_targets[a]); - return reverse_execute_members (a, t, p.data (), c == 0 ? p.size () : c); + return reverse_execute_members (a, t, + p.data (), + c == 0 ? p.size () : c, + p.size ()); } inline target_state @@ -505,6 +512,39 @@ namespace build2 : reverse_execute_prerequisites (a, t, c); } + inline target_state + straight_execute_prerequisites_inner (action a, const target& t, + size_t c, size_t s) + { + assert (a.outer ()); + auto& p (t.prerequisite_targets[a]); + return straight_execute_members (a.inner_action (), + t[a].task_count, + p.data (), + c == 0 ? p.size () - s : c, + s); + } + + inline target_state + reverse_execute_prerequisites_inner (action a, const target& t, size_t c) + { + assert (a.outer ()); + auto& p (t.prerequisite_targets[a]); + return reverse_execute_members (a.inner_action (), + t[a].task_count, + p.data (), + c == 0 ? p.size () : c, + p.size ()); + } + + inline target_state + execute_prerequisites_inner (action a, const target& t, size_t c) + { + return current_mode == execution_mode::first + ? straight_execute_prerequisites_inner (a, t, c) + : reverse_execute_prerequisites_inner (a, t, c); + } + // If the first argument is NULL, then the result is treated as a boolean // value. // @@ -559,7 +599,7 @@ namespace build2 execute_members (action a, const target& t, const target* ts[], size_t n) { return current_mode == execution_mode::first - ? straight_execute_members (a, t, ts, n) - : reverse_execute_members (a, t, ts, n); + ? straight_execute_members (a, t, ts, n, 0) + : reverse_execute_members (a, t, ts, n, n); } } diff --git a/build2/bin/init.cxx b/build2/bin/init.cxx index 565936f..e7509d3 100644 --- a/build2/bin/init.cxx +++ b/build2/bin/init.cxx @@ -462,6 +462,8 @@ namespace build2 // Similar to alias. // + + //@@ outer r.insert (perform_id, 0, "bin.lib", lib_); r.insert (configure_id, 0, "bin.lib", lib_); diff --git a/build2/bin/rule.cxx b/build2/bin/rule.cxx index 79270c3..cde2d9a 100644 --- a/build2/bin/rule.cxx +++ b/build2/bin/rule.cxx @@ -65,6 +65,8 @@ namespace build2 { lib& t (xt.as ()); + //@@ outer: also prerequisites (if outer) or not? + const target* m[] = {t.a, t.s}; match_members (a, t, m); diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx index d06a835..b6b707d 100644 --- a/build2/cc/link-rule.cxx +++ b/build2/cc/link-rule.cxx @@ -56,6 +56,8 @@ namespace build2 // if (lt.library ()) { + //@@ inner/outer race (see install-rule)? + if (t.group == nullptr) t.group = &search (t, lt.utility ? libu::static_type : lib::static_type, @@ -621,7 +623,22 @@ namespace build2 // if (p.is_a ()) pt = &search (t, tt.obj, p.key ()); else if (p.is_a ()) pt = &search (t, tt.bmi, p.key ()); - else pt = &p.search (t); + // + // Something else. This could be something unrelated that the user + // tacked on (e.g., a doc{}). Or it could be some ad hoc input to + // the linker (say a linker script or some such). + // + else + { + // @@ Temporary hack until we get the default outer operation for + // update. This allows operations like test and install to skip + // such tacked on stuff. + // + if (current_outer_oif != nullptr) + continue; + + pt = &p.search (t); + } if (skip (pt)) continue; @@ -676,7 +693,7 @@ namespace build2 // If this obj*{} already has prerequisites, then verify they are // "compatible" with what we are doing here. Otherwise, synthesize // the dependency. Note that we may also end up synthesizing with - // someone beating up to it. In this case also verify. + // someone beating us to it. In this case also verify. // bool verify (true); @@ -709,6 +726,9 @@ namespace build2 { const target* pt (pts[j++]); + if (pt == nullptr) + continue; + if (p.is_a () || p.is_a () || p.is_a () || p.is_a () || p.is_a () || p.is_a (tt.bmi)) @@ -1155,6 +1175,9 @@ namespace build2 for (const prerequisite_target& pt: t.prerequisite_targets[a]) { + if (pt == nullptr) + continue; + bool la; const file* f; @@ -1525,6 +1548,9 @@ namespace build2 { const target* pt (p.target); + if (pt == nullptr) + continue; + // If this is bmi*{}, then obj*{} is its ad hoc member. // if (modules) @@ -1810,6 +1836,9 @@ namespace build2 { const target* pt (p.target); + if (pt == nullptr) + continue; + if (modules) { if (pt->is_a () || pt->is_a () || pt->is_a ()) diff --git a/build2/cli/init.cxx b/build2/cli/init.cxx index df123ba..ede0db1 100644 --- a/build2/cli/init.cxx +++ b/build2/cli/init.cxx @@ -319,6 +319,8 @@ namespace build2 // resolved/linked up. Looks like a general pattern: groups should // resolve on *(update). // + // @@ meta-op wildcard? + // reg (configure_id, update_id); reg (dist_id, update_id); } diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx index 42f2176..94b2d24 100644 --- a/build2/cli/rule.cxx +++ b/build2/cli/rule.cxx @@ -167,15 +167,15 @@ namespace build2 // inject_fsdir (a, t); - // Match prerequisite members. + // Match prerequisites. // match_prerequisite_members (a, t); switch (a) { case perform_update_id: return &perform_update; - case perform_clean_id: return &perform_clean_group; // Standard impl. - default: return noop_recipe; // Configure update. + case perform_clean_id: return &perform_clean_group; // Standard impl. + default: return noop_recipe; // Configure update. } } else diff --git a/build2/config/init.cxx b/build2/config/init.cxx index 2552e77..35bc94c 100644 --- a/build2/config/init.cxx +++ b/build2/config/init.cxx @@ -121,19 +121,25 @@ namespace build2 // Register alias and fallback rule for the configure meta-operation. // + // We need this rule for out-of-any-project dependencies (e.g., + // libraries imported from /usr/lib). We are registring it on the + // global scope similar to builtin rules. + // { - // We need this rule for out-of-any-project dependencies (e.g., - // libraries imported from /usr/lib). Registring it on the global - // scope smells a bit but seems harmless. - // - rs.global ().rules.insert ( + auto& r (rs.global ().rules); + r.insert ( configure_id, 0, "config.file", file_rule::instance); - + } + { auto& r (rs.rules); - r.insert (configure_id, 0, "config", fallback_rule::instance); - r.insert (configure_id, 0, "config.file", fallback_rule::instance); + //@@ outer r.insert (configure_id, 0, "config.alias", alias_rule::instance); + + // This allows a custom configure rule while doing nothing by default. + // + r.insert (configure_id, 0, "config", noop_rule::instance); + r.insert (configure_id, 0, "config.file", noop_rule::instance); } return true; diff --git a/build2/context.cxx b/build2/context.cxx index 3bb07a8..6f3abcc 100644 --- a/build2/context.cxx +++ b/build2/context.cxx @@ -697,8 +697,9 @@ namespace build2 // Register builtin rules. // { - rule_map& r (gs.rules); + rule_map& r (gs.rules); // Note: global scope! + //@@ outer r.insert (perform_id, 0, "alias", alias_rule::instance); r.insert (perform_update_id, "fsdir", fsdir_rule::instance); diff --git a/build2/dist/init.cxx b/build2/dist/init.cxx index 818570a..fe0e9a9 100644 --- a/build2/dist/init.cxx +++ b/build2/dist/init.cxx @@ -94,7 +94,7 @@ namespace build2 // something like insert(dist_id, test_id) taking precedence. // rs.rules.insert (dist_id, 0, "dist", rule_); - rs.rules.insert (dist_id, 0, "dist.alias", rule_); + rs.rules.insert (dist_id, 0, "dist.alias", rule_); //@@ outer? // Configuration. // diff --git a/build2/operation.hxx b/build2/operation.hxx index 8c9818e..e79a57c 100644 --- a/build2/operation.hxx +++ b/build2/operation.hxx @@ -70,6 +70,12 @@ namespace build2 bool inner () const {return outer_id == 0;} bool outer () const {return outer_id != 0;} + action + inner_action () const + { + return action (meta_operation (), operation ()); + } + // Implicit conversion operator to action_id for the switch() statement, // etc. Most places only care about the inner operation. // diff --git a/build2/rule.cxx b/build2/rule.cxx index ef1d3a4..e215846 100644 --- a/build2/rule.cxx +++ b/build2/rule.cxx @@ -98,6 +98,11 @@ namespace build2 recipe file_rule:: apply (action a, target& t) const { + /* + @@ outer + return noop_recipe; + */ + // Update triggers the update of this target's prerequisites so it would // seem natural that we should also trigger their cleanup. However, this // possibility is rather theoretical so until we see a real use-case for @@ -220,6 +225,8 @@ namespace build2 // First update prerequisites (e.g. create parent directories) then create // this directory. // + // @@ outer: should we assume for simplicity its only prereqs are fsdir{}? + // if (!t.prerequisite_targets[a].empty ()) ts = straight_execute_prerequisites (a, t); @@ -232,6 +239,8 @@ namespace build2 // better performance by first checking if it indeed exists. See // butl::try_mkdir() for details. // + // @@ Also skip prerequisites? Can't we return noop in apply? + // if (!exists (d) && fsdir_mkdir (t, d)) ts |= target_state::changed; @@ -280,7 +289,19 @@ namespace build2 const fsdir_rule fsdir_rule::instance; - // fallback_rule + // noop_rule // - const fallback_rule fallback_rule::instance; + bool noop_rule:: + match (action, target&, const string&) const + { + return true; + } + + recipe noop_rule:: + apply (action, target&) const + { + return noop_recipe; + } + + const noop_rule noop_rule::instance; } diff --git a/build2/rule.hxx b/build2/rule.hxx index 8e43ca6..f4a7798 100644 --- a/build2/rule.hxx +++ b/build2/rule.hxx @@ -29,41 +29,38 @@ namespace build2 apply (action, target&) const = 0; }; - // Fallback rule that only matches if the file exists. + // Fallback rule that only matches if the file exists. It will also match + // an mtime_target provided it has a set timestamp. // class file_rule: public rule { public: - file_rule () {} - virtual bool match (action, target&, const string&) const override; virtual recipe apply (action, target&) const override; + file_rule () {} static const file_rule instance; }; class alias_rule: public rule { public: - alias_rule () {} - virtual bool match (action, target&, const string&) const override; virtual recipe apply (action, target&) const override; + alias_rule () {} static const alias_rule instance; }; class fsdir_rule: public rule { public: - fsdir_rule () {} - virtual bool match (action, target&, const string&) const override; @@ -82,26 +79,23 @@ namespace build2 static void perform_update_direct (action, const target&); + fsdir_rule () {} static const fsdir_rule instance; }; // Fallback rule that always matches and does nothing. // - class fallback_rule: public build2::rule + class noop_rule: public rule { public: - fallback_rule () {} - virtual bool - match (action, target&, const string&) const override - { - return true; - } + match (action, target&, const string&) const override; virtual recipe - apply (action, target&) const override {return noop_recipe;} + apply (action, target&) const override; - static const fallback_rule instance; + noop_rule () {} + static const noop_rule instance; }; } diff --git a/build2/test/rule.cxx b/build2/test/rule.cxx index 7c55445..86062f9 100644 --- a/build2/test/rule.cxx +++ b/build2/test/rule.cxx @@ -142,16 +142,32 @@ namespace build2 break; } - // If this is the test operation, collect testscripts after the - // pass-through prerequisites. + // Collect testscripts after the pass-through prerequisites. // - // Note that we don't match nor execute them relying on update to - // assign their paths and make sure they are up to date. + const target& pt (p.search (t)); + + // Note that for the test operation itself we don't match nor + // execute them relying on update to assign their paths. + // + // Causing update for test inputs/scripts is tricky: we cannot + // match for update_for_install because this same rule will match + // and since the target is not testable, it will return the noop + // recipe. + // + // So what we are going to do is directly match (and also execute; + // see below) a recipe for the inner update (who thought we could + // do that... but it seems we can). While at first it might feel + // iffy, it does make sense: the outer rule we would have matched + // would have simply delegated to the inner so we might as well + // take a shortcut. The only potential drawback of this approach + // is that we won't be able to provide any for_test customizations + // when updating test inputs/scripts. But such a need seems rather + // far fetched. // - if (a.operation () != test_id) - break; + if (a.operation () == update_id) + match_inner (a, pt); - pts.push_back (&p.search (t)); + pts.push_back (&pt); } } @@ -196,14 +212,9 @@ namespace build2 if (si || so || in) { - // Note that we don't match nor execute them relying on update - // to assign their paths and make sure they are up to date. - // - const target& pt (p.search (t)); - // Verify it is file-based. // - if (!pt.is_a ()) + if (!p.is_a ()) { fail << "test." << (si ? "stdin" : so ? "stdout" : "input") << " prerequisite " << p << " of target " << t @@ -214,9 +225,6 @@ namespace build2 { test = true; - if (a.operation () != test_id) - break; - // First matching prerequisite. Establish the structure in // pts: the first element (after pass_n) is stdin (can be // NULL), the second is stdout (can be NULL), and everything @@ -226,13 +234,31 @@ namespace build2 pts.push_back (nullptr); // stdout } + // Collect them after the pass-through prerequisites. + // + // Note that for the test operation itself we don't match nor + // execute them relying on update to assign their paths. + // + auto match = [a, &p, &t] () -> const target* + { + const target& pt (p.search (t)); + + // The same match_inner() rationale as for the testcript + // prerequisites above. + // + if (a.operation () == update_id) + match_inner (a, pt); + + return &pt; + }; + if (si) { if (pts[pass_n] != nullptr) fail << "multiple test.stdin prerequisites for target " << t; - pts[pass_n] = &pt; + pts[pass_n] = match (); } if (so) @@ -241,11 +267,11 @@ namespace build2 fail << "multiple test.stdout prerequisites for target " << t; - pts[pass_n + 1] = &pt; + pts[pass_n + 1] = match (); } if (in) - pts.push_back (&pt); + pts.push_back (match ()); } } @@ -274,11 +300,13 @@ namespace build2 if (a.operation () == update_id) { // For the update pre-operation match the inner rule (actual update). - // Note that here we assume it will update (if required) all the - // testscript and input/output prerequisites. // match_inner (a, t); - return &perform_update; + + return [pass_n, this] (action a, const target& t) + { + return perform_update (a, t, pass_n); + }; } else { @@ -300,15 +328,16 @@ namespace build2 } target_state rule:: - perform_update (action a, const target& t) + perform_update (action a, const target& t, size_t pass_n) { - // First execute the inner recipe, then, if passing-through, execute - // prerequisites. + // First execute the inner recipe then execute prerequisites. // target_state ts (execute_inner (a, t)); - if (t.prerequisite_targets[a].size () != 0) - ts |= straight_execute_prerequisites (a, t); + if (pass_n != 0) + ts |= straight_execute_prerequisites (a, t, pass_n); + + ts |= straight_execute_prerequisites_inner (a, t, 0, pass_n); return ts; } diff --git a/build2/test/rule.hxx b/build2/test/rule.hxx index 4f02390..85fa1cd 100644 --- a/build2/test/rule.hxx +++ b/build2/test/rule.hxx @@ -27,7 +27,7 @@ namespace build2 apply (action, target&) const override; static target_state - perform_update (action, const target&); + perform_update (action, const target&, size_t); target_state perform_test (action, const target&, size_t) const; -- cgit v1.1