Refactoring: include exec_properties as a separate field in Spawn and…

… change the code to use that instead of the platform.

This is in preparation for target level execution properties
(See https://docs.google.com/document/d/1w3fu8zu_sRw_gK1dFAvkY2suhbQQ82tc0zdjet-dpCI/edit#heading=h.5mcn15i0e1ch)

RELNOTES: None
PiperOrigin-RevId: 265386594

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

@@ -16,6 +16,7 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
@@ -549,6 +550,11 @@ public SkylarkDict<String, String> getEnv() {
     return SkylarkDict.copyOf(null, env.getFixedEnv().toMap());
   }
 
+  @Override
+  public ImmutableMap<String, String> getExecProperties() {
+    return getOwner().getExecProperties();
+  }
+
   @Override
   @Nullable
   public PlatformInfo getExecutionPlatform() {

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

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
 import javax.annotation.Nullable;
@@ -250,6 +251,9 @@ default boolean hasLooseHeaders() {
     return false;
   }
 
+  /** Returns a String to String map containing the execution properties of this action. */
+  ImmutableMap<String, String> getExecProperties();
+
   /**
    * Returns the {@link PlatformInfo} platform this action should be executed on. If the execution
    * platform is {@code null}, then the host platform is assumed.

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

@@ -40,6 +40,8 @@ public final String getKey(ActionKeyContext actionKeyContext) {
               getExecutionPlatform().addTo(fp);
             }
 
+            fp.addStringMap(getExecProperties());
+
             // Compute the actual key and store it.
             cachedKey = fp.hexDigestAndReset();
           } catch (CommandLineExpansionException e) {

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

@@ -16,6 +16,7 @@
 import com.google.auto.value.AutoValue;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 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.Immutable;
@@ -46,6 +47,7 @@ public abstract class ActionOwner {
           "system",
           null,
           null,
+          ImmutableMap.<String, String>of(),
           null);
 
   @AutoCodec.Instantiator
@@ -58,6 +60,7 @@ public static ActionOwner create(
       String configurationChecksum,
       @Nullable BuildConfigurationEvent configuration,
       @Nullable String additionalProgressInfo,
+      ImmutableMap<String, String> execProperties,
       @Nullable PlatformInfo executionPlatform) {
     return new AutoValue_ActionOwner(
         location,
@@ -68,6 +71,7 @@ public static ActionOwner create(
         configuration,
         targetKind,
         additionalProgressInfo,
+        execProperties,
         executionPlatform);
   }
 
@@ -111,6 +115,9 @@ public static ActionOwner create(
   @Nullable
   abstract String getAdditionalProgressInfo();
 
+  /** Returns a String to String map containing the execution properties of this action. */
+  abstract ImmutableMap<String, String> getExecProperties();
+
   /**
    * Returns the {@link PlatformInfo} platform this action should be executed on. If the execution
    * platform is {@code null}, then the host platform is assumed.

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

@@ -13,6 +13,7 @@
 // limitations under the License.
 package com.google.devtools.build.lib.actions;
 
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.actions.ActionLookupValue.ActionLookupKey;
 import com.google.devtools.build.lib.actions.Artifact.TreeFileArtifact;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
@@ -88,6 +89,11 @@ Iterable<T> generateActionForInputArtifacts(
   /** Returns the output TreeArtifact. */
   Artifact getOutputTreeArtifact();
 
+  @Override
+  default ImmutableMap<String, String> getExecProperties() {
+    return ImmutableMap.<String, String>of();
+  }
+
   @Override
   @Nullable
   default PlatformInfo getExecutionPlatform() {

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

@@ -150,6 +150,11 @@ public String getMnemonic() {
     return action.getMnemonic();
   }
 
+  @Override
+  public ImmutableMap<String, String> getCombinedExecProperties() {
+    return action.getOwner().getExecProperties();
+  }
+
   @Override
   @Nullable
   public PlatformInfo getExecutionPlatform() {

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

@@ -87,6 +87,11 @@ public String getMnemonic() {
     return spawn.getMnemonic();
   }
 
+  @Override
+  public ImmutableMap<String, String> getCombinedExecProperties() {
+    return spawn.getCombinedExecProperties();
+  }
+
   @Override
   @Nullable
   public PlatformInfo getExecutionPlatform() {

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

@@ -139,6 +139,11 @@ public String getMnemonic() {
     return owner.getMnemonic();
   }
 
+  @Override
+  public ImmutableMap<String, String> getCombinedExecProperties() {
+    return owner.getExecProperties();
+  }
+
   @Override
   @Nullable
   public PlatformInfo getExecutionPlatform() {

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

@@ -111,6 +111,14 @@ public interface Spawn {
    */
   String getMnemonic();
 
+  /**
+   * Returns execution properties related to this spawn.
+   *
+   * <p>Note that this includes data from the execution platform's exec_properties as well as
+   * target-level exec_properties. TODO(agoulti): implement target-level exec_properties.
+   */
+  ImmutableMap<String, String> getCombinedExecProperties();
+
   @Nullable
   PlatformInfo getExecutionPlatform();
 }

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -514,6 +514,14 @@ public static ActionOwner createActionOwner(
       ImmutableList<AspectDescriptor> aspectDescriptors,
       BuildConfiguration configuration,
       @Nullable PlatformInfo executionPlatform) {
+    ImmutableMap<String, String> execProperties;
+    if (executionPlatform != null) {
+      execProperties = executionPlatform.execProperties();
+    } else {
+      execProperties = ImmutableMap.of();
+    }
+    // TODO(agoulti): Insert logic to include per-target execution properties
+
     return ActionOwner.create(
         rule.getLabel(),
         aspectDescriptors,
@@ -523,6 +531,7 @@ public static ActionOwner createActionOwner(
         configuration.checksum(),
         configuration.toBuildEvent(),
         configuration.isHostConfiguration() ? HOST_CONFIGURATION_PROGRESS_TAG : null,
+        execProperties,
         executionPlatform);
   }
 

src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java

@@ -19,6 +19,7 @@
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.common.collect.Ordering;
+import com.google.devtools.build.lib.actions.Spawn;
 import com.google.devtools.build.lib.actions.UserExecException;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import com.google.protobuf.TextFormat;
@@ -33,35 +34,36 @@
 public final class PlatformUtils {
 
   @Nullable
-  public static Platform getPlatformProto(
-      @Nullable PlatformInfo executionPlatform, @Nullable RemoteOptions remoteOptions)
+  public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions remoteOptions)
       throws UserExecException {
     SortedMap<String, String> defaultExecProperties =
         remoteOptions != null
             ? remoteOptions.getRemoteDefaultExecProperties()
             : ImmutableSortedMap.of();
 
-    if (executionPlatform == null && defaultExecProperties.isEmpty()) {
+    if (spawn.getExecutionPlatform() == null
+        && spawn.getCombinedExecProperties().isEmpty()
+        && defaultExecProperties.isEmpty()) {
       return null;
     }
 
     Platform.Builder platformBuilder = Platform.newBuilder();
 
-    if (executionPlatform != null && !executionPlatform.execProperties().isEmpty()) {
-      for (Map.Entry<String, String> entry : executionPlatform.execProperties().entrySet()) {
+    if (!spawn.getCombinedExecProperties().isEmpty()) {
+      for (Map.Entry<String, String> entry : spawn.getCombinedExecProperties().entrySet()) {
         platformBuilder.addPropertiesBuilder().setName(entry.getKey()).setValue(entry.getValue());
       }
-    } else if (executionPlatform != null
-        && !Strings.isNullOrEmpty(executionPlatform.remoteExecutionProperties())) {
+    } else if (spawn.getExecutionPlatform() != null
+        && !Strings.isNullOrEmpty(spawn.getExecutionPlatform().remoteExecutionProperties())) {
       // Try and get the platform info from the execution properties.
       try {
         TextFormat.getParser()
-            .merge(executionPlatform.remoteExecutionProperties(), platformBuilder);
+            .merge(spawn.getExecutionPlatform().remoteExecutionProperties(), platformBuilder);
       } catch (ParseException e) {
         throw new UserExecException(
             String.format(
                 "Failed to parse remote_execution_properties from platform %s",
-                executionPlatform.label()),
+                spawn.getExecutionPlatform().label()),
             e);
       }
     } else {

src/main/java/com/google/devtools/build/lib/exec/SpawnLogContext.java

@@ -129,8 +129,7 @@ public void logSpawn(
     }
     builder.setRemotable(Spawns.mayBeExecutedRemotely(spawn));
 
-    Platform execPlatform =
-        PlatformUtils.getPlatformProto(spawn.getExecutionPlatform(), remoteOptions);
+    Platform execPlatform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
     if (execPlatform != null) {
       builder.setPlatform(buildPlatform(execPlatform));
     }

src/main/java/com/google/devtools/build/lib/includescanning/SpawnIncludeScanner.java

@@ -194,6 +194,11 @@ public RunfilesSupplier getRunfilesSupplier() {
       throw new UnsupportedOperationException();
     }
 
+    @Override
+    public ImmutableMap<String, String> getExecProperties() {
+      return actionExecutionMetadata.getExecProperties();
+    }
+
     @Override
     @Nullable
     public PlatformInfo getExecutionPlatform() {

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

@@ -132,7 +132,7 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context)
     Digest merkleTreeRoot = merkleTree.getRootDigest();
 
     // Get the remote platform properties.
-    Platform platform = PlatformUtils.getPlatformProto(spawn.getExecutionPlatform(), options);
+    Platform platform = PlatformUtils.getPlatformProto(spawn, options);
 
     Command command =
         RemoteSpawnRunner.buildCommand(

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

@@ -169,7 +169,7 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context)
     maybeWriteParamFilesLocally(spawn);
 
     // Get the remote platform properties.
-    Platform platform = PlatformUtils.getPlatformProto(spawn.getExecutionPlatform(), remoteOptions);
+    Platform platform = PlatformUtils.getPlatformProto(spawn, remoteOptions);
 
     Command command =
         buildCommand(

src/main/java/com/google/devtools/build/lib/sandbox/DockerSandboxedSpawnRunner.java

@@ -377,8 +377,7 @@ private String executeCommand(List<String> cmdLine, InputStream stdIn) throws Us
 
   private Optional<String> dockerContainerFromSpawn(Spawn spawn) throws ExecException {
     Platform platform =
-        PlatformUtils.getPlatformProto(
-            spawn.getExecutionPlatform(), cmdEnv.getOptions().getOptions(RemoteOptions.class));
+        PlatformUtils.getPlatformProto(spawn, cmdEnv.getOptions().getOptions(RemoteOptions.class));
 
     if (platform != null) {
       try {

src/test/java/com/google/devtools/build/lib/actions/ResourceManagerTest.java

@@ -17,6 +17,7 @@
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.actions.ResourceManager.ResourceHandle;
 import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
@@ -547,6 +548,11 @@ public MiddlemanType getActionType() {
       throw new IllegalStateException();
     }
 
+    @Override
+    public ImmutableMap<String, String> getExecProperties() {
+      throw new IllegalStateException();
+    }
+
     @Nullable
     @Override
     public PlatformInfo getExecutionPlatform() {

src/test/java/com/google/devtools/build/lib/actions/util/ActionsTestUtil.java

@@ -346,6 +346,7 @@ public Label getLabel() {
           "dummy-configuration",
           null,
           null,
+          ImmutableMap.<String, String>of(),
           null);
 
   @AutoCodec

src/test/java/com/google/devtools/build/lib/analysis/platform/BUILD

@@ -23,6 +23,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
         "//src/main/java/com/google/devtools/build/lib:syntax",
         "//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/analysis/platform:platform_utils",
         "//src/main/java/com/google/devtools/build/lib/analysis/platform:utils",