commit 5539265e162ee2cafaecbb4c22bc5e5e7014c44c
author Googler <leba@google.com> Mon Jun 05 08:18:48 2023 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Jun 05 08:20:33 2023 -0700
tree fccc5dc067fdbe546cf8071e7e14848664892390
parent f4a2984fc10c06b6f63a49aa6d46865937ac3178

Minor refactoring for RemoteOutputChecker.

IMO some of the old method names e.g. `shouldDownloadOutputForTopLevel` reads like it'd return the value of a `--download_top_level_output` flag, while in this situation we want to answer the question: "is this output a top-level output? If so, it should be downloaded".

PiperOrigin-RevId: 537880537
Change-Id: I960eac6add5aad5c23408f7cd4770d1d8dcd3231

src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
index a09bc94..c06e7e2 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOutputChecker.java
@@ -194,15 +194,15 @@
     }
   }
 
-  private boolean shouldDownloadOutputForToplevel(ActionInput output) {
-    return shouldDownloadOutputFor(output, toplevelArtifactsToDownload);
+  private boolean isTopLevelArtifact(ActionInput output) {
+    return isPartOfCollectedSet(output, toplevelArtifactsToDownload);
   }
 
-  private boolean shouldDownloadOutputForLocalAction(ActionInput output) {
-    return shouldDownloadOutputFor(output, inputsToDownload);
+  private boolean isInputToLocalAction(ActionInput output) {
+    return isPartOfCollectedSet(output, inputsToDownload);
   }
 
-  private boolean shouldDownloadOutputForRegex(ActionInput output) {
+  private boolean matchesRegex(ActionInput output) {
     if (output instanceof Artifact && ((Artifact) output).isTreeArtifact()) {
       return false;
     }
@@ -216,26 +216,19 @@
     return false;
   }
 
-  private static boolean shouldDownloadOutputFor(
-      ActionInput output, Set<ActionInput> artifactCollection) {
-    if (output instanceof TreeFileArtifact) {
-      if (artifactCollection.contains(((Artifact) output).getParent())) {
-        return true;
-      }
-    } else if (artifactCollection.contains(output)) {
-      return true;
-    }
-
-    return false;
+  private static boolean isPartOfCollectedSet(
+      ActionInput actionInput, Set<ActionInput> artifactSet) {
+    return artifactSet.contains(
+        actionInput instanceof TreeFileArtifact
+            ? ((Artifact) actionInput).getParent()
+            : actionInput);
   }
 
   /**
    * Returns {@code true} if Bazel should download this {@link ActionInput} during spawn execution.
    */
   public boolean shouldDownloadOutput(ActionInput output) {
-    return shouldDownloadOutputForToplevel(output)
-        || shouldDownloadOutputForLocalAction(output)
-        || shouldDownloadOutputForRegex(output);
+    return isTopLevelArtifact(output) || isInputToLocalAction(output) || matchesRegex(output);
   }
 
   @Override