Unify input prefetching

All prefetching now goes through AbstractSpawnStrategy's implementation of
SpawnExecutionPolicy. Make sure the sandbox runners also do this consistently.

PiperOrigin-RevId: 164836877

4 files changed, 8 insertions, 37 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
index e822904a17..dd5c231036 100644
--- a/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/exec/local/LocalSpawnRunnerTest.java
@@ -22,12 +22,10 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
-import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.io.ByteStreams;
 import com.google.devtools.build.lib.actions.ActionInput;
 import com.google.devtools.build.lib.actions.ActionInputFileCache;
-import com.google.devtools.build.lib.actions.ActionInputHelper;
 import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander;
 import com.google.devtools.build.lib.actions.ExecutionRequirements;
 import com.google.devtools.build.lib.actions.ResourceManager;
@@ -145,7 +143,7 @@ public class LocalSpawnRunnerTest {
     private final TreeMap<PathFragment, ActionInput> inputMapping = new TreeMap<>();
 
     private long timeoutMillis;
-    private final List<Iterable<ActionInput>> prefetched = new ArrayList<>();
+    private boolean prefetchCalled;
     private boolean lockOutputFilesCalled;
 
     @Override
@@ -154,8 +152,8 @@ public class LocalSpawnRunnerTest {
     }
 
     @Override
-    public void prefetchInputs(Iterable<ActionInput> inputs) throws IOException {
-      prefetched.add(Preconditions.checkNotNull(inputs));
+    public void prefetchInputs() throws IOException {
+      prefetchCalled = true;
     }
 
     @Override
@@ -435,7 +433,7 @@ public class LocalSpawnRunnerTest {
     policy.timeoutMillis = 123 * 1000L;
     outErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
     runner.exec(SIMPLE_SPAWN, policy);
-    assertThat(policy.prefetched).isNotEmpty();
+    assertThat(policy.prefetchCalled).isTrue();
   }
 
   @Test
@@ -455,34 +453,7 @@ public class LocalSpawnRunnerTest {
     Spawn spawn = new SpawnBuilder("/bin/echo", "Hi!")
         .withExecutionInfo(ExecutionRequirements.DISABLE_LOCAL_PREFETCH, "").build();
     runner.exec(spawn, policy);
-    assertThat(policy.prefetched).isEmpty();
-  }
-
-  /**
-   * Regression test: the SpawnInputExpander can return null values for empty files, but the
-   * ActionInputPrefetcher expects no null values.
-   */
-  @Test
-  public void checkPrefetchCalledNonNull() throws Exception {
-    Subprocess.Factory factory = mock(Subprocess.Factory.class);
-    when(factory.create(any())).thenReturn(new FinishedSubprocess(0));
-    SubprocessBuilder.setSubprocessFactory(factory);
-
-    LocalExecutionOptions options = Options.getDefaults(LocalExecutionOptions.class);
-    LocalSpawnRunner runner = new LocalSpawnRunner(
-        fs.getPath("/execroot"), options, resourceManager, USE_WRAPPER, OS.LINUX,
-        "product-name", LocalEnvProvider.UNMODIFIED);
-
-    policy.inputMapping.put(PathFragment.create("relative/path"), null);
-    policy.inputMapping.put(
-        PathFragment.create("another/relative/path"), ActionInputHelper.fromPath("/absolute/path"));
-    policy.timeoutMillis = 123 * 1000L;
-    outErr = new FileOutErr(fs.getPath("/out/stdout"), fs.getPath("/out/stderr"));
-    runner.exec(SIMPLE_SPAWN, policy);
-    assertThat(policy.prefetched).hasSize(1);
-    Iterable<ActionInput> prefetched = policy.prefetched.get(0);
-    assertThat(prefetched).doesNotContain(null);
-    assertThat(prefetched).containsExactly(ActionInputHelper.fromPath("/absolute/path"));
+    assertThat(policy.prefetchCalled).isFalse();
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunnerTest.java
index 6a1fdb4bb4..746447b1a6 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/CachedLocalSpawnRunnerTest.java
@@ -87,7 +87,7 @@ public class CachedLocalSpawnRunnerTest {
         }
 
         @Override
-        public void prefetchInputs(Iterable<ActionInput> inputs) {
+        public void prefetchInputs() {
           // CachedLocalSpawnRunner should never prefetch itself, though the nested SpawnRunner may.
           throw new UnsupportedOperationException();
         }
diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
index 6dc3f15e97..e04630a8c0 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteExecutionClientTest.java
@@ -119,7 +119,7 @@ public class GrpcRemoteExecutionClientTest {
         }
 
         @Override
-        public void prefetchInputs(Iterable<ActionInput> inputs) {
+        public void prefetchInputs() {
           throw new UnsupportedOperationException();
         }
 
diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
index 4c7fa6364c..2475b2cb1e 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteSpawnRunnerTest.java
@@ -232,7 +232,7 @@ public class RemoteSpawnRunnerTest {
     }
 
     @Override
-    public void prefetchInputs(Iterable<ActionInput> inputs) throws IOException {
+    public void prefetchInputs() throws IOException {
       throw new UnsupportedOperationException();
     }