Move the remote initialization code out of RemoteSpawnStrategy

This is in preparation for making it use the RemoteSpawnRunner, as part of
which it will no longer need to do that. Also, Java style says you shouldn't
do work in the constructor, and it's better dependency injection.

PiperOrigin-RevId: 161071134

4 files changed, 58 insertions, 58 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
index af0f1a81d4..af15906e97 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java
@@ -53,11 +53,43 @@ final class RemoteActionContextProvider extends ActionContextProvider {
             executionOptions.verboseFailures,
             env.getRuntime().getProductName(),
             ResourceManager.instance());
+
+    RemoteOptions remoteOptions = env.getOptions().getOptions(RemoteOptions.class);
+    AuthAndTLSOptions authAndTlsOptions = env.getOptions().getOptions(AuthAndTLSOptions.class);
+    ChannelOptions channelOptions = ChannelOptions.create(authAndTlsOptions);
+
+    // Initialize remote cache and execution handlers. We use separate handlers for every
+    // action to enable server-side parallelism (need a different gRPC channel per action).
+    RemoteActionCache remoteCache;
+    if (SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions)) {
+      remoteCache = new SimpleBlobStoreActionCache(SimpleBlobStoreFactory.create(remoteOptions));
+    } else if (GrpcRemoteCache.isRemoteCacheOptions(remoteOptions)) {
+      remoteCache =
+          new GrpcRemoteCache(
+              GrpcUtils.createChannel(remoteOptions.remoteCache, channelOptions),
+              channelOptions,
+              remoteOptions);
+    } else {
+      remoteCache = null;
+    }
+
+    // Otherwise remoteCache remains null and remote caching/execution are disabled.
+    GrpcRemoteExecutor remoteExecutor;
+    if (remoteCache != null && GrpcRemoteExecutor.isRemoteExecutionOptions(remoteOptions)) {
+      remoteExecutor =
+          new GrpcRemoteExecutor(
+              GrpcUtils.createChannel(remoteOptions.remoteExecutor, channelOptions),
+              channelOptions,
+              remoteOptions);
+    } else {
+      remoteExecutor = null;
+    }
     spawnStrategy =
         new RemoteSpawnStrategy(
             env.getExecRoot(),
-            env.getOptions().getOptions(RemoteOptions.class),
-            env.getOptions().getOptions(AuthAndTLSOptions.class),
+            remoteOptions,
+            remoteCache,
+            remoteExecutor,
             executionOptions.verboseFailures,
             fallbackStrategy);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
