Don't delete everything in apexdata/com.android.art.

Rather than deleting everything we now specifically delete the
artifacts directory when validation fails or forcing compilation. This
allows the pending directory, with CompOS artifacts, to be preserved
and used as an alternative.

As a slightly gratuitous bonus, delete the staging directory when
we're done with it, rather than just its contents.

Test: Manual, create pending directory, see it preserved.
Test: atest --host art_odrefresh_tests
Test: Presubmits
Bug: 190166662
Change-Id: Ic6f0cb15eb22b3a235d9df90653c6e6d63ea80bc

4 files changed, 22 insertions, 14 deletions

diff --git a/odrefresh/odr_fs_utils.cc b/odrefresh/odr_fs_utils.cc
index 9f3a68bda8..22cc1b6a5d 100644
--- a/odrefresh/odr_fs_utils.cc
+++ b/odrefresh/odr_fs_utils.cc
@@ -56,7 +56,7 @@ static int NftwCleanUpCallback(const char* fpath,
   }
 }
 
-WARN_UNUSED bool CleanDirectory(const std::string& dir_path) {
+WARN_UNUSED bool RemoveDirectory(const std::string& dir_path) {
   if (!OS::DirectoryExists(dir_path.c_str())) {
     return true;
   }
@@ -66,6 +66,11 @@ WARN_UNUSED bool CleanDirectory(const std::string& dir_path) {
     LOG(ERROR) << "Failed to clean-up '" << dir_path << "'";
     return false;
   }
+
+  if (rmdir(dir_path.c_str()) != 0) {
+    LOG(ERROR) << "Failed to delete '" << dir_path << "'";
+    return false;
+  }
   return true;
 }
 
diff --git a/odrefresh/odr_fs_utils.h b/odrefresh/odr_fs_utils.h
index cefa11e1f4..61f86515e3 100644
--- a/odrefresh/odr_fs_utils.h
+++ b/odrefresh/odr_fs_utils.h
@@ -25,9 +25,9 @@
 namespace art {
 namespace odrefresh {
 
-// Cleans directory by removing all files and sub-directories under `dir_path`.
+// Removes the directory at `dir_path`, first removing all files and sub-directories in it.
 // Returns true on success, false otherwise.
-WARN_UNUSED bool CleanDirectory(const std::string& dir_path);
+WARN_UNUSED bool RemoveDirectory(const std::string& dir_path);
 
 // Create all directories on `absolute_dir_path`.
 // Returns true on success, false otherwise.
diff --git a/odrefresh/odr_fs_utils_test.cc b/odrefresh/odr_fs_utils_test.cc
index 6c4326505a..d460be28a0 100644
--- a/odrefresh/odr_fs_utils_test.cc
+++ b/odrefresh/odr_fs_utils_test.cc
@@ -58,7 +58,7 @@ static bool CreateFile(const char* file_path, size_t bytes) {
 
 }  // namespace
 
-TEST_F(OdrFsUtilsTest, CleanDirectory) {
+TEST_F(OdrFsUtilsTest, RemoveDirectory) {
   ScratchDir scratch_dir(/*keep_files=*/false);
 
   // Create some sub-directories and files
@@ -83,8 +83,8 @@ TEST_F(OdrFsUtilsTest, CleanDirectory) {
     ASSERT_TRUE(CreateFile(file_path.c_str(), 4096));
   }
 
-  // Clean all files and sub-directories
-  ASSERT_TRUE(CleanDirectory(scratch_dir.GetPath()));
+  // Remove directory, all files and sub-directories
+  ASSERT_TRUE(RemoveDirectory(scratch_dir.GetPath()));
 
   // Check nothing we created remains.
   for (const auto& dir_path : dir_paths) {
@@ -94,6 +94,9 @@ TEST_F(OdrFsUtilsTest, CleanDirectory) {
   for (const auto& file_path : file_paths) {
     ASSERT_FALSE(OS::FileExists(file_path.c_str(), true));
   }
+
+  // And the original directory is also gone
+  ASSERT_FALSE(OS::DirectoryExists(scratch_dir.GetPath().c_str()));
 }
 
 TEST_F(OdrFsUtilsTest, EnsureDirectoryExistsBadPath) {
diff --git a/odrefresh/odrefresh.cc b/odrefresh/odrefresh.cc
index e7eba5684d..4cb15b94ae 100644
--- a/odrefresh/odrefresh.cc
+++ b/odrefresh/odrefresh.cc
@@ -487,7 +487,7 @@ class OnDeviceRefresh final {
 
     // Clean-up helper used to simplify clean-ups and handling failures there.
     auto cleanup_return = [this](ExitCode exit_code) {
-      return CleanApexdataDirectory() ? exit_code : ExitCode::kCleanupFailed;
+      return RemoveArtifactsDirectory() ? exit_code : ExitCode::kCleanupFailed;
     };
 
     const auto apex_info = GetArtApexInfo();
@@ -943,13 +943,13 @@ class OnDeviceRefresh final {
     return exit_code;
   }
 
-  WARN_UNUSED bool CleanApexdataDirectory() const {
-    const std::string& apex_data_path = GetArtApexData();
+  WARN_UNUSED bool RemoveArtifactsDirectory() const {
     if (config_.GetDryRun()) {
-      LOG(INFO) << "Files under `" << QuotePath(apex_data_path) << " would be removed (dry-run).";
+      LOG(INFO) << "Directory " << QuotePath(kOdrefreshArtifactDirectory)
+                << " and contents would be removed (dry-run).";
       return true;
     }
-    return CleanDirectory(apex_data_path);
+    return RemoveDirectory(kOdrefreshArtifactDirectory);
   }
 
   WARN_UNUSED bool RemoveArtifacts(const OdrArtifacts& artifacts) const {
@@ -1343,7 +1343,7 @@ class OnDeviceRefresh final {
     const char* staging_dir = nullptr;
     metrics.SetStage(OdrMetrics::Stage::kPreparation);
     // Clean-up existing files.
-    if (force_compile && !CleanApexdataDirectory()) {
+    if (force_compile && !RemoveArtifactsDirectory()) {
       metrics.SetStatus(OdrMetrics::Status::kIoError);
       return ExitCode::kCleanupFailed;
     }
@@ -1385,7 +1385,7 @@ class OnDeviceRefresh final {
         if (!CompileBootExtensionArtifacts(
                 isa, staging_dir, metrics, &dex2oat_invocation_count, &error_msg)) {
           LOG(ERROR) << "Compilation of BCP failed: " << error_msg;
-          if (!config_.GetDryRun() && !CleanDirectory(staging_dir)) {
+          if (!config_.GetDryRun() && !RemoveDirectory(staging_dir)) {
             return ExitCode::kCleanupFailed;
           }
           return ExitCode::kCompilationFailed;
@@ -1405,7 +1405,7 @@ class OnDeviceRefresh final {
       if (!CompileSystemServerArtifacts(
               staging_dir, metrics, &dex2oat_invocation_count, &error_msg)) {
         LOG(ERROR) << "Compilation of system_server failed: " << error_msg;
-        if (!config_.GetDryRun() && !CleanDirectory(staging_dir)) {
+        if (!config_.GetDryRun() && !RemoveDirectory(staging_dir)) {
           return ExitCode::kCleanupFailed;
         }
         return ExitCode::kCompilationFailed;