Normalize platform related options.

- extra_toolchains: order matters but multiplicity does not. So keep only the first copy of each value.
- platforms: only the first value matters

See 3a8da92 for why we should normalize.

RELNOTES: None.
PiperOrigin-RevId: 505201389
Change-Id: Ieb55cc085d01fb922c78a5863e8e7d305f94f738

src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java

@@ -242,6 +242,18 @@ public PlatformOptions getExec() {
     return exec;
   }
 
+  @Override
+  public PlatformOptions getNormalized() {
+    PlatformOptions result = (PlatformOptions) clone();
+    result.extraToolchains = dedupeOnly(result.extraToolchains);
+    // Only the first entry of platforms is used (it should have been Label and not List<Label>)
+    // So drop all but the first entry.
+    if (result.platforms.size() > 1) {
+      result.platforms = ImmutableList.of(result.platforms.get(0));
+    }
+    return result;
+  }
+
   /** Returns the intended target platform value based on options defined in this fragment. */
   public Label computeTargetPlatform() {
     // Handle default values for the host and target platform.

src/test/java/com/google/devtools/build/lib/analysis/PlatformOptionsTest.java

@@ -0,0 +1,80 @@
+// Copyright 2015 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.analysis;
+
+import com.google.devtools.build.lib.analysis.util.OptionsTestCase;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public final class PlatformOptionsTest extends OptionsTestCase<PlatformOptions> {
+
+  private static final String EXTRA_PLATFORMS_PREFIX = "--extra_execution_platforms=";
+  private static final String EXTRA_TOOLCHAINS_PREFIX = "--extra_toolchains=";
+  private static final String PLATFORMS_PREFIX = "--platforms=";
+
+  @Override
+  protected Class<PlatformOptions> getOptionsClass() {
+    return PlatformOptions.class;
+  }
+
+  @Test
+  public void testExtraPlatforms_orderMatters() throws Exception {
+    // It seems that the platforms are considered in order. Picking the wrong one results in broken
+    // builds. So add test asserting that order matters.
+    PlatformOptions one = createWithPrefix(EXTRA_PLATFORMS_PREFIX, "platform1", "platform2");
+    PlatformOptions two = createWithPrefix(EXTRA_PLATFORMS_PREFIX, "platform2", "platform1");
+    assertDifferent(one, two);
+  }
+
+  @Test
+  public void testExtraToolchains_ordering() throws Exception {
+    // The ordering matters for tool chains, since we pick the first available toolchain.
+    PlatformOptions one = createWithPrefix(EXTRA_TOOLCHAINS_PREFIX, "one", "two");
+    PlatformOptions two = createWithPrefix(EXTRA_TOOLCHAINS_PREFIX, "two", "one");
+    assertDifferent(one, two);
+  }
+
+  @Test
+  public void testExtraToolchains_duplicates() throws Exception {
+    // Specifying the same tool chain multiple times is a no-op.
+    PlatformOptions one = createWithPrefix(EXTRA_TOOLCHAINS_PREFIX, "one", "one");
+    PlatformOptions two = createWithPrefix(EXTRA_TOOLCHAINS_PREFIX, "one");
+    assertSame(one, two);
+  }
+
+  @Test
+  public void testPlatforms_duplicates() throws Exception {
+    PlatformOptions one = createWithPrefix(PLATFORMS_PREFIX, "//p:one,//p:one");
+    PlatformOptions two = createWithPrefix(PLATFORMS_PREFIX, "//p:one");
+    assertSame(one, two);
+  }
+
+  @Test
+  public void testPlatforms_extraValues() throws Exception {
+    // Only the first value matters.
+    PlatformOptions one = createWithPrefix(PLATFORMS_PREFIX, "//one,//two");
+    PlatformOptions two = createWithPrefix(PLATFORMS_PREFIX, "//one");
+    assertSame(one, two);
+  }
+
+  @Test
+  public void testPlatforms_orderMatters() throws Exception {
+    // Changing the order changes the semantics.
+    PlatformOptions foo = createWithPrefix(PLATFORMS_PREFIX, "//one,//two");
+    PlatformOptions bar = createWithPrefix(PLATFORMS_PREFIX, "//two,//one");
+    assertDifferent(foo, bar);
+  }
+}