index 203128dab1..1706f7ea98 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteOptions.java
@@ -18,6 +18,9 @@ import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.proto.OptionFilters.OptionEffectTag;
+import com.google.devtools.remoteexecution.v1test.Platform;
+import com.google.protobuf.TextFormat;
+import com.google.protobuf.TextFormat.ParseException;
 
 /** Options for remote execution and distributed caching. */
 public final class RemoteOptions extends OptionsBase {
@@ -214,4 +217,19 @@ public final class RemoteOptions extends OptionsBase {
     help = "The random factor to apply to retry delays on transient errors."
   )
   public double experimentalRemoteRetryJitter;
+
+  public Platform parseRemotePlatformOverride() {
+    if (experimentalRemotePlatformOverride != null) {
+      Platform.Builder platformBuilder = Platform.newBuilder();
+      try {
+        TextFormat.getParser().merge(experimentalRemotePlatformOverride, platformBuilder);
+      } catch (ParseException e) {
+        throw new IllegalArgumentException(
+            "Failed to parse --experimental_remote_platform_override", e);
+      }
+      return platformBuilder.build();
+    } else {
+      return null;
+    }
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
index 4ec3610db0..0c1a6cf63f 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java
@@ -35,8 +35,6 @@ import com.google.devtools.remoteexecution.v1test.Digest;
 import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
 import com.google.devtools.remoteexecution.v1test.Platform;
 import com.google.protobuf.Duration;
-import com.google.protobuf.TextFormat;
-import com.google.protobuf.TextFormat.ParseException;
 import io.grpc.StatusRuntimeException;
 import java.io.IOException;
 import java.util.Collection;
@@ -62,17 +60,7 @@ final class RemoteSpawnRunner implements SpawnRunner {
       GrpcRemoteCache remoteCache) {
     this.execRoot = execRoot;
     this.options = options;
-    if (options.experimentalRemotePlatformOverride != null) {
-      Platform.Builder platformBuilder = Platform.newBuilder();
-      try {
-        TextFormat.getParser().merge(options.experimentalRemotePlatformOverride, platformBuilder);
-      } catch (ParseException e) {
-        throw new RuntimeException("Failed to parse --experimental_remote_platform_override", e);
-      }
-      platform = platformBuilder.build();
-    } else {
-      platform = null;
-    }
+    this.platform = options.parseRemotePlatformOverride();
     this.executor = executor;
     this.remoteCache = remoteCache;
   }
diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java
index 871bc0cff5..346d43e1bb 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java
@@ -26,7 +26,6 @@ import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.SpawnActionContext;
 import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.actions.UserExecException;
-import com.google.devtools.build.lib.authandtls.AuthAndTLSOptions;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.exec.SpawnInputExpander;
@@ -44,8 +43,6 @@ import com.google.devtools.remoteexecution.v1test.ExecuteRequest;
 import com.google.devtools.remoteexecution.v1test.ExecuteResponse;
 import com.google.devtools.remoteexecution.v1test.Platform;
 import com.google.protobuf.Duration;
-import com.google.protobuf.TextFormat;
-import com.google.protobuf.TextFormat.ParseException;
 import io.grpc.StatusRuntimeException;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -69,7 +66,6 @@ final class RemoteSpawnStrategy implements SpawnActionContext {
   private final RemoteOptions remoteOptions;
   // TODO(olaola): This will be set on a per-action basis instead.
   private final Platform platform;
-  private final ChannelOptions channelOptions;
   private final SpawnInputExpander spawnInputExpander = new SpawnInputExpander(/*strict=*/ false);
 
   private final RemoteActionCache remoteCache;
@@ -78,51 +74,17 @@ final class RemoteSpawnStrategy implements SpawnActionContext {
   RemoteSpawnStrategy(
       Path execRoot,
       RemoteOptions remoteOptions,
-      AuthAndTLSOptions authTlsOptions,
+      RemoteActionCache remoteCache,
+      GrpcRemoteExecutor remoteExecutor,
       boolean verboseFailures,
       SpawnActionContext fallbackStrategy) {
     this.execRoot = execRoot;
     this.fallbackStrategy = fallbackStrategy;
     this.verboseFailures = verboseFailures;
     this.remoteOptions = remoteOptions;
-    channelOptions = ChannelOptions.create(authTlsOptions);
-    if (remoteOptions.experimentalRemotePlatformOverride != null) {
-      Platform.Builder platformBuilder = Platform.newBuilder();
-      try {
-        TextFormat.getParser()
-            .merge(remoteOptions.experimentalRemotePlatformOverride, platformBuilder);
-      } catch (ParseException e) {
-        throw new IllegalArgumentException(
-            "Failed to parse --experimental_remote_platform_override", e);
-      }
-      platform = platformBuilder.build();
-    } else {
-      platform = null;
-    }
-    // Initialize remote cache and execution handlers. We use separate handlers for every
-    // action to enable server-side parallelism (need a different gRPC channel per action).
-    if (SimpleBlobStoreFactory.isRemoteCacheOptions(remoteOptions)) {
-      remoteCache = new SimpleBlobStoreActionCache(SimpleBlobStoreFactory.create(remoteOptions));
-    } else if (GrpcRemoteCache.isRemoteCacheOptions(remoteOptions)) {
-      remoteCache =
-          new GrpcRemoteCache(
-              GrpcUtils.createChannel(remoteOptions.remoteCache, channelOptions),
-              channelOptions,
-              remoteOptions);
-    } else {
-      remoteCache = null;
-    }
-    // Otherwise remoteCache remains null and remote caching/execution are disabled.
-
-    if (remoteCache != null && GrpcRemoteExecutor.isRemoteExecutionOptions(remoteOptions)) {
-      workExecutor =
-          new GrpcRemoteExecutor(
-              GrpcUtils.createChannel(remoteOptions.remoteExecutor, channelOptions),
-              channelOptions,
-              remoteOptions);
-    } else {
-      workExecutor = null;
-    }
+    this.platform = remoteOptions.parseRemotePlatformOverride();
+    this.remoteCache = remoteCache;
+    this.workExecutor = remoteExecutor;
   }
 
   /** Release resources associated with this spawn strategy. */