From cfe15fb80ba2f859d67ef12a8a78b298d69db568 Mon Sep 17 00:00:00 2001 From: David Rose Date: Mon, 25 Oct 2004 20:47:38 +0000 Subject: [PATCH] v1.18: address bad pp.dep caches? --- ppremake/config_msvc.h | 2 +- ppremake/configure.in | 2 +- ppremake/ppDependableFile.cxx | 113 +++++++++++++++++++++++++--------- ppremake/ppDependableFile.h | 5 +- ppremake/ppDirectory.cxx | 110 ++++++++++++++++++++++++++------- 5 files changed, 176 insertions(+), 56 deletions(-) diff --git a/ppremake/config_msvc.h b/ppremake/config_msvc.h index 50a383f23d..bc90297bd1 100644 --- a/ppremake/config_msvc.h +++ b/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 ****************/ diff --git a/ppremake/configure.in b/ppremake/configure.in index 5aa574139b..985573539d 100644 --- a/ppremake/configure.in +++ b/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) diff --git a/ppremake/ppDependableFile.cxx b/ppremake/ppDependableFile.cxx index 1fdb083008..41ce914eb6 100644 --- a/ppremake/ppDependableFile.cxx +++ b/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 &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 &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::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::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; } diff --git a/ppremake/ppDependableFile.h b/ppremake/ppDependableFile.h index a4339435f9..84c959a323 100644 --- a/ppremake/ppDependableFile.h +++ b/ppremake/ppDependableFile.h @@ -29,7 +29,8 @@ class PPDirectory; class PPDependableFile { public: PPDependableFile(PPDirectory *directory, const string &filename); - void update_from_cache(const vector &words); + bool update_from_cache(const vector &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; diff --git a/ppremake/ppDirectory.cxx b/ppremake/ppDirectory.cxx index 0333eaae81..fd808293d7 100644 --- a/ppremake/ppDirectory.cxx +++ b/ppremake/ppDirectory.cxx @@ -23,6 +23,8 @@ #include #endif +#include + #include #include @@ -31,6 +33,9 @@ #include #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 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 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(); } }