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 79227355b01b5d..503b649886b819 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 @@ -29,6 +29,7 @@ import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import java.util.Comparator; +import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.SortedMap; @@ -78,7 +79,19 @@ public static Platform getPlatformProto(Spawn spawn, @Nullable RemoteOptions rem Platform.Builder platformBuilder = Platform.newBuilder(); if (!spawn.getCombinedExecProperties().isEmpty()) { - for (Map.Entry entry : spawn.getCombinedExecProperties().entrySet()) { + Map combinedExecProperties; + // Apply default exec properties if the execution platform does not already set + // exec_properties + if (spawn.getExecutionPlatform() == null + || spawn.getExecutionPlatform().execProperties().isEmpty()) { + combinedExecProperties = new HashMap<>(); + combinedExecProperties.putAll(defaultExecProperties); + combinedExecProperties.putAll(spawn.getCombinedExecProperties()); + } else { + combinedExecProperties = spawn.getCombinedExecProperties(); + } + + for (Map.Entry entry : combinedExecProperties.entrySet()) { platformBuilder.addPropertiesBuilder().setName(entry.getKey()).setValue(entry.getValue()); } } else if (spawn.getExecutionPlatform() != null 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 0b013fbdb81706..fc15ae5d79f493 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,12 +17,14 @@ import static com.google.common.truth.Truth.assertThat; import build.bazel.remote.execution.v2.Platform; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.actions.ExecutionRequirements; import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.exec.util.SpawnBuilder; import com.google.devtools.build.lib.remote.options.RemoteOptions; import com.google.devtools.common.options.Options; +import java.util.AbstractMap; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -45,8 +47,9 @@ private static String platformOptionsString() { private static RemoteOptions remoteOptions() { RemoteOptions remoteOptions = Options.getDefaults(RemoteOptions.class); - remoteOptions.remoteDefaultPlatformProperties = platformOptionsString(); - + remoteOptions.remoteDefaultExecProperties = + ImmutableList.of( + new AbstractMap.SimpleEntry<>("b", "2"), new AbstractMap.SimpleEntry<>("a", "1")); return remoteOptions; } @@ -93,7 +96,7 @@ public void testParsePlatformSortsProperties_execProperties() throws Exception { .addProperties(Platform.Property.newBuilder().setName("zz").setValue("66")) .build(); // execProperties are sorted by key - assertThat(PlatformUtils.getPlatformProto(s, remoteOptions())).isEqualTo(expected); + assertThat(PlatformUtils.getPlatformProto(s, null)).isEqualTo(expected); } @Test @@ -115,4 +118,28 @@ public void testGetPlatformProto_differentiateWorkspace() throws Exception { // execProperties are sorted by key assertThat(PlatformUtils.getPlatformProto(s, remoteOptions())).isEqualTo(expected); } + + @Test + public void getPlatformProto_mergeTargetExecPropertiesWithPlatform() throws Exception { + Spawn spawn = new SpawnBuilder("dummy").withExecProperties(ImmutableMap.of("c", "3")).build(); + Platform expected = + Platform.newBuilder() + .addProperties(Platform.Property.newBuilder().setName("a").setValue("1")) + .addProperties(Platform.Property.newBuilder().setName("b").setValue("2")) + .addProperties(Platform.Property.newBuilder().setName("c").setValue("3")) + .build(); + assertThat(PlatformUtils.getPlatformProto(spawn, remoteOptions())).isEqualTo(expected); + } + + @Test + public void getPlatformProto_targetExecPropertiesConflictWithPlatform_override() + throws Exception { + Spawn spawn = new SpawnBuilder("dummy").withExecProperties(ImmutableMap.of("b", "3")).build(); + Platform expected = + Platform.newBuilder() + .addProperties(Platform.Property.newBuilder().setName("a").setValue("1")) + .addProperties(Platform.Property.newBuilder().setName("b").setValue("3")) + .build(); + assertThat(PlatformUtils.getPlatformProto(spawn, remoteOptions())).isEqualTo(expected); + } }