Manage temporary files inside AbstractActionInputPrefetcher

so that we can share this code among implementations. Also, always delete tempDir after build PiperOrigin-RevId: 449221017
bazelbuild · May 17, 2022 · 43cb21a · 43cb21a
1 parent 8179e78
commit 43cb21a
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 17 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -28,12 +28,20 @@
 import com.google.devtools.build.lib.actions.FileArtifactValue;
 import com.google.devtools.build.lib.actions.MetadataProvider;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.profiler.Profiler;
 import com.google.devtools.build.lib.profiler.ProfilerTask;
 import com.google.devtools.build.lib.profiler.SilentCloseable;
 import com.google.devtools.build.lib.remote.util.AsyncTaskCache;
 import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult;
 import com.google.devtools.build.lib.remote.util.TempPathGenerator;
+import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
+import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution;
+import com.google.devtools.build.lib.server.FailureDetails.RemoteExecution.Code;
+import com.google.devtools.build.lib.util.AbruptExitException;
+import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.util.ExitCode;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import io.reactivex.rxjava3.core.Completable;
@@ -60,6 +68,36 @@ protected AbstractActionInputPrefetcher(Path execRoot, TempPathGenerator tempPat
     this.tempPathGenerator = tempPathGenerator;
   }
 
+  public void startBuild(EventHandler eventHandler) throws AbruptExitException {
+    Path tempDir = tempPathGenerator.getTempDir();
+    if (tempDir.exists()) {
+      eventHandler.handle(Event.warn("Found stale downloads from previous build, deleting..."));
+      try {
+        tempDir.deleteTree();
+      } catch (IOException e) {
+        throw new AbruptExitException(
+            DetailedExitCode.of(
+                ExitCode.LOCAL_ENVIRONMENTAL_ERROR,
+                FailureDetail.newBuilder()
+                    .setMessage(
+                        String.format("Failed to delete stale downloads: %s", e.getMessage()))
+                    .setRemoteExecution(
+                        RemoteExecution.newBuilder()
+                            .setCode(Code.DOWNLOADED_INPUTS_DELETION_FAILURE))
+                    .build()));
+      }
+    }
+  }
+
+  public void finalizeBuild() {
+    Path tempDir = tempPathGenerator.getTempDir();
+    try {
+      tempDir.deleteTree();
+    } catch (IOException ignored) {
+      // Intentionally left empty.
+    }
+  }
+
   protected abstract boolean shouldDownloadInput(
       ActionInput input, @Nullable FileArtifactValue metadata);
 

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteModule.java
@@ -921,19 +921,6 @@ public void executorInit(CommandEnvironment env, BuildRequest request, ExecutorB
     RemoteOutputsMode remoteOutputsMode = remoteOptions.remoteOutputsMode;
     if (!remoteOutputsMode.downloadAllOutputs() && actionContextProvider.getRemoteCache() != null) {
       Path tempDir = env.getActionTempsDirectory().getChild("remote");
-      try {
-        if (tempDir.exists()
-            && (!tempDir.isDirectory() || !tempDir.getDirectoryEntries().isEmpty())) {
-          env.getReporter()
-              .handle(Event.warn("Found incomplete downloads from previous build, deleting..."));
-          tempDir.deleteTree();
-        }
-      } catch (IOException e) {
-        throw createExitException(
-            e.getMessage(),
-            ExitCode.LOCAL_ENVIRONMENTAL_ERROR,
-            Code.DOWNLOADED_INPUTS_DELETION_FAILURE);
-      }
       actionInputFetcher =
           new RemoteActionInputFetcher(
               env.getBuildRequestId(),

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputService.java
@@ -24,6 +24,7 @@
 import com.google.devtools.build.lib.actions.FilesetOutputSymlink;
 import com.google.devtools.build.lib.actions.cache.MetadataHandler;
 import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.util.AbruptExitException;
 import com.google.devtools.build.lib.vfs.BatchStat;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.ModifiedFileSet;
@@ -76,13 +77,18 @@ public String getFilesSystemName() {
 
   @Override
   public ModifiedFileSet startBuild(
-      EventHandler eventHandler, UUID buildId, boolean finalizeActions) {
+      EventHandler eventHandler, UUID buildId, boolean finalizeActions) throws AbruptExitException {
+    if (actionInputFetcher != null) {
+      actionInputFetcher.startBuild(eventHandler);
+    }
     return ModifiedFileSet.EVERYTHING_MODIFIED;
   }
 
   @Override
   public void finalizeBuild(boolean buildSuccessful) {
-    // Intentionally left empty.
+    if (actionInputFetcher != null) {
+      actionInputFetcher.finalizeBuild();
+    }
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java b/src/main/java/com/google/devtools/build/lib/remote/util/TempPathGenerator.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.remote.util;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.devtools.build.lib.vfs.Path;
 import java.util.concurrent.atomic.AtomicInteger;
 import javax.annotation.concurrent.ThreadSafe;
@@ -33,7 +32,6 @@ public Path generateTempPath() {
     return tempDir.getChild(index.getAndIncrement() + ".tmp");
   }
 
-  @VisibleForTesting
   public Path getTempDir() {
     return tempDir;
   }