remove RemovePath
Generally:
- we do overwrite files (perhaps should stop, but for legacy
constraints)
- we ask build systems to clean the output directory before we run code
- if an error happens in a backend, it should be AIDL_FATAL, and we
should attempt to fix things in the validation phase of the compiler
- if there is an I/O error, we should crash/report a fatal error, but
avoid removing paths (for debuggability, and since there is an I/O
issue, avoid doing additional I/O)
- if a user tries to use this file, and it's incomplete, then this is
a way they can discover they aren't checking the error code from their
compiler
Fixes: 201507504
Test: aidl_unittests
Change-Id: Id0360c2253d4e53a5e76bfbc1b5c83dc0aa491c4
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index bf3190b..e3a41d5 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -1508,10 +1508,7 @@
auto gen = [&](auto file, GenFn fn) {
unique_ptr<CodeWriter> writer(io_delegate.GetCodeWriter(file));
fn(*writer, defined_type, typenames, options);
- if (!writer->Close()) {
- io_delegate.RemovePath(file);
- return false;
- }
+ AIDL_FATAL_IF(!writer->Close(), defined_type) << "I/O Error!";
return true;
};
diff --git a/generate_cpp_unittest.cpp b/generate_cpp_unittest.cpp
index a1be5d8..96d85c6 100644
--- a/generate_cpp_unittest.cpp
+++ b/generate_cpp_unittest.cpp
@@ -131,11 +131,10 @@
StringPrintf("%s%c%s", kHeaderDir, OS_PATH_SEPARATOR,
kInterfaceHeaderRelPath);
io_delegate_.AddBrokenFilePath(header_path);
- ASSERT_FALSE(GenerateCpp(options_.OutputFile(), options_, typenames_, *interface, io_delegate_));
+ ASSERT_DEATH(GenerateCpp(options_.OutputFile(), options_, typenames_, *interface, io_delegate_),
+ "I/O Error!");
// We should never attempt to write the C++ file if we fail writing headers.
ASSERT_FALSE(io_delegate_.GetWrittenContents(kOutputPath, nullptr));
- // We should remove partial results.
- ASSERT_TRUE(io_delegate_.PathWasRemoved(header_path));
}
TEST_F(IoErrorHandlingTest, HandlesBadCppWrite) {
@@ -145,9 +144,8 @@
// Simulate issues closing the cpp file.
io_delegate_.AddBrokenFilePath(kOutputPath);
- ASSERT_FALSE(GenerateCpp(options_.OutputFile(), options_, typenames_, *interface, io_delegate_));
- // We should remove partial results.
- ASSERT_TRUE(io_delegate_.PathWasRemoved(kOutputPath));
+ ASSERT_DEATH(GenerateCpp(options_.OutputFile(), options_, typenames_, *interface, io_delegate_),
+ "I/O Error!");
}
} // namespace cpp
diff --git a/io_delegate.cpp b/io_delegate.cpp
index e054a6b..d40bf1d 100644
--- a/io_delegate.cpp
+++ b/io_delegate.cpp
@@ -183,14 +183,6 @@
}
}
-void IoDelegate::RemovePath(const std::string& file_path) const {
-#ifdef _WIN32
- _unlink(file_path.c_str());
-#else
- unlink(file_path.c_str());
-#endif
-}
-
#ifdef _WIN32
Result<vector<string>> IoDelegate::ListFiles(const string&) const {
return Error() << "File listing not implemented on Windows";
diff --git a/io_delegate.h b/io_delegate.h
index 48d2807..a92a9b9 100644
--- a/io_delegate.h
+++ b/io_delegate.h
@@ -57,8 +57,6 @@
virtual std::unique_ptr<CodeWriter> GetCodeWriter(
const std::string& file_path) const;
- virtual void RemovePath(const std::string& file_path) const;
-
virtual android::base::Result<std::vector<std::string>> ListFiles(const std::string& dir) const;
private:
diff --git a/tests/fake_io_delegate.cpp b/tests/fake_io_delegate.cpp
index 4633282..6bd2984 100644
--- a/tests/fake_io_delegate.cpp
+++ b/tests/fake_io_delegate.cpp
@@ -66,15 +66,10 @@
if (broken_files_.count(file_path) > 0) {
return unique_ptr<CodeWriter>(new BrokenCodeWriter);
}
- removed_files_.erase(file_path);
written_file_contents_[file_path] = "";
return CodeWriter::ForString(&written_file_contents_[file_path]);
}
-void FakeIoDelegate::RemovePath(const std::string& file_path) const {
- removed_files_.insert(file_path);
-}
-
void FakeIoDelegate::SetFileContents(const string& filename,
const string& contents) {
file_contents_[filename] = contents;
@@ -150,13 +145,6 @@
return written_file_contents_;
}
-bool FakeIoDelegate::PathWasRemoved(const std::string& path) {
- if (removed_files_.count(path) > 0) {
- return true;
- }
- return false;
-}
-
} // namespace test
} // namespace aidl
} // namespace android
diff --git a/tests/fake_io_delegate.h b/tests/fake_io_delegate.h
index 951c32e..81d6f89 100644
--- a/tests/fake_io_delegate.h
+++ b/tests/fake_io_delegate.h
@@ -45,7 +45,6 @@
bool FileIsReadable(const std::string& path) const override;
std::unique_ptr<CodeWriter> GetCodeWriter(
const std::string& file_path) const override;
- void RemovePath(const std::string& file_path) const override;
android::base::Result<std::vector<std::string>> ListFiles(const std::string& dir) const override;
// Methods added to facilitate testing.
@@ -63,8 +62,6 @@
const std::map<std::string, std::string>& InputFiles() const;
const std::map<std::string, std::string>& OutputFiles() const;
- bool PathWasRemoved(const std::string& path);
-
private:
std::map<std::string, std::string> file_contents_;
// Normally, writing to files leaves the IoDelegate unchanged, so
@@ -75,7 +72,6 @@
// We normally just write to strings in |written_file_contents_| but for
// files in this list, we simulate I/O errors.
std::set<std::string> broken_files_;
- mutable std::set<std::string> removed_files_;
}; // class FakeIoDelegate
} // namespace test