Improve workaround for backwards modification time issue

3 files changed, 46 insertions, 25 deletions

diff --git a/build2/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index c4fd82b1..981358ed 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -2071,12 +2071,12 @@ namespace build2
       if (dd.writing () || dd.mtime > mt)
         scratch = update = true;
 
-#define MTIME_SANITY_CHECK
-#ifdef MTIME_SANITY_CHECK
+#define BUILD2_MTIME_CHECK
+#ifdef BUILD2_MTIME_CHECK
       timestamp dd_tt (system_clock::now ());
 #endif
 
-      timestamp dd_ct (dd.close ());
+      dd.close ();
 
       // If nothing changed, then we are done.
       //
@@ -2560,7 +2560,7 @@ namespace build2
 
       rm.cancel ();
 
-#ifdef MTIME_SANITY_CHECK
+#ifdef BUILD2_MTIME_CHECK
       {
         timestamp tp_mt (file_mtime (tp));
         timestamp dd_mt (file_mtime (dd.path));
@@ -2568,11 +2568,11 @@ namespace build2
 
         if (dd_mt > tp_mt)
           fail << "backwards modification times:\n"
-               << dd_tt << " window start\n"
-               << dd_ct << " close mtime\n"
-               << dd_mt << " " << dd.path.string () << '\n'
-               << tp_mt << " " << tp.string () << '\n'
-               << tp_tt << " window end";
+               << dd_tt    << " window start\n"
+               << dd.mtime << " close mtime\n"
+               << dd_mt    << " " << dd.path.string () << '\n'
+               << tp_mt    << " " << tp.string () << '\n'
+               << tp_tt    << " window end";
       }
 #endif
 
diff --git a/build2/depdb.cxx b/build2/depdb.cxx
index 8abd5f2e..f0b0d217 100644
--- a/build2/depdb.cxx
+++ b/build2/depdb.cxx
@@ -201,7 +201,7 @@ namespace build2
       fs_.put ('\n');
   }
 
-  timestamp depdb::
+  void depdb::
   close ()
   {
     // If we are at eof, then it means all lines are good, there is the "end
@@ -225,9 +225,7 @@ namespace build2
         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.
+        state_ = state::write;       // Write end marker below.
       }
     }
     else
@@ -238,8 +236,6 @@ namespace build2
         change (false); // Don't flush.
       }
 
-      fs_.put ('\0'); // The "end marker".
-
       // Truncating an fstream is actually a non-portable pain in the butt.
       // What if we leave the junk after the "end marker"? These files are
       // pretty small and chances are they will occupy the filesystem's block
@@ -247,25 +243,50 @@ namespace build2
       // actually be faster not to truncate.
     }
 
-    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.
+    // 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.
+    // 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.
+    //
+    // Well, this seems to work on FreeBSD but on Windows we may still end up
+    // getting the old mtime if we ask for it too soon. For such cases we are
+    // going to just set it ourselves.
     //
 #if defined(_WIN32) || defined(__FreeBSD__)
+#  define BUILD2_NEED_MTIME_FIX
+#endif
+
+#ifdef BUILD2_NEED_MTIME_FIX
+    timestamp mt;
+#endif
+
     if (state_ == state::write)
-      mtime = file_mtime (path);
+    {
+#ifdef BUILD2_NEED_MTIME_FIX
+      mt = system_clock::now ();
 #endif
+      fs_.put ('\0'); // The "end marker".
+    }
+
+    fs_.close ();
+
+#ifdef BUILD2_NEED_MTIME_FIX
+    if (state_ == state::write)
+    {
+      // Save the original returned time for debugging.
+      //
+      mtime = file_mtime (path);
 
-    return mtime;
+      if (mtime < mt)
+        file_mtime (path, mt);
+    }
+#endif
   }
 }
diff --git a/build2/depdb.hxx b/build2/depdb.hxx
index ffb3b890..7c9e464e 100644
--- a/build2/depdb.hxx
+++ b/build2/depdb.hxx
@@ -90,7 +90,7 @@ namespace build2
     // may be left in the old/currupt state. Note that in the read mode this
     // function will "chop off" lines that haven't been read.
     //
-    timestamp
+    void
     close ();
 
     // Read the next line. If the result is not NULL, then it is a pointer to