v1.18: address bad pp.dep caches?

ppremake/config_msvc.h

@@ -86,5 +86,5 @@
  **         Also be sure to change the version number        **
  **             at the beginning of configure.in.            **
  ****************                              ****************/
-#define VERSION "1.17"
+#define VERSION "1.18"
 /****************  UPDATE VERSION NUMBER HERE  ****************/

ppremake/configure.in

@@ -5,7 +5,7 @@ dnl ****************  UPDATE VERSION NUMBER HERE  ****************
 dnl **         Also be sure to change the version number        **
 dnl **                at the end of config_msvc.h.              **
 dnl ****************                              ****************
-AM_INIT_AUTOMAKE(ppremake, 1.17)
+AM_INIT_AUTOMAKE(ppremake, 1.18)
 dnl ****************  UPDATE VERSION NUMBER HERE  ****************
 
 AM_CONFIG_HEADER(config.h)

ppremake/ppDependableFile.cxx

@@ -60,8 +60,11 @@ PPDependableFile(PPDirectory *directory, const string &filename) :
 //               cached modification time against the file's actual
 //               modification time, and storing the cached
 //               dependencies if they match.
+//
+//               The return value is true if the cache is valid, false
+//               if something appears to be wrong.
 ////////////////////////////////////////////////////////////////////
