Record per-file mtimes in .kati_stamp for accurate regen detection

src-rs/file.rs

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-use std::{ffi::OsStr, os::unix::ffi::OsStrExt, sync::Arc};
+use std::{ffi::OsStr, io::Read, os::unix::ffi::OsStrExt, sync::Arc, time::SystemTime};
 
 use anyhow::Result;
 use bytes::Bytes;
@@ -28,20 +28,29 @@ use crate::{
 
 pub struct Makefile {
     pub filename: Symbol,
+    pub mtime: SystemTime,
     pub stmts: Arc<Mutex<Vec<Stmt>>>,
 }
 
 impl Makefile {
     pub fn from_file(filename: &OsStr) -> Result<Option<Arc<Makefile>>> {
-        if !std::fs::exists(filename)? {
-            return Ok(None);
-        }
-
-        let buf = Bytes::from(std::fs::read(filename)?);
+        let mut file = match std::fs::File::open(filename) {
+            Ok(f) => f,
+            Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None),
+            Err(e) => return Err(e.into()),
+        };
+        let mtime = file.metadata()?.modified()?;
+        let mut buf = Vec::new();
+        file.read_to_end(&mut buf)?;
+        let buf = Bytes::from(buf);
 
         let filename = intern(filename.as_bytes().to_vec());
         let stmts = parse_file(&buf, filename)?;
 
-        Ok(Some(Arc::new(Makefile { filename, stmts })))
+        Ok(Some(Arc::new(Makefile {
+            filename,
+            mtime,
+            stmts,
+        })))
     }
 }

src-rs/file_cache.rs

@@ -15,9 +15,10 @@ limitations under the License.
 */
 
 use std::{
-    collections::{HashMap, HashSet},
+    collections::HashMap,
     ffi::{OsStr, OsString},
     sync::{Arc, LazyLock},
+    time::{SystemTime, UNIX_EPOCH},
 };
 
 use anyhow::Result;
