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

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;
 }
 

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.

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) {

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;