From 8396c2f6cde8c15913b5e9deed89b8d7a45dbb15 Mon Sep 17 00:00:00 2001 From: schmitt Date: Mon, 22 Apr 2019 11:20:43 -0700 Subject: [PATCH] Reset platform on cpu transitions. With platform mappings about to be enabled we need to make sure that platforms are reset in any configuration transitions that write flags which may impact the platform. Otherwise a platform may be set before the transition that matches the old flags but platform mapping would not be triggered for the new flag values (because platform mapping only happens if no platform is set). Step 6/N towards the platforms mapping functionality for https://github.com/bazelbuild/bazel/issues/6426 RELNOTES: None. PiperOrigin-RevId: 244696779 --- .../lib/rules/android/AndroidRuleClasses.java | 41 ++++++++++--------- .../rules/objc/AppleCrosstoolTransition.java | 4 ++ 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java index fc4f6178b02499..d3cf348a992940 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidRuleClasses.java @@ -30,6 +30,7 @@ import com.google.common.collect.ImmutableSortedSet; import com.google.common.collect.Lists; import com.google.devtools.build.lib.analysis.BaseRuleClasses; +import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; @@ -212,22 +213,19 @@ public static LabelLateBoundDefault getAndroidSdkLabel(Label androidSdk) { /** Android Split configuration transition for properly handling native dependencies */ public static final class AndroidSplitTransition implements SplitTransition, AndroidSplitTransititionApi { - private static void setCrosstoolToAndroid(BuildOptions output, BuildOptions input) { - AndroidConfiguration.Options inputAndroidOptions = - input.get(AndroidConfiguration.Options.class); - AndroidConfiguration.Options outputAndroidOptions = - output.get(AndroidConfiguration.Options.class); - - CppOptions cppOptions = output.get(CppOptions.class); - if (inputAndroidOptions.androidCrosstoolTop != null - && !cppOptions.crosstoolTop.equals(inputAndroidOptions.androidCrosstoolTop)) { + private static void setCrosstoolToAndroid(BuildOptions options) { + AndroidConfiguration.Options androidOptions = options.get(AndroidConfiguration.Options.class); + + CppOptions cppOptions = options.get(CppOptions.class); + if (androidOptions.androidCrosstoolTop != null + && !cppOptions.crosstoolTop.equals(androidOptions.androidCrosstoolTop)) { if (cppOptions.hostCrosstoolTop == null) { cppOptions.hostCrosstoolTop = cppOptions.crosstoolTop; } - cppOptions.crosstoolTop = inputAndroidOptions.androidCrosstoolTop; + cppOptions.crosstoolTop = androidOptions.androidCrosstoolTop; } - outputAndroidOptions.configurationDistinguisher = ConfigurationDistinguisher.ANDROID; + androidOptions.configurationDistinguisher = ConfigurationDistinguisher.ANDROID; } @Override @@ -248,11 +246,8 @@ public List split(BuildOptions buildOptions) { } else { BuildOptions splitOptions = buildOptions.clone(); - splitOptions.get(CppOptions.class).cppCompiler = androidOptions.cppCompiler; - splitOptions.get(CppOptions.class).libcTopLabel = androidOptions.androidLibcTopLabel; splitOptions.get(BuildConfiguration.Options.class).cpu = androidOptions.cpu; - splitOptions.get(CppOptions.class).dynamicMode = androidOptions.dynamicMode; - setCrosstoolToAndroid(splitOptions, buildOptions); + setCommonAndroidOptions(androidOptions, splitOptions); return ImmutableList.of(splitOptions); } @@ -268,16 +263,24 @@ public List split(BuildOptions buildOptions) { // TODO(bazel-team): --android_cpu doesn't follow --cpu right now; it should. splitOptions.get(AndroidConfiguration.Options.class).cpu = cpu; splitOptions.get(BuildConfiguration.Options.class).cpu = cpu; - splitOptions.get(CppOptions.class).cppCompiler = androidOptions.cppCompiler; - splitOptions.get(CppOptions.class).libcTopLabel = androidOptions.androidLibcTopLabel; - splitOptions.get(CppOptions.class).dynamicMode = androidOptions.dynamicMode; - setCrosstoolToAndroid(splitOptions, buildOptions); + setCommonAndroidOptions(androidOptions, splitOptions); result.add(splitOptions); } return result.build(); } } + private void setCommonAndroidOptions( + AndroidConfiguration.Options androidOptions, BuildOptions newOptions) { + newOptions.get(CppOptions.class).cppCompiler = androidOptions.cppCompiler; + newOptions.get(CppOptions.class).libcTopLabel = androidOptions.androidLibcTopLabel; + newOptions.get(CppOptions.class).dynamicMode = androidOptions.dynamicMode; + setCrosstoolToAndroid(newOptions); + + // Ensure platforms aren't set so that platform mapping can take place. + newOptions.get(PlatformOptions.class).platforms = ImmutableList.of(); + } + @Override public boolean isImmutable() { return true; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java index 938bc61eba545e..1d837432f596cc 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java @@ -16,6 +16,7 @@ import com.google.common.base.Strings; import com.google.common.collect.ImmutableList; +import com.google.devtools.build.lib.analysis.PlatformOptions; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.BuildOptions; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; @@ -96,6 +97,9 @@ public static void setAppleCrosstoolTransitionConfiguration(BuildOptions from, // OSX toolchains do not support fission. to.get(CppOptions.class).fissionModes = ImmutableList.of(); + + // Ensure platforms aren't set so that platform mapping can take place. + to.get(PlatformOptions.class).platforms = ImmutableList.of(); } /**