diff options
Diffstat (limited to 'build')
| -rw-r--r-- | build/algorithm | 10 | ||||
| -rw-r--r-- | build/algorithm.cxx | 2 | ||||
| -rw-r--r-- | build/algorithm.ixx | 31 | ||||
| -rw-r--r-- | build/b.cxx | 3 | ||||
| -rw-r--r-- | build/context | 9 | ||||
| -rw-r--r-- | build/context.cxx | 1 | ||||
| -rw-r--r-- | build/install/rule.cxx | 2 | ||||
| -rw-r--r-- | build/operation.cxx | 36 | ||||
| -rw-r--r-- | build/rule.cxx | 30 | ||||
| -rw-r--r-- | build/target | 31 | ||||
| -rw-r--r-- | build/target.cxx | 2 | ||||
| -rw-r--r-- | build/test/rule.cxx | 6 |
12 files changed, 88 insertions, 75 deletions
diff --git a/build/algorithm b/build/algorithm index 99e6c2b..af461dc 100644 --- a/build/algorithm +++ b/build/algorithm @@ -67,13 +67,15 @@ namespace build // Match and apply a rule to the action/target with ambiguity // detection. Increment the target's dependents count, which // means that you should call this function with the intent - // to also call execute(). In case of optimizations that - // would avoid calling execute(), decrement the dependents - // cound manually to compensate. + // to also call execute(). In case of optimizations that would + // avoid calling execute(), call unmatch() to indicate this. // void match (action, target&); + void + unmatch (action, target&); + // Match (but do not apply) a rule to the action/target with // ambiguity detection. Note that this function does not touch // the dependents count. @@ -84,7 +86,7 @@ namespace build // Match a "delegate rule" from withing another rules' apply() // function. Return recipe and recipe action (if any). Note // that unlike match(), this call doesn't increment the - // dependents count. See also execute_delegate(). + // dependents count. See also the companion execute_delegate(). // std::pair<recipe, action> match_delegate (action, target&); diff --git a/build/algorithm.cxx b/build/algorithm.cxx index 94c84b8..e09d125 100644 --- a/build/algorithm.cxx +++ b/build/algorithm.cxx @@ -338,7 +338,7 @@ namespace build // switch (t.raw_state) { - case target_state::group: + case target_state::group: // Means group's state is unknown. case target_state::unknown: case target_state::postponed: { diff --git a/build/algorithm.ixx b/build/algorithm.ixx index bb66b53..e0b9364 100644 --- a/build/algorithm.ixx +++ b/build/algorithm.ixx @@ -60,6 +60,19 @@ namespace build match_impl (a, t, true); t.dependents++; + dependency_count++; + + // text << "M " << t << ": " << t.dependents << " " << dependency_count; + } + + inline void + unmatch (action, target& t) + { + assert (t.dependents != 0 && dependency_count != 0); + t.dependents--; + dependency_count--; + + // text << "U " << t << ": " << t.dependents << " " << dependency_count; } inline void @@ -118,10 +131,14 @@ namespace build inline target_state execute (action a, target& t) { - // This can happen when we re-examine the state after being postponed. - // - if (t.dependents != 0) + if (dependency_count != 0) // Re-examination of a postponed target? + { + assert (t.dependents != 0); t.dependents--; + dependency_count--; + } + + // text << "E " << t << ": " << t.dependents << " " << dependency_count; switch (target_state ts = t.state ()) { @@ -138,10 +155,10 @@ namespace build // clean the group via three of its members, only the last // attempt will actually execute the clean. This means that // when we match a group member, inside we should also match - // the group in order to increment the dependents count. - // Though this seems to be a natural requirement (if we - // are delegating to the group, we need to find a recipe - // for it, just like we would for a prerequisite). + // the group in order to increment the dependents count. This + // seems to be a natural requirement: if we are delegating to + // the group, we need to find a recipe for it, just like we + // would for a prerequisite. // // Note that below we are going to change the group state // to postponed. This is not a mistake: until we execute diff --git a/build/b.cxx b/build/b.cxx index fea5690..5500c9e 100644 --- a/build/b.cxx +++ b/build/b.cxx @@ -744,6 +744,7 @@ main (int argc, char* argv[]) current_inner_oif = pre_oif; current_outer_oif = oif; current_mode = pre_oif->mode; + dependency_count = 0; action a (mid, pre_oid, oid); @@ -760,6 +761,7 @@ main (int argc, char* argv[]) current_inner_oif = oif; current_outer_oif = nullptr; current_mode = oif->mode; + dependency_count = 0; action a (mid, oid, 0); @@ -777,6 +779,7 @@ main (int argc, char* argv[]) current_inner_oif = post_oif; current_outer_oif = oif; current_mode = post_oif->mode; + dependency_count = 0; action a (mid, post_oid, oid); diff --git a/build/context b/build/context index c8200b7..b8d178b 100644 --- a/build/context +++ b/build/context @@ -7,6 +7,7 @@ #include <string> #include <ostream> +#include <cstdint> // uint64_t #include <butl/filesystem> @@ -33,6 +34,14 @@ namespace build extern execution_mode current_mode; + // Total number of dependency relationships in the current action. + // Together with the target::dependents count it is incremented + // during the rule search & match phase and is decremented during + // execution with the expectation of it reaching 0. Used as a sanity + // check. + // + extern std::uint64_t dependency_count; + // Reset the dependency state. In particular, this removes all the // targets, scopes, and variable names. // diff --git a/build/context.cxx b/build/context.cxx index 5a9452b..e8ecf74 100644 --- a/build/context.cxx +++ b/build/context.cxx @@ -28,6 +28,7 @@ namespace build const operation_info* current_inner_oif; const operation_info* current_outer_oif; execution_mode current_mode; + uint64_t dependency_count; void reset () diff --git a/build/install/rule.cxx b/build/install/rule.cxx index 890228c..f14547c 100644 --- a/build/install/rule.cxx +++ b/build/install/rule.cxx @@ -130,7 +130,7 @@ namespace build if (pt.state () != target_state::unchanged) t.prerequisite_targets.push_back (&pt); else - pt.dependents--; // No intent to execute, so compensate. + unmatch (a, pt); // No intent to execute. } // This is where we diverge depending on the operation. In the diff --git a/build/operation.cxx b/build/operation.cxx index 9bfa8d2..694ad1c 100644 --- a/build/operation.cxx +++ b/build/operation.cxx @@ -114,12 +114,11 @@ namespace build { tracer trace ("execute"); - // Build collecting postponed targets (to be re-examined later). + // Execute collecting postponed targets (to be re-examined later). + // Do it in reverse order if the execution mode is 'last'. // vector<reference_wrapper<target>> psp; - // Execute targets in reverse if the execution mode is 'last'. - // auto body ( [a, &psp, &trace] (void* v) { @@ -129,11 +128,6 @@ namespace build switch (execute (a, t)) { - case target_state::postponed: - { - psp.push_back (t); - break; - } case target_state::unchanged: { // Be quiet in pre/post operations. @@ -142,6 +136,9 @@ namespace build info << diag_done (a, t); break; } + case target_state::postponed: + psp.push_back (t); + break; case target_state::changed: break; case target_state::failed: @@ -156,32 +153,27 @@ namespace build else for (void* v: reverse_iterate (ts)) body (v); - // Re-examine postponed targets. Note that we will do it in the - // order added, so no need to check the execution mode. + // We should have executed every target that we matched. + // + assert (dependency_count == 0); + + // Re-examine postponed targets. This is the only reliable way to + // find out whether the target has changed. // for (target& t: psp) { - if (t.state () == target_state::postponed) - execute (a, t); // Re-examine the state. - - switch (t.state ()) + switch (execute (a, t)) { - case target_state::postponed: - { - info << "unable to " << diag_do (a, t) << " at this time"; - break; - } case target_state::unchanged: { - // Be quiet in pre/post operations. - // if (a.outer_operation () == 0) info << diag_done (a, t); break; } - case target_state::unknown: // Assume something was done to it. case target_state::changed: break; + case target_state::postponed: + assert (false); case target_state::failed: //@@ This could probably happen in a parallel build. default: diff --git a/build/rule.cxx b/build/rule.cxx index 97cd20f..7bf030d 100644 --- a/build/rule.cxx +++ b/build/rule.cxx @@ -193,7 +193,7 @@ namespace build // First update prerequisites (e.g. create parent directories) // then create this directory. // - if (t.has_prerequisites ()) + if (!t.prerequisite_targets.empty ()) ts = execute_prerequisites (a, t); const path& d (t.dir); // Everything is in t.dir. @@ -231,26 +231,16 @@ namespace build // The reverse order of update: first delete this directory, // then clean prerequisites (e.g., delete parent directories). // - rmdir_status rs (rmdir (t.dir, t)); - - target_state ts (target_state::unchanged); - if (t.has_prerequisites ()) - ts = reverse_execute_prerequisites (a, t); - - // If we couldn't remove the directory, return postponed meaning - // that the operation could not be performed at this time. + // Don't fail if we couldn't remove the directory because it + // is not empty (or is current working directory). In this + // case rmdir() will issue a warning when appropriate. // - switch (rs) - { - case rmdir_status::success: ts |= target_state::changed; break; - case rmdir_status::not_empty: - { - if (!work.sub (t.dir)) // No use postponing removing working directory. - ts |= target_state::postponed; - break; - } - default: break; - } + target_state ts (rmdir (t.dir, t) + ? target_state::changed + : target_state::unchanged); + + if (!t.prerequisite_targets.empty ()) + ts |= reverse_execute_prerequisites (a, t); return ts; } diff --git a/build/target b/build/target index 2aa3b5f..dd58b24 100644 --- a/build/target +++ b/build/target @@ -41,14 +41,14 @@ namespace build // enum class target_state: std::uint8_t { - // The order of the enumerators is arranged so that their - // inegral values indicate whether one "overrides" the other - // in the merge operator (see below). + // The order of the enumerators is arranged so that their integral + // values indicate whether one "overrides" the other in the merge + // operator (see below). // unknown, unchanged, - changed, postponed, + changed, failed, group // Target's state is the group's state. }; @@ -68,22 +68,15 @@ namespace build // Recipe. // // The returned target state should be changed, unchanged, or - // postponed. If there is an error, then the recipe should throw - // rather than returning failed. + // postponed, though you shouldn't be returning postponed + // directly. If there is an error, then the recipe should + // throw rather than returning failed. // - // The recipe execution protocol is as follows: before executing - // the recipe, the caller sets the target's state to failed. If - // the recipe returns normally and the target's state is still - // failed, then the caller sets it to the returned value. This - // means that the recipe can set the target's state manually to - // some other value. For example, setting it to unknown will - // result in the recipe to be executed again if this target is a - // prerequisite of another target. Note that in this case the - // returned by the recipe value is still used (by the caller) as - // the resulting target state for this execution of the recipe. - // Returning postponed from the last call to the recipe means - // that the action could not be executed at this time (see fsdir - // clean for an example). + // The return value of the recipe is used to update the target + // state except if the state was manually set by the recipe to + // target_state::group. Note that in this case the returned by + // the recipe value is still used as the resulting target state + // so it should match the group's state. // using recipe_function = target_state (action, target&); using recipe = std::function<recipe_function>; diff --git a/build/target.cxx b/build/target.cxx index cf8747f..033a7f9 100644 --- a/build/target.cxx +++ b/build/target.cxx @@ -32,7 +32,7 @@ namespace build // target_state // static const char* target_state_[] = { - "unknown", "unchanged", "changed", "postponed", "failed", "group"}; + "unknown", "unchanged", "postponed", "changed", "failed", "group"}; ostream& operator<< (ostream& os, target_state ts) diff --git a/build/test/rule.cxx b/build/test/rule.cxx index 6e82d8d..30de93c 100644 --- a/build/test/rule.cxx +++ b/build/test/rule.cxx @@ -210,7 +210,10 @@ namespace build build::match (a, *it); if (it->state () == target_state::unchanged) + { + unmatch (a, *it); it = nullptr; + } } if (ot != nullptr && in == on) @@ -218,7 +221,10 @@ namespace build build::match (a, *ot); if (ot->state () == target_state::unchanged) + { + unmatch (a, *ot); ot = nullptr; + } } else ot = it; |