Skip to content

Commit

Permalink
Flipping the flag --incompatible_use_platforms_repo_for_constraints
Browse files Browse the repository at this point in the history
Closes: #8622
PiperOrigin-RevId: 466348165
Change-Id: I26caec587d2d7dd8dde445ac1ed58f315b47c96f
  • Loading branch information
Googler authored and copybara-github committed Aug 9, 2022
1 parent c12dd9b commit 3469784
Show file tree
Hide file tree
Showing 13 changed files with 135 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,9 @@ public class CoreOptions extends FragmentOptions implements Cloneable {
"If true, the target platform is used in the output directory name instead of the CPU.")
public boolean platformInOutputDir;

// TODO(b/231200175): @aranguyen flip this flag
@Option(
name = "incompatible_use_platforms_repo_for_constraints",
defaultValue = "false",
defaultValue = "true",
documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2410,7 +2410,8 @@ public void testImplicitPlatformsChange() throws Exception {
// --platforms to PlatformOptions.computeTargetPlatform(), which defaults to the host.
assertThat(getConfiguration(dep).getOptions().get(PlatformOptions.class).platforms)
.containsExactly(
Label.parseAbsoluteUnchecked(TestConstants.PLATFORM_PACKAGE_ROOT + ":default_host"));
Label.parseAbsoluteUnchecked(
TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host"));
}

@Test
Expand Down Expand Up @@ -2465,7 +2466,7 @@ public void testNoPlatformChange() throws Exception {
scratch.file(
"platforms/BUILD",
"platform(name = 'my_platform',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + ":default_host'],",
" parents = ['" + TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host'],",
" constraint_values = [],",
")");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1654,7 +1654,8 @@ public void testImplicitPlatformsChange() throws Exception {
.get(PlatformOptions.class)
.platforms)
.containsExactly(
Label.parseAbsoluteUnchecked(TestConstants.PLATFORM_PACKAGE_ROOT + ":default_host"));
Label.parseAbsoluteUnchecked(
TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,10 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
/** The rest of platforms is initialized in {@link MockPlatformSupport}. */
config.create("platforms_workspace/WORKSPACE", "workspace(name = 'platforms')");
config.create("platforms_workspace/MODULE.bazel", "module(name = 'platforms')");
config.create(
"local_config_platform_workspace/WORKSPACE", "workspace(name = 'local_config_platform')");
config.create(
"local_config_platform_workspace/MODULE.bazel", "module(name = 'local_config_platform')");
config.create("embedded_tools/WORKSPACE", "workspace(name = 'bazel_tools')");
Runfiles runfiles = Runfiles.create();
for (String filename : Arrays.asList("tools/jdk/java_toolchain_alias.bzl")) {
Expand Down Expand Up @@ -216,14 +220,12 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
")",
"java_plugins_flag_alias(name = 'java_plugins_flag_alias')");

// Create a default Android target platform.
// Any tests that create platforms should inherit from this.
config.create(
TestConstants.PLATFORMS_PATH + "/android/BUILD",
TestConstants.CONSTRAINTS_PATH + "/android/BUILD",
"package(default_visibility=['//visibility:public'])",
"platform(",
" name = 'armeabi-v7a',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + ":default_target'],",
" parents = ['" + TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host'],",
" constraint_values = [",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:android',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:armv7',",
Expand All @@ -241,8 +243,37 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
"embedded_tools/tools/android/emulator/BUILD",
Iterables.toArray(createToolsAndroidEmulatorContents(), String.class));
// Create a dummy toolchain to make toolchain resolution happy.
config.create(
"embedded_tools/tools/android/dummy_sdk/dummy-cc-toolchain-config.bzl",
"def _impl(ctx):",
" out = ctx.actions.declare_file(ctx.label.name)",
" ctx.actions.write(out, 'Fake executable')",
" return [",
" cc_common.create_cc_toolchain_config_info(",
" ctx = ctx,",
" toolchain_identifier = 'dummy-toolchain',",
" host_system_name = 'nothing',",
" target_system_name = 'nothing',",
" target_cpu = 'nothing',",
" target_libc = 'nothing',",
" cc_target_os = 'nothing',",
" compiler = 'bin-false',",
" abi_version = 'nothing',",
" abi_libc_version = 'eleventy',",
" ),",
" DefaultInfo(",
" executable = out,",
" ),",
" ]",
"dummy_cc_toolchain_config = rule(",
" implementation = _impl,",
" attrs = {},",
" provides = [CcToolchainConfigInfo],",
" executable = True,",
")");
config.create(
"embedded_tools/tools/android/dummy_sdk/BUILD",
"load(':dummy-cc-toolchain-config.bzl'," + " 'dummy_cc_toolchain_config')",
"package(default_visibility=['//visibility:public'])",
"toolchain(",
" name = 'dummy-sdk',",
Expand Down Expand Up @@ -276,6 +307,25 @@ public void setupMockClient(MockToolsConfig config, List<String> workspaceConten
" shrinked_android_jar = 'dummy.jar',",
" zipalign = ':empty_binary',",
" tags = ['__ANDROID_RULES_MIGRATION__'],",
")",
"toolchain(",
" name = 'dummy-cc_toolchain',",
" toolchain = ':dummy_cc_toolchain_impl',",
" toolchain_type = '@bazel_tools//tools/cpp:toolchain_type',",
")",
"cc_toolchain(",
" name = 'dummy_cc_toolchain_impl',",
" all_files = ':nothing',",
" as_files = ':nothing',",
" compiler_files = ':nothing',",
" dwp_files = ':nothing',",
" linker_files = ':nothing',",
" objcopy_files = ':nothing',",
" strip_files = ':nothing',",
" toolchain_config = ':dummy-cc-toolchain-config',",
")",
"dummy_cc_toolchain_config(",
" name = 'dummy-cc-toolchain-config',",
")");
config.create(
"android_gmaven_r8/jar/BUILD",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@ public void setup() throws Exception {
mockToolsConfig,
CcToolchainConfig.builder()
.withToolchainTargetConstraints("@platforms//os:linux", "@platforms//cpu:x86_64"));
BazelMockCcSupport.INSTANCE.setupCcToolchainConfig(
mockToolsConfig,
CcToolchainConfig.builder()
.withToolchainTargetConstraints("@platforms//os:android", "@platforms//cpu:armv7"));
scratch.overwriteFile("embedded_tools/tools/build_defs/cc/BUILD");
scratch.overwriteFile(
"embedded_tools/tools/build_defs/cc/action_names.bzl",
Expand All @@ -88,14 +92,19 @@ public void setup() throws Exception {
// Set up a fake @platforms repository.
scratch.appendFile(
"WORKSPACE", "local_repository(name = 'platforms', path = 'platforms_workspace')");
if (!scratch.resolve("platforms_workspace/WORKSPACE").exists()) {
scratch.appendFile("WORKSPACE", "local_config_platform(name='local_config_platform')");

if (!scratch.resolve("platforms_workspace/WORKSPACE").exists()
|| !scratch.resolve("local_config_platform_workspace/WORKSPACE").exists()) {
// Create the needed platforms and constraints if they don't already exist.
scratch.file("platforms_workspace/WORKSPACE", "workspace(name = 'platforms')");
MockPlatformSupport.setup(
mockToolsConfig,
"embedded_tools/platforms",
"@platforms//",
"platforms_workspace");
"platforms_workspace",
"@local_config_platform//",
"local_config_platform_workspace");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,9 @@ public static void setup(MockToolsConfig config) throws IOException {
// Any device, simulator or maccatalyst platforms created by Apple tests should consider
// building on one of these targets as parents, to ensure that the proper constraints are set.
config.create(
TestConstants.PLATFORMS_PATH + "/apple/BUILD",
TestConstants.CONSTRAINTS_PATH + "/apple/BUILD",
"package(default_visibility=['//visibility:public'])",
"licenses(['notice'])",
"platform(",
" name = 'darwin_x86_64',",
" constraint_values = [",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,15 +27,19 @@ public static void setup(MockToolsConfig mockToolsConfig) throws IOException {
mockToolsConfig,
TestConstants.PLATFORMS_PATH,
TestConstants.CONSTRAINTS_PACKAGE_ROOT,
TestConstants.CONSTRAINTS_PATH);
TestConstants.CONSTRAINTS_PATH,
TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT,
TestConstants.LOCAL_CONFIG_PLATFORM_PATH);
}

/** Adds mocks for basic host and target platform. */
public static void setup(
MockToolsConfig mockToolsConfig,
String platformsPath,
String constraintsPackageRoot,
String constraintsPath)
String constraintsPath,
String localConfigPlatformPackageRoot,
String localConfigPlatformPath)
throws IOException {
mockToolsConfig.create(
constraintsPath + "/BUILD",
Expand Down Expand Up @@ -150,6 +154,32 @@ public static void setup(
" '" + constraintsPackageRoot + "os:linux',",
" ],",
")");
mockToolsConfig.create(
localConfigPlatformPath + "/BUILD",
"package(default_visibility=['//visibility:public'])",
"licenses(['notice'])",
"platform(",
" name = 'host',",
" constraint_values = [",
// Regardless of the actual machine the tests are run on, hardcode everything to a single
// default value for simplicity.
" '" + constraintsPackageRoot + "cpu:x86_64',",
" '" + constraintsPackageRoot + "os:linux',",
" ],",
")");

mockToolsConfig.create(
"third_party/bazel_platforms/android/BUILD",
"licenses(['notice'])",
"package(default_visibility=['//visibility:public'])",
"platform(",
" name = 'armeabi-v7a',",
" parents = ['" + TestConstants.LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host'],",
" constraint_values = [",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:android',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:armv7',",
" ],",
")");
}

/** Adds a mock K8 platform. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ protected boolean platformBasedToolchains() {
@Override
protected String defaultPlatformFlag() {
return String.format(
"--android_platforms=%s/android:armeabi-v7a", TestConstants.PLATFORM_PACKAGE_ROOT);
"--android_platforms=%sandroid:armeabi-v7a", TestConstants.CONSTRAINTS_PACKAGE_ROOT);
}

@Test
Expand Down Expand Up @@ -220,12 +220,12 @@ public void setup() throws Exception {
"java/android/platforms/BUILD",
"platform(",
" name = 'x86',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + "/android:armeabi-v7a'],",
" parents = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "android:armeabi-v7a'],",
" constraint_values = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_32'],",
")",
"platform(",
" name = 'armeabi-v7a',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + "/android:armeabi-v7a'],",
" parents = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "android:armeabi-v7a'],",
" constraint_values = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:armv7'],",
")");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ protected boolean platformBasedToolchains() {
}

protected String defaultPlatformFlag() {
return String.format("--platforms=%s/android:armeabi-v7a", TestConstants.PLATFORM_PACKAGE_ROOT);
return String.format(
"--platforms=%sandroid:armeabi-v7a", TestConstants.CONSTRAINTS_PACKAGE_ROOT);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,12 +173,12 @@ public void setUp() throws Exception {
"java/android/platforms/BUILD",
"platform(",
" name = 'x86',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + "/android:armeabi-v7a'],",
" parents = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "android:armeabi-v7a'],",
" constraint_values = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_32'],",
")",
"platform(",
" name = 'armeabi-v7a',",
" parents = ['" + TestConstants.PLATFORM_PACKAGE_ROOT + "/android:armeabi-v7a'],",
" parents = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "android:armeabi-v7a'],",
" constraint_values = ['" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:armv7'],",
")");
scratch.file(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public void testMacOsToolchainSetup() throws Exception {
assertThat(darwinToolchainInfo.toolchainType()).isEqualTo(CPP_TOOLCHAIN_TYPE);

// Verify the macOS platform.
ConfiguredTarget darwinPlatform = getConfiguredTarget(
TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:darwin_x86_64");
ConfiguredTarget darwinPlatform =
getConfiguredTarget(TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:darwin_x86_64");
PlatformInfo darwinPlatformInfo = PlatformProviderUtils.platform(darwinPlatform);
assertThat(darwinPlatformInfo).isNotNull();
}
Expand All @@ -92,8 +92,8 @@ public void testIosDeviceToolchainSetup() throws Exception {
assertThat(iosDeviceToolchainInfo.toolchainType()).isEqualTo(CPP_TOOLCHAIN_TYPE);

// Verify the iOS 64 bit device platform.
ConfiguredTarget iosDevicePlatform = getConfiguredTarget(
TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:ios_arm64");
ConfiguredTarget iosDevicePlatform =
getConfiguredTarget(TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:ios_arm64");
PlatformInfo iosDevicePlatformInfo = PlatformProviderUtils.platform(iosDevicePlatform);
assertThat(iosDevicePlatformInfo).isNotNull();
}
Expand All @@ -105,16 +105,18 @@ public void testToolchainSelectionMacOs() throws Exception {
useConfiguration(
EXTRA_SDK_TOOLCHAINS_FLAG,
"--apple_platform_type=macos",
"--apple_platforms=" + TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:darwin_x86_64",
"--platforms=" + TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:darwin_x86_64");
"--apple_platforms=" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:darwin_x86_64",
"--platforms=" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:darwin_x86_64");
createLibraryTargetWriter("//a:lib").write();

BuildConfigurationValue config = getAppleCrosstoolConfiguration();
BuildOptions crosstoolBuildOptions = config.getOptions();
assertThat(crosstoolBuildOptions.contains(PlatformOptions.class)).isNotNull();
List<Label> platforms = crosstoolBuildOptions.get(PlatformOptions.class).platforms;
assertThat(platforms).containsExactly(Label.parseAbsoluteUnchecked(
TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:darwin_x86_64"));
assertThat(platforms)
.containsExactly(
Label.parseAbsoluteUnchecked(
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:darwin_x86_64"));

ConfiguredTarget darwinPlatform = getConfiguredTarget(platforms.get(0).toString());
PlatformInfo darwinPlatformInfo = PlatformProviderUtils.platform(darwinPlatform);
Expand Down Expand Up @@ -144,16 +146,18 @@ public void testToolchainSelectionIosDevice() throws Exception {
useConfiguration(
EXTRA_SDK_TOOLCHAINS_FLAG,
"--apple_platform_type=ios",
"--apple_platforms=" + TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:ios_arm64",
"--platforms=" + TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:ios_arm64");
"--apple_platforms=" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:ios_arm64",
"--platforms=" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:ios_arm64");
createLibraryTargetWriter("//a:lib").write();

BuildConfigurationValue config = getAppleCrosstoolConfiguration();
BuildOptions crosstoolBuildOptions = config.getOptions();
assertThat(crosstoolBuildOptions.contains(PlatformOptions.class)).isNotNull();
List<Label> platforms = crosstoolBuildOptions.get(PlatformOptions.class).platforms;
assertThat(platforms).containsExactly(Label.parseAbsoluteUnchecked(
TestConstants.PLATFORM_PACKAGE_ROOT + "/apple:ios_arm64"));
assertThat(platforms)
.containsExactly(
Label.parseAbsoluteUnchecked(
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "apple:ios_arm64"));

ConfiguredTarget iosPlatform = getConfiguredTarget(platforms.get(0).toString());
PlatformInfo iosPlatformInfo = PlatformProviderUtils.platform(iosPlatform);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ private TestConstants() {
*/
public static final ImmutableList<String> PRODUCT_SPECIFIC_FLAGS =
ImmutableList.of(
"--target_platform_fallback=@bazel_tools//platforms:default_target",
"--platforms=@bazel_tools//platforms:default_target",
"--host_platform=@bazel_tools//platforms:default_host",
"--target_platform_fallback=@local_config_platform//:host",
"--platforms=@local_config_platform//:host",
"--host_platform=@local_config_platform//:host",
// TODO(#7849): Remove after flag flip.
"--incompatible_use_toolchain_resolution_for_java_rules");

Expand All @@ -143,13 +143,15 @@ private TestConstants() {

public static final String PLATFORM_PACKAGE_ROOT = "@bazel_tools//platforms";
public static final String CONSTRAINTS_PACKAGE_ROOT = "@platforms//";
public static final String LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT =
"@local_config_platform//";

public static final String PLATFORMS_PATH = "embedded_tools/platforms";
public static final String CONSTRAINTS_PATH = "platforms_workspace";
public static final String LOCAL_CONFIG_PLATFORM_PATH = "local_config_platform_workspace";

public static final String PLATFORM_LABEL =
PLATFORM_PACKAGE_ROOT + ":default_host + " + PLATFORM_PACKAGE_ROOT + ":default_target";
LOCAL_CONFIG_PLATFORM_PACKAGE_ROOT + ":host";

/** What toolchain type do Android rules use for platform-based toolchain resolution? */
public static final String ANDROID_TOOLCHAIN_TYPE_LABEL =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -495,7 +495,7 @@ cc_binary(
platform(
name = 'android_arm',
constraint_values = ['@platforms//cpu:armv7', '@bazel_tools//platforms:android'],
constraint_values = ['@platforms//cpu:armv7', '@platforms//os:android'],
visibility = ['//visibility:public']
)
EOF
Expand Down

0 comments on commit 3469784

Please sign in to comment.