From e4c4fec8c9097722ee5ee94f2ce5ad0313ed8d7b Mon Sep 17 00:00:00 2001 From: Boris Kolpackov Date: Wed, 1 Jul 2015 16:45:34 +0200 Subject: Clean up group state, mtime design --- build/algorithm | 6 ++- build/b.cxx | 8 ++-- build/cli/rule.cxx | 106 +++++++++++++++++++++------------------------------ build/cli/target | 7 +++- build/cli/target.cxx | 14 ++++++- build/cxx/rule.cxx | 4 +- build/rule | 4 +- build/rule.cxx | 21 +++++++--- build/search.cxx | 4 +- build/target | 36 ++++++++++++++--- 10 files changed, 124 insertions(+), 86 deletions(-) diff --git a/build/algorithm b/build/algorithm index 1cfd15b..f108a8a 100644 --- a/build/algorithm +++ b/build/algorithm @@ -134,7 +134,11 @@ namespace build 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 timestamp. + // needs to be executed on the target based on the passed mtime + // timestamp. + // + // Note that because we use mtime, this function should normally + // only be used in the perform_update action. // bool execute_prerequisites (action, target&, const timestamp&); diff --git a/build/b.cxx b/build/b.cxx index ef39446..d7fb9d3 100644 --- a/build/b.cxx +++ b/build/b.cxx @@ -212,10 +212,10 @@ main (int argc, char* argv[]) rules[update_id][typeid (fsdir)].emplace ("fsdir", fsdir_r); rules[clean_id][typeid (fsdir)].emplace ("fsdir", fsdir_r); - path_rule path_r; - rules[default_id][typeid (path_target)].emplace ("path", path_r); - rules[update_id][typeid (path_target)].emplace ("path", path_r); - rules[clean_id][typeid (path_target)].emplace ("path", path_r); + file_rule file_r; + rules[default_id][typeid (file)].emplace ("file", file_r); + rules[update_id][typeid (file)].emplace ("file", file_r); + rules[clean_id][typeid (file)].emplace ("file", file_r); // Figure out work and home directories. // diff --git a/build/cli/rule.cxx b/build/cli/rule.cxx index 6377b29..3bcd074 100644 --- a/build/cli/rule.cxx +++ b/build/cli/rule.cxx @@ -210,78 +210,65 @@ namespace build // Execute our prerequsites and check if we are out of date. // - cli* s (execute_prerequisites (a, t, t.h ()->mtime ())); - - target_state ts; + cli* s (execute_prerequisites (a, t, t.mtime ())); if (s == nullptr) - ts = target_state::unchanged; - else - { - // Translate source path to relative (to working directory). This - // results in easier to read diagnostics. - // - path relo (relative (t.dir)); - path rels (relative (s->path ())); + return target_state::unchanged; - scope& rs (t.root_scope ()); - const string& cli (rs["config.cli"].as ()); - - vector args {cli.c_str ()}; + // Translate source path to relative (to working directory). This + // results in easier to read diagnostics. + // + path relo (relative (t.dir)); + path rels (relative (s->path ())); - // See if we need to pass any --?xx-suffix options. - // - append_extension (args, *t.h (), "--hxx-suffix", "hxx"); - append_extension (args, *t.c (), "--cxx-suffix", "cxx"); - if (t.i () != nullptr) - append_extension (args, *t.i (), "--ixx-suffix", "ixx"); + scope& rs (t.root_scope ()); + const string& cli (rs["config.cli"].as ()); - append_options (args, t, "cli.options"); + vector args {cli.c_str ()}; - if (!relo.empty ()) - { - args.push_back ("-o"); - args.push_back (relo.string ().c_str ()); - } + // See if we need to pass any --?xx-suffix options. + // + append_extension (args, *t.h (), "--hxx-suffix", "hxx"); + append_extension (args, *t.c (), "--cxx-suffix", "cxx"); + if (t.i () != nullptr) + append_extension (args, *t.i (), "--ixx-suffix", "ixx"); - args.push_back (rels.string ().c_str ()); - args.push_back (nullptr); + append_options (args, t, "cli.options"); - if (verb) - print_process (args); - else - text << "cli " << *s; + if (!relo.empty ()) + { + args.push_back ("-o"); + args.push_back (relo.string ().c_str ()); + } - try - { - process pr (args.data ()); + args.push_back (rels.string ().c_str ()); + args.push_back (nullptr); - if (!pr.wait ()) - throw failed (); + if (verb) + print_process (args); + else + text << "cli " << *s; - timestamp s (system_clock::now ()); + try + { + process pr (args.data ()); - // Update member timestamps. - // - t.h ()->mtime (s); - t.c ()->mtime (s); - if (t.i () != nullptr) - t.i ()->mtime (s); + if (!pr.wait ()) + throw failed (); - ts = target_state::changed; - } - catch (const process_error& e) - { - error << "unable to execute " << args[0] << ": " << e.what (); + t.mtime (system_clock::now ()); + } + catch (const process_error& e) + { + error << "unable to execute " << args[0] << ": " << e.what (); - if (e.child ()) - exit (1); + if (e.child ()) + exit (1); - throw failed (); - } + throw failed (); } - return ts; + return target_state::changed; } target_state compile:: @@ -297,16 +284,11 @@ namespace build bool r (false); if (t.i () != nullptr) - { r = rmfile (t.i ()->path (), *t.i ()) || r; - t.i ()->mtime (timestamp_nonexistent); - } - r = rmfile (t.c ()->path (), *t.c ()) || r; - t.c ()->mtime (timestamp_nonexistent); - r = rmfile (t.h ()->path (), *t.h ()) || r; - t.h ()->mtime (timestamp_nonexistent); + + t.mtime (timestamp_nonexistent); // Clean prerequisites. // diff --git a/build/cli/target b/build/cli/target index e72d4e0..d733668 100644 --- a/build/cli/target +++ b/build/cli/target @@ -23,10 +23,10 @@ namespace build static const target_type static_type; }; - class cli_cxx: public target + class cli_cxx: public mtime_target { public: - using target::target; + using mtime_target::mtime_target; target* m[3] {nullptr, nullptr, nullptr}; @@ -41,6 +41,9 @@ namespace build virtual group_view group_members (action) const; + virtual timestamp + load_mtime () const; + public: virtual const target_type& type () const {return static_type;} static const target_type static_type; diff --git a/build/cli/target.cxx b/build/cli/target.cxx index aded1ff..2f8b54c 100644 --- a/build/cli/target.cxx +++ b/build/cli/target.cxx @@ -4,7 +4,10 @@ #include +#include + using namespace std; +using namespace butl; namespace build { @@ -34,11 +37,20 @@ namespace build : group_view {nullptr, 0}; } + timestamp cli_cxx:: + load_mtime () const + { + // The rule has been matched which means the members should + // be resolved and paths assigned. + // + return file_mtime (h ()->path ()); + } + const target_type cli_cxx::static_type { typeid (cli_cxx), "cli.cxx", - &target::static_type, + &mtime_target::static_type, &target_factory, nullptr, &search_target, diff --git a/build/cxx/rule.cxx b/build/cxx/rule.cxx index 08a9202..7547201 100644 --- a/build/cxx/rule.cxx +++ b/build/cxx/rule.cxx @@ -650,13 +650,13 @@ namespace build // expensive. At the same time, most of these headers are // existing files that we will never be updating (again, // system headers, for example) and the rule that will match - // them is fallback path_rule. So we are going to do a little + // them is fallback file_rule. So we are going to do a little // fast-path optimization by detecting this common case. // recipe_function* const* recipe ( pt.recipe (a).target ()); - if (recipe == nullptr || *recipe != &path_rule::perform_update) + if (recipe == nullptr || *recipe != &file_rule::perform_update) { // We only want to restart if our call to execute() actually // caused an update. In particular, the target could already diff --git a/build/rule b/build/rule index 8c7e4c1..eda6ed3 100644 --- a/build/rule +++ b/build/rule @@ -64,10 +64,10 @@ namespace build extern operation_rule_map rules; - // Fallback rule that on update verifies that the path exists and is + // Fallback rule that on update verifies that the file exists and is // not older than any of its prerequisites. // - class path_rule: public rule + class file_rule: public rule { public: virtual match_result diff --git a/build/rule.cxx b/build/rule.cxx index ce8ef01..a8c77a8 100644 --- a/build/rule.cxx +++ b/build/rule.cxx @@ -21,7 +21,7 @@ namespace build { operation_rule_map rules; - // path_rule + // file_rule // // Note that this rule is special. It is the last, fallback rule. If // it doesn't match, then no other rule can possibly match and we have @@ -30,9 +30,11 @@ namespace build // that normal implementations should follow. So you probably shouldn't // use it as a guide to implement your own, normal, rules. // - match_result path_rule:: + match_result file_rule:: match (action a, target& t, const string&) const { + tracer trace ("file_rule::match"); + // While strictly speaking we should check for the file's existence // for every action (because that's the condition for us matching), // for some actions this is clearly a waste. Say, perform_clean: we @@ -57,14 +59,23 @@ namespace build if (pt.path ().empty ()) pt.derive_path (); - return pt.mtime () != timestamp_nonexistent ? &t : nullptr; + // We cannot just call pt.mtime() since we haven't matched yet. + // + timestamp ts (file_mtime (pt.path ())); + pt.mtime (ts); + + if (ts != timestamp_nonexistent) + return t; + + level3 ([&]{trace << "no existing file for target " << t;}); + return nullptr; } default: return t; } } - recipe path_rule:: + recipe file_rule:: apply (action a, target& t, const match_result&) const { // Update triggers the update of this target's prerequisites @@ -87,7 +98,7 @@ namespace build : t.has_prerequisites () ? default_recipe : noop_recipe; } - target_state path_rule:: + target_state file_rule:: perform_update (action a, target& t) { // Make sure the target is not older than any of its prerequisites. diff --git a/build/search.cxx b/build/search.cxx index 872a0d6..dfb603d 100644 --- a/build/search.cxx +++ b/build/search.cxx @@ -116,9 +116,9 @@ namespace build // auto r (targets.insert (*tk.type, f.directory (), *tk.name, ext, trace)); - // Has to be a path_target. + // Has to be a file_target. // - path_target& t (dynamic_cast (r.first)); + file& t (dynamic_cast (r.first)); level4 ([&]{trace << (r.second ? "new" : "existing") << " target " << t << " for prerequisite " << pk;}); diff --git a/build/target b/build/target index 000496c..39f6679 100644 --- a/build/target +++ b/build/target @@ -782,17 +782,40 @@ namespace build public: using target::target; + // Target mtime is only available after a rule has been matched + // (because this is when we know if we should get our mtime from + // the group and where the path which we need to load mtime is + // normally assigned). The mtime is also unavailable while the + // execution of the target is postponed (because we temporarily + // loose our group state). + // + // The rule for groups that utilize the group state is as follows: + // if it has any members that are mtime_targets, then the group + // should be mtime_target and the members get the mtime from it. + // timestamp mtime () const { - if (mtime_ == timestamp_unknown) - mtime_ = load_mtime (); + assert (raw_state != target_state::postponed); + + const mtime_target* t (raw_state == target_state::group + ? static_cast (group) + : this); + + if (t->mtime_ == timestamp_unknown) + t->mtime_ = t->load_mtime (); - return mtime_; + return t->mtime_; } void - mtime (timestamp mt) {mtime_ = mt;} + mtime (timestamp mt) + { + // While we can cache the mtime at any time, it may be ignored + // if the target state is group (see the mtime() accessor). + // + mtime_ = mt; + } protected: virtual timestamp @@ -853,8 +876,11 @@ namespace build using path_target::path_target; protected: + // Note that it is final in order to be consistent with file_rule, + // search_existing_file(). + // virtual timestamp - load_mtime () const; + load_mtime () const final; public: virtual const target_type& type () const {return static_type;} -- cgit v1.1