summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNicolas Despres <nicolas.despres@gmail.com>2011-05-02 14:46:31 (GMT)
committerNicolas Despres <nicolas.despres@gmail.com>2011-05-02 15:21:21 (GMT)
commit9412cb05008437b2ce534871ec86772b107ce23c (patch)
tree6f4c1eac7f0d6e4608d398b5f415b60c93a780e1
parent5113d8c29f2e165dabb05c5624055020704ba5fc (diff)
downloadNinja-9412cb05008437b2ce534871ec86772b107ce23c.zip
Ninja-9412cb05008437b2ce534871ec86772b107ce23c.tar.gz
Ninja-9412cb05008437b2ce534871ec86772b107ce23c.tar.bz2
Return non-zero status when errors occur.
Clean all was not returning non-zero when an error occur (like when failing to remove a directory). This patch fix the problem and add a test for it.
-rw-r--r--src/clean.cc50
-rw-r--r--src/clean.h13
-rw-r--r--src/clean_test.cc16
-rw-r--r--src/ninja.cc3
-rw-r--r--src/ninja.h8
-rw-r--r--src/ninja_jumble.cc8
-rw-r--r--src/ninja_test.cc9
-rw-r--r--src/test.cc9
-rw-r--r--src/test.h2
9 files changed, 78 insertions, 40 deletions
diff --git a/src/clean.cc b/src/clean.cc
index 5072484..e86793e 100644
--- a/src/clean.cc
+++ b/src/clean.cc
@@ -31,6 +31,7 @@ Cleaner::Cleaner(State* state, const BuildConfig& config)
, removed_()
, cleaned_files_count_(0)
, disk_interface_(new RealDiskInterface)
+ , status_(0)
{
}
@@ -42,10 +43,11 @@ Cleaner::Cleaner(State* state,
, removed_()
, cleaned_files_count_(0)
, disk_interface_(disk_interface)
+ , status_(0)
{
}
-bool Cleaner::RemoveFile(const string& path) {
+int Cleaner::RemoveFile(const string& path) {
return disk_interface_->RemoveFile(path);
}
@@ -66,8 +68,11 @@ void Cleaner::Remove(const string& path) {
if (FileExists(path))
Report(path);
} else {
- if (RemoveFile(path))
+ int ret = RemoveFile(path);
+ if (ret == 0)
Report(path);
+ else if (ret == -1)
+ status_ = 1;
}
}
}
@@ -93,7 +98,8 @@ void Cleaner::PrintFooter() {
printf("%d files.\n", cleaned_files_count_);
}
-void Cleaner::CleanAll() {
+int Cleaner::CleanAll() {
+ Reset();
PrintHeader();
for (vector<Edge*>::iterator e = state_->edges_.begin();
e != state_->edges_.end();
@@ -103,6 +109,7 @@ void Cleaner::CleanAll() {
++out_node)
Remove((*out_node)->file_->path_);
PrintFooter();
+ return status_;
}
void Cleaner::DoCleanTarget(Node* target) {
@@ -116,28 +123,32 @@ void Cleaner::DoCleanTarget(Node* target) {
}
}
-void Cleaner::CleanTarget(Node* target) {
+int Cleaner::CleanTarget(Node* target) {
assert(target);
+ Reset();
PrintHeader();
DoCleanTarget(target);
PrintFooter();
+ return status_;
}
int Cleaner::CleanTarget(const char* target) {
assert(target);
+
+ Reset();
Node* node = state_->LookupNode(target);
if (node) {
CleanTarget(node);
- return 0;
} else {
Error("unknown target '%s'", target);
- return 1;
+ status_ = 1;
}
+ return status_;
}
int Cleaner::CleanTargets(int target_count, char* targets[]) {
- int status = 0;
+ Reset();
PrintHeader();
for (int i = 0; i < target_count; ++i) {
const char* target_name = targets[i];
@@ -148,11 +159,11 @@ int Cleaner::CleanTargets(int target_count, char* targets[]) {
DoCleanTarget(target);
} else {
Error("unknown target '%s'", target_name);
- status = 1;
+ status_ = 1;
}
}
PrintFooter();
- return status;
+ return status_;
}
void Cleaner::DoCleanRule(const Rule* rule) {
@@ -168,31 +179,34 @@ void Cleaner::DoCleanRule(const Rule* rule) {
Remove((*out_node)->file_->path_);
}
-void Cleaner::CleanRule(const Rule* rule) {
+int Cleaner::CleanRule(const Rule* rule) {
assert(rule);
+ Reset();
PrintHeader();
DoCleanRule(rule);
PrintFooter();
+ return status_;
}
int Cleaner::CleanRule(const char* rule) {
assert(rule);
+ Reset();
const Rule* r = state_->LookupRule(rule);
if (r) {
CleanRule(r);
- return 0;
} else {
Error("unknown rule '%s'", rule);
- return 1;
+ status_ = 1;
}
+ return status_;
}
int Cleaner::CleanRules(int rule_count, char* rules[]) {
assert(rules);
- int status = 0;
+ Reset();
PrintHeader();
for (int i = 0; i < rule_count; ++i) {
const char* rule_name = rules[i];
@@ -203,9 +217,15 @@ int Cleaner::CleanRules(int rule_count, char* rules[]) {
DoCleanRule(rule);
} else {
Error("unknown rule '%s'", rule_name);
- status = 1;
+ status_ = 1;
}
}
PrintFooter();
- return status;
+ return status_;
+}
+
+void Cleaner::Reset() {
+ status_ = 0;
+ cleaned_files_count_ = 0;
+ removed_.clear();
}
diff --git a/src/clean.h b/src/clean.h
index 6194aa0..8f0cfa4 100644
--- a/src/clean.h
+++ b/src/clean.h
@@ -40,7 +40,8 @@ public:
DiskInterface* disk_interface);
/// Clean the given @a target and all the file built for it.
- void CleanTarget(Node* target);
+ /// @return non-zero if an error occurs.
+ int CleanTarget(Node* target);
/// Clean the given target @a target.
/// @return non-zero if an error occurs.
int CleanTarget(const char* target);
@@ -49,10 +50,12 @@ public:
int CleanTargets(int target_count, char* targets[]);
/// Clean all built files.
- void CleanAll();
+ /// @return non-zero if an error occurs.
+ int CleanAll();
/// Clean all the file built with the given rule @a rule.
- void CleanRule(const Rule* rule);
+ /// @return non-zero if an error occurs.
+ int CleanRule(const Rule* rule);
/// Clean the file produced by the given @a rule.
/// @return non-zero if an error occurs.
int CleanRule(const char* rule);
@@ -74,7 +77,7 @@ public:
private:
/// Remove the file @a path.
/// @return whether the file has been removed.
- bool RemoveFile(const string& path);
+ int RemoveFile(const string& path);
/// @returns whether the file @a path exists.
bool FileExists(const string& path);
void Report(const string& path);
@@ -87,6 +90,7 @@ private:
void PrintHeader();
void PrintFooter();
void DoCleanRule(const Rule* rule);
+ void Reset();
private:
State* state_;
@@ -94,6 +98,7 @@ private:
set<string> removed_;
int cleaned_files_count_;
DiskInterface* disk_interface_;
+ int status_;
};
#endif // NINJA_CLEAN_H_
diff --git a/src/clean_test.cc b/src/clean_test.cc
index 0e2114e..d16941c 100644
--- a/src/clean_test.cc
+++ b/src/clean_test.cc
@@ -40,7 +40,7 @@ TEST_F(CleanTest, CleanAll) {
Cleaner cleaner(&state_, config_, &fs_);
ASSERT_EQ(0, cleaner.cleaned_files_count());
- cleaner.CleanAll();
+ EXPECT_EQ(0, cleaner.CleanAll());
EXPECT_EQ(4, cleaner.cleaned_files_count());
EXPECT_EQ(4, fs_.files_removed_.size());
}
@@ -56,7 +56,7 @@ TEST_F(CleanTest, CleanAll) {
Cleaner cleaner(&state_, config_, &fs_);
ASSERT_EQ(0, cleaner.cleaned_files_count());
- cleaner.CleanAll();
+ EXPECT_EQ(0, cleaner.CleanAll());
EXPECT_EQ(0, cleaner.cleaned_files_count());
EXPECT_EQ(0, fs_.files_removed_.size());
}
@@ -78,7 +78,7 @@ TEST_F(CleanTest, CleanAllDryRun) {
Cleaner cleaner(&state_, config_, &fs_);
ASSERT_EQ(0, cleaner.cleaned_files_count());
- cleaner.CleanAll();
+ EXPECT_EQ(0, cleaner.CleanAll());
EXPECT_EQ(4, cleaner.cleaned_files_count());
EXPECT_EQ(0, fs_.files_removed_.size());
}
@@ -94,7 +94,7 @@ TEST_F(CleanTest, CleanAllDryRun) {
Cleaner cleaner(&state_, config_, &fs_);
ASSERT_EQ(0, cleaner.cleaned_files_count());
- cleaner.CleanAll();
+ EXPECT_EQ(0, cleaner.CleanAll());
EXPECT_EQ(4, cleaner.cleaned_files_count());
EXPECT_EQ(0, fs_.files_removed_.size());
}
@@ -253,3 +253,11 @@ TEST_F(CleanTest, CleanRuleDryRun) {
EXPECT_EQ(0, fs_.files_removed_.size());
}
}
+
+TEST_F(CleanTest, CleanFailure) {
+ ASSERT_NO_FATAL_FAILURE(AssertParse(&state_,
+ "build dir: cat src1\n"));
+ fs_.MakeDir("dir");
+ Cleaner cleaner(&state_, config_, &fs_);
+ EXPECT_NE(0, cleaner.CleanAll());
+}
diff --git a/src/ninja.cc b/src/ninja.cc
index fbae091..26b6e5e 100644
--- a/src/ninja.cc
+++ b/src/ninja.cc
@@ -335,8 +335,7 @@ int CmdClean(State* state,
}
}
else {
- cleaner.CleanAll();
- return 0;
+ return cleaner.CleanAll();
}
}
diff --git a/src/ninja.h b/src/ninja.h
index 0f4a029..8927768 100644
--- a/src/ninja.h
+++ b/src/ninja.h
@@ -51,8 +51,10 @@ struct DiskInterface {
virtual string ReadFile(const string& path, string* err) = 0;
/// Remove the file named @a path. It behaves like 'rm -f path' so no errors
/// are reported if it does not exists.
- /// @returns whether the file has been removed.
- virtual bool RemoveFile(const string& path) = 0;
+ /// @returns 0 if the file has been removed,
+ /// 1 if the file does not exists, and
+ /// -1 if an error occurs.
+ virtual int RemoveFile(const string& path) = 0;
/// Create all the parent directories for path; like mkdir -p
/// `basename path`.
@@ -65,7 +67,7 @@ struct RealDiskInterface : public DiskInterface {
virtual int Stat(const string& path);
virtual bool MakeDir(const string& path);
virtual string ReadFile(const string& path, string* err);
- virtual bool RemoveFile(const string& path);
+ virtual int RemoveFile(const string& path);
};
/// Mapping of path -> FileStat.
diff --git a/src/ninja_jumble.cc b/src/ninja_jumble.cc
index 1230546..537f48e 100644
--- a/src/ninja_jumble.cc
+++ b/src/ninja_jumble.cc
@@ -107,17 +107,17 @@ bool RealDiskInterface::MakeDir(const string& path) {
return true;
}
-bool RealDiskInterface::RemoveFile(const string& path) {
+int RealDiskInterface::RemoveFile(const string& path) {
if (remove(path.c_str()) < 0) {
switch (errno) {
case ENOENT:
- return false;
+ return 1;
default:
Error("remove(%s): %s", path.c_str(), strerror(errno));
- return false;
+ return -1;
}
} else {
- return true;
+ return 0;
}
}
diff --git a/src/ninja_test.cc b/src/ninja_test.cc
index e8e7f08..8d53343 100644
--- a/src/ninja_test.cc
+++ b/src/ninja_test.cc
@@ -114,9 +114,9 @@ struct StatTest : public StateTestWithBuiltinRules,
assert(false);
return "";
}
- virtual bool RemoveFile(const string& path) {
+ virtual int RemoveFile(const string& path) {
assert(false);
- return false;
+ return 0;
}
map<string, time_t> mtimes_;
@@ -262,6 +262,7 @@ TEST_F(DiskInterfaceTest, RemoveFile) {
string cmd = "touch ";
cmd += kFileName;
ASSERT_EQ(0, system(cmd.c_str()));
- EXPECT_TRUE(disk_.RemoveFile(kFileName));
- EXPECT_FALSE(disk_.RemoveFile(kFileName));
+ EXPECT_EQ(0, disk_.RemoveFile(kFileName));
+ EXPECT_EQ(1, disk_.RemoveFile(kFileName));
+ EXPECT_EQ(-1, disk_.RemoveFile("does not exist"));
}
diff --git a/src/test.cc b/src/test.cc
index 4dc2f47..719cec3 100644
--- a/src/test.cc
+++ b/src/test.cc
@@ -59,13 +59,16 @@ string VirtualFileSystem::ReadFile(const string& path, string* err) {
return "";
}
-bool VirtualFileSystem::RemoveFile(const string& path) {
+int VirtualFileSystem::RemoveFile(const string& path) {
+ if (find(directories_made_.begin(), directories_made_.end(), path)
+ != directories_made_.end())
+ return -1;
FileMap::iterator i = files_.find(path);
if (i != files_.end()) {
files_.erase(i);
files_removed_.insert(path);
- return true;
+ return 0;
} else {
- return false;
+ return 1;
}
}
diff --git a/src/test.h b/src/test.h
index 0af8874..c516508 100644
--- a/src/test.h
+++ b/src/test.h
@@ -45,7 +45,7 @@ struct VirtualFileSystem : public DiskInterface {
virtual int Stat(const string& path);
virtual bool MakeDir(const string& path);
virtual string ReadFile(const string& path, string* err);
- virtual bool RemoveFile(const string& path);
+ virtual int RemoveFile(const string& path);
/// An entry for a single in-memory file.
struct Entry {