Using execProperties in platform generation.

Checking to ensure that execProperties are not conflicting with remoteExecutionProperties will be done at PlatformInfo creation time (in a later cl). RELNOTES: None PiperOrigin-RevId: 260930416
bazelbuild · Jul 31, 2019 · e4ce176 · e4ce176
1 parent f1ee74d
commit e4ce176
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformUtils.java
@@ -23,6 +23,7 @@
 import com.google.protobuf.TextFormat.ParseException;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Map;
 import javax.annotation.Nullable;
 
 /** Utilities for accessing platform properties. */
@@ -43,7 +44,11 @@ public static Platform getPlatformProto(
 
     Platform.Builder platformBuilder = Platform.newBuilder();
 
-    if (executionPlatform != null
+    if (executionPlatform != null && !executionPlatform.execProperties().isEmpty()) {
+      for (Map.Entry<String, String> entry : executionPlatform.execProperties().entrySet()) {
+        platformBuilder.addPropertiesBuilder().setName(entry.getKey()).setValue(entry.getValue());
+      }
+    } else if (executionPlatform != null
         && !Strings.isNullOrEmpty(executionPlatform.remoteExecutionProperties())) {
       // Try and get the platform info from the execution properties.
       try {
@@ -70,10 +75,11 @@ public static Platform getPlatformProto(
     }
 
     // Sort the properties.
-    List<Platform.Property> properties = platformBuilder.getPropertiesList();
+    List<Platform.Property> properties =
+        Ordering.from(Comparator.comparing(Platform.Property::getName))
+            .sortedCopy(platformBuilder.getPropertiesList());
     platformBuilder.clearProperties();
-    platformBuilder.addAllProperties(
-        Ordering.from(Comparator.comparing(Platform.Property::getName)).sortedCopy(properties));
+    platformBuilder.addAllProperties(properties);
     return platformBuilder.build();
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformUtilsTest.java b/src/test/java/com/google/devtools/build/lib/analysis/platform/PlatformUtilsTest.java
@@ -17,6 +17,7 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import build.bazel.remote.execution.v2.Platform;
+import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.remote.options.RemoteOptions;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -25,9 +26,9 @@
 /** Tests for {@link PlatformUtils } */
 @RunWith(JUnit4.class)
 public final class PlatformUtilsTest {
-  @Test
-  public void testParsePlatformSortsProperties() throws Exception {
-    String s =
+  private static RemoteOptions remoteOptions() {
+    RemoteOptions remoteOptions = new RemoteOptions();
+    remoteOptions.remoteDefaultPlatformProperties =
         String.join(
             "\n",
             "properties: {",
@@ -38,19 +39,38 @@ public void testParsePlatformSortsProperties() throws Exception {
             " name: \"a\"",
             " value: \"1\"",
             "}");
-    RemoteOptions remoteOptions = new RemoteOptions();
-    remoteOptions.remoteDefaultPlatformProperties = s;
 
+    return remoteOptions;
+  }
+
+  @Test
+  public void testParsePlatformSortsProperties() throws Exception {
     Platform expected =
         Platform.newBuilder()
             .addProperties(Platform.Property.newBuilder().setName("a").setValue("1"))
             .addProperties(Platform.Property.newBuilder().setName("b").setValue("2"))
             .build();
-    assertThat(PlatformUtils.getPlatformProto(null, remoteOptions)).isEqualTo(expected);
+    assertThat(PlatformUtils.getPlatformProto(null, remoteOptions())).isEqualTo(expected);
   }
 
   @Test
   public void testParsePlatformHandlesNull() throws Exception {
     assertThat(PlatformUtils.getPlatformProto(null, null)).isEqualTo(null);
   }
+
+  @Test
+  public void testParsePlatformSortsProperties_ExecProperties() throws Exception {
+    // execProperties are chosen even if there are remoteOptions
+    ImmutableMap<String, String> map = ImmutableMap.of("aa", "99", "zz", "66", "dd", "11");
+    PlatformInfo platformInfo = PlatformInfo.builder().setExecProperties(map).build();
+
+    Platform expected =
+        Platform.newBuilder()
+            .addProperties(Platform.Property.newBuilder().setName("aa").setValue("99"))
+            .addProperties(Platform.Property.newBuilder().setName("dd").setValue("11"))
+            .addProperties(Platform.Property.newBuilder().setName("zz").setValue("66"))
+            .build();
+    // execProperties are sorted by key
+    assertThat(PlatformUtils.getPlatformProto(platformInfo, remoteOptions())).isEqualTo(expected);
+  }
 }