-void PPDependableFile::
+bool PPDependableFile::
 update_from_cache(const vector<string> &words) {
   // Shouldn't call this once the file has actually been read.
   assert((_flags & F_updated) == 0);
@@ -69,44 +72,63 @@ update_from_cache(const vector<string> &words) {
   assert((_flags & F_from_cache) == 0);
   assert(words.size() >= 2);
 
-  // The second parameter is the cached modification time.
-  time_t mtime = strtol(words[1].c_str(), (char **)NULL, 10);
-  if (mtime == get_mtime()) {
-    // The modification matches; preserve the cache information.
-    PPDirectoryTree *tree = _directory->get_tree();
+  if (!exists()) {
+    // The file doesn't even exist; clearly the cache is bad.
+    _flags |= F_bad_cache;
 
-    _dependencies.clear();
-    vector<string>::const_iterator wi;
-    for (wi = words.begin() + 2; wi != words.end(); ++wi) {
-      string dirpath = (*wi);
+  } else {
+    // The second parameter is the cached modification time.
+    time_t mtime = strtol(words[1].c_str(), (char **)NULL, 10);
+    if (mtime == get_mtime()) {
+      // The modification matches; preserve the cache information.
+      PPDirectoryTree *tree = _directory->get_tree();
 
-      Dependency dep;
-      dep._okcircular = false;
+      _dependencies.clear();
+      vector<string>::const_iterator wi;
+      for (wi = words.begin() + 2; wi != words.end(); ++wi) {
+        string dirpath = (*wi);
 
-      if (dirpath.length() > 1 && dirpath[0] == '/') {
-        // If the first character is '/', it means that the file has
-        // been marked okcircular.
-        dep._okcircular = true;
-        dirpath = dirpath.substr(1);
-      }
+        Dependency dep;
+        dep._okcircular = false;
 
-      if (dirpath.length() > 2 && dirpath.substr(0, 2) == "*/") {
-        // This is an extra include file, not a file in this source
-        // tree.
-        _extra_includes.push_back(dirpath.substr(2));
+        if (dirpath.length() > 1 && dirpath[0] == '/') {
+          // If the first character is '/', it means that the file has
+          // been marked okcircular.
+          dep._okcircular = true;
+          dirpath = dirpath.substr(1);
+        }
 
-      } else {
-        dep._file = 
-          tree->get_dependable_file_by_dirpath(dirpath, false);
-        if (dep._file != (PPDependableFile *)NULL) {
-          _dependencies.push_back(dep);
+        if (dirpath.length() > 2 && dirpath.substr(0, 2) == "*/") {
+          // This is an extra include file, not a file in this source
+          // tree.
+          _extra_includes.push_back(dirpath.substr(2));
+
+        } else {
+          dep._file = 
+            tree->get_dependable_file_by_dirpath(dirpath, false);
+          if (dep._file != (PPDependableFile *)NULL) {
+            _dependencies.push_back(dep);
+          }
         }
       }
-    }
 
-    _flags |= F_from_cache;
-    sort(_dependencies.begin(), _dependencies.end());
+      _flags |= F_from_cache;
+      sort(_dependencies.begin(), _dependencies.end());
+    }
   }
+
+  return ((_flags & F_bad_cache) == 0);
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: PPDependableFile::clear_cache
+//       Access: Public
+//  Description: Forgets the cache we just read.
+////////////////////////////////////////////////////////////////////
+void PPDependableFile::
+clear_cache() {
+  _dependencies.clear();
+  _flags &= ~(F_bad_cache | F_from_cache);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -317,6 +339,17 @@ was_examined() const {
   return ((_flags & F_updated) != 0);
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: PPDependableFile::was_cached
+//       Access: Public
+//  Description: Returns true if this file was found in the cache,
+//               false otherwise.
+////////////////////////////////////////////////////////////////////
+bool PPDependableFile::
+was_cached() const {
+  return ((_flags & F_from_cache) != 0);
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: PPDependableFile::update_dependencies
 //       Access: Private
@@ -375,6 +408,7 @@ compute_dependencies(string &circularity) {
       } else {
         cerr << "Warning: dependent file " << filename
              << " does not exist.\n";
+        _flags |= F_bad_cache;
       }
 
     } else {
@@ -427,6 +461,12 @@ compute_dependencies(string &circularity) {
     // with an "okcircular" comment.
     if (!(*di)._okcircular) {
       circ = (*di)._file->compute_dependencies(circularity);
+      if (((*di)._file->_flags & F_bad_cache) != 0) {
+        // If our dependent file had a broken cache, our own cache is
+        // suspect.
+        _flags |= F_bad_cache;
+      }
+
       if (circ != (PPDependableFile *)NULL) {
         // Oops, a circularity.  Silly user.
         circularity = get_dirpath() + " => " + circularity;
@@ -441,6 +481,19 @@ compute_dependencies(string &circularity) {
 
   _flags = (_flags & ~F_updating) | F_updated;
   sort(_dependencies.begin(), _dependencies.end());
+
+  if ((_flags & (F_bad_cache | F_from_cache)) == (F_bad_cache | F_from_cache)) {
+    // Our cache is suspect.  Re-read the file to flush the cache.
+    if (verbose || true) {
+      cerr << "Dependency cache for \"" << get_fullpath() << "\" is suspect.\n";
+    }
+
+    clear_cache();
+    _flags &= ~F_updated;
+
+    return compute_dependencies(circularity);
+  }
+
   return circ;
 }
 

ppremake/ppDependableFile.h

@@ -29,7 +29,8 @@ class PPDirectory;
 class PPDependableFile {
 public:
   PPDependableFile(PPDirectory *directory, const string &filename);
-  void update_from_cache(const vector<string> &words);
+  bool update_from_cache(const vector<string> &words);
+  void clear_cache();
   void write_cache(ostream &out);
 
   PPDirectory *get_directory() const;
@@ -51,6 +52,7 @@ public:
   string get_circularity();
 
   bool was_examined() const;
+  bool was_cached() const;
 
 private:
   void update_dependencies();
@@ -67,6 +69,7 @@ private:
     F_statted     = 0x008,
     F_exists      = 0x010,
     F_from_cache  = 0x020,
+    F_bad_cache   = 0x040,
   };
   int _flags;
   string _circularity;

ppremake/ppDirectory.cxx

@@ -23,6 +23,8 @@
 #include <sys/types.h>
 #endif
 
+#include <sys/stat.h>
+
 #include <algorithm>
 #include <assert.h>
 
@@ -31,6 +33,9 @@
 #include <windows.h>
 #endif
 
+// How new must a pp.dep cache file be before we will believe it?
+static const int max_cache_minutes = 60;
+
 PPDirectory *current_output_directory = (PPDirectory *)NULL;
 
 // An STL object to sort directories in order by dependency and then
@@ -715,26 +720,86 @@ read_file_dependencies(const string &cache_filename) {
   cache_pathname.set_text();
   ifstream in;
 
-  if (!cache_pathname.open_read(in)) {
-    // Can't read it.  Maybe it's not there.  No problem.
+  // Does the cache file exist, and is it recent enough?  We don't
+  // trust old cache files on principle.
+
+  string os_specific = cache_pathname.to_os_specific();
+  time_t now = time(NULL);
+
+#ifdef WIN32_VC
+  struct _stat this_buf;
+  bool this_exists = false;
+
+  if (_stat(os_specific.c_str(), &this_buf) == 0) {
+    this_exists = true;
+  }
+#else  // WIN32_VC
+  struct stat this_buf;
+  bool this_exists = false;
+
+  if (stat(os_specific.c_str(), &this_buf) == 0) {
+    this_exists = true;
+  }
+#endif
+
+  if (!this_exists) {
+    // The cache file doesn't exist.  That's OK.
     if (verbose) {
-      cerr << "Couldn't read \"" << cache_pathname << "\"\n";
-    }
-  } else {
-    if (verbose) {
-      cerr << "Loading cache \"" << cache_pathname << "\"\n";
+      cerr << "No cache file: \"" << cache_pathname << "\"\n";
     }
 
-    string line;
-    getline(in, line);
-    while (!in.fail() && !in.eof()) {
-      vector<string> words;
-      tokenize_whitespace(line, words);
-      if (words.size() >= 2) {
-        PPDependableFile *file = get_dependable_file(words[0], false);
-        file->update_from_cache(words);
+  } else if (this_buf.st_mtime < now - 60 * max_cache_minutes) {
+    // It exists, but it's too old.
+    if (verbose) {
+      cerr << "Cache file too old: \"" << cache_pathname << "\"\n";
+    }
+
+  } else {
+    // It exists and is new enough; use it.
+    if (!cache_pathname.open_read(in)) {
+      cerr << "Couldn't read \"" << cache_pathname << "\"\n";
+  
+    } else {
+      if (verbose) {
+        cerr << "Loading cache \"" << cache_pathname << "\"\n";
       }
+
+      bool okcache = true;
+      
+      string line;
       getline(in, line);
+      while (!in.fail() && !in.eof()) {
+        vector<string> words;
+        tokenize_whitespace(line, words);
+        if (words.size() >= 2) {
+          PPDependableFile *file = get_dependable_file(words[0], false);
+          if (!file->update_from_cache(words)) {
+            // Hey, we asked for an invalid or absent file.  Phooey.
+            // Invalidate the cache, and also make sure that this
+            // particular file (which maybe doesn't even exist) isn't
+            // mentioned in the cache file any more.
+            Dependables::iterator di;
+            di = _dependables.find(words[0]);
+            if (di != _dependables.end()) {
+              _dependables.erase(di);
+            }
+
+            okcache = false;
+            break;
+          }
+        }
+        getline(in, line);
+      }
+
+      if (!okcache) {
+        if (verbose || true) {
+          cerr << "Cache \"" << cache_pathname << "\" is stale.\n";
+        }
+        Dependables::iterator di;
+        for (di = _dependables.begin(); di != _dependables.end(); ++di) {
+          (*di).second->clear_cache();
+        }
+      }
     }
   }
     
@@ -773,9 +838,8 @@ update_file_dependencies(const string &cache_filename) {
     cache_pathname.unlink();
 
     // If we have no files, don't bother writing the cache.
+    bool wrote_anything = false;
     if (!_dependables.empty()) {
-      bool wrote_anything = false;
-      
       ofstream out;
       if (!cache_pathname.open_write(out)) {
         cerr << "Cannot update cache dependency file " << cache_pathname << "\n";
@@ -792,7 +856,7 @@ update_file_dependencies(const string &cache_filename) {
       Dependables::const_iterator di;
       for (di = _dependables.begin(); di != _dependables.end(); ++di) {
         PPDependableFile *file = (*di).second;
-        if (file->was_examined() || external_tree) {
+        if (file->was_examined() || (external_tree && file->was_cached())) {
           if (file->is_circularity()) {
             cerr << "Warning: circular #include directives:\n"
                  << "  " << file->get_circularity() << "\n";
@@ -803,12 +867,12 @@ update_file_dependencies(const string &cache_filename) {
       }
       
       out.close();
+    }
       
-      if (!wrote_anything) {
-        // Well, if we didn't write anything, remove the cache file
-        // after all.
-        cache_pathname.unlink();
-      }
+    if (!wrote_anything) {
+      // Well, if we didn't write anything, remove the cache file
+      // after all.
+      cache_pathname.unlink();
     }
   }