From ad874816a8d323c71fae3a970a6826d3691628a7 Mon Sep 17 00:00:00 2001
From: schmitt <schmitt@google.com>
Date: Fri, 29 Mar 2019 12:40:38 -0700
Subject: [PATCH] Remove explicitly set target platform value and replace it by
 fallback.

The original value for the target platform was set at the dawn of time when we didn't have auto-calculation for it yet. Now that we do the explicit setting interferes with platform mapping which assumes that the target platform value is empty unless the user explicitly requests otherwise.

This change introduces a new flag, --target_platform_fallsback which can be used to set the fallback in case there is no matching mapping. Because it uses the old defaults no behavioral change should be observed as long as no mappings exist.

Step 5/N towards the platforms mapping functionality for https://github.com/bazelbuild/bazel/issues/6426

RELNOTES: None.
PiperOrigin-RevId: 241031610
---
 .../build/lib/analysis/PlatformOptions.java   | 19 ++++++++++++++++---
 .../skyframe/PlatformMappingFunctionTest.java |  5 +++--
 .../skyframe/PlatformMappingValueTest.java    |  7 ++++---
 3 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
index 89d0b9056ad009..926f69b08982a4 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java
@@ -38,8 +38,6 @@ public class PlatformOptions extends FragmentOptions {
       Label.parseAbsoluteUnchecked("@bazel_tools//platforms:host_platform");
   public static final Label DEFAULT_HOST_PLATFORM =
       Label.parseAbsoluteUnchecked("@local_config_platform//:host");
-  public static final Label LEGACY_DEFAULT_TARGET_PLATFORM =
-      Label.parseAbsoluteUnchecked("@bazel_tools//platforms:target_platform");
 
   /**
    * Main workspace-relative location to use when the user does not explicitly set {@code
@@ -92,6 +90,21 @@ public class PlatformOptions extends FragmentOptions {
               + "command.")
   public List<Label> platforms;
 
+  @Option(
+      name = "target_platform_fallback",
+      converter = BuildConfiguration.EmptyToNullLabelConverter.class,
+      defaultValue = "@bazel_tools//platforms:target_platform",
+      documentationCategory = OptionDocumentationCategory.TOOLCHAIN,
+      effectTags = {
+        OptionEffectTag.AFFECTS_OUTPUTS,
+        OptionEffectTag.CHANGES_INPUTS,
+        OptionEffectTag.LOADING_AND_ANALYSIS
+      },
+      help =
+          "The label of a platform rule that should be used if no target platform is set and no"
+              + " platform mapping matches the current set of flags.")
+  public Label targetPlatformFallback;
+
   @Option(
       name = "extra_toolchains",
       defaultValue = "",
@@ -229,7 +242,7 @@ public Label computeTargetPlatform() {
       return computeHostPlatform();
     } else {
       // Use the legacy target platform
-      return LEGACY_DEFAULT_TARGET_PLATFORM;
+      return targetPlatformFallback;
     }
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java
index 194fa03a2a0345..5a0867e2733b55 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java
@@ -15,7 +15,6 @@
 package com.google.devtools.build.lib.skyframe;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.analysis.PlatformOptions.LEGACY_DEFAULT_TARGET_PLATFORM;
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.base.Optional;
@@ -64,6 +63,8 @@ public class PlatformMappingFunctionTest extends BuildViewTestCase {
   private static final BuildOptions.OptionsDiffForReconstruction EMPTY_DIFF =
       BuildOptions.diffForReconstruction(
           DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
+  private static final Label DEFAULT_TARGET_PLATFORM =
+      Label.parseAbsoluteUnchecked("@bazel_tools//platforms:target_platform");
 
   @Test
   public void testMappingFileDoesNotExist() throws Exception {
@@ -88,7 +89,7 @@ public void testMappingFileDoesNotExistDefaultLocation() throws Exception {
         platformMappingValue.map(key, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
 
     assertThat(toMappedOptions(mapped).get(PlatformOptions.class).platforms)
-        .containsExactly(LEGACY_DEFAULT_TARGET_PLATFORM);
+        .containsExactly(DEFAULT_TARGET_PLATFORM);
   }
 
   @Test
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java
index 88cdf7be4093e1..af8c114bd61793 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java
@@ -14,7 +14,6 @@
 package com.google.devtools.build.lib.skyframe;
 
 import static com.google.common.truth.Truth.assertThat;
-import static com.google.devtools.build.lib.analysis.PlatformOptions.LEGACY_DEFAULT_TARGET_PLATFORM;
 import static com.google.devtools.build.lib.testutil.MoreAsserts.assertThrows;
 
 import com.google.common.collect.ImmutableList;
@@ -57,6 +56,8 @@ public class PlatformMappingValueTest {
   private static final BuildOptions.OptionsDiffForReconstruction EMPTY_DIFF =
       BuildOptions.diffForReconstruction(
           DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
+  private static final Label DEFAULT_TARGET_PLATFORM =
+      Label.parseAbsoluteUnchecked("@bazel_tools//platforms:target_platform");
 
   @Test
   public void testMapNoMappings() throws OptionsParsingException {
@@ -70,7 +71,7 @@ public void testMapNoMappings() throws OptionsParsingException {
         mappingValue.map(key, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
 
     assertThat(toMappedOptions(mapped).get(PlatformOptions.class).platforms)
-        .containsExactly(LEGACY_DEFAULT_TARGET_PLATFORM);
+        .containsExactly(DEFAULT_TARGET_PLATFORM);
   }
 
   @Test
@@ -148,7 +149,7 @@ public void testMapFlagsToPlatformNoneMatching() throws Exception {
         mappingValue.map(keyForOptions(modifiedOptions), DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
 
     assertThat(toMappedOptions(mapped).get(PlatformOptions.class).platforms)
-        .containsExactly(LEGACY_DEFAULT_TARGET_PLATFORM);
+        .containsExactly(DEFAULT_TARGET_PLATFORM);
   }
 
   @Test