aboutsummaryrefslogtreecommitdiff
path: root/build
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2015-08-13 14:48:41 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2015-08-13 14:48:41 +0200
commit9fa5209175dffb881e8ec6c5f6ad4fc54448244a (patch)
treeb937763e9605832f9cac87b846a2996c8727af12 /build
parent467d700c66582471013a07384318d0142d2f3de2 (diff)
Rework postponed logic
Specifically, now postponed is only used by the execution mode logic and rules should not return it directly.
Diffstat (limited to 'build')
-rw-r--r--build/algorithm10
-rw-r--r--build/algorithm.cxx2
-rw-r--r--build/algorithm.ixx31
-rw-r--r--build/b.cxx3
-rw-r--r--build/context9
-rw-r--r--build/context.cxx1
-rw-r--r--build/install/rule.cxx2
-rw-r--r--build/operation.cxx36
-rw-r--r--build/rule.cxx30
-rw-r--r--build/target31
-rw-r--r--build/target.cxx2
-rw-r--r--build/test/rule.cxx6
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;