From 5b4de12b37cb4ba921b6bdfe810bbb47921ab3a6 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 10 Jan 2023 09:52:36 -0800 Subject: [PATCH] Do not clear `--platforms` on no-op change to `--cpu` Resolves a TODO in the code to not unnecessarily fork the configuration when a transition has `--cpu` as a declared output but doesn't change it. Closes #17158. PiperOrigin-RevId: 501021431 Change-Id: Ib942ea9584b2ceb3083b5fec71e2e11b89dcfdb6 --- .../starlark/FunctionTransitionUtil.java | 30 +++++------ .../StarlarkAttrTransitionProviderTest.java | 50 +++++++++++++++++++ 2 files changed, 66 insertions(+), 14 deletions(-) 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 c6903b01cc68d0..db271bf339065f 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 @@ -99,7 +99,8 @@ static ImmutableMap applyAndValidate( } for (Map.Entry> entry : transitions.entrySet()) { - Map newValues = handleImplicitPlatformChange(entry.getValue()); + Map newValues = + handleImplicitPlatformChange(buildOptions, entry.getValue()); BuildOptions transitionedOptions = applyTransition(buildOptions, newValues, optionInfoMap, starlarkTransition); splitBuildOptions.put(entry.getKey(), transitionedOptions); @@ -127,21 +128,22 @@ static ImmutableMap applyAndValidate( *

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

Platform mappings: https://bazel.build/concepts/platforms-intro#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.