From 8901004f9e783fb3a30e2ddb1e69e8f8a7d085f5 Mon Sep 17 00:00:00 2001
From: Boris Kolpackov <boris@codesynthesis.com>
Date: Wed, 28 Nov 2018 11:11:10 +0200
Subject: Add --[no-]mtime-check options to control this behavior at runtime

By default the checks are enabled only for the staged toolchain.
---
 build2/b-options.cxx       | 15 +++++++++++++++
 build2/b-options.hxx       |  8 ++++++++
 build2/b-options.ixx       | 12 ++++++++++++
 build2/b.cli               | 13 +++++++++++++
 build2/b.cxx               |  5 ++++-
 build2/cc/compile-rule.cxx |  9 +++++++--
 build2/cc/link-rule.cxx    |  2 +-
 build2/cli/rule.cxx        |  2 +-
 build2/config.hxx.in       |  9 +++++++++
 build2/depdb.cxx           | 35 ++++++++++++++++-------------------
 build2/depdb.hxx           | 36 +++++++++++++++++++++---------------
 build2/depdb.ixx           | 23 +++++++++++++++++++----
 build2/in/rule.cxx         |  2 +-
 13 files changed, 127 insertions(+), 44 deletions(-)

diff --git a/build2/b-options.cxx b/build2/b-options.cxx
index 23ebae73..6409f3e7 100644
--- a/build2/b-options.cxx
+++ b/build2/b-options.cxx
@@ -612,6 +612,8 @@ namespace build2
     max_stack_ (),
     max_stack_specified_ (false),
     serial_stop_ (),
+    mtime_check_ (),
+    no_mtime_check_ (),
     structured_result_ (),
     match_only_ (),
     no_column_ (),
@@ -792,6 +794,15 @@ namespace build2
        << "                     parallel execution, add --jobs|-j\033[0m (for example -j 0\033[0m for" << ::std::endl
        << "                     default concurrency)." << ::std::endl;
 
+    os << std::endl
+       << "\033[1m--mtime-check\033[0m        Perform file modification time sanity checks. These checks" << ::std::endl
+       << "                     can be helpful in diagnosing spurious rebuilds and are" << ::std::endl
+       << "                     enabled by default for the staged version of the build" << ::std::endl
+       << "                     system. Use \033[1m--no-mtime-check\033[0m to disable." << ::std::endl;
+
+    os << std::endl
+       << "\033[1m--no-mtime-check\033[0m     Don't perform file modification time sanity checks." << ::std::endl;
+
     os << std::endl
        << "\033[1m--structured-result\033[0m  Write the result of execution in a structured form. In" << ::std::endl
        << "                     this mode, instead of printing to \033[1mSTDERR\033[0m diagnostics" << ::std::endl
@@ -926,6 +937,10 @@ namespace build2
       &::build2::cl::thunk< options, bool, &options::serial_stop_ >;
       _cli_options_map_["-s"] = 
       &::build2::cl::thunk< options, bool, &options::serial_stop_ >;
+      _cli_options_map_["--mtime-check"] = 
+      &::build2::cl::thunk< options, bool, &options::mtime_check_ >;
+      _cli_options_map_["--no-mtime-check"] = 
+      &::build2::cl::thunk< options, bool, &options::no_mtime_check_ >;
       _cli_options_map_["--structured-result"] = 
       &::build2::cl::thunk< options, bool, &options::structured_result_ >;
       _cli_options_map_["--match-only"] = 
diff --git a/build2/b-options.hxx b/build2/b-options.hxx
index 0aad125c..94a87abc 100644
--- a/build2/b-options.hxx
+++ b/build2/b-options.hxx
@@ -472,6 +472,12 @@ namespace build2
     const bool&
     serial_stop () const;
 
+    const bool&
+    mtime_check () const;
+
+    const bool&
+    no_mtime_check () const;
+
     const bool&
     structured_result () const;
 
@@ -558,6 +564,8 @@ namespace build2
     size_t max_stack_;
     bool max_stack_specified_;
     bool serial_stop_;
+    bool mtime_check_;
+    bool no_mtime_check_;
     bool structured_result_;
     bool match_only_;
     bool no_column_;
diff --git a/build2/b-options.ixx b/build2/b-options.ixx
index 2b6e8b04..b05ed668 100644
--- a/build2/b-options.ixx
+++ b/build2/b-options.ixx
@@ -345,6 +345,18 @@ namespace build2
     return this->serial_stop_;
   }
 
+  inline const bool& options::
+  mtime_check () const
+  {
+    return this->mtime_check_;
+  }
+
+  inline const bool& options::
+  no_mtime_check () const
+  {
+    return this->no_mtime_check_;
+  }
+
   inline const bool& options::
   structured_result () const
   {
diff --git a/build2/b.cli b/build2/b.cli
index 77f50ba9..68ef0fec 100644
--- a/build2/b.cli
+++ b/build2/b.cli
@@ -508,6 +508,19 @@ namespace build2
        \c{-j 0} for default concurrency)."
     }
 
