Skip to content

Commit

Permalink
Fix unit tests to work with cc toolchain resolution enabled.
Browse files Browse the repository at this point in the history
Most of the change is straightforward - adding platform configuration and `--platforms` flags.

ObjcRuleTestCase has a platform/no-platform test setup, however only non-platform test is running and a lot is broken on the platform test.

PiperOrigin-RevId: 470948924
Change-Id: I7d2f7f8c7a836672b0faf83ecf494d340d513ed4
  • Loading branch information
comius authored and copybara-github committed Aug 30, 2022
1 parent 85463a8 commit 5a73201
Show file tree
Hide file tree
Showing 12 changed files with 118 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -124,8 +124,14 @@ public static class Builder {
private String targetLibc = "local";
private String abiVersion = "local";
private String abiLibcVersion = "local";
private ImmutableList<String> toolchainExecConstraints = ImmutableList.of();
private ImmutableList<String> toolchainTargetConstraints = ImmutableList.of();
private ImmutableList<String> toolchainExecConstraints =
ImmutableList.of(
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64",
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:linux");
private ImmutableList<String> toolchainTargetConstraints =
ImmutableList.of(
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64",
TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:linux");

@CanIgnoreReturnValue
public Builder withCpu(String cpu) {
Expand Down Expand Up @@ -710,8 +716,8 @@ public void writeOSX() throws IOException {
: " static_runtime_lib = '" + staticRuntimeLabel + "',",
")",
"toolchain(name = 'cc-toolchain-" + toolchainConfig.getTargetCpu() + "',",
" exec_compatible_with = [],",
" target_compatible_with = [],",
toolchainConfig.getToolchainExecConstraints(),
toolchainConfig.getToolchainTargetConstraints(),
" toolchain = ':cc-compiler-" + toolchainConfig.getTargetCpu() + "',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type'",
")");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ private void writeToolchainsForRealFilesystemTools(
" toolchain = '//" + crosstoolTopPath + ":cc-compiler-arm-llvm',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" target_compatible_with = [",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:arm',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:armv7',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:android',",
" ],",
")");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ public static ImmutableList<String> requiredObjcCrosstoolFlags() {
public static ImmutableList<String> requiredObjcPlatformFlagsNoXcodeConfig() {
ImmutableList.Builder<String> argsBuilder = ImmutableList.builder();

argsBuilder.add("--platforms=" + TestConstants.CONSTRAINTS_PATH + "/apple:darwin_x86_64");

// Set a crosstool_top that is compatible with Apple transitions. Currently, even though this
// references the old cc_toolchain_suite, it's still required of cc builds even when the
// incompatible_enable_cc_toolchain_resolution flag is active.
Expand All @@ -99,8 +101,8 @@ public static ImmutableList<String> requiredObjcCrosstoolFlagsNoXcodeConfig() {
// AppleCrosstoolTransition
argsBuilder
.add("--apple_crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);

.add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.add("--noincompatible_enable_cc_toolchain_resolution");
return argsBuilder.build();
}

Expand Down Expand Up @@ -157,6 +159,20 @@ public static void setup(MockToolsConfig config) throws IOException {
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:ios',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:arm64',",
" ],",
")",
"platform(",
" name = 'ios_x86_64',",
" constraint_values = [",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:ios',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64',",
" ],",
")",
"platform(",
" name = 'watchos_x86_64',",
" constraint_values = [",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "os:watchos',",
" '" + TestConstants.CONSTRAINTS_PACKAGE_ROOT + "cpu:x86_64',",
" ],",
")");

for (String tool :
Expand Down Expand Up @@ -198,8 +214,7 @@ public static void setup(MockToolsConfig config) throws IOException {
"xcode_version(",
" name = 'version5',",
" version = '5',",
")",
"objc_library(name = 'dummy_lib', srcs = ['objc_dummy.mm'])");
")");
// If the bazel tools repository is not in the workspace, also create a workspace tools/objc
// package with a few lingering dependencies.
// TODO(b/64537078): Move these dependencies underneath the tools workspace.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,26 @@ public static void addMockK8Platform(MockToolsConfig mockToolsConfig, Label cros
" target_compatible_with = [':mock_value'],",
")");
}

/** Adds a mock PPC platform. */
public static void addMockPPCPlatform(MockToolsConfig mockToolsConfig, Label crosstoolLabel)
throws Exception {
mockToolsConfig.create(
"mock_platform/BUILD",
"package(default_visibility=['//visibility:public'])",
"constraint_setting(name = 'mock_setting')",
"constraint_value(name = 'mock_value', constraint_setting = ':mock_setting')",
"platform(",
" name = 'mock-ppc-platform',",
" constraint_values = [':mock_value'],",
")",
"toolchain(",
" name = 'toolchain_cc-compiler-ppc',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" toolchain = '"
+ crosstoolLabel.getRelativeWithRemapping("cc-compiler-ppc-compiler", ImmutableMap.of())
+ "',",
" target_compatible_with = [':mock_value'],",
")");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -73,16 +73,21 @@ protected boolean platformBasedToolchains() {
}

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

@Override
protected void useConfiguration(ImmutableMap<String, Object> starlarkOptions, String... args)
throws Exception {

if (!platformBasedToolchains()) {
super.useConfiguration(starlarkOptions, args);
super.useConfiguration(
starlarkOptions,
ImmutableList.builder()
.add((Object[]) args)
.add("--noincompatible_enable_cc_toolchain_resolution")
.build()
.toArray(new String[0]));
return;
}

Expand Down Expand Up @@ -116,8 +121,12 @@ protected void useConfiguration(ImmutableMap<String, Object> starlarkOptions, St
}
}
if (!hasPlatform) {
fullArgs.add(
String.format(
"--platforms=%sandroid:armeabi-v7a", TestConstants.CONSTRAINTS_PACKAGE_ROOT));
fullArgs.add(defaultPlatformFlag());
}
fullArgs.add("--incompatible_enable_cc_toolchain_resolution");
super.useConfiguration(starlarkOptions, fullArgs.build().toArray(new String[0]));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.packages.util.BazelMockAndroidSupport;
import com.google.devtools.build.lib.rules.android.AndroidDataBindingV2Test.WithPlatforms;
import com.google.devtools.build.lib.rules.android.AndroidDataBindingV2Test.WithoutPlatforms;
import com.google.devtools.build.lib.rules.android.databinding.DataBinding;
Expand Down Expand Up @@ -59,6 +60,12 @@ public static class WithPlatforms extends AndroidDataBindingV2Test {
protected boolean platformBasedToolchains() {
return true;
}

@Before
public void setupCcToolchain() throws Exception {
BazelMockAndroidSupport.setupNdk(mockToolsConfig);
invalidatePackages(false);
}
}

@Before
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -378,6 +378,7 @@ java_test(
"//src/main/java/com/google/devtools/build/lib/rules/java:java-compilation",
"//src/main/protobuf:extra_actions_base_java_proto",
"//src/test/java/com/google/devtools/build/lib/actions/util",
"//src/test/java/com/google/devtools/build/lib/packages:testutil",
"//src/test/java/com/google/devtools/build/lib/rules/java:java_compile_action_test_helper",
"//third_party:guava",
"//third_party:junit4",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,10 @@ public void testThatHostCrosstoolTopCommandLineArgumentWorks() throws Exception

scratch.file("a/BUILD", "cc_host_toolchain_alias(name='current_cc_host_toolchain')");

useConfiguration("--host_crosstool_top=//b:my_custom_toolchain_suite", "--host_cpu=k8");
useConfiguration(
"--noincompatible_enable_cc_toolchain_resolution",
"--host_crosstool_top=//b:my_custom_toolchain_suite",
"--host_cpu=k8");
ConfiguredTarget target = getConfiguredTarget("//a:current_cc_host_toolchain");

CcToolchainProvider ccToolchainProvider = target.get(CcToolchainProvider.PROVIDER);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -751,14 +751,25 @@ public void testWhenStaticRuntimeLibAttributeMandatoryWhenSupportsEmbeddedRuntim
CcToolchainConfig.builder()
.withFeatures(CppRuleClasses.STATIC_LINK_CPP_RUNTIMES)
.build()
.getCcToolchainConfigRule());
.getCcToolchainConfigRule(),
"toolchain(",
" name = 'cc-toolchain-b',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" toolchain = ':b',",
" target_compatible_with = [],",
" exec_compatible_with = [],",
")");
analysisMock.ccSupport().setupCcToolchainConfig(mockToolsConfig, CcToolchainConfig.builder());
mockToolsConfig.create(
"a/cc_toolchain_config.bzl",
ResourceLoader.readFromResources(
"com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl"));
reporter.removeHandler(failFastHandler);
useConfiguration("--crosstool_top=//a:a", "--cpu=k8", "--host_cpu=k8");
useConfiguration(
"--extra_toolchains=//a:cc-toolchain-b",
"--crosstool_top=//a:a",
"--cpu=k8",
"--host_cpu=k8");
assertThat(getConfiguredTarget("//a:main")).isNull();
assertContainsEvent(
"Toolchain supports embedded runtimes, but didn't provide static_runtime_lib attribute.");
Expand Down Expand Up @@ -795,14 +806,26 @@ public void testWhenDynamicRuntimeLibAttributeMandatoryWhenSupportsEmbeddedRunti
.withFeatures(
CppRuleClasses.STATIC_LINK_CPP_RUNTIMES, CppRuleClasses.SUPPORTS_DYNAMIC_LINKER)
.build()
.getCcToolchainConfigRule());
.getCcToolchainConfigRule(),
"toolchain(",
" name = 'cc-toolchain-b',",
" toolchain_type = '" + TestConstants.TOOLS_REPOSITORY + "//tools/cpp:toolchain_type',",
" toolchain = ':b',",
" target_compatible_with = [],",
" exec_compatible_with = [],",
")");
analysisMock.ccSupport().setupCcToolchainConfig(mockToolsConfig, CcToolchainConfig.builder());
mockToolsConfig.create(
"a/cc_toolchain_config.bzl",
ResourceLoader.readFromResources(
"com/google/devtools/build/lib/analysis/mock/cc_toolchain_config.bzl"));
reporter.removeHandler(failFastHandler);
useConfiguration("--crosstool_top=//a:a", "--cpu=k8", "--host_cpu=k8", "--dynamic_mode=fully");
useConfiguration(
"--extra_toolchains=//a:cc-toolchain-b",
"--crosstool_top=//a:a",
"--cpu=k8",
"--host_cpu=k8",
"--dynamic_mode=fully");
assertThat(getConfiguredTarget("//a:test")).isNull();
assertContainsEvent(
"Toolchain supports embedded runtimes, but didn't provide dynamic_runtime_lib attribute.");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1150,7 +1150,9 @@ public void testJ2ObjcDeadCodeRemovalActionWithOptFlag() throws Exception {
"--output_archive",
removeConfigFragment(prunedArchive.getExecPathString()),
"--dummy_archive",
execPath + TestConstants.TOOLS_REPOSITORY_PATH_PREFIX + "tools/objc/libdummy_lib.a",
execPath
+ TestConstants.TOOLS_REPOSITORY_PATH_PREFIX
+ "tools/objc/dummy/libdummy_lib.a",
"--xcrunwrapper",
removeConfigFragment(MOCK_XCRUNWRAPPER_EXECUTABLE_PATH),
"--dependency_mapping_files",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,14 @@ protected void initializeMockClient() throws IOException {

@Override
protected void useConfiguration(String... args) throws Exception {
ImmutableList<String> extraArgs = ImmutableList.<String>builder()
.add("--xcode_version_config=" + MockObjcSupport.XCODE_VERSION_CONFIG)
.add("--apple_crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.addAll(ImmutableList.copyOf(args))
.build();
ImmutableList<String> extraArgs =
ImmutableList.<String>builder()
.add("--noincompatible_enable_cc_toolchain_resolution")
.add("--xcode_version_config=" + MockObjcSupport.XCODE_VERSION_CONFIG)
.add("--apple_crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.add("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL)
.addAll(ImmutableList.copyOf(args))
.build();

super.useConfiguration(extraArgs.toArray(new String[extraArgs.size()]));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -506,6 +506,9 @@ protected static void addAppleBinaryStarlarkRule(Scratch scratch) throws Excepti
scratch.file("test_starlark/BUILD");
RepositoryName toolsRepo = TestConstants.TOOLS_REPOSITORY;
String toolsLoc = toolsRepo + "//tools/objc";
scratch.file(
TestConstants.TOOLS_REPOSITORY_SCRATCH + "tools/objc/dummy/BUILD",
"objc_library(name = 'dummy_lib', srcs = ['objc_dummy.mm'])");

scratch.file(
"test_starlark/apple_binary_starlark.bzl",
Expand Down Expand Up @@ -576,7 +579,7 @@ protected static void addAppleBinaryStarlarkRule(Scratch scratch) throws Excepti
" cfg=apple_common.multi_arch_split,",
" default=Label('" + toolsRepo + "//tools/cpp:current_cc_toolchain'),),",
" '_dummy_lib': attr.label(",
" default = Label('" + toolsLoc + ":dummy_lib'),),",
" default = Label('" + toolsLoc + "/dummy:dummy_lib'),),",
" '_grep_includes': attr.label(",
" cfg = 'host',",
" allow_single_file = True,",
Expand Down

0 comments on commit 5a73201

Please sign in to comment.