Have the RemoteSpawnRunner use the execution platform present in the Spawn to get the remote execution properties.

Fixes #4128.

This reverts commit 3ce42ef3074ee6d3ac7d9968381c8c0a51d9d38d.

Change-Id: I8b9ad5099f6334c2488a22baf05d0b273e10f776
PiperOrigin-RevId: 181550828

4 files changed, 30 insertions, 39 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/remote/BUILD b/src/main/java/com/google/devtools/build/lib/remote/BUILD
index 8ac275eea1..51c33841ae 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/remote/BUILD
@@ -22,6 +22,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:runtime",
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/actions",
+        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
         "//src/main/java/com/google/devtools/build/lib/authandtls",
         "//src/main/java/com/google/devtools/build/lib/buildeventstream",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
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 4b257d7ac4..b9b1f55d79 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
@@ -20,9 +20,6 @@ import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDocumentationCategory;
 import com.google.devtools.common.options.OptionEffectTag;
 import com.google.devtools.common.options.OptionsBase;
-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 {
@@ -113,16 +110,6 @@ public final class RemoteOptions extends OptionsBase {
   public boolean remoteUploadLocalResults;
 
   @Option(
-    name = "experimental_remote_platform_override",
-    defaultValue = "null",
-    category = "remote",
-    documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
-    effectTags = {OptionEffectTag.UNKNOWN},
-    help = "Temporary, for testing only. Manually set a Platform to pass to remote execution."
-  )
-  public String experimentalRemotePlatformOverride;
-
-  @Option(
     name = "remote_instance_name",
     defaultValue = "",
     category = "remote",
@@ -224,19 +211,4 @@ public final class RemoteOptions extends OptionsBase {
     help = "A file path to a local disk cache."
   )
   public PathFragment experimentalLocalDiskCachePath;
-
-  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/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
index 9cf68d114c..e8af197907 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java
@@ -32,7 +32,6 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.remoteexecution.v1test.Action;
 import com.google.devtools.remoteexecution.v1test.ActionResult;
 import com.google.devtools.remoteexecution.v1test.Command;
-import com.google.devtools.remoteexecution.v1test.Platform;
 import io.grpc.Context;
 import java.io.IOException;
 import java.util.Collection;
@@ -52,8 +51,6 @@ import javax.annotation.Nullable;
 final class RemoteSpawnCache implements SpawnCache {
   private final Path execRoot;
   private final RemoteOptions options;
-  // TODO(olaola): This will be set on a per-action basis instead.
-  private final Platform platform;
 
   private final AbstractRemoteActionCache remoteCache;
   private final String buildRequestId;
@@ -78,7 +75,6 @@ final class RemoteSpawnCache implements SpawnCache {
       DigestUtil digestUtil) {
     this.execRoot = execRoot;
     this.options = options;
-    this.platform = options.parseRemotePlatformOverride();
     this.remoteCache = remoteCache;
     this.verboseFailures = verboseFailures;
     this.cmdlineReporter = cmdlineReporter;
@@ -103,7 +99,7 @@ final class RemoteSpawnCache implements SpawnCache {
             spawn.getOutputFiles(),
             digestUtil.compute(command),
             repository.getMerkleDigest(inputRoot),
-            platform,
+            spawn.getExecutionPlatform(),
             policy.getTimeout(),
             Spawns.mayBeCached(spawn));
 
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 b8a1080c7b..e0ba1fb4f4 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
@@ -26,6 +26,8 @@ import com.google.devtools.build.lib.actions.SpawnResult;
 import com.google.devtools.build.lib.actions.SpawnResult.Status;
 import com.google.devtools.build.lib.actions.Spawns;
 import com.google.devtools.build.lib.actions.cache.VirtualActionInput;
+import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.Reporter;
@@ -45,6 +47,8 @@ import com.google.devtools.remoteexecution.v1test.Digest;
 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.TextFormat;
+import com.google.protobuf.TextFormat.ParseException;
 import io.grpc.Context;
 import io.grpc.Status.Code;
 import java.io.IOException;
@@ -69,8 +73,6 @@ class RemoteSpawnRunner implements SpawnRunner {
 
   private final Path execRoot;
   private final RemoteOptions options;
-  // TODO(olaola): This will be set on a per-action basis instead.
-  private final Platform platform;
   private final SpawnRunner fallbackRunner;
   private final boolean verboseFailures;
 
@@ -97,7 +99,6 @@ class RemoteSpawnRunner implements SpawnRunner {
       DigestUtil digestUtil) {
     this.execRoot = execRoot;
     this.options = options;
-    this.platform = options.parseRemotePlatformOverride();
     this.fallbackRunner = fallbackRunner;
     this.remoteCache = remoteCache;
     this.remoteExecutor = remoteExecutor;
@@ -129,7 +130,7 @@ class RemoteSpawnRunner implements SpawnRunner {
             spawn.getOutputFiles(),
             digestUtil.compute(command),
             repository.getMerkleDigest(inputRoot),
-            platform,
+            spawn.getExecutionPlatform(),
             policy.getTimeout(),
             Spawns.mayBeCached(spawn));
 
@@ -263,9 +264,10 @@ class RemoteSpawnRunner implements SpawnRunner {
       Collection<? extends ActionInput> outputs,
       Digest command,
       Digest inputRoot,
-      Platform platform,
+      @Nullable PlatformInfo executionPlatform,
       Duration timeout,
       boolean cacheable) {
+
     Action.Builder action = Action.newBuilder();
     action.setCommandDigest(command);
     action.setInputRootDigest(inputRoot);
@@ -282,9 +284,14 @@ class RemoteSpawnRunner implements SpawnRunner {
     Collections.sort(outputPaths);
     Collections.sort(outputDirectoryPaths);
     action.addAllOutputFiles(outputPaths);
-    if (platform != null) {
+
+    // Get the remote platform properties.
+    if (executionPlatform != null) {
+      Platform platform =
+          parsePlatform(executionPlatform.label(), executionPlatform.remoteExecutionProperties());
       action.setPlatform(platform);
     }
+
     if (!timeout.isZero()) {
       action.setTimeout(com.google.protobuf.Duration.newBuilder().setSeconds(timeout.getSeconds()));
     }
@@ -294,6 +301,21 @@ class RemoteSpawnRunner implements SpawnRunner {
     return action.build();
   }
 
+  static Platform parsePlatform(Label platformLabel, @Nullable String platformDescription) {
+    Platform.Builder platformBuilder = Platform.newBuilder();
+    try {
+      if (platformDescription != null) {
+        TextFormat.getParser().merge(platformDescription, platformBuilder);
+      }
+    } catch (ParseException e) {
+      throw new IllegalArgumentException(
+          String.format(
+              "Failed to parse remote_execution_properties from platform %s", platformLabel),
+          e);
+    }
+    return platformBuilder.build();
+  }
+
   static Command buildCommand(List<String> arguments, ImmutableMap<String, String> env) {
     Command.Builder command = Command.newBuilder();
     command.addAllArguments(arguments);