+    bool --mtime-check
+    {
+      "Perform file modification time sanity checks. These checks can be
+       helpful in diagnosing spurious rebuilds and are enabled by default
+       for the staged version of the build system. Use \cb{--no-mtime-check}
+       to disable."
+    }
+
+    bool --no-mtime-check
+    {
+      "Don't perform file modification time sanity checks."
+    }
+
     bool --structured-result
     {
       "Write the result of execution in a structured form. In this mode,
diff --git a/build2/b.cxx b/build2/b.cxx
index 74dc7a21..bb7ec418 100644
--- a/build2/b.cxx
+++ b/build2/b.cxx
@@ -327,7 +327,10 @@ main (int argc, char* argv[])
     // Validate options.
     //
     if (ops.progress () && ops.no_progress ())
-      fail << "inconsistent progress display options";
+      fail << "both --progress and --no-progress specified";
+
+    if (ops.mtime_check () && ops.no_mtime_check ())
+      fail << "both --mtime-check and --no-mtime-check specified";
 
     // Global initializations.
     //
diff --git a/build2/cc/compile-rule.cxx b/build2/cc/compile-rule.cxx
index 93261936..897a9f16 100644
--- a/build2/cc/compile-rule.cxx
+++ b/build2/cc/compile-rule.cxx
@@ -4250,8 +4250,13 @@ namespace build2
       }
 
       // Make sure depdb is no older than any of our prerequisites (see md.mt
-      // logic description above for details).
+      // logic description above for details). Also save the sequence start
+      // time if doing mtime checks (see the depdb::check_mtime() call below).
       //
+      timestamp start (depdb::mtime_check ()
+                       ? system_clock::now ()
+                       : timestamp_unknown);
+
       touch (md.dd, false, verb_never);
 
       const scope& bs (t.base_scope ());
@@ -4670,7 +4675,7 @@ namespace build2
       }
 
       timestamp now (system_clock::now ());
-      depdb::verify (timestamp_unknown, md.dd, tp, now);
+      depdb::check_mtime (start, md.dd, tp, now);
 
       // 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/cc/link-rule.cxx b/build2/cc/link-rule.cxx
index 47c058d0..ca517000 100644
--- a/build2/cc/link-rule.cxx
+++ b/build2/cc/link-rule.cxx
@@ -2554,7 +2554,7 @@ namespace build2
       }
 
       rm.cancel ();
-      dd.verify (tp);
+      dd.check_mtime (tp);
 
       // 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 d07af158..626d0ee0 100644
--- a/build2/cli/rule.cxx
+++ b/build2/cli/rule.cxx
@@ -324,7 +324,7 @@ namespace build2
 
       run (cli, args);
 
-      dd.verify (tp);
+      dd.check_mtime (tp);
 
       t.mtime (system_clock::now ());
       return target_state::changed;
diff --git a/build2/config.hxx.in b/build2/config.hxx.in
index af42fcd0..e84fd6ae 100644
--- a/build2/config.hxx.in
+++ b/build2/config.hxx.in
@@ -21,6 +21,15 @@
 //
 #define BUILD2_STAGE true
 
+// Modification time sanity checks are by default only enabled for the staged
+// version but this can be overridden at runtime with --[no-]mtime-check.
+//
+#if BUILD2_STAGE
+#  define BUILD2_MTIME_CHECK true
+#else
+#  define BUILD2_MTIME_CHECK false
+#endif
+
 #ifdef BUILD2_BOOTSTRAP
 #else
 #endif
diff --git a/build2/depdb.cxx b/build2/depdb.cxx
index 94b845de..f89daba9 100644
--- a/build2/depdb.cxx
+++ b/build2/depdb.cxx
@@ -272,9 +272,8 @@ namespace build2
       change (true /* truncate */);
     }
 
-#ifdef BUILD2_MTIME_CHECK
-    start_ = system_clock::now ();
-#endif
+    if (mtime_check ())
+      start_ = system_clock::now ();
 
     os_.put ('\0'); // The "end marker".
     os_.close ();
@@ -296,13 +295,9 @@ namespace build2
 #endif
   }
 
-#ifdef BUILD2_MTIME_CHECK
   void depdb::
-  verify (const path_type& t, timestamp e)
+  check_mtime_ (const path_type& t, timestamp e)
   {
-    if (state_ != state::write)
-      return;
-
     // We could call the static version but then we would have lost additional
     // information for some platforms.
     //
@@ -315,18 +310,21 @@ namespace build2
         e = system_clock::now ();
 
       fail << "backwards modification times detected:\n"
-           << start_ << " sequence start\n"
+           << "    " << start_ << " sequence start\n"
 #if defined(__FreeBSD__)
-           << mtime  << " close mtime\n"
+           << "    " << mtime  << " close mtime\n"
 #endif
-           << d_mt   << " " << path.string () << '\n'
-           << t_mt   << " " << t.string () << '\n'
-           << e      << " sequence end";
+           << "    " << d_mt   << " " << path.string () << '\n'
+           << "    " << t_mt   << " " << t.string () << '\n'
+           << "    " << e      << " sequence end";
     }
   }
 
   void depdb::
