Refactor ImportantOutputHandler into separate methods for top-level…

… outputs and runfiles.

PiperOrigin-RevId: 644046863
Change-Id: I345ac75d684fdd2bef408b8e5c56dce9d2bcafd2

src/main/java/com/google/devtools/build/lib/actions/BUILD

@@ -471,6 +471,7 @@ java_library(
         ":artifacts",
         "//src/main/java/com/google/devtools/build/lib/skyframe:detailed_exceptions",
         "//src/main/java/com/google/devtools/build/lib/util:detailed_exit_code",
+        "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/protobuf:failure_details_java_proto",
         "//third_party:guava",
     ],

src/main/java/com/google/devtools/build/lib/actions/ImportantOutputHandler.java

@@ -17,27 +17,58 @@
 import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
 import com.google.devtools.build.lib.skyframe.DetailedException;
 import com.google.devtools.build.lib.util.DetailedExitCode;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.Map;
 
 /** Context to be informed of top-level outputs and their runfiles. */
 public interface ImportantOutputHandler extends ActionContext {
 
   /**
-   * Informs this handler that top-level outputs or their runfiles have been built.
+   * Informs this handler that top-level outputs have been built.
    *
    * <p>The handler may verify that remotely stored outputs are still available. Returns a map from
    * digest to output for any artifacts that need to be regenerated via action rewinding.
    *
-   * <p>{@code outputs} may contain {@linkplain Artifact#isDirectory directory artifacts}, in which
-   * case the handler is responsible for expanding them using {@code expander}.
-   *
+   * @param outputs top-level outputs
+   * @param expander used to expand {@linkplain Artifact#isDirectory directory artifacts} in {@code
+   *     outputs}
+   * @param metadataProvider provides metadata for artifacts in {@code outputs} and their expansions
+   * @return a map from digest to output for any artifacts that need to be regenerated via action
+   *     rewinding
    * @throws ImportantOutputException for an issue processing the outputs, not including lost
    *     outputs which are reported in the returned map
    */
-  ImmutableMap<String, ActionInput> processAndGetLostArtifacts(
+  ImmutableMap<String, ActionInput> processOutputsAndGetLostArtifacts(
       Iterable<Artifact> outputs, ArtifactExpander expander, InputMetadataProvider metadataProvider)
       throws ImportantOutputException, InterruptedException;
 
-  /** Represents an exception encountered during {@link #processAndGetLostArtifacts}. */
+  /**
+   * Informs this handler that the runfiles of a top-level target have been built.
+   *
+   * <p>The handler may verify that remotely stored outputs are still available. Returns a map from
+   * digest to output for any artifacts that need to be regenerated via action rewinding.
+   *
+   * @param runfilesDir exec path of the runfiles directory
+   * @param runfiles mapping from {@code runfilesDir}-relative path to target artifact; values may
+   *     be {@code null} to represent an empty file (can happen with {@code __init__.py} files, see
+   *     {@link com.google.devtools.build.lib.rules.python.PythonUtils.GetInitPyFiles})
+   * @param expander used to expand {@linkplain Artifact#isDirectory directory artifacts} in {@code
+   *     runfiles}
+   * @param metadataProvider provides metadata for artifacts in {@code runfiles} and their
+   *     expansions
+   * @return a map from digest to output for any artifacts that need to be regenerated via action
+   *     rewinding
+   * @throws ImportantOutputException for an issue processing the runfiles, not including lost
+   *     outputs which are reported in the returned map
+   */
+  ImmutableMap<String, ActionInput> processRunfilesAndGetLostArtifacts(
+      PathFragment runfilesDir,
+      Map<PathFragment, Artifact> runfiles,
+      ArtifactExpander expander,
+      InputMetadataProvider metadataProvider)
+      throws ImportantOutputException, InterruptedException;
+
+  /** Represents an exception encountered during processing of important outputs. */
   final class ImportantOutputException extends Exception implements DetailedException {
     private final FailureDetail failureDetail;
 

src/main/java/com/google/devtools/build/lib/skyframe/CompletionFunction.java

@@ -509,7 +509,7 @@ Pair<ValueT, ArtifactsToBuild> getValueAndArtifactsToBuild(
   }
 
   /**
-   * Calls {@link ImportantOutputHandler#processAndGetLostArtifacts}.
+   * Calls {@link ImportantOutputHandler#processOutputsAndGetLostArtifacts}.
    *
    * <p>If any outputs are lost, returns a {@link Reset} which can be used to initiate action
    * rewinding and regenerate the lost outputs. Otherwise, returns {@code null}.
@@ -544,7 +544,7 @@ private Reset informImportantOutputHandler(
               "Informing important output handler of top-level outputs for " + label,
               IMPORTANT_OUTPUT_HANDLER_LOGGING_THRESHOLD)) {
         lostOutputs =
-            importantOutputHandler.processAndGetLostArtifacts(
+            importantOutputHandler.processOutputsAndGetLostArtifacts(
                 key.topLevelArtifactContext().expandFilesets()
                     ? importantArtifacts
                     : Iterables.filter(importantArtifacts, artifact -> !artifact.isFileset()),

src/test/java/com/google/devtools/build/lib/skyframe/rewinding/LostImportantOutputHandlerModule.java

@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.runtime.CommandEnvironment;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import java.io.IOException;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Stream;
@@ -73,7 +74,26 @@ public void registerActionContexts(
       CommandEnvironment env,
       BuildRequest buildRequest) {
     execRoot = env.getExecRoot().asFragment();
-    registryBuilder.register(ImportantOutputHandler.class, this::getLostOutputs);
+    registryBuilder.register(
+        ImportantOutputHandler.class,
+        new ImportantOutputHandler() {
+          @Override
+          public ImmutableMap<String, ActionInput> processOutputsAndGetLostArtifacts(
+              Iterable<Artifact> outputs,
+              ArtifactExpander expander,
+              InputMetadataProvider metadataProvider) {
+            return getLostOutputs(outputs, expander, metadataProvider);
+          }
+
+          @Override
+          public ImmutableMap<String, ActionInput> processRunfilesAndGetLostArtifacts(
+              PathFragment runfilesDir,
+              Map<PathFragment, Artifact> runfiles,
+              ArtifactExpander expander,
+              InputMetadataProvider metadataProvider) {
+            return getLostOutputs(runfiles.values(), expander, metadataProvider);
+          }
+        });
   }
 
   private ImmutableMap<String, ActionInput> getLostOutputs(