Let Stat() have an err outparam instead of writing to stderr.

Also check for Stat() failure in a few more places.

This way, ninja doesn't print two "ninja: error: " lines if stat() fails
during a build.  It also makes it easier to keep the stat tests quiet.
Every caller of Stat() needs to explicitly log the error string if
that's desired.

diff --git a/src/build.cc b/src/build.cc
index 1e10c7c..d898929 100644 (file)
--- a/src/build.cc
+++ b/src/build.cc
@@ -560,10 +560,12 @@ void Builder::Cleanup() {
         // need to rebuild an output because of a modified header file
         // mentioned in a depfile, and the command touches its depfile
         // but is interrupted before it touches its output file.)
-        if (!depfile.empty() ||
-            (*o)->mtime() != disk_interface_->Stat((*o)->path())) {
+        string err;
+        TimeStamp new_mtime = disk_interface_->Stat((*o)->path(), &err);
+        if (new_mtime == -1)  // Log and ignore Stat() errors.
+          Error("%s", err.c_str());
+        if (!depfile.empty() || (*o)->mtime() != new_mtime)
           disk_interface_->RemoveFile((*o)->path());
-        }
       }
       if (!depfile.empty())
         disk_interface_->RemoveFile(depfile);
@@ -760,7 +762,9 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
 
     for (vector<Node*>::iterator o = edge->outputs_.begin();
          o != edge->outputs_.end(); ++o) {
-      TimeStamp new_mtime = disk_interface_->Stat((*o)->path());
+      TimeStamp new_mtime = disk_interface_->Stat((*o)->path(), err);
+      if (new_mtime == -1)
+        return false;
       if ((*o)->mtime() == new_mtime) {
         // The rule command did not change the output.  Propagate the clean
         // state through the build graph.
@@ -776,14 +780,18 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
       // (existing) non-order-only input or the depfile.
       for (vector<Node*>::iterator i = edge->inputs_.begin();
            i != edge->inputs_.end() - edge->order_only_deps_; ++i) {
-        TimeStamp input_mtime = disk_interface_->Stat((*i)->path());
+        TimeStamp input_mtime = disk_interface_->Stat((*i)->path(), err);
+        if (input_mtime == -1)
+          return false;
         if (input_mtime > restat_mtime)
           restat_mtime = input_mtime;
       }
 
       string depfile = edge->GetUnescapedDepfile();
       if (restat_mtime != 0 && deps_type.empty() && !depfile.empty()) {
-        TimeStamp depfile_mtime = disk_interface_->Stat(depfile);
+        TimeStamp depfile_mtime = disk_interface_->Stat(depfile, err);
+        if (depfile_mtime == -1)
+          return false;
         if (depfile_mtime > restat_mtime)
           restat_mtime = depfile_mtime;
       }
@@ -812,12 +820,9 @@ bool Builder::FinishCommand(CommandRunner::Result* result, string* err) {
   if (!deps_type.empty() && !config_.dry_run) {
     assert(edge->outputs_.size() == 1 && "should have been rejected by parser");
     Node* out = edge->outputs_[0];
-    TimeStamp deps_mtime = disk_interface_->Stat(out->path());
-    if (deps_mtime == -1) {
-      // TODO: Let DiskInterface::Stat() take err instead of it calling Error().
-      *err = "stat failed";
+    TimeStamp deps_mtime = disk_interface_->Stat(out->path(), err);
+    if (deps_mtime == -1)
       return false;
-    }
     if (!scan_.deps_log()->RecordDeps(out, deps_mtime, deps_nodes)) {
       *err = string("Error writing to deps log: ") + strerror(errno);
       return false;

diff --git a/src/build_test.cc b/src/build_test.cc
index 0cdcd87..13d1e7e 100644 (file)
--- a/src/build_test.cc
+++ b/src/build_test.cc
@@ -1483,7 +1483,7 @@ TEST_F(BuildTest, InterruptCleanup) {
   EXPECT_FALSE(builder_.Build(&err));
   EXPECT_EQ("interrupted by user", err);
   builder_.Cleanup();
-  EXPECT_GT(fs_.Stat("out1"), 0);
+  EXPECT_GT(fs_.Stat("out1", &err), 0);
   err = "";
 
   // A touched output of an interrupted command should be deleted.
@@ -1492,7 +1492,7 @@ TEST_F(BuildTest, InterruptCleanup) {
   EXPECT_FALSE(builder_.Build(&err));
   EXPECT_EQ("interrupted by user", err);
   builder_.Cleanup();
-  EXPECT_EQ(0, fs_.Stat("out2"));
+  EXPECT_EQ(0, fs_.Stat("out2", &err));
 }
 
 TEST_F(BuildTest, StatFailureAbortsBuild) {
@@ -1503,6 +1503,7 @@ TEST_F(BuildTest, StatFailureAbortsBuild) {
 
   // This simulates a stat failure:
   fs_.files_[kTooLongToStat].mtime = -1;
+  fs_.files_[kTooLongToStat].stat_error = "stat failed";
 
   string err;
   EXPECT_FALSE(builder_.AddTarget(kTooLongToStat, &err));
@@ -1626,7 +1627,7 @@ TEST_F(BuildWithDepsLogTest, Straightforward) {
     EXPECT_EQ("", err);
 
     // The deps file should have been removed.
-    EXPECT_EQ(0, fs_.Stat("in1.d"));
+    EXPECT_EQ(0, fs_.Stat("in1.d", &err));
     // Recreate it for the next step.
     fs_.Create("in1.d", "out: in2");
     deps_log.Close();
@@ -1706,7 +1707,7 @@ TEST_F(BuildWithDepsLogTest, ObsoleteDeps) {
   fs_.Create("out", "");
 
   // The deps file should have been removed, so no need to timestamp it.
-  EXPECT_EQ(0, fs_.Stat("in1.d"));
+  EXPECT_EQ(0, fs_.Stat("in1.d", &err));
 
   {
     State state;

diff --git a/src/clean.cc b/src/clean.cc
index 7b044c5..1d6ba9e 100644 (file)
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -49,7 +49,11 @@ int Cleaner::RemoveFile(const string& path) {
 }
 
 bool Cleaner::FileExists(const string& path) {
-  return disk_interface_->Stat(path) > 0;
+  string err;
+  TimeStamp mtime = disk_interface_->Stat(path, &err);
+  if (mtime == -1)
+    Error("%s", err.c_str());
+  return mtime > 0;  // Treat Stat() errors as "file does not exist".
 }
 
 void Cleaner::Report(const string& path) {

diff --git a/src/clean_test.cc b/src/clean_test.cc
index 5869bbb..395343b 100644 (file)
--- a/src/clean_test.cc
+++ b/src/clean_test.cc
@@ -44,10 +44,11 @@ TEST_F(CleanTest, CleanAll) {
   EXPECT_EQ(4u, fs_.files_removed_.size());
 
   // Check they are removed.
-  EXPECT_EQ(0, fs_.Stat("in1"));
-  EXPECT_EQ(0, fs_.Stat("out1"));
-  EXPECT_EQ(0, fs_.Stat("in2"));
-  EXPECT_EQ(0, fs_.Stat("out2"));
+  string err;
+  EXPECT_EQ(0, fs_.Stat("in1", &err));
+  EXPECT_EQ(0, fs_.Stat("out1", &err));
+  EXPECT_EQ(0, fs_.Stat("in2", &err));
+  EXPECT_EQ(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   EXPECT_EQ(0, cleaner.CleanAll());
@@ -75,10 +76,11 @@ TEST_F(CleanTest, CleanAllDryRun) {
   EXPECT_EQ(0u, fs_.files_removed_.size());
 
   // Check they are not removed.
-  EXPECT_NE(0, fs_.Stat("in1"));
-  EXPECT_NE(0, fs_.Stat("out1"));
-  EXPECT_NE(0, fs_.Stat("in2"));
-  EXPECT_NE(0, fs_.Stat("out2"));
+  string err;
+  EXPECT_LT(0, fs_.Stat("in1", &err));
+  EXPECT_LT(0, fs_.Stat("out1", &err));
+  EXPECT_LT(0, fs_.Stat("in2", &err));
+  EXPECT_LT(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   EXPECT_EQ(0, cleaner.CleanAll());
@@ -105,10 +107,11 @@ TEST_F(CleanTest, CleanTarget) {
   EXPECT_EQ(2u, fs_.files_removed_.size());
 
   // Check they are removed.
-  EXPECT_EQ(0, fs_.Stat("in1"));
-  EXPECT_EQ(0, fs_.Stat("out1"));
-  EXPECT_NE(0, fs_.Stat("in2"));
-  EXPECT_NE(0, fs_.Stat("out2"));
+  string err;
+  EXPECT_EQ(0, fs_.Stat("in1", &err));
+  EXPECT_EQ(0, fs_.Stat("out1", &err));
+  EXPECT_LT(0, fs_.Stat("in2", &err));
+  EXPECT_LT(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   ASSERT_EQ(0, cleaner.CleanTarget("out1"));
@@ -135,11 +138,12 @@ TEST_F(CleanTest, CleanTargetDryRun) {
   EXPECT_EQ(2, cleaner.cleaned_files_count());
   EXPECT_EQ(0u, fs_.files_removed_.size());
 
-  // Check they are removed.
-  EXPECT_NE(0, fs_.Stat("in1"));
-  EXPECT_NE(0, fs_.Stat("out1"));
-  EXPECT_NE(0, fs_.Stat("in2"));
-  EXPECT_NE(0, fs_.Stat("out2"));
+  // Check they are not removed.
+  string err;
+  EXPECT_LT(0, fs_.Stat("in1", &err));
+  EXPECT_LT(0, fs_.Stat("out1", &err));
+  EXPECT_LT(0, fs_.Stat("in2", &err));
+  EXPECT_LT(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   ASSERT_EQ(0, cleaner.CleanTarget("out1"));
@@ -168,10 +172,11 @@ TEST_F(CleanTest, CleanRule) {
   EXPECT_EQ(2u, fs_.files_removed_.size());
 
   // Check they are removed.
-  EXPECT_EQ(0, fs_.Stat("in1"));
-  EXPECT_NE(0, fs_.Stat("out1"));
-  EXPECT_EQ(0, fs_.Stat("in2"));
-  EXPECT_NE(0, fs_.Stat("out2"));
+  string err;
+  EXPECT_EQ(0, fs_.Stat("in1", &err));
+  EXPECT_LT(0, fs_.Stat("out1", &err));
+  EXPECT_EQ(0, fs_.Stat("in2", &err));
+  EXPECT_LT(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
@@ -200,11 +205,12 @@ TEST_F(CleanTest, CleanRuleDryRun) {
   EXPECT_EQ(2, cleaner.cleaned_files_count());
   EXPECT_EQ(0u, fs_.files_removed_.size());
 
-  // Check they are removed.
-  EXPECT_NE(0, fs_.Stat("in1"));
-  EXPECT_NE(0, fs_.Stat("out1"));
-  EXPECT_NE(0, fs_.Stat("in2"));
-  EXPECT_NE(0, fs_.Stat("out2"));
+  // Check they are not removed.
+  string err;
+  EXPECT_LT(0, fs_.Stat("in1", &err));
+  EXPECT_LT(0, fs_.Stat("out1", &err));
+  EXPECT_LT(0, fs_.Stat("in2", &err));
+  EXPECT_LT(0, fs_.Stat("out2", &err));
   fs_.files_removed_.clear();
 
   ASSERT_EQ(0, cleaner.CleanRule("cat_e"));
@@ -328,12 +334,13 @@ TEST_F(CleanTest, CleanRsp) {
   EXPECT_EQ(6u, fs_.files_removed_.size());
 
   // Check they are removed.
-  EXPECT_EQ(0, fs_.Stat("in1"));
-  EXPECT_EQ(0, fs_.Stat("out1"));
-  EXPECT_EQ(0, fs_.Stat("in2"));
-  EXPECT_EQ(0, fs_.Stat("out2"));
-  EXPECT_EQ(0, fs_.Stat("in2.rsp"));
-  EXPECT_EQ(0, fs_.Stat("out2.rsp"));
+  string err;
+  EXPECT_EQ(0, fs_.Stat("in1", &err));
+  EXPECT_EQ(0, fs_.Stat("out1", &err));
+  EXPECT_EQ(0, fs_.Stat("in2", &err));
+  EXPECT_EQ(0, fs_.Stat("out2", &err));
+  EXPECT_EQ(0, fs_.Stat("in2.rsp", &err));
+  EXPECT_EQ(0, fs_.Stat("out2.rsp", &err));
 }
 
 TEST_F(CleanTest, CleanFailure) {
@@ -345,6 +352,7 @@ TEST_F(CleanTest, CleanFailure) {
 }
 
 TEST_F(CleanTest, CleanPhony) {
+  string err;
   ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
 "build phony: phony t1 t2\n"
 "build t1: cat\n"
@@ -358,7 +366,7 @@ TEST_F(CleanTest, CleanPhony) {
   Cleaner cleaner(&state_, config_, &fs_);
   EXPECT_EQ(0, cleaner.CleanAll());
   EXPECT_EQ(2, cleaner.cleaned_files_count());
-  EXPECT_NE(0, fs_.Stat("phony"));
+  EXPECT_LT(0, fs_.Stat("phony", &err));
 
   fs_.Create("t1", "");
   fs_.Create("t2", "");
@@ -366,7 +374,7 @@ TEST_F(CleanTest, CleanPhony) {
   // Check that CleanTarget does not remove "phony".
   EXPECT_EQ(0, cleaner.CleanTarget("phony"));
   EXPECT_EQ(2, cleaner.cleaned_files_count());
-  EXPECT_NE(0, fs_.Stat("phony"));
+  EXPECT_LT(0, fs_.Stat("phony", &err));
 }
 
 TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) {
@@ -391,8 +399,9 @@ TEST_F(CleanTest, CleanDepFileAndRspFileWithSpaces) {
   EXPECT_EQ(4, cleaner.cleaned_files_count());
   EXPECT_EQ(4u, fs_.files_removed_.size());
 
-  EXPECT_EQ(0, fs_.Stat("out 1"));
-  EXPECT_EQ(0, fs_.Stat("out 2"));
-  EXPECT_EQ(0, fs_.Stat("out 1.d"));
-  EXPECT_EQ(0, fs_.Stat("out 2.rsp"));
+  string err;
+  EXPECT_EQ(0, fs_.Stat("out 1", &err));
+  EXPECT_EQ(0, fs_.Stat("out 2", &err));
+  EXPECT_EQ(0, fs_.Stat("out 1.d", &err));
+  EXPECT_EQ(0, fs_.Stat("out 2.rsp", &err));
 }

diff --git a/src/disk_interface.cc b/src/disk_interface.cc
index 9810708..70282c0 100644 (file)
--- a/src/disk_interface.cc
+++ b/src/disk_interface.cc
@@ -23,6 +23,7 @@
 #include <sys/types.h>
 
 #ifdef _WIN32
+#include <sstream>
 #include <windows.h>
 #include <direct.h>  // _mkdir
 #endif
@@ -67,16 +68,13 @@ TimeStamp TimeStampFromFileTime(const FILETIME& filetime) {
   return (TimeStamp)mtime;
 }
 
-TimeStamp StatSingleFile(const string& path, bool quiet) {
+TimeStamp StatSingleFile(const string& path, string* err) {
   WIN32_FILE_ATTRIBUTE_DATA attrs;
   if (!GetFileAttributesEx(path.c_str(), GetFileExInfoStandard, &attrs)) {
-    DWORD err = GetLastError();
-    if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
+    DWORD win_err = GetLastError();
+    if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND)
       return 0;
-    if (!quiet) {
-      Error("GetFileAttributesEx(%s): %s", path.c_str(),
-            GetLastErrorString().c_str());
-    }
+    *err = "GetFileAttributesEx(" + path + "): " + GetLastErrorString();
     return -1;
   }
   return TimeStampFromFileTime(attrs.ftLastWriteTime);
@@ -98,7 +96,7 @@ bool IsWindows7OrLater() {
 #endif
 
 bool StatAllFilesInDir(const string& dir, map<string, TimeStamp>* stamps,
-                       bool quiet) {
+                       string* err) {
   // FindExInfoBasic is 30% faster than FindExInfoStandard.
   static bool can_use_basic_info = IsWindows7OrLater();
   // This is not in earlier SDKs.
@@ -111,13 +109,10 @@ bool StatAllFilesInDir(const string& dir, map<string, TimeStamp>* stamps,
                                         FindExSearchNameMatch, NULL, 0);
 
   if (find_handle == INVALID_HANDLE_VALUE) {
-    DWORD err = GetLastError();
-    if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND)
+    DWORD win_err = GetLastError();
+    if (win_err == ERROR_FILE_NOT_FOUND || win_err == ERROR_PATH_NOT_FOUND)
       return true;
-    if (!quiet) {
-      Error("FindFirstFileExA(%s): %s", dir.c_str(),
-            GetLastErrorString().c_str());
-    }
+    *err = "FindFirstFileExA(" + dir + "): " + GetLastErrorString();
     return false;
   }
   do {
@@ -139,9 +134,12 @@ bool DiskInterface::MakeDirs(const string& path) {
   string dir = DirName(path);
   if (dir.empty())
     return true;  // Reached root; assume it's there.
-  TimeStamp mtime = Stat(dir);
-  if (mtime < 0)
-    return false;  // Error.
+  string err;
+  TimeStamp mtime = Stat(dir, &err);
+  if (mtime < 0) {
+    Error("%s", err.c_str());
+    return false;
+  }
   if (mtime > 0)
     return true;  // Exists already; we're done.
 
@@ -154,19 +152,19 @@ bool DiskInterface::MakeDirs(const string& path) {
 
 // RealDiskInterface -----------------------------------------------------------
 
-TimeStamp RealDiskInterface::Stat(const string& path) const {
+TimeStamp RealDiskInterface::Stat(const string& path, string* err) const {
 #ifdef _WIN32
   // MSDN: "Naming Files, Paths, and Namespaces"
   // http://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx
   if (!path.empty() && path[0] != '\\' && path.size() > MAX_PATH) {
-    if (!quiet_) {
-      Error("Stat(%s): Filename longer than %i characters",
-            path.c_str(), MAX_PATH);
-    }
+    ostringstream err_stream;
+    err_stream << "Stat(" << path << "): Filename longer than " << MAX_PATH
+               << " characters";
+    *err = err_stream.str();
     return -1;
   }
   if (!use_cache_)
-    return StatSingleFile(path, quiet_);
+    return StatSingleFile(path, err);
 
   string dir = DirName(path);
   string base(path.substr(dir.size() ? dir.size() + 1 : 0));
@@ -177,7 +175,7 @@ TimeStamp RealDiskInterface::Stat(const string& path) const {
   Cache::iterator ci = cache_.find(dir);
   if (ci == cache_.end()) {
     ci = cache_.insert(make_pair(dir, DirCache())).first;
-    if (!StatAllFilesInDir(dir.empty() ? "." : dir, &ci->second, quiet_)) {
+    if (!StatAllFilesInDir(dir.empty() ? "." : dir, &ci->second, err)) {
       cache_.erase(ci);
       return -1;
     }
@@ -189,9 +187,7 @@ TimeStamp RealDiskInterface::Stat(const string& path) const {
   if (stat(path.c_str(), &st) < 0) {
     if (errno == ENOENT || errno == ENOTDIR)
       return 0;
-    if (!quiet_) {
-      Error("stat(%s): %s", path.c_str(), strerror(errno));
-    }
+    *err = "stat(" + path + "): " + strerror(errno);
     return -1;
   }
   return st.st_mtime;

diff --git a/src/disk_interface.h b/src/disk_interface.h
index a13bced..b61d192 100644 (file)
--- a/src/disk_interface.h
+++ b/src/disk_interface.h
@@ -30,7 +30,7 @@ struct DiskInterface {
 
   /// stat() a file, returning the mtime, or 0 if missing and -1 on
   /// other errors.
-  virtual TimeStamp Stat(const string& path) const = 0;
+  virtual TimeStamp Stat(const string& path, string* err) const = 0;
 
   /// Create a directory, returning false on failure.
   virtual bool MakeDir(const string& path) = 0;
@@ -56,21 +56,18 @@ struct DiskInterface {
 
 /// Implementation of DiskInterface that actually hits the disk.
 struct RealDiskInterface : public DiskInterface {
-  RealDiskInterface() : quiet_(false)
+  RealDiskInterface()
 #ifdef _WIN32
-                      , use_cache_(false)
+                      : use_cache_(false)
 #endif
                       {}
   virtual ~RealDiskInterface() {}
-  virtual TimeStamp Stat(const string& path) const;
+  virtual TimeStamp Stat(const string& path, string* err) const;
   virtual bool MakeDir(const string& path);
   virtual bool WriteFile(const string& path, const string& contents);
   virtual string ReadFile(const string& path, string* err);
   virtual int RemoveFile(const string& path);
 
-  /// Whether to print on errors.  Used to make a test quieter.
-  bool quiet_;
-
   /// Whether stat information can be cached.  Only has an effect on Windows.
   void AllowStatCache(bool allow);
 

diff --git a/src/disk_interface_test.cc b/src/disk_interface_test.cc
index 658fffd..9d210b4 100644 (file)
--- a/src/disk_interface_test.cc
+++ b/src/disk_interface_test.cc
@@ -47,49 +47,64 @@ struct DiskInterfaceTest : public testing::Test {
 };
 
 TEST_F(DiskInterfaceTest, StatMissingFile) {
-  EXPECT_EQ(0, disk_.Stat("nosuchfile"));
+  string err;
+  EXPECT_EQ(0, disk_.Stat("nosuchfile", &err));
+  EXPECT_EQ("", err);
 
   // On Windows, the errno for a file in a nonexistent directory
   // is different.
-  EXPECT_EQ(0, disk_.Stat("nosuchdir/nosuchfile"));
+  EXPECT_EQ(0, disk_.Stat("nosuchdir/nosuchfile", &err));
+  EXPECT_EQ("", err);
 
   // On POSIX systems, the errno is different if a component of the
   // path prefix is not a directory.
   ASSERT_TRUE(Touch("notadir"));
-  EXPECT_EQ(0, disk_.Stat("notadir/nosuchfile"));
+  EXPECT_EQ(0, disk_.Stat("notadir/nosuchfile", &err));
+  EXPECT_EQ("", err);
 }
 
 TEST_F(DiskInterfaceTest, StatBadPath) {
-  disk_.quiet_ = true;
+  string err;
 #ifdef _WIN32
   string bad_path("cc:\\foo");
-  EXPECT_EQ(-1, disk_.Stat(bad_path));
+  EXPECT_EQ(-1, disk_.Stat(bad_path, &err));
+  EXPECT_NE("", err);
 #else
   string too_long_name(512, 'x');
-  EXPECT_EQ(-1, disk_.Stat(too_long_name));
+  EXPECT_EQ(-1, disk_.Stat(too_long_name, &err));
+  EXPECT_NE("", err);
 #endif
-  disk_.quiet_ = false;
 }
 
 TEST_F(DiskInterfaceTest, StatExistingFile) {
+  string err;
   ASSERT_TRUE(Touch("file"));
-  EXPECT_GT(disk_.Stat("file"), 1);
+  EXPECT_GT(disk_.Stat("file", &err), 1);
+  EXPECT_EQ("", err);
 }
 
 TEST_F(DiskInterfaceTest, StatExistingDir) {
+  string err;
   ASSERT_TRUE(disk_.MakeDir("subdir"));
   ASSERT_TRUE(disk_.MakeDir("subdir/subsubdir"));
-  EXPECT_GT(disk_.Stat("."), 1);
-  EXPECT_GT(disk_.Stat("subdir"), 1);
-  EXPECT_GT(disk_.Stat("subdir/subsubdir"), 1);
+  EXPECT_GT(disk_.Stat(".", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("subdir", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("subdir/subsubdir", &err), 1);
+  EXPECT_EQ("", err);
 
-  EXPECT_EQ(disk_.Stat("subdir"), disk_.Stat("subdir/."));
-  EXPECT_EQ(disk_.Stat("subdir"), disk_.Stat("subdir/subsubdir/.."));
-  EXPECT_EQ(disk_.Stat("subdir/subsubdir"), disk_.Stat("subdir/subsubdir/."));
+  EXPECT_EQ(disk_.Stat("subdir", &err),
+            disk_.Stat("subdir/.", &err));
+  EXPECT_EQ(disk_.Stat("subdir", &err),
+            disk_.Stat("subdir/subsubdir/..", &err));
+  EXPECT_EQ(disk_.Stat("subdir/subsubdir", &err),
+            disk_.Stat("subdir/subsubdir/.", &err));
 }
 
 #ifdef _WIN32
 TEST_F(DiskInterfaceTest, StatCache) {
+  string err;
   disk_.AllowStatCache(true);
 
   ASSERT_TRUE(Touch("file1"));
@@ -100,27 +115,43 @@ TEST_F(DiskInterfaceTest, StatCache) {
   ASSERT_TRUE(Touch("subdir\\SUBFILE2"));
   ASSERT_TRUE(Touch("subdir\\SUBFILE3"));
 
-  EXPECT_GT(disk_.Stat("FIle1"), 1);
-  EXPECT_GT(disk_.Stat("file1"), 1);
+  EXPECT_GT(disk_.Stat("FIle1", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("file1", &err), 1);
+  EXPECT_EQ("", err);
 
-  EXPECT_GT(disk_.Stat("subdir/subfile2"), 1);
-  EXPECT_GT(disk_.Stat("sUbdir\\suBFile1"), 1);
+  EXPECT_GT(disk_.Stat("subdir/subfile2", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("sUbdir\\suBFile1", &err), 1);
+  EXPECT_EQ("", err);
 
-  EXPECT_GT(disk_.Stat("."), 1);
-  EXPECT_GT(disk_.Stat("subdir"), 1);
-  EXPECT_GT(disk_.Stat("subdir/subsubdir"), 1);
+  EXPECT_GT(disk_.Stat(".", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("subdir", &err), 1);
+  EXPECT_EQ("", err);
+  EXPECT_GT(disk_.Stat("subdir/subsubdir", &err), 1);
+  EXPECT_EQ("", err);
 
-  EXPECT_EQ(disk_.Stat("subdir"), disk_.Stat("subdir/."));
-  EXPECT_EQ(disk_.Stat("subdir"), disk_.Stat("subdir/subsubdir/.."));
-  EXPECT_EQ(disk_.Stat("subdir/subsubdir"), disk_.Stat("subdir/subsubdir/."));
+  EXPECT_EQ(disk_.Stat("subdir", &err),
+            disk_.Stat("subdir/.", &err));
+  EXPECT_EQ("", err);
+  EXPECT_EQ(disk_.Stat("subdir", &err),
+            disk_.Stat("subdir/subsubdir/..", &err));
+  EXPECT_EQ("", err);
+  EXPECT_EQ(disk_.Stat("subdir/subsubdir", &err),
+            disk_.Stat("subdir/subsubdir/.", &err));
+  EXPECT_EQ("", err);
 
   // Test error cases.
-  disk_.quiet_ = true;
   string bad_path("cc:\\foo");
-  EXPECT_EQ(-1, disk_.Stat(bad_path));
-  EXPECT_EQ(-1, disk_.Stat(bad_path));
-  EXPECT_EQ(0, disk_.Stat("nosuchfile"));
-  EXPECT_EQ(0, disk_.Stat("nosuchdir/nosuchfile"));
+  EXPECT_EQ(-1, disk_.Stat(bad_path, &err));
+  EXPECT_NE("", err); err.clear();
+  EXPECT_EQ(-1, disk_.Stat(bad_path, &err));
+  EXPECT_NE("", err); err.clear();
+  EXPECT_EQ(0, disk_.Stat("nosuchfile", &err));
+  EXPECT_EQ("", err);
+  EXPECT_EQ(0, disk_.Stat("nosuchdir/nosuchfile", &err));
+  EXPECT_EQ("", err);
 }
 #endif
 
@@ -168,7 +199,7 @@ struct StatTest : public StateTestWithBuiltinRules,
   StatTest() : scan_(&state_, NULL, NULL, this) {}
 
   // DiskInterface implementation.
-  virtual TimeStamp Stat(const string& path) const;
+  virtual TimeStamp Stat(const string& path, string* err) const;
   virtual bool WriteFile(const string& path, const string& contents) {
     assert(false);
     return true;
@@ -191,7 +222,7 @@ struct StatTest : public StateTestWithBuiltinRules,
   mutable vector<string> stats_;
 };
 
-TimeStamp StatTest::Stat(const string& path) const {
+TimeStamp StatTest::Stat(const string& path, string* err) const {
   stats_.push_back(path);
   map<string, TimeStamp>::const_iterator i = mtimes_.find(path);
   if (i == mtimes_.end())

diff --git a/src/graph.cc b/src/graph.cc
index d99c8bd..fcfeba0 100644 (file)
--- a/src/graph.cc
+++ b/src/graph.cc
@@ -29,13 +29,7 @@
 
 bool Node::Stat(DiskInterface* disk_interface, string* err) {
   METRIC_RECORD("node stat");
-  mtime_ = disk_interface->Stat(path_);
-  if (mtime_ == -1) {
-    // TODO: Let DiskInterface::Stat() take err instead of it calling Error().
-    *err = "stat failed";
-    return false;
-  }
-  return true;
+  return (mtime_ = disk_interface->Stat(path_, err)) != -1;
 }
 
 bool DependencyScan::RecomputeDirty(Edge* edge, string* err) {

diff --git a/src/manifest_parser_perftest.cc b/src/manifest_parser_perftest.cc
index ca62fb2..6b56ab0 100644 (file)
--- a/src/manifest_parser_perftest.cc
+++ b/src/manifest_parser_perftest.cc
@@ -42,15 +42,19 @@ struct RealFileReader : public ManifestParser::FileReader {
   }
 };
 
-bool WriteFakeManifests(const string& dir) {
+bool WriteFakeManifests(const string& dir, string* err) {
   RealDiskInterface disk_interface;
-  if (disk_interface.Stat(dir + "/build.ninja") > 0)
-    return true;
+  TimeStamp mtime = disk_interface.Stat(dir + "/build.ninja", err);
+  if (mtime != 0)  // 0 means that the file doesn't exist yet.
+    return mtime != -1;
 
+  string command = "python misc/write_fake_manifests.py " + dir;
   printf("Creating manifest data..."); fflush(stdout);
-  int err = system(("python misc/write_fake_manifests.py " + dir).c_str());
+  int exit_code = system(command.c_str());
   printf("done.\n");
-  return err == 0;
+  if (exit_code != 0)
+    *err = "Failed to run " + command;
+  return exit_code == 0;
 }
 
 int LoadManifests(bool measure_command_evaluation) {
@@ -93,8 +97,9 @@ int main(int argc, char* argv[]) {
 
   const char kManifestDir[] = "build/manifest_perftest";
 
-  if (!WriteFakeManifests(kManifestDir)) {
-    fprintf(stderr, "Failed to write test data\n");
+  string err;
+  if (!WriteFakeManifests(kManifestDir, &err)) {
+    fprintf(stderr, "Failed to write test data: %s\n", err.c_str());
     return 1;
   }
 

diff --git a/src/ninja.cc b/src/ninja.cc
index 1e43137..e5bf98d 100644 (file)
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -143,6 +143,8 @@ struct NinjaMain : public BuildLogUser {
 
   virtual bool IsPathDead(StringPiece s) const {
     Node* n = state_.LookupNode(s);
+    if (!n || !n->in_edge())
+      return false;
     // Just checking n isn't enough: If an old output is both in the build log
     // and in the deps log, it will have a Node object in state_.  (It will also
     // have an in edge if one of its inputs is another output that's in the deps
@@ -152,7 +154,11 @@ struct NinjaMain : public BuildLogUser {
     // which seems good enough for this corner case.)
     // Do keep entries around for files which still exist on disk, for
     // generators that want to use this information.
-    return (!n || !n->in_edge()) && disk_interface_.Stat(s.AsString()) == 0;
+    string err;
+    TimeStamp mtime = disk_interface_.Stat(s.AsString(), &err);
+    if (mtime == -1)
+      Error("%s", err.c_str());  // Log and ignore Stat() errors.
+    return mtime == 0;
   }
 };
 
@@ -481,7 +487,10 @@ int NinjaMain::ToolDeps(int argc, char** argv) {
       continue;
     }
 
-    TimeStamp mtime = disk_interface.Stat((*it)->path());
+    string err;
+    TimeStamp mtime = disk_interface.Stat((*it)->path(), &err);
+    if (mtime == -1)
+      Error("%s", err.c_str());  // Log and ignore Stat() errors;
     printf("%s: #deps %d, deps mtime %d (%s)\n",
            (*it)->path().c_str(), deps->node_count, deps->mtime,
            (!mtime || mtime > deps->mtime ? "STALE":"VALID"));

diff --git a/src/test.cc b/src/test.cc
index ddecd3d..aed8db7 100644 (file)
--- a/src/test.cc
+++ b/src/test.cc
@@ -145,10 +145,12 @@ void VirtualFileSystem::Create(const string& path,
   files_created_.insert(path);
 }
 
-TimeStamp VirtualFileSystem::Stat(const string& path) const {
+TimeStamp VirtualFileSystem::Stat(const string& path, string* err) const {
   FileMap::const_iterator i = files_.find(path);
-  if (i != files_.end())
+  if (i != files_.end()) {
+    *err = i->second.stat_error;
     return i->second.mtime;
+  }
   return 0;
 }
 

diff --git a/src/test.h b/src/test.h
index 3ce510d..156e68a 100644 (file)
--- a/src/test.h
+++ b/src/test.h
@@ -142,7 +142,7 @@ struct VirtualFileSystem : public DiskInterface {
   }
 
   // DiskInterface
-  virtual TimeStamp Stat(const string& path) const;
+  virtual TimeStamp Stat(const string& path, string* err) const;
   virtual bool WriteFile(const string& path, const string& contents);
   virtual bool MakeDir(const string& path);
   virtual string ReadFile(const string& path, string* err);
@@ -151,6 +151,7 @@ struct VirtualFileSystem : public DiskInterface {
   /// An entry for a single in-memory file.
   struct Entry {
     int mtime;
+    string stat_error;  // If mtime is -1.
     string contents;
   };