aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBoris Kolpackov <boris@codesynthesis.com>2018-11-19 14:58:20 +0200
committerBoris Kolpackov <boris@codesynthesis.com>2018-11-19 14:58:20 +0200
commitac5ef35b5fa8b938ed427df4b0ad44e5b6b52cff (patch)
treefa1544e9ba088d4e054a095eec6a196502361d4e
parent5136a6b723e2eeb27718747f1bfd822d0e790b86 (diff)
Finalize workaround for backwards modification time issue
-rw-r--r--build2/cc/compile-rule.cxx10
-rw-r--r--build2/cc/link-rule.cxx25
-rw-r--r--build2/cli/rule.cxx2
-rw-r--r--build2/depdb.cxx37
-rw-r--r--build2/depdb.hxx48
-rw-r--r--build2/in/rule.cxx2
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.