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 index 739b25661343b7..765294ead8fb09 100644 --- 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 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 properties = platformBuilder.getPropertiesList(); + List 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 index 985fc272441b19..45df840a9af984 100644 --- 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 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); + } }