Raise an error if platform mappings have duplicate keys.
It should not be possible to create two platform mappings with either the same platform as key or the same set of flags.
RELNOTES: None.
PiperOrigin-RevId: 244227201
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java
index 33694f4..5ae21de 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunction.java
@@ -185,7 +185,12 @@
platformsToFlags.put(platform, flags);
}
- return platformsToFlags.build();
+ try {
+ return platformsToFlags.build();
+ } catch (IllegalArgumentException e) {
+ throw throwParsingException(
+ e, "Got duplicate platform entries but each platform key must be unique");
+ }
}
private Label platform() throws PlatformMappingException {
@@ -202,9 +207,7 @@
// mappings however only apply within a repository imported by the root repository.
platform = Label.parseAbsolute(label, emptyRepositoryMapping);
} catch (LabelSyntaxException e) {
- throw new PlatformMappingException(
- new PlatformMappingParsingException("Expected platform label but got " + label, e),
- SkyFunctionException.Transience.PERSISTENT);
+ throw throwParsingException(e, "Expected platform label but got " + label);
}
return platform;
}
@@ -234,7 +237,12 @@
Label platform = platform();
flagsToPlatforms.put(flags, platform);
}
- return flagsToPlatforms.build();
+ try {
+ return flagsToPlatforms.build();
+ } catch (IllegalArgumentException e) {
+ throw throwParsingException(
+ e, "Got duplicate flags entries but each flags key must be unique");
+ }
}
private String consume() {
@@ -251,6 +259,13 @@
return line.get();
}
+ private AssertionError throwParsingException(Exception e, String message)
+ throws PlatformMappingException {
+ throw new PlatformMappingException(
+ new PlatformMappingParsingException(message, e),
+ SkyFunctionException.Transience.PERSISTENT);
+ }
+
private void throwParsingException(String message) throws PlatformMappingException {
throw new PlatformMappingException(
new PlatformMappingParsingException(message), SkyFunctionException.Transience.PERSISTENT);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionParserTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionParserTest.java
index 6d7a9fa..f4796f9 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionParserTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionParserTest.java
@@ -274,17 +274,16 @@
assertThat(exception).hasMessageThat().contains("platform:");
- PlatformMappingFunction.PlatformMappingException exception2 =
- assertThrows(
- PlatformMappingFunction.PlatformMappingException.class,
- () ->
- parse(
- "platforms:",
- " //platforms:one",
- " --cpu=one",
- "flag:",
- " --cpu=one",
- " //platforms:one"));
+ assertThrows(
+ PlatformMappingFunction.PlatformMappingException.class,
+ () ->
+ parse(
+ "platforms:",
+ " //platforms:one",
+ " --cpu=one",
+ "flag:",
+ " --cpu=one",
+ " //platforms:one"));
assertThat(exception).hasMessageThat().contains("platform");
}
@@ -347,6 +346,51 @@
assertThat(exception).hasMessageThat().contains("-cpu");
}
+ @Test
+ public void testParsePlatformsDuplicatePlatform() throws Exception {
+ PlatformMappingFunction.PlatformMappingException exception =
+ assertThrows(
+ PlatformMappingFunction.PlatformMappingException.class,
+ () ->
+ parse(
+ "platforms:", // Force line break
+ " //platforms:one", // Force line break
+ " --cpu=one", // Force line break
+ " //platforms:one", // Force line break
+ " --cpu=two"));
+
+ assertThat(exception).hasMessageThat().contains("duplicate");
+ assertThat(exception)
+ .hasCauseThat()
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains("//platforms:one");
+ }
+
+ @Test
+ public void testParseFlagsDuplicateFlags() throws Exception {
+ PlatformMappingFunction.PlatformMappingException exception =
+ assertThrows(
+ PlatformMappingFunction.PlatformMappingException.class,
+ () ->
+ parse(
+ "flags:", // Force line break
+ " --compilation_mode=dbg", // Force line break
+ " --cpu=one", // Force line break
+ " //platforms:one", // Force line break
+ " --compilation_mode=dbg", // Force line break
+ " --cpu=one", // Force line break
+ " //platforms:two"));
+
+ assertThat(exception).hasMessageThat().contains("duplicate");
+ assertThat(exception).hasCauseThat().hasCauseThat().hasMessageThat().contains("--cpu=one");
+ assertThat(exception)
+ .hasCauseThat()
+ .hasCauseThat()
+ .hasMessageThat()
+ .contains("--compilation_mode=dbg");
+ }
+
private PlatformMappingFunction.Mappings parse(String... lines)
throws PlatformMappingFunction.PlatformMappingException {
return new PlatformMappingFunction.Parser(ImmutableList.copyOf(lines).iterator()).parse();
