Skip to content

Commit 2f87d62

Browse files
author
Luca Di Grazia
committed
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 bazelbuild/bazel#6426 RELNOTES: None. PiperOrigin-RevId: 241031610
1 parent 2c074c3 commit 2f87d62

File tree

3 files changed

+18
-17
lines changed

3 files changed

+18
-17
lines changed

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/analysis/PlatformOptions.java

+1
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@ public class PlatformOptions extends FragmentOptions {
183183

184184
@Option(
185185
name = "incompatible_use_toolchain_resolution_for_java_rules",
186+
oldName = "experimental_use_toolchain_resolution_for_java_rules",
186187
defaultValue = "false",
187188
documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
188189
effectTags = OptionEffectTag.UNKNOWN,

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingFunctionTest.java

+5-5
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import com.google.devtools.build.lib.analysis.PlatformOptions;
2626
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
2727
import com.google.devtools.build.lib.analysis.config.BuildOptions;
28-
import com.google.devtools.build.lib.analysis.config.CoreOptions;
2928
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
3029
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
3130
import com.google.devtools.build.lib.cmdline.Label;
@@ -54,7 +53,8 @@ public class PlatformMappingFunctionTest extends BuildViewTestCase {
5453
ImmutableSet.of(PlatformConfiguration.class);
5554

5655
private static final ImmutableList<Class<? extends FragmentOptions>>
57-
BUILD_CONFIG_PLATFORM_OPTIONS = ImmutableList.of(CoreOptions.class, PlatformOptions.class);
56+
BUILD_CONFIG_PLATFORM_OPTIONS =
57+
ImmutableList.of(BuildConfiguration.Options.class, PlatformOptions.class);
5858

5959
private static final Label PLATFORM1 = Label.parseAbsoluteUnchecked("//platforms:one");
6060

@@ -83,7 +83,7 @@ public void testMappingFileDoesNotExistDefaultLocation() throws Exception {
8383
executeFunction(PlatformMappingValue.Key.create(null));
8484

8585
BuildConfigurationValue.Key key =
86-
BuildConfigurationValue.keyWithoutPlatformMapping(PLATFORM_FRAGMENT_CLASS, EMPTY_DIFF);
86+
BuildConfigurationValue.key(PLATFORM_FRAGMENT_CLASS, EMPTY_DIFF);
8787

8888
BuildConfigurationValue.Key mapped =
8989
platformMappingValue.map(key, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
@@ -121,7 +121,7 @@ public void testMappingFileIsRead() throws Exception {
121121
platformMappingValue.map(
122122
keyForOptions(modifiedOptions), DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
123123

124-
assertThat(toMappedOptions(mapped).get(CoreOptions.class).cpu).isEqualTo("one");
124+
assertThat(toMappedOptions(mapped).get(BuildConfiguration.Options.class).cpu).isEqualTo("one");
125125
}
126126

127127
private PlatformMappingValue executeFunction(PlatformMappingValue.Key key) throws Exception {
@@ -155,6 +155,6 @@ private BuildConfigurationValue.Key keyForOptions(BuildOptions modifiedOptions)
155155
BuildOptions.OptionsDiffForReconstruction diff =
156156
BuildOptions.diffForReconstruction(DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS, modifiedOptions);
157157

158-
return BuildConfigurationValue.keyWithoutPlatformMapping(PLATFORM_FRAGMENT_CLASS, diff);
158+
return BuildConfigurationValue.key(PLATFORM_FRAGMENT_CLASS, diff);
159159
}
160160
}

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/skyframe/PlatformMappingValueTest.java

+12-12
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
2525
import com.google.devtools.build.lib.analysis.config.BuildOptions;
2626
import com.google.devtools.build.lib.analysis.config.CompilationMode;
27-
import com.google.devtools.build.lib.analysis.config.CoreOptions;
2827
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
2928
import com.google.devtools.build.lib.cmdline.Label;
3029
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -46,7 +45,8 @@ public class PlatformMappingValueTest {
4645
ImmutableSet.of(PlatformConfiguration.class);
4746

4847
private static final ImmutableList<Class<? extends FragmentOptions>>
49-
BUILD_CONFIG_PLATFORM_OPTIONS = ImmutableList.of(CoreOptions.class, PlatformOptions.class);
48+
BUILD_CONFIG_PLATFORM_OPTIONS =
49+
ImmutableList.of(BuildConfiguration.Options.class, PlatformOptions.class);
5050

5151
private static final Label PLATFORM1 = Label.parseAbsoluteUnchecked("//platforms:one");
5252
private static final Label PLATFORM2 = Label.parseAbsoluteUnchecked("//platforms:two");
@@ -65,7 +65,7 @@ public void testMapNoMappings() throws OptionsParsingException {
6565
new PlatformMappingValue(ImmutableMap.of(), ImmutableMap.of());
6666

6767
BuildConfigurationValue.Key key =
68-
BuildConfigurationValue.keyWithoutPlatformMapping(PLATFORM_FRAGMENT_CLASS, EMPTY_DIFF);
68+
BuildConfigurationValue.key(PLATFORM_FRAGMENT_CLASS, EMPTY_DIFF);
6969

7070
BuildConfigurationValue.Key mapped =
7171
mappingValue.map(key, DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
@@ -90,7 +90,7 @@ public void testMapPlatformToFlags() throws Exception {
9090

9191
assertThat(mapped.getFragments()).isEqualTo(PLATFORM_FRAGMENT_CLASS);
9292

93-
assertThat(toMappedOptions(mapped).get(CoreOptions.class).cpu).isEqualTo("one");
93+
assertThat(toMappedOptions(mapped).get(BuildConfiguration.Options.class).cpu).isEqualTo("one");
9494
}
9595

9696
@Test
@@ -102,8 +102,8 @@ public void testMapFlagsToPlatform() throws Exception {
102102
new PlatformMappingValue(ImmutableMap.of(), flagsToPlatforms);
103103

104104
BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
105-
modifiedOptions.get(CoreOptions.class).cpu = "one";
106-
modifiedOptions.get(CoreOptions.class).compilationMode = CompilationMode.DBG;
105+
modifiedOptions.get(BuildConfiguration.Options.class).cpu = "one";
106+
modifiedOptions.get(BuildConfiguration.Options.class).compilationMode = CompilationMode.DBG;
107107

108108
BuildConfigurationValue.Key mapped =
109109
mappingValue.map(keyForOptions(modifiedOptions), DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
@@ -125,7 +125,7 @@ public void testMapFlagsToPlatformPriority() throws Exception {
125125
new PlatformMappingValue(ImmutableMap.of(), flagsToPlatforms);
126126

127127
BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
128-
modifiedOptions.get(CoreOptions.class).cpu = "foo";
128+
modifiedOptions.get(BuildConfiguration.Options.class).cpu = "foo";
129129

130130
BuildConfigurationValue.Key mapped =
131131
mappingValue.map(keyForOptions(modifiedOptions), DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
@@ -143,7 +143,7 @@ public void testMapFlagsToPlatformNoneMatching() throws Exception {
143143
new PlatformMappingValue(ImmutableMap.of(), flagsToPlatforms);
144144

145145
BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
146-
modifiedOptions.get(CoreOptions.class).cpu = "bar";
146+
modifiedOptions.get(BuildConfiguration.Options.class).cpu = "bar";
147147

148148
BuildConfigurationValue.Key mapped =
149149
mappingValue.map(keyForOptions(modifiedOptions), DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS);
@@ -160,7 +160,7 @@ public void testMapNoPlatformOptions() throws Exception {
160160
PlatformMappingValue mappingValue =
161161
new PlatformMappingValue(ImmutableMap.of(), flagsToPlatforms);
162162

163-
BuildOptions options = BuildOptions.of(ImmutableList.of(CoreOptions.class));
163+
BuildOptions options = BuildOptions.of(ImmutableList.of(BuildConfiguration.Options.class));
164164

165165
assertThrows(
166166
IllegalArgumentException.class,
@@ -175,7 +175,7 @@ public void testMapNoMappingIfPlatformIsSetButNotMatching() throws Exception {
175175
ImmutableMap.of(ImmutableList.of("--cpu=one"), PLATFORM1);
176176

177177
BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
178-
modifiedOptions.get(CoreOptions.class).cpu = "one";
178+
modifiedOptions.get(BuildConfiguration.Options.class).cpu = "one";
179179
modifiedOptions.get(PlatformOptions.class).platforms = ImmutableList.of(PLATFORM2);
180180

181181
PlatformMappingValue mappingValue =
@@ -193,7 +193,7 @@ public void testMapNoMappingIfPlatformIsSetAndNoPlatformMapping() throws Excepti
193193
ImmutableMap.of(ImmutableList.of("--cpu=one"), PLATFORM1);
194194

195195
BuildOptions modifiedOptions = DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS.clone();
196-
modifiedOptions.get(CoreOptions.class).cpu = "one";
196+
modifiedOptions.get(BuildConfiguration.Options.class).cpu = "one";
197197
modifiedOptions.get(PlatformOptions.class).platforms = ImmutableList.of(PLATFORM2);
198198

199199
PlatformMappingValue mappingValue =
@@ -238,6 +238,6 @@ private BuildConfigurationValue.Key keyForOptions(BuildOptions modifiedOptions)
238238
BuildOptions.OptionsDiffForReconstruction diff =
239239
BuildOptions.diffForReconstruction(DEFAULT_BUILD_CONFIG_PLATFORM_OPTIONS, modifiedOptions);
240240

241-
return BuildConfigurationValue.keyWithoutPlatformMapping(PLATFORM_FRAGMENT_CLASS, diff);
241+
return BuildConfigurationValue.key(PLATFORM_FRAGMENT_CLASS, diff);
242242
}
243243
}

0 commit comments

Comments
 (0)