aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2017-01-18 13:50:58 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2017-01-18 13:50:58 +0200
commitd402bc96297c6ed3dd6ee883dcff8cc39bd01030 (patch)
tree634d397f48022935926e26123c13ab8adad6796e
parent34be21a72a396240642acf3050eead875d3ed4b4 (diff)
Ignore prerequisite mtimes that are not linker inputs
This makes sure, for example, that we don't unnecessarily re-link an executable when its testscript prerequisite is changes.
-rw-r--r--build2/algorithm48
-rw-r--r--build2/algorithm.cxx71
-rw-r--r--build2/algorithm.ixx40
-rw-r--r--build2/cc/compile.cxx25
-rw-r--r--build2/cc/link.cxx26
-rw-r--r--build2/cli/rule.cxx14
-rw-r--r--build2/rule6
-rw-r--r--build2/rule.cxx61
-rw-r--r--build2/target15
9 files changed, 155 insertions, 151 deletions
diff --git a/build2/algorithm b/build2/algorithm
index 9401915..3dbef4c 100644
--- a/build2/algorithm
+++ b/build2/algorithm
@@ -180,15 +180,31 @@ namespace build2
target_state
reverse_execute_prerequisites (action, target&);
- // A version of the above that also determines whether the action
- // needs to be executed on the target based on the passed mtime
- // timestamp.
+ // 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.
//
- // Note that because we use mtime, this function should normally
- // only be used in the perform_update action.
+ // The filter is passed each prerequisite target and is expected to signal
+ // which ones should be used for timestamp comparison. If the filter is
+ // NULL, then all the prerequisites are used.
//
- bool
- execute_prerequisites (action, target&, const timestamp&);
+ // Note that the return value is a pair with the second half indicating
+ // whether any prerequisites were updated. This is used to handle the
+ // situation where some prerequisites were updated but no update of the
+ // target is necessary. In this case we still signal that the target was
+ // (conceptually, but not physically) changed. This is important both to
+ // propagate the fact that some work has been done and to also allow our
+ // dependents to detect this case if they are up to something tricky (like
+ // recursively linking liba{} prerequisites).
+ //
+ // Note that because we use mtime, this function should normally only be
+ // used in the perform_update action.
+ //
+ using prerequisite_filter = function<bool (target&)>;
+
+ pair<bool, target_state>
+ execute_prerequisites (action, target&,
+ const timestamp&,
+ const prerequisite_filter& = nullptr);
// Another version of the above that does two extra things for the caller:
// it determines whether the action needs to be executed on the target based
@@ -198,17 +214,23 @@ namespace build2
// prerequisites of the same type may get injected).
//
template <typename T>
- T*
- execute_prerequisites (action, target&, const timestamp&);
+ pair<T*, target_state>
+ execute_prerequisites (action, target&,
+ const timestamp&,
+ const prerequisite_filter& = nullptr);
- target*
+ pair<target*, target_state>
execute_prerequisites (const target_type&,
- action, target&, const timestamp&);
+ action, target&,
+ const timestamp&,
+ const prerequisite_filter& = nullptr);
template <typename T>
- T*
+ pair<T*, target_state>
execute_prerequisites (const target_type&,
- action, target&, const timestamp&);
+ action, target&,
+ const timestamp&,
+ const prerequisite_filter& = nullptr);
// Return noop_recipe instead of using this function directly.
//
diff --git a/build2/algorithm.cxx b/build2/algorithm.cxx
index 638afd0..8874e96 100644
--- a/build2/algorithm.cxx
+++ b/build2/algorithm.cxx
@@ -413,54 +413,15 @@ namespace build2
return r;
}
- bool
- execute_prerequisites (action a, target& t, const timestamp& mt)
+ pair<target*, target_state>
+ execute_prerequisites (const target_type* tt,
+ action a, target& t,
+ const timestamp& mt, const prerequisite_filter& pf)
{
bool e (mt == timestamp_nonexistent);
- for (target* pt: t.prerequisite_targets)
- {
- if (pt == nullptr) // Skipped.
- continue;
-
- target_state ts (execute (a, *pt));
-
- if (!e)
- {
- // If this is an mtime-based target, then compare timestamps.
- //
- if (auto mpt = dynamic_cast<const mtime_target*> (pt))
- {
- timestamp mp (mpt->mtime ());
-
- // What do we do if timestamps are equal? This can happen, for
- // example, on filesystems that don't have subsecond resolution.
- // There is not much we can do here except detect the case where
- // the prerequisite was changed in this run which means the
- // action must be executed on the target as well.
- //
- if (mt < mp || (mt == mp && ts == target_state::changed))
- e = true;
- }
- else
- {
- // Otherwise we assume the prerequisite is newer if it was changed.
- //
- if (ts == target_state::changed)
- e = true;
- }
- }
- }
-
- return e;
- }
-
- target*
- execute_prerequisites (const target_type& tt,
- action a, target& t, const timestamp& mt)
- {
- bool e (mt == timestamp_nonexistent);
- target* r (nullptr);
+ target* rt (tt != nullptr ? nullptr : &t);
+ target_state rs (target_state::unchanged);
for (target* pt: t.prerequisite_targets)
{
@@ -468,8 +429,11 @@ namespace build2
continue;
target_state ts (execute (a, *pt));
+ rs |= ts;
- if (!e)
+ // Should we compare the timestamp to this target's?
+ //
+ if (!e && (!pf || pf (*pt)))
{
// If this is an mtime-based target, then compare timestamps.
//
@@ -477,11 +441,8 @@ namespace build2
{
timestamp mp (mpt->mtime ());
- // What do we do if timestamps are equal? This can happen, for
- // example, on filesystems that don't have subsecond resolution.
- // There is not much we can do here except detect the case where
- // the prerequisite was changed in this run which means the
- // action must be executed on the target as well.
+ // The same logic as in mtime_target::newer() (but avoids a call to
+ // state()).
//
if (mt < mp || (mt == mp && ts == target_state::changed))
e = true;
@@ -495,12 +456,12 @@ namespace build2
}
}
- if (r == nullptr && pt->is_a (tt))
- r = pt;
+ if (rt == nullptr && pt->is_a (*tt))
+ rt = pt;
}
- assert (r != nullptr);
- return e ? r : nullptr;
+ assert (rt != nullptr);
+ return make_pair (e ? rt : nullptr, rs);
}
target_state
diff --git a/build2/algorithm.ixx b/build2/algorithm.ixx
index 3128365..09d522f 100644
--- a/build2/algorithm.ixx
+++ b/build2/algorithm.ixx
@@ -217,18 +217,46 @@ namespace build2
}
}
+ // If the first argument is NULL, then the result is treated as a boolean
+ // value.
+ //
+ pair<target*, target_state>
+ execute_prerequisites (const target_type*,
+ action, target&,
+ const timestamp&, const prerequisite_filter&);
+
+ inline pair<bool, target_state>
+ execute_prerequisites (action a, target& t,
+ const timestamp& mt, const prerequisite_filter& pf)
+ {
+ auto p (execute_prerequisites (nullptr, a, t, mt, pf));
+ return make_pair (p.first != nullptr, p.second);
+ }
+
template <typename T>
- inline T*
- execute_prerequisites (action a, target& t, const timestamp& mt)
+ inline pair<T*, target_state>
+ execute_prerequisites (action a, target& t,
+ const timestamp& mt, const prerequisite_filter& pf)
+ {
+ auto p (execute_prerequisites (T::static_type, a, t, mt, pf));
+ return make_pair (static_cast<T*> (p.first), p.second);
+ }
+
+ inline pair<target*, target_state>
+ execute_prerequisites (const target_type& tt,
+ action a, target& t,
+ const timestamp& mt, const prerequisite_filter& pf)
{
- return static_cast<T*> (execute_prerequisites (T::static_type, a, t, mt));
+ return execute_prerequisites (&tt, a, t, mt, pf);
}
template <typename T>
- inline T*
+ inline pair<T*, target_state>
execute_prerequisites (const target_type& tt,
- action a, target& t, const timestamp& mt)
+ action a, target& t,
+ const timestamp& mt, const prerequisite_filter& pf)
{
- return static_cast<T*> (execute_prerequisites (tt, a, t, mt));
+ auto p (execute_prerequisites (tt, a, t, mt, pf));
+ return make_pair (static_cast<T*> (p.first), p.second);
}
}
diff --git a/build2/cc/compile.cxx b/build2/cc/compile.cxx
index 6fd64c2..3318f21 100644
--- a/build2/cc/compile.cxx
+++ b/build2/cc/compile.cxx
@@ -888,16 +888,7 @@ namespace build2
}
}
- if (ts != timestamp_unknown)
- {
- timestamp mt (pt.mtime ());
-
- // See execute_prerequisites() for rationale behind the equal part.
- //
- return ts < mt || (ts == mt && pt.state () != target_state::changed);
- }
-
- return false;
+ return ts != timestamp_unknown ? pt.newer (ts) : false;
};
// Update and add a header file to the list of prerequisite targets.
@@ -1394,10 +1385,18 @@ namespace build2
perform_update (action a, target& xt) const
{
file& t (static_cast<file&> (xt));
- file* s (execute_prerequisites<file> (x_src, a, t, t.mtime ()));
- if (s == nullptr)
- return target_state::unchanged;
+ // Update prerequisites and determine if any relevant ones render us
+ // out-of-date. Note that currently we treat all the prerequisites
+ // as potentially affecting the result (for simplicity/performance).
+ //
+ file* s;
+ {
+ auto p (execute_prerequisites<file> (x_src, a, t, t.mtime ()));
+
+ if ((s = p.first) == nullptr)
+ return p.second;
+ }
scope& bs (t.base_scope ());
scope& rs (*bs.root_scope ());
diff --git a/build2/cc/link.cxx b/build2/cc/link.cxx
index 9dbfa36..a6b67d5 100644
--- a/build2/cc/link.cxx
+++ b/build2/cc/link.cxx
@@ -890,9 +890,12 @@ namespace build2
otype lt (link_type (t));
lorder lo (link_order (bs, lt));
- // Update prerequisites.
+ // Update prerequisites. We determine if any relevant ones render us
+ // out-of-date manually below.
//
- bool update (execute_prerequisites (a, t, t.mtime ()));
+ bool update (false);
+ timestamp mt (t.mtime ());
+ target_state ts (execute_prerequisites (a, t));
// If targeting Windows, take care of the manifest.
//
@@ -913,7 +916,7 @@ namespace build2
t,
rpath_timestamp != timestamp_nonexistent));
- timestamp mt (file_mtime (mf));
+ timestamp mf_mt (file_mtime (mf));
if (tsys == "mingw32")
{
@@ -923,7 +926,7 @@ namespace build2
//
manifest = mf + ".o";
- if (mt > file_mtime (manifest))
+ if (mf_mt > file_mtime (manifest))
{
path of (relative (manifest));
@@ -998,7 +1001,7 @@ namespace build2
{
manifest = move (mf); // Save for link.exe's /MANIFESTINPUT.
- if (mt > t.mtime ())
+ if (mf_mt > mt)
update = true; // Manifest changed, force update.
}
}
@@ -1204,9 +1207,16 @@ namespace build2
else
cs.append (f->path ().string ());
}
+ else
+ f = pt->is_a<exe> (); // Consider executable mtime (e.g., linker).
+
+ // Check if this input renders us out-of-date.
+ //
+ if (f != nullptr)
+ update = update || f->newer (mt);
}
- // Treat it as input for both MinGW and VC.
+ // Treat it as input for both MinGW and VC (mtime checked above).
//
if (!manifest.empty ())
cs.append (manifest.string ());
@@ -1229,7 +1239,7 @@ namespace build2
// this situation in the "from scratch" flag.
//
bool scratch (false);
- if (dd.writing () || dd.mtime () > t.mtime ())
+ if (dd.writing () || dd.mtime () > mt)
scratch = update = true;
dd.close ();
@@ -1237,7 +1247,7 @@ namespace build2
// If nothing changed, then we are done.
//
if (!update)
- return target_state::unchanged;
+ return ts;
// Ok, so we are updating. Finish building the command line.
//
diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx
index 5b78723..b97bea2 100644
--- a/build2/cli/rule.cxx
+++ b/build2/cli/rule.cxx
@@ -231,12 +231,18 @@ namespace build2
{
cli_cxx& t (static_cast<cli_cxx&> (xt));
- // Execute our prerequsites and check if we are out of date.
+ // Update prerequisites and determine if any relevant ones render us
+ // out-of-date. Note that currently we treat all the prerequisites
+ // as potentially affecting the result (think prologues/epilogues,
+ // etc).
//
- cli* s (execute_prerequisites<cli> (a, t, t.mtime ()));
+ cli* s;
+ {
+ auto p (execute_prerequisites<cli> (a, t, t.mtime ()));
- if (s == nullptr)
- return target_state::unchanged;
+ if ((s = p.first) == nullptr)
+ return p.second;
+ }
// Translate paths to relative (to working directory). This
// results in easier to read diagnostics.
diff --git a/build2/rule b/build2/rule
index 8090fa0..015cf1a 100644
--- a/build2/rule
+++ b/build2/rule
@@ -39,8 +39,7 @@ namespace build2
apply (action, target&) const = 0;
};
- // Fallback rule that on update verifies that the file exists and is
- // not older than any of its prerequisites.
+ // Fallback rule that only matches if the file exists.
//
class file_rule: public rule
{
@@ -51,9 +50,6 @@ namespace build2
virtual recipe
apply (action, target&) const override;
- static target_state
- perform_update (action, target&);
-
static file_rule instance;
};
diff --git a/build2/rule.cxx b/build2/rule.cxx
index a9b7c4e..1c0e08c 100644
--- a/build2/rule.cxx
+++ b/build2/rule.cxx
@@ -81,22 +81,18 @@ namespace build2
recipe file_rule::
apply (action a, target& t) const
{
- // 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
- // since such an update would render this target out of date
- // which in turn would lead to an error. So until we see a
- // real use-case for this functionality, we simply ignore
- // the clean operation.
+ // 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
+ // this functionality, we simply ignore the clean operation.
//
if (a.operation () == clean_id)
return noop_recipe;
- // If we have no prerequisites, then this means this file
- // is up to date. Return noop_recipe which will also cause
- // the target's state to be set to unchanged. This is an
- // important optimization on which quite a few places that
- // deal with predominantly static content rely.
+ // If we have no prerequisites, then this means this file is up to date.
+ // Return noop_recipe which will also cause the target's state to be set
+ // to unchanged. This is an important optimization on which quite a few
+ // places that deal with predominantly static content rely.
//
if (!t.has_prerequisites ())
return noop_recipe;
@@ -104,43 +100,14 @@ namespace build2
// Search and match all the prerequisites.
//
search_and_match_prerequisites (a, t);
- return a == perform_update_id ? &perform_update : default_recipe;
- }
- target_state file_rule::
- perform_update (action a, target& t)
- {
- // Make sure the target is not older than any of its prerequisites.
+ // Note that we used to provide perform_update() which checked that this
+ // target is not older than any of its prerequisites. However, later we
+ // realized this is probably wrong: consider a script with a testscript as
+ // a prerequisite; chances are the testscript will be newer than the
+ // script and there is nothing wrong with that.
//
- timestamp mt (dynamic_cast<path_target&> (t).mtime ());
-
- for (target* pt: t.prerequisite_targets)
- {
- target_state ts (execute (a, *pt));
-
- // If this is an mtime-based target, then compare timestamps.
- //
- if (auto mpt = dynamic_cast<const mtime_target*> (pt))
- {
- timestamp mp (mpt->mtime ());
-
- if (mt < mp)
- fail << "no recipe to " << diag_do (a, t) <<
- info << "prerequisite " << *pt << " is ahead of " << t
- << " by " << (mp - mt);
- }
- else
- {
- // Otherwise we assume the prerequisite is newer if it was changed.
- //
- if (ts == target_state::changed)
- fail << "no recipe to " << diag_do (a, t) <<
- info << "prerequisite " << *pt << " is ahead of " << t
- << " because it was updated";
- }
- }
-
- return target_state::unchanged;
+ return default_recipe;
}
file_rule file_rule::instance;
diff --git a/build2/target b/build2/target
index 1b75da3..dd9461c 100644
--- a/build2/target
+++ b/build2/target
@@ -1098,6 +1098,21 @@ namespace build2
mtime_ = mt;
}
+ // Return true if this target is newer than the specified timestamp.
+ //
+ bool
+ newer (timestamp mt)
+ {
+ timestamp mp (mtime ());
+
+ // What do we do if timestamps are equal? This can happen, for example,
+ // on filesystems that don't have subsecond resolution. There is not
+ // much we can do here except detect the case where the target was
+ // changed on this run.
+ //
+ return mt < mp || (mt == mp && state () == target_state::changed);
+ }
+
protected:
// Return timestamp_unknown if the mtime cannot be loaded.
//