When creating a BuildConfigurationKey, check the target platform for …

…flags.

Fixes platform-based flags: #19409.

PiperOrigin-RevId: 641217049
Change-Id: I0a873eb708f8919a5bd5d62ca35dba5e51b80888

src/main/java/com/google/devtools/build/lib/analysis/producers/BUILD

@@ -101,8 +101,10 @@ java_library(
     name = "build_configuration_key_producer",
     srcs = ["BuildConfigurationKeyProducer.java"],
     deps = [
+        ":platform_flags_producer",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
+        "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/skyframe/config",
         "//src/main/java/com/google/devtools/build/lib/skyframe/config:exceptions",
         "//src/main/java/com/google/devtools/build/lib/skyframe/toolchains:platform_lookup_util",

src/main/java/com/google/devtools/build/lib/analysis/producers/BuildConfigurationKeyProducer.java

@@ -16,7 +16,9 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.devtools.build.lib.analysis.PlatformOptions;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.skyframe.config.BuildConfigurationKey;
+import com.google.devtools.build.lib.skyframe.config.NativeAndStarlarkFlags;
 import com.google.devtools.build.lib.skyframe.config.PlatformMappingException;
 import com.google.devtools.build.lib.skyframe.config.PlatformMappingValue;
 import com.google.devtools.build.lib.skyframe.toolchains.PlatformLookupUtil.InvalidPlatformException;
@@ -25,6 +27,9 @@
 import com.google.devtools.build.skyframe.state.StateMachine;
 import com.google.devtools.build.skyframe.state.StateMachine.ValueOrExceptionSink;
 import com.google.devtools.common.options.OptionsParsingException;
+import com.google.devtools.common.options.OptionsParsingResult;
+import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -37,7 +42,9 @@
  * <p>The output preserves the iteration order of the input.
  */
 public class BuildConfigurationKeyProducer
-    implements StateMachine, ValueOrExceptionSink<PlatformMappingException> {
+    implements StateMachine,
+        ValueOrExceptionSink<PlatformMappingException>,
+        PlatformFlagsProducer.ResultSink {
 
   /** Interface for clients to accept results of this computation. */
   public interface ResultSink {
@@ -61,6 +68,7 @@ void acceptTransitionedConfigurations(
   // There is only ever a single PlatformMappingValue in use, as the `--platform_mappings` flag
   // can not be changed in a transition.
   private PlatformMappingValue platformMappingValue;
+  private final Map<Label, NativeAndStarlarkFlags> platformFlags = new HashMap<>();
 
   public BuildConfigurationKeyProducer(
       ResultSink sink, StateMachine runAfter, Map<String, BuildOptions> options) {
@@ -71,6 +79,12 @@ public BuildConfigurationKeyProducer(
 
   @Override
   public StateMachine step(Tasks tasks) {
+    findPlatformMappings(tasks);
+    findTargetPlatformInfos(tasks);
+    return this::applyFlags;
+  }
+
+  private void findPlatformMappings(Tasks tasks) {
     // Use any configuration, since all configurations will have the same platform mapping.
     Optional<PathFragment> platformMappingsPath =
         options.values().stream()
@@ -81,9 +95,17 @@ public StateMachine step(Tasks tasks) {
     PlatformMappingValue.Key platformMappingValueKey =
         PlatformMappingValue.Key.create(platformMappingsPath.orElse(null));
     tasks.lookUp(platformMappingValueKey, PlatformMappingException.class, this);
-    return this::applyMappings;
   }
 
+  private void findTargetPlatformInfos(Tasks tasks) {
+    this.options.values().stream()
+        .filter(opts -> opts.contains(PlatformOptions.class))
+        .flatMap(opts -> opts.get(PlatformOptions.class).platforms.stream())
+        .map(targetPlatform -> new PlatformFlagsProducer(targetPlatform, this, StateMachine.DONE))
+        .forEach(tasks::enqueue);
+  }
+
+  // Handles results from the PlatformMappingValueKey lookup.
   @Override
   public void acceptValueOrException(
       @Nullable SkyValue value, @Nullable PlatformMappingException exception) {
@@ -99,7 +121,26 @@ public void acceptValueOrException(
     throw new IllegalStateException("No value or exception was provided");
   }
 
-  private StateMachine applyMappings(Tasks tasks) {
+  // Handle results from PlatformFlagsProducer.
+  @Override
+  public void acceptPlatformFlags(Label platform, NativeAndStarlarkFlags flags) {
+    BuildConfigurationKeyProducer.this.platformFlags.put(platform, flags);
+  }
+
+  @Override
+  public void acceptPlatformFlagsError(Label unusedPlatform, InvalidPlatformException error) {
+    // The requested platform is in the exception already, so it's fine to drop.
+    sink.acceptPlatformFlagsError(error);
+  }
+
+  @Override
+  public void acceptPlatformFlagsError(Label unusedPlatform, OptionsParsingException error) {
+    // TODO: blaze-configurability-team - See if this should include the requested platform in the
+    // error message.
+    sink.acceptTransitionError(error);
+  }
+
+  private StateMachine applyFlags(Tasks tasks) {
     if (this.platformMappingValue == null) {
       return DONE; // There was an error.
     }
@@ -108,17 +149,52 @@ private StateMachine applyMappings(Tasks tasks) {
         ImmutableMap.<String, BuildConfigurationKey>builderWithExpectedSize(options.size());
     for (Map.Entry<String, BuildOptions> entry : options.entrySet()) {
       String transitionKey = entry.getKey();
-      BuildConfigurationKey newConfigurationKey;
       try {
-        BuildOptions mappedOptions = this.platformMappingValue.map(entry.getValue());
-        newConfigurationKey = BuildConfigurationKey.create(mappedOptions);
+        BuildConfigurationKey newConfigurationKey = applyFlagsForOptions(entry.getValue());
+        result.put(transitionKey, newConfigurationKey);
       } catch (OptionsParsingException e) {
         sink.acceptTransitionError(e);
         return runAfter;
       }
-      result.put(transitionKey, newConfigurationKey);
     }
     sink.acceptTransitionedConfigurations(result.buildOrThrow());
     return runAfter;
   }
+
+  /**
+   * Apply discovered flags from platforms or platform mappings to the given options, and return the
+   * {@link BuildConfigurationKey}.
+   *
+   * <p>Platform-based flags and platform mappings are mutually exclusive: only one will be applied
+   * if they are present. Trying to mix and match would be possible but confusing, especially if
+   * they try to change the same flag.
+   */
+  private BuildConfigurationKey applyFlagsForOptions(BuildOptions options)
+      throws OptionsParsingException {
+    // Does the target platform provide any flags?
+    if (options.contains(PlatformOptions.class)) {
+      List<Label> targetPlatforms = options.get(PlatformOptions.class).platforms;
+      if (targetPlatforms != null && targetPlatforms.size() == 1) {
+        // TODO: https://github.com/bazelbuild/bazel/issues/19807 - We define this flag to only use
+        // the first value and ignore any subsequent ones. Remove this check as part of cleanup.
+        Label targetPlatform = targetPlatforms.get(0);
+
+        if (this.platformFlags.containsKey(targetPlatform)) {
+          NativeAndStarlarkFlags platformBasedFlags = this.platformFlags.get(targetPlatform);
+          OptionsParsingResult parsingResult = platformBasedFlags.parse();
+          BuildOptions updatedOptions = options.applyParsingResult(parsingResult);
+          return BuildConfigurationKey.create(updatedOptions);
+        }
+      }
+    }
+
+    // Is there a platform mapping?
+    if (this.platformMappingValue != null) {
+      BuildOptions mappedOptions = this.platformMappingValue.map(options);
+      return BuildConfigurationKey.create(mappedOptions);
+    }
+
+    // Just use the original options.
+    return BuildConfigurationKey.create(options);
+  }
 }