@@ -28,13 +29,13 @@ use crate::file::Makefile;
 static CACHE: LazyLock<Mutex<MakefileCacheManager>> = LazyLock::new(|| {
     Mutex::new(MakefileCacheManager {
         cache: HashMap::new(),
-        extra_file_deps: HashSet::new(),
+        extra_file_deps: HashMap::new(),
     })
 });
 
 struct MakefileCacheManager {
     cache: HashMap<OsString, Option<Arc<Makefile>>>,
-    extra_file_deps: HashSet<OsString>,
+    extra_file_deps: HashMap<OsString, SystemTime>,
 }
 
 impl MakefileCacheManager {
@@ -54,17 +55,25 @@ pub fn get_makefile(filename: &OsStr) -> Result<Option<Arc<Makefile>>> {
 }
 
 pub fn add_extra_file_dep(filename: OsString) {
-    CACHE.lock().extra_file_deps.insert(filename);
+    let mtime = std::fs::metadata(&filename)
+        .and_then(|m| m.modified())
+        .unwrap_or(UNIX_EPOCH);
+    CACHE.lock().extra_file_deps.insert(filename, mtime);
 }
 
-pub fn get_all_filenames() -> HashSet<OsString> {
+pub fn get_all_filenames() -> HashMap<OsString, SystemTime> {
     let manager = CACHE.lock();
-    let mut ret = HashSet::new();
-    for p in manager.cache.keys() {
-        ret.insert(p.clone());
+    let mut ret = HashMap::new();
+    for (p, mk) in &manager.cache {
+        let mtime = mk
+            .as_ref()
+            .map(|m| m.mtime)
+            .or_else(|| std::fs::metadata(p).and_then(|m| m.modified()).ok())
+            .unwrap_or(UNIX_EPOCH);
+        ret.insert(p.clone(), mtime);
     }
-    for f in &manager.extra_file_deps {
-        ret.insert(f.clone());
+    for (f, mtime) in &manager.extra_file_deps {
+        ret.insert(f.clone(), *mtime);
     }
     ret
 }

src-rs/ninja.rs

@@ -743,8 +743,13 @@ impl<'a> NinjaGenerator<'a> {
             let makefiles = file_cache::get_all_filenames();
             dump_usize(&mut out, makefiles.len() + 1)?;
             dump_string(&mut out, self.kati_binary.as_bytes())?;
-            for makefile in makefiles {
+            let kati_binary_ts = std::fs::metadata(&self.kati_binary)
+                .and_then(|m| m.modified())
+                .unwrap_or(self.start_time);
+            dump_systemtime(&mut out, &kati_binary_ts)?;
+            for (makefile, mtime) in makefiles {
                 dump_string(&mut out, makefile.as_bytes())?;
+                dump_systemtime(&mut out, &mtime)?;
             }
 
             dump_usize(&mut out, Evaluator::used_undefined_vars().len())?;

src-rs/regen.rs

@@ -148,10 +148,15 @@ impl StampChecker {
             println!("Generated time: {:?}", self.gen_time);
         }
 
-        let files = load!(load_vec_string(fp));
-        for s in files {
+        let num_files = load!(load_usize(fp));
+        for _ in 0..num_files {
+            let s = load!(load_string(fp));
+            let recorded_ts = load!(load_systemtime(fp));
             let ts = std::fs::metadata(&s).and_then(|m| m.modified());
-            if ts.as_ref().is_ok_and(|ts| gen_time >= *ts) {
+            if ts
+                .as_ref()
+                .is_ok_and(|ts| recorded_ts + std::time::Duration::from_micros(1) >= *ts)
+            {
                 if FLAGS.dump_kati_stamp {
                     println!("file {s:?}: clean ({:?})", ts.unwrap())
                 }
@@ -162,12 +167,12 @@ impl StampChecker {
                 }
                 if should_ignore_dirty(s.as_bytes()) {
                     if FLAGS.regen_debug {
-                        println!("file {s:?}: ignored ({:?})", ts.unwrap());
+                        println!("file {s:?}: ignored ({:?})", ts.ok());
                     }
                     continue;
                 }
                 if FLAGS.dump_kati_stamp {
-                    println!("file {s:?}: dirty ({:?})", ts.unwrap());
+                    println!("file {s:?}: dirty ({:?} > {:?})", ts, recorded_ts);
                 } else {
                     eprintln!("{} was modified, regenerating...", s.to_string_lossy());
                 }

src-rs/regen_dump.rs

@@ -85,9 +85,11 @@ fn inner(
     //
 
     {
-        let files = load_vec_string(fp)?;
-        if dump_files {
-            for file in files {
+        let num_files = load_usize(fp)?;
+        for _ in 0..num_files {
+            let file = load_string(fp)?;
+            let _ts = load_systemtime(fp)?;
+            if dump_files {
                 println!("{}", file.display());
             }
         }

src/file.cc

@@ -39,7 +39,7 @@ Makefile::Makefile(const std::string& filename)
   }
 
   size_t len = st.st_size;
-  mtime_ = st.st_mtime;
+  mtime_ = GetTimestampFromStat(st);
   buf_.resize(len);
   exists_ = true;
   size_t remaining = len;

src/file.h

@@ -34,10 +34,11 @@ class Makefile {
   std::vector<Stmt*>* mutable_stmts() { return &stmts_; }
 
   bool Exists() const { return exists_; }
+  double mtime() const { return mtime_; }
 
  private:
   std::string buf_;
-  uint64_t mtime_;
+  double mtime_;
   std::string filename_;
   std::vector<Stmt*> stmts_;
   bool exists_;

src/file_cache.cc

@@ -18,6 +18,7 @@
 
 #include "file.h"
 #include "file_cache.h"
+#include "fileutil.h"
 
 const Makefile& MakefileCacheManager::ReadMakefile(
     const std::string& filename) {
@@ -29,15 +30,16 @@ const Makefile& MakefileCacheManager::ReadMakefile(
 }
 
 void MakefileCacheManager::GetAllFilenames(
-    std::unordered_set<std::string>* out) {
+    std::unordered_map<std::string, double>* out) const {
   for (const auto& p : cache_)
-    out->insert(p.first);
+    (*out)[p.first] = p.second.mtime();
   for (const auto& f : extra_file_deps_)
-    out->insert(f);
+    (*out)[f.first] = f.second;
 }
 
 void MakefileCacheManager::AddExtraFileDep(std::string_view dep) {
-  extra_file_deps_.emplace(dep);
+  std::string s(dep);
+  extra_file_deps_[s] = GetTimestamp(s);
 }
 
 MakefileCacheManager& MakefileCacheManager::Get() {

src/file_cache.h

@@ -16,14 +16,15 @@
 #define FILE_CACHE_H_
 
 #include <string>
+#include <unordered_map>
 #include <unordered_set>
 
 #include "file.h"
 
 class MakefileCacheManager {
  public:
   const Makefile& ReadMakefile(const std::string& filename);
-  void GetAllFilenames(std::unordered_set<std::string>* out);
+  void GetAllFilenames(std::unordered_map<std::string, double>* out) const;
   void AddExtraFileDep(std::string_view dep);
 
   static MakefileCacheManager& Get();
@@ -33,7 +34,7 @@ class MakefileCacheManager {
   MakefileCacheManager(const MakefileCacheManager&) = delete;
   MakefileCacheManager(MakefileCacheManager&&) = delete;
   std::unordered_map<std::string, Makefile> cache_;
-  std::unordered_set<std::string> extra_file_deps_;
+  std::unordered_map<std::string, double> extra_file_deps_;
 };
 
 #endif  // FILE_CACHE_H_

src/ninja.cc

@@ -674,12 +674,17 @@ class NinjaGenerator {
     size_t r = fwrite(&start_time_, sizeof(start_time_), 1, fp);
     CHECK(r == 1);
 
-    std::unordered_set<std::string> makefiles;
+    std::unordered_map<std::string, double> makefiles;
     MakefileCacheManager::Get().GetAllFilenames(&makefiles);
     DumpInt(fp, makefiles.size() + 1);
     DumpString(fp, kati_binary_);
-    for (const std::string& makefile : makefiles) {
+    double kati_binary_ts = GetTimestamp(kati_binary_);
+    r = fwrite(&kati_binary_ts, sizeof(kati_binary_ts), 1, fp);
+    CHECK(r == 1);
+    for (const auto& [makefile, mtime] : makefiles) {
       DumpString(fp, makefile);
+      r = fwrite(&mtime, sizeof(mtime), 1, fp);
+      CHECK(r == 1);
     }
 
     DumpInt(fp, Evaluator::used_undefined_vars().size());

src/regen.cc

@@ -160,10 +160,15 @@ class StampChecker {
     int num_files = LOAD_INT(fp);
     for (int i = 0; i < num_files; i++) {
       LOAD_STRING(fp, &s);
+      double recorded_ts;
+      if (fread(&recorded_ts, sizeof(recorded_ts), 1, fp) != 1) {
+        fprintf(stderr, "incomplete kati_stamp, regenerating...\n");
+        RETURN_TRUE;
+      }
       double ts = GetTimestamp(s);
       // GetTimestamp returns < 0 when there's an error reading the file, like
-      // when its been removed.
-      if (gen_time < ts || ts < 0) {
+      // when its been removed. Allow 1e-6s tolerance for float representation.
+      if (recorded_ts + 1e-6 < ts || ts < 0) {
         if (g_flags.regen_ignoring_kati_binary) {
           if (s == GetExecutablePath()) {
             fprintf(stderr, "%s was modified, ignored.\n", s.c_str());
@@ -176,7 +181,7 @@ class StampChecker {
           continue;
         }
         if (g_flags.dump_kati_stamp)
-          printf("file %s: dirty (%f)\n", s.c_str(), ts);
+          printf("file %s: dirty (%f > %f)\n", s.c_str(), ts, recorded_ts);
         else
           fprintf(stderr, "%s was modified, regenerating...\n", s.c_str());
         RETURN_TRUE;

src/regen_dump.cc

@@ -95,9 +95,17 @@ int stamp_dump_main(int argc, char* argv[]) {
   //
 
   {
-    auto files = LoadVecString(fp);
-    if (dump_files) {
-      for (const auto& f : files) {
+    int num_files = LoadInt(fp);
+    if (num_files < 0)
+      ERROR("Incomplete stamp file");
+    for (int i = 0; i < num_files; i++) {
+      std::string f;
+      if (!LoadString(fp, &f))
+        ERROR("Incomplete stamp file");
+      double ts;
+      if (fread(&ts, sizeof(ts), 1, fp) != 1)
+        ERROR("Incomplete stamp file");
+      if (dump_files) {
         printf("%s\n", f.c_str());
       }
     }