Error out when mixing the two modes for specifying platforms.
RELNOTES: None
PiperOrigin-RevId: 261017812
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
index dc84c24..1d09ae7 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/platform/PlatformInfo.java
@@ -237,13 +237,40 @@
return this;
}
+ private void checkRemoteExecutionProperties() throws ExecPropertiesException {
+ if (execProperties != null && !Strings.isNullOrEmpty(remoteExecutionProperties)) {
+ throw new ExecPropertiesException(
+ "Platform contains both remote_execution_properties and exec_properties. Prefer"
+ + " exec_properties over the deprecated remote_execution_properties.");
+ }
+ if (execProperties != null
+ && parent != null
+ && !Strings.isNullOrEmpty(parent.remoteExecutionProperties())) {
+ throw new ExecPropertiesException(
+ "Platform specifies exec_properties but its parent "
+ + parent.label()
+ + " specifies remote_execution_properties. Prefer exec_properties over the"
+ + " deprecated remote_execution_properties.");
+ }
+ if (!Strings.isNullOrEmpty(remoteExecutionProperties)
+ && parent != null
+ && !parent.execProperties().isEmpty()) {
+ throw new ExecPropertiesException(
+ "Platform specifies remote_execution_properties but its parent specifies"
+ + " exec_properties. Prefer exec_properties over the deprecated"
+ + " remote_execution_properties.");
+ }
+ }
+
/**
* Returns the new {@link PlatformInfo} instance.
*
* @throws DuplicateConstraintException if more than one constraint value exists for the same
* constraint setting
*/
- public PlatformInfo build() throws DuplicateConstraintException {
+ public PlatformInfo build() throws DuplicateConstraintException, ExecPropertiesException {
+ checkRemoteExecutionProperties();
+
// Merge the remote execution properties.
String remoteExecutionProperties =
mergeRemoteExecutionProperties(parent, this.remoteExecutionProperties);
@@ -315,4 +342,11 @@
public int hashCode() {
return Objects.hash(label, constraints, remoteExecutionProperties);
}
+
+ /** Exception that indicates something is wrong in exec_properties configuration. */
+ public static class ExecPropertiesException extends Exception {
+ ExecPropertiesException(String message) {
+ super(message);
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java
index e1e870f..991e952 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/platform/Platform.java
@@ -81,7 +81,9 @@
Map<String, String> execProperties =
ruleContext.attributes().get(PlatformRule.EXEC_PROPS_ATTR, Type.STRING_DICT);
- platformBuilder.setExecProperties(ImmutableMap.copyOf(execProperties));
+ if (execProperties != null && !execProperties.isEmpty()) {
+ platformBuilder.setExecProperties(ImmutableMap.copyOf(execProperties));
+ }
PlatformInfo platformInfo;
try {
@@ -89,6 +91,8 @@
} catch (ConstraintCollection.DuplicateConstraintException e) {
throw ruleContext.throwWithAttributeError(
PlatformRule.CONSTRAINT_VALUES_ATTR, e.getMessage());
+ } catch (PlatformInfo.ExecPropertiesException e) {
+ throw ruleContext.throwWithAttributeError(PlatformRule.EXEC_PROPS_ATTR, e.getMessage());
}
return new RuleConfiguredTargetBuilder(ruleContext)
diff --git a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java
index 9d6e045..39dc909 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/platform/PlatformInfoApiTest.java
@@ -187,6 +187,22 @@
}
@Test
+ public void testPlatform_conflictingProperties_errorsOut() throws Exception {
+ ImmutableMap<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
+ PlatformBuilder builder =
+ platformBuilder("//foo:my_platform")
+ .setExecProperties(props)
+ .setRemoteExecutionProperties("child props");
+
+ checkError(
+ "foo",
+ "my_platform",
+ "Platform contains both remote_execution_properties and exec_properties. Prefer"
+ + " exec_properties over the deprecated remote_execution_properties.",
+ builder.lines().toArray(new String[] {}));
+ }
+
+ @Test
public void testPlatform_execProperties_parent() throws Exception {
ImmutableMap<String, String> props = ImmutableMap.of("k1", "v1", "k2", "v2");
platformBuilder("//foo:parent_platform").setExecProperties(props).write();
@@ -215,4 +231,41 @@
ImmutableMap.of("k1", "v1", "k2", "child_v2", "k3", "child_v3");
assertThat(platformInfo.execProperties()).isEqualTo(expected);
}
+
+ @Test
+ public void testPlatform_execProperties_parentSpecifiesRemoteExecutionProperties_errorsOut()
+ throws Exception {
+ ImmutableMap<String, String> propsParent = ImmutableMap.of("k1", "v1", "k2", "v2");
+ platformBuilder("//foo:parent_platform").setExecProperties(propsParent).write();
+ PlatformBuilder builder =
+ platformBuilder("//bar:my_platform")
+ .setParent("//foo:parent_platform")
+ .setRemoteExecutionProperties("properties");
+
+ checkError(
+ "bar",
+ "my_platform",
+ "Platform specifies remote_execution_properties but its parent specifies exec_properties."
+ + " Prefer exec_properties over the deprecated remote_execution_properties.",
+ builder.lines().toArray(new String[] {}));
+ }
+
+ @Test
+ public void testPlatform_remoteExecutionProperties_parentSpecifiesExecProperties_errorsOut()
+ throws Exception {
+ ImmutableMap<String, String> propsChild = ImmutableMap.of("k2", "child_v2", "k3", "child_v3");
+ platformBuilder("//foo:parent_platform").setRemoteExecutionProperties("properties").write();
+ PlatformBuilder builder =
+ platformBuilder("//bar:my_platform")
+ .setParent("//foo:parent_platform")
+ .setExecProperties(propsChild);
+
+ checkError(
+ "bar",
+ "my_platform",
+ "Platform specifies exec_properties but its parent //foo:parent_platform specifies"
+ + " remote_execution_properties. Prefer exec_properties over the deprecated"
+ + " remote_execution_properties.",
+ builder.lines().toArray(new String[] {}));
+ }
}
