diff options
| -rw-r--r-- | build2/cc/compile-rule.cxx | 10 | ||||
| -rw-r--r-- | build2/cc/link-rule.cxx | 25 | ||||
| -rw-r--r-- | build2/cli/rule.cxx | 2 | ||||
| -rw-r--r-- | build2/depdb.cxx | 37 | ||||
| -rw-r--r-- | build2/depdb.hxx | 48 | ||||
| -rw-r--r-- | build2/in/rule.cxx | 2 |
6 files changed, 69 insertions, 55 deletions
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx index 634a586..e361e9d 100644 --- a/build2/cc/compile-rule.cxx +++ b/build2/cc/compile-rule.cxx @@ -751,8 +751,7 @@ namespace build2 // Note: the leading '@' is reserved for the module map prefix (see // extract_modules()) and no other line must start with it. // - md.dd = tp + ".d"; - depdb dd (md.dd); + depdb dd (tp + ".d"); // First should come the rule name/version. // @@ -824,7 +823,7 @@ namespace build2 // the target (interrupted update), then do unconditional update. // timestamp mt; - bool u (dd.writing () || dd.mtime () > (mt = file_mtime (tp))); + bool u (dd.writing () || dd.mtime > (mt = file_mtime (tp))); if (u) mt = timestamp_nonexistent; // Treat as if it doesn't exist. @@ -996,9 +995,10 @@ namespace build2 // we will keep re-validating the cached data over and over again. // if (u && dd.reading ()) - dd.touch (); + dd.touch = true; dd.close (); + md.dd = move (dd.path); // If the preprocessed output is suitable for compilation, then pass // it along. @@ -1036,7 +1036,7 @@ namespace build2 // store the first in the target file (so we do touch it) and the // second in depdb (which is never newer that the target). // - md.mt = u ? timestamp_nonexistent : dd.mtime (); + md.mt = u ? timestamp_nonexistent : dd.mtime; } switch (a) diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx index 90d290d..77619d1 100644 --- a/build2/cc/link-rule.cxx +++ b/build2/cc/link-rule.cxx @@ -2068,11 +2068,14 @@ namespace build2 // this situation in the "from scratch" flag. // bool scratch (false); - if (dd.writing () || dd.mtime () > mt) + if (dd.writing () || dd.mtime > mt) scratch = update = true; +#define MTIME_SANITY_CHECK +#ifdef MTIME_SANITY_CHECK timestamp dd_tt (system_clock::now ()); - bool dd_rd (dd.reading ()); +#endif + dd.close (); // If nothing changed, then we are done. @@ -2080,9 +2083,6 @@ namespace build2 if (!update) return ts; - path ddp (tp + ".d"); - timestamp dd_mt (file_mtime (ddp)); - // Ok, so we are updating. Finish building the command line. // string in, out, out1, out2, out3; // Storage. @@ -2544,19 +2544,20 @@ namespace build2 rm.cancel (); +#ifdef MTIME_SANITY_CHECK { - timestamp tp_tt (system_clock::now ()); timestamp tp_mt (file_mtime (tp)); + timestamp dd_mt (file_mtime (dd.path)); + timestamp tp_tt (system_clock::now ()); if (dd_mt > tp_mt) - { fail << "backwards modification times:\n" - << ddp.string () << " " << dd_mt << " (" << dd_rd << ")\n" - << tp.string () << " " << tp_mt << '\n' - << dd_tt << '\n' - << tp_tt; - } + << dd_tt << " window start\n" + << dd_mt << " " << dd.path.string () << '\n' + << tp_mt << " " << tp.string () << '\n' + << tp_tt << " window end"; } +#endif // Should we go to the filesystem and get the new mtime? We know the // file has been modified, so instead just use the current clock time. diff --git a/build2/cli/rule.cxx b/build2/cli/rule.cxx index 27668c1..6c87490 100644 --- a/build2/cli/rule.cxx +++ b/build2/cli/rule.cxx @@ -262,7 +262,7 @@ namespace build2 // Update if depdb mismatch. // - if (dd.writing () || dd.mtime () > mt) + if (dd.writing () || dd.mtime > mt) update = true; dd.close (); diff --git a/build2/depdb.cxx b/build2/depdb.cxx index df73dfd..416f1f4 100644 --- a/build2/depdb.cxx +++ b/build2/depdb.cxx @@ -14,15 +14,15 @@ using namespace butl; namespace build2 { depdb:: - depdb (const path& f) - : mtime_ (file_mtime (f)), touch_ (false) + depdb (path_type p) + : path (move (p)), mtime (file_mtime (path)), touch (false) { fstream::openmode om (fstream::out | fstream::binary); fstream::iostate em (fstream::badbit); - if (mtime_ == timestamp_nonexistent) + if (mtime == timestamp_nonexistent) { - mtime_ = timestamp_unknown; + mtime = timestamp_unknown; state_ = state::write; em |= fstream::failbit; } @@ -32,13 +32,13 @@ namespace build2 om |= fstream::in; } - fs_.open (f.string (), om); + fs_.open (path.string (), om); if (!fs_.is_open ()) { bool c (state_ == state::write); diag_record dr (fail); - dr << "unable to " << (c ? "create" : "open") << ' ' << f; + dr << "unable to " << (c ? "create" : "open") << ' ' << path; if (c) dr << info << "did you forget to add fsdir{} prerequisite for " @@ -93,7 +93,7 @@ namespace build2 fs_.seekp (pos_); // Must be done when changing from read to write. state_ = state::write; - mtime_ = timestamp_unknown; + mtime = timestamp_unknown; } string* depdb:: @@ -217,15 +217,17 @@ namespace build2 // and skip updating mtime (which would probably be incorrect). // // It would be interesting to one day write an implementation that uses - // POSIX file OI, futimens(), and ftruncate() and see how much better it + // POSIX file IO, futimens(), and ftruncate() and see how much better it // performs. // - if (touch_) + if (touch) { fs_.clear (); fs_.exceptions (fstream::failbit | fstream::badbit); fs_.seekp (0, fstream::cur); // Required to switch from read to write. fs_.put ('\0'); + + state_ = state::write; // See below. } } else @@ -246,5 +248,22 @@ namespace build2 } fs_.close (); + + // On some platforms (currently confirmed on Windows and FreeBSD, both + // running as VMs) one can sometimes end up with a modification time that + // is quite a bit after the call to close(). And this messes with our + // arrangement that a valid depdb should be no older than the target it + // is for. + // + // Note that this does not seem to be related to clock adjustments but + // rather feels like the modification time is set when the changes + // actually hit some lower-level layer (e.g., OS or filesystem driver). + // One workaround that appears to work is to query the mtime. This seems + // to force that layer to commit to a timestamp. + // +#if defined(_WIN32) || defined(__FreeBSD__) + if (state_ == state::write) + file_mtime (path); +#endif } } diff --git a/build2/depdb.hxx b/build2/depdb.hxx index 9885723..7c9e464 100644 --- a/build2/depdb.hxx +++ b/build2/depdb.hxx @@ -61,6 +61,19 @@ namespace build2 class depdb { public: + using path_type = build2::path; + + // The modification time of the database only makes sense while reading + // (in the write mode it will be set to timestamp_unknown). + // + // If touch is set to true, update the database modification time in + // close() even if otherwise no modifications are necessary (i.e., the + // database is in the read mode and is at eof). + // + path_type path; + timestamp mtime; + bool touch; + // Open the database for reading. Note that if the file does not exist, // has wrong format version, or is corrupt, then the database will be // immediately switched to writing. @@ -71,20 +84,7 @@ namespace build2 // prerequisite. Handling this as io_error in every rule that uses depdb // would be burdensome thus we issue the diagnostics here. // - depdb (const path&); - - // Return the modification time of the database. This value only makes - // sense while reading (in the write mode it will be timestamp_unknown). - // - timestamp - mtime () const {return mtime_;} - - // Update the database modification time in close() even if otherwise - // no modifications are necessary (i.e., the database is in the read - // mode and is at eof). - // - void - touch () {touch_ = true;} + depdb (path_type); // Close the database. If this function is not called, then the database // may be left in the old/currupt state. Note that in the read mode this @@ -121,9 +121,6 @@ namespace build2 bool writing () const {return state_ == state::write;} - bool - touched () const {return touch_;} - // Skip to the end of the database and return true if it is valid. // Otherwise, return false, in which case the database must be // overwritten. Note that this function expects the database to be in the @@ -140,7 +137,7 @@ namespace build2 write (const string& l, bool nl = true) {write (l.c_str (), l.size (), nl);} void - write (const path& p, bool nl = true) {write (p.string (), nl);} + write (const path_type& p, bool nl = true) {write (p.string (), nl);} void write (const char* s, bool nl = true) {write (s, std::strlen (s), nl);} @@ -178,10 +175,10 @@ namespace build2 } string* - expect (const path& v) + expect (const path_type& v) { string* l (read ()); - if (l == nullptr || path::traits::compare (*l, v.string ()) != 0) + if (l == nullptr || path_type::traits::compare (*l, v.string ()) != 0) { write (v); return l; @@ -211,14 +208,11 @@ namespace build2 read_ (); private: - timestamp mtime_; - std::fstream fs_; - - std::fstream::pos_type pos_; // Start of the last returned line. - string line_; - enum class state {read, read_eof, write} state_; - bool touch_; + + std::fstream fs_; + std::fstream::pos_type pos_; // Start of the last returned line. + string line_; // Current line. }; } diff --git a/build2/in/rule.cxx b/build2/in/rule.cxx index 450f398..86b8ffa 100644 --- a/build2/in/rule.cxx +++ b/build2/in/rule.cxx @@ -157,7 +157,7 @@ namespace build2 // Update if any mismatch or depdb is newer that the output. // - if (dd.writing () || dd.mtime () > mt) + if (dd.writing () || dd.mtime > mt) update = true; // Substituted variable values. |