Use ArtifactPathResolver when deleting outputs.

PiperOrigin-RevId: 338961970
hrfuller · Oct 26, 2020 · 4009b17 · 4009b17
1 parent 364a867
commit 4009b17
Show file tree

Hide file tree

Showing 9 changed files with 28 additions and 15 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java
@@ -353,15 +353,16 @@ public void repr(Printer printer) {
    * @param execRoot the exec root in which this action is executed
    * @param bulkDeleter a helper to bulk delete outputs to avoid delegating to the filesystem
    */
-  protected void deleteOutputs(Path execRoot, @Nullable BulkDeleter bulkDeleter)
+  protected void deleteOutputs(
+      Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
       throws IOException, InterruptedException {
     if (bulkDeleter != null) {
       bulkDeleter.bulkDelete(Artifact.asPathFragments(getOutputs()));
       return;
     }
 
     for (Artifact output : getOutputs()) {
-      deleteOutput(output.getPath(), output.getRoot());
+      deleteOutput(pathResolver.toPath(output), output.getRoot());
     }
   }
 
@@ -457,9 +458,10 @@ protected void checkOutputsForDirectories(ActionExecutionContext actionExecution
   }
 
   @Override
-  public void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
+  public void prepare(
+      Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
       throws IOException, InterruptedException {
-    deleteOutputs(execRoot, bulkDeleter);
+    deleteOutputs(execRoot, pathResolver, bulkDeleter);
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/actions/Action.java b/src/main/java/com/google/devtools/build/lib/actions/Action.java
@@ -85,7 +85,7 @@ public interface Action extends ActionExecutionMetadata {
    * @throws IOException if there is an error deleting the outputs.
    * @throws InterruptedException if the execution is interrupted
    */
-  void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
+  void prepare(Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
       throws IOException, InterruptedException;
 
   /**

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java
@@ -482,9 +482,10 @@ protected String getRawProgressMessage() {
    * the test log base name with arbitrary prefix and extension.
    */
   @Override
-  protected void deleteOutputs(Path execRoot, @Nullable BulkDeleter bulkDeleter)
+  protected void deleteOutputs(
+      Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
       throws IOException, InterruptedException {
-    super.deleteOutputs(execRoot, bulkDeleter);
+    super.deleteOutputs(execRoot, pathResolver, bulkDeleter);
 
     // We do not rely on globs, as it causes quadratic behavior in --runs_per_test and test
     // shard count.

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelWorkspaceStatusModule.java
@@ -26,6 +26,7 @@
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.ActionResult;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.BuildInfo;
 import com.google.devtools.build.lib.analysis.BuildInfoEvent;
@@ -154,11 +155,13 @@ private static byte[] printStatusMap(Map<String, String> map) {
     }
 
     @Override
-    public void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter) throws IOException {
+    public void prepare(
+        Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
+        throws IOException {
       // The default implementation of this method deletes all output files; override it to keep
       // the old stableStatus around. This way we can reuse the existing file (preserving its mtime)
       // if the contents haven't changed.
-      deleteOutput(volatileStatus.getPath(), volatileStatus.getRoot());
+      deleteOutput(pathResolver.toPath(volatileStatus), volatileStatus.getRoot());
     }
 
     @Override

diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkAction.java
@@ -1,4 +1,3 @@
-
 // Copyright 2016 The Bazel Authors. All rights reserved.
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
@@ -25,6 +24,7 @@
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.ActionResult;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.collect.nestedset.Order;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
@@ -65,12 +65,13 @@ public CreateIncSymlinkAction(
   }
 
   @Override
-  public void prepare(Path execRoot, @Nullable BulkDeleter bulkDeleter)
+  public void prepare(
+      Path execRoot, ArtifactPathResolver pathResolver, @Nullable BulkDeleter bulkDeleter)
       throws IOException, InterruptedException {
     if (includePath.isDirectory(Symlinks.NOFOLLOW)) {
       includePath.deleteTree();
     }
-    super.prepare(execRoot, bulkDeleter);
+    super.prepare(execRoot, pathResolver, bulkDeleter);
   }
 
   @Override

diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaCompileAction.java
@@ -621,7 +621,10 @@ public ActionContinuationOrResult execute()
         // Fall back to running with the full classpath. This requires first deleting potential
         // artifacts generated by the reduced action and clearing the metadata caches.
         try {
-          deleteOutputs(actionExecutionContext.getExecRoot(), /* bulkDeleter= */ null);
+          deleteOutputs(
+              actionExecutionContext.getExecRoot(),
+              actionExecutionContext.getPathResolver(),
+              /*bulkDeleter=*/ null);
         } catch (IOException e) {
           throw new EnvironmentalExecException(
                   e,

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java
@@ -24,6 +24,7 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.actions.CommandLineExpansionException;
 import com.google.devtools.build.lib.actions.EnvironmentalExecException;
@@ -437,7 +438,7 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti
       workingDir = env.getExecRoot();
 
       try {
-        testAction.prepare(env.getExecRoot(), /* bulkDeleter= */ null);
+        testAction.prepare(env.getExecRoot(), ArtifactPathResolver.IDENTITY, /*bulkDeleter=*/ null);
       } catch (IOException e) {
         return reportAndCreateFailureResult(
             env,

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeActionExecutor.java
@@ -931,6 +931,7 @@ public ActionStepOrResult run(Environment env) throws InterruptedException {
               // keep previous outputs in place.
               action.prepare(
                   actionExecutionContext.getExecRoot(),
+                  actionExecutionContext.getPathResolver(),
                   outputService != null ? outputService.bulkDeleter() : null);
             } catch (IOException e) {
               logger.atWarning().withCause(e).log(

diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CreateIncSymlinkActionTest.java
@@ -22,6 +22,7 @@
 import com.google.devtools.build.lib.actions.ActionExecutionContext.LostInputsCheck;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.ArtifactPathResolver;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
 import com.google.devtools.build.lib.actions.util.ActionsTestUtil;
 import com.google.devtools.build.lib.actions.util.DummyExecutor;
@@ -154,7 +155,7 @@ public void testFileRemoved() throws Exception {
     Path extra = rootDirectory.getRelative("out/extra");
     FileSystemUtils.createEmptyFile(extra);
     assertThat(extra.exists()).isTrue();
-    action.prepare(rootDirectory, /*bulkDeleter=*/ null);
+    action.prepare(rootDirectory, ArtifactPathResolver.IDENTITY, /*bulkDeleter=*/ null);
     assertThat(extra.exists()).isFalse();
   }