diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java index 4d463944612614..bb3b0b294ad913 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/FunctionTransitionUtil.java @@ -77,7 +77,7 @@ static Map applyAndValidate( StructImpl attrObject, EventHandler eventHandler) throws EvalException, InterruptedException { - checkForBlacklistedOptions(starlarkTransition); + checkForDenylistedOptions(starlarkTransition); // TODO(waltl): consider building this once and use it across different split // transitions. @@ -91,14 +91,48 @@ static Map applyAndValidate( validateFunctionOutputsMatchesDeclaredOutputs(transitions.values(), starlarkTransition); for (Map.Entry> entry : transitions.entrySet()) { + Map newValues = handleImplicitPlatformChange(entry.getValue()); BuildOptions transitionedOptions = - applyTransition(buildOptions, entry.getValue(), optionInfoMap, starlarkTransition); + applyTransition(buildOptions, newValues, optionInfoMap, starlarkTransition); splitBuildOptions.put(entry.getKey(), transitionedOptions); } return splitBuildOptions.build(); } - private static void checkForBlacklistedOptions(StarlarkDefinedConfigTransition transition) + /** + * If the transition changes --cpu but not --platforms, clear out --platforms. + * + *

Purpose: + * + *

    + *
  1. A platform mapping sets --cpu=foo when --platforms=foo. + *
  2. A transition sets --cpu=bar. + *
  3. Because --platforms=foo, the platform mapping kicks in to set --cpu back to foo. + *
  4. Result: the mapping accidentally overrides the transition + *
+ * + *

Transitions can alo explicitly set --platforms to be clear what platform they set. + * + *

Platform mappings: + * https://docs.bazel.build/versions/master/platforms-intro.html#platform-mappings. + * + *

This doesn't check that the changed value is actually different than the source (i.e. + * setting {@code --cpu=foo} when {@code --cpu} is already {@code foo}). That could unnecessarily + * fork configurations that are really the same. That's a possible optimization TODO. + */ + private static Map handleImplicitPlatformChange( + Map originalOutput) { + boolean changesCpu = originalOutput.containsKey(COMMAND_LINE_OPTION_PREFIX + "cpu"); + boolean changesPlatforms = originalOutput.containsKey(COMMAND_LINE_OPTION_PREFIX + "platforms"); + return changesCpu && !changesPlatforms + ? ImmutableMap.builder() + .putAll(originalOutput) + .put(COMMAND_LINE_OPTION_PREFIX + "platforms", ImmutableList.