-  verify (timestamp s, const path_type& d, const path_type& t, timestamp e)
+  check_mtime_ (timestamp s,
+                const path_type& d,
+                const path_type& t,
+                timestamp e)
   {
     timestamp t_mt (file_mtime (t));
     timestamp d_mt (file_mtime (d));
@@ -334,11 +332,10 @@ namespace build2
     if (d_mt > t_mt)
     {
       fail << "backwards modification times detected:\n"
-           << s    << " sequence start\n"
-           << d_mt << " " << d.string () << '\n'
-           << t_mt << " " << t.string () << '\n'
-           << e    << " sequence end";
+           << "    " << s    << " sequence start\n"
+           << "    " << d_mt << " " << d.string () << '\n'
+           << "    " << t_mt << " " << t.string () << '\n'
+           << "    " << e    << " sequence end";
     }
   }
-#endif // BUILD2_MTIME_CHECK
 }
diff --git a/build2/depdb.hxx b/build2/depdb.hxx
index 66d89e99..f93f16c4 100644
--- a/build2/depdb.hxx
+++ b/build2/depdb.hxx
@@ -10,8 +10,6 @@
 #include <build2/types.hxx>
 #include <build2/utility.hxx>
 
-#define BUILD2_MTIME_CHECK
-
 namespace build2
 {
   // Auxiliary dependency database (those .d files). Uses io_error and
@@ -110,8 +108,8 @@ 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.
     //
-    // Make sure to also call verify() after updating the target to perform
-    // the target/database modification times sanity check.
+    // Make sure to also call check_mtime() after updating the target to
+    // perform the target/database modification times sanity checks.
     //
     void
     close ();
@@ -119,13 +117,18 @@ namespace build2
     // Perform target/database modification times sanity check.
     //
     void
-    verify (const path_type& target, timestamp end = timestamp_unknown);
+    check_mtime (const path_type& target, timestamp end = timestamp_unknown);
 
     static void
-    verify (timestamp start,
-            const path_type& db,
-            const path_type& target,
-            timestamp end);
+    check_mtime (timestamp start,
+                 const path_type& db,
+                 const path_type& target,
+                 timestamp end);
+
+    // Return true if mtime checks are enabled.
+    //
+    static bool
+    mtime_check ();
 
     // Read the next line. If the result is not NULL, then it is a pointer to
     // the next line in the database (which you are free to move from). If you
@@ -251,13 +254,16 @@ namespace build2
     string*
     read_ ();
 
-  private:
-    uint64_t pos_;  // Start of the last returned line.
-    string   line_; // Current line.
+    void
+    check_mtime_ (const path_type&, timestamp);
+
+    static void
+    check_mtime_ (timestamp, const path_type&, const path_type&, timestamp);
 
-#ifdef BUILD2_MTIME_CHECK
-    timestamp start_;
-#endif
+  private:
+    uint64_t  pos_;   // Start of the last returned line.
+    string    line_;  // Current line.
+    timestamp start_; // Sequence start (mtime check).
   };
 }
 
diff --git a/build2/depdb.ixx b/build2/depdb.ixx
index 46fdb286..ecaf102b 100644
--- a/build2/depdb.ixx
+++ b/build2/depdb.ixx
@@ -13,15 +13,30 @@ namespace build2
       os_.~ofdstream ();
   }
 
-#ifndef BUILD2_MTIME_CHECK
+  inline bool depdb::
+  mtime_check ()
+  {
+    // Note: options were validated in main().
+    //
+    return (ops.   mtime_check () ? true  :
+            ops.no_mtime_check () ? false :
+            BUILD2_MTIME_CHECK);
+  }
+
   inline void depdb::
-  verify (const path_type&, timestamp)
+  check_mtime (const path_type& t, timestamp e)
   {
+    if (state_ == state::write && mtime_check ())
+      check_mtime_ (t, e);
   }
 
   inline void depdb::
-  verify (timestamp, const path_type&, const path_type&, timestamp)
+  check_mtime (timestamp s,
+               const path_type& d,
+               const path_type& t,
+               timestamp e)
   {
+    if (mtime_check ())
+      check_mtime_ (s, d, t, e);
   }
-#endif
 }
diff --git a/build2/in/rule.cxx b/build2/in/rule.cxx
index 1395958c..924bf954 100644
--- a/build2/in/rule.cxx
+++ b/build2/in/rule.cxx
@@ -401,7 +401,7 @@ namespace build2
         fail << "unable to " << what << ' ' << *whom << ": " << e;
       }
 
-      dd.verify (tp);
+      dd.check_mtime (tp);
 
       t.mtime (system_clock::now ());
       return target_state::changed;
-- 
cgit