Skip to content

Commit

Permalink
Fix tests for Starlark cc_binary.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 426882171
  • Loading branch information
Googler authored and copybara-github committed Feb 7, 2022
1 parent 473d147 commit 4f341a3
Show file tree
Hide file tree
Showing 8 changed files with 46 additions and 23 deletions.
11 changes: 8 additions & 3 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,11 @@ _TVOS_DEVICE_TARGET_CPUS = ["tvos_arm64"]
_CATALYST_TARGET_CPUS = ["catalyst_x86_64"]
_MACOS_TARGET_CPUS = ["darwin_x86_64", "darwin_arm64", "darwin_arm64e", "darwin"]

def _grep_includes_executable(grep_includes):
if grep_includes == None:
return None
return grep_includes.files_to_run.executable

def _new_dwp_action(ctx, cc_toolchain, dwp_tools):
return {
"tools": dwp_tools,
Expand Down Expand Up @@ -600,7 +605,7 @@ def _create_transitive_linking_actions(
feature_configuration = feature_configuration,
cc_toolchain = cc_toolchain,
compilation_outputs = cc_compilation_outputs_with_only_objects,
grep_includes = ctx.attr._grep_includes.files_to_run.executable,
grep_includes = _grep_includes_executable(ctx.attr._grep_includes),
stamp = _is_stamping_enabled(ctx),
additional_inputs = additional_linker_inputs,
linking_contexts = [cc_linking_context],
Expand Down Expand Up @@ -773,7 +778,7 @@ def cc_binary_impl(ctx):
copts_filter = common.copts_filter,
srcs = common.srcs,
compilation_contexts = compilation_context_deps,
grep_includes = ctx.attr._grep_includes.files_to_run.executable,
grep_includes = _grep_includes_executable(ctx.attr._grep_includes),
code_coverage_enabled = cc_helper.is_code_coverage_enabled(ctx = ctx),
hdrs_checking_mode = semantics.determine_headers_checking_mode(ctx),
)
Expand Down Expand Up @@ -807,7 +812,7 @@ def cc_binary_impl(ctx):
cc_toolchain = cc_toolchain,
compilation_outputs = cc_compilation_outputs,
name = ctx.label.name,
grep_includes = ctx.attr._grep_includes.files_to_run.executable,
grep_includes = _grep_includes_executable(ctx.attr._grep_includes),
linking_contexts = [cc_helper.get_linking_context_from_deps(_malloc_for_target(ctx, cpp_config))] + cc_helper.get_linking_context_from_deps(ctx.attr.deps),
stamp = _is_stamping_enabled(ctx),
test_only_target = cc_helper.is_test_target(ctx) or ctx.attr._is_test,
Expand Down
7 changes: 1 addition & 6 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary_attrs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -83,12 +83,7 @@ cc_binary_attrs_with_aspects = {
"distribs": attr.string_list(),
"_cc_binary": attr.bool(),
"_is_test": attr.bool(default = False),
"_grep_includes": attr.label(
allow_files = True,
executable = True,
cfg = "exec",
default = Label("@" + semantics.get_repo() + "//tools/cpp:grep-includes"),
),
"_grep_includes": semantics.get_grep_includes(),
"_stl": semantics.get_stl(),
"_cc_toolchain": attr.label(default = "@" + semantics.get_repo() + "//tools/cpp:current_cc_toolchain"),
"_cc_toolchain_type": attr.label(default = "@" + semantics.get_repo() + "//tools/cpp:toolchain_type"),
Expand Down
4 changes: 4 additions & 0 deletions src/main/starlark/builtins_bzl/common/cc/semantics.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,9 @@ def _get_def_parser():
cfg = "exec",
)

def _get_grep_includes():
return attr.label()

semantics = struct(
ALLOWED_RULES_IN_DEPS = [
"cc_library",
Expand All @@ -84,4 +87,5 @@ semantics = struct(
get_def_parser = _get_def_parser,
get_stl = _get_stl,
should_create_empty_archive = _should_create_empty_archive,
get_grep_includes = _get_grep_includes,
)
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ build_failure_test(

build_failure_test(
name = "two_dynamic_deps_same_export_in_binary_test",
message = "Two shared libraries in dependencies export the same symbols",
message = "Two shared libraries in dependencies link the same library statically",
target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:two_dynamic_deps_same_export_in_binary",
)

Expand Down
1 change: 1 addition & 0 deletions src/main/starlark/tests/builtins_bzl/cc_builtin_tests.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ EOF
--experimental_cc_shared_library_debug \
--experimental_link_static_libraries_once \
--experimental_enable_target_export_check --experimental_cc_shared_library \
--experimental_builtins_injection_override=+cc_binary \
//src/main/starlark/tests/builtins_bzl/cc/... || fail "expected success"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import java.util.List;
import java.util.Set;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

/**
Expand Down Expand Up @@ -593,6 +594,7 @@ public void testPathOperatorsWithOutputFile() throws Exception {
}

@Test
@Ignore("b/198254254")
public void testDeps() throws Exception {
writeBuildFiles3();
writeBuildFilesWithConfigurableAttributes();
Expand Down Expand Up @@ -636,7 +638,8 @@ public void testDeps() throws Exception {
helper.getToolsRepository()
+ "//tools/cpp:malloc + //configurable:main + "
+ "//configurable:main.cc + //configurable:adep + //configurable:bdep + "
+ "//configurable:defaultdep + //conditions:a + //conditions:b"
+ "//configurable:defaultdep + //conditions:a + //conditions:b + "
+ "//tools/cpp:toolchain_type + //tools/cpp:current_cc_toolchain"
+ implicitDeps));
}
}
Expand Down Expand Up @@ -952,6 +955,7 @@ public void testBuildFilesDoesNotReturnVisibilityOfBUILD() throws Exception {
}

@Test
@Ignore("b/198254254")
public void testNoImplicitDeps() throws Exception {
writeFile("x/BUILD", "cc_binary(name='x', srcs=['x.cc'])");

Expand All @@ -972,10 +976,11 @@ public void testNoImplicitDeps() throws Exception {
}

String targetDepsExpr = "//x:x + //x:x.cc";
String toolchainDepsExpr = "//tools/cpp:toolchain_type + //tools/cpp:current_cc_toolchain";

// Test all combinations of --[no]host_deps and --[no]implicit_deps on //x:x
assertEqualsFiltered(
targetDepsExpr + " + " + hostDepsExpr + implicitDepsExpr,
targetDepsExpr + " + " + hostDepsExpr + implicitDepsExpr + " + " + toolchainDepsExpr,
"deps(//x)" + TestConstants.CC_DEPENDENCY_CORRECTION);
assertEqualsFiltered(
targetDepsExpr + " + " + hostDepsExpr,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ private CcStarlarkApiProvider getApi(String label) throws Exception {
return (CcStarlarkApiProvider) rule.get(CcStarlarkApiProvider.NAME);
}

private CcStarlarkApiInfo getApiForCcBinary(String label) throws Exception {
RuleConfiguredTarget rule = (RuleConfiguredTarget) getConfiguredTarget(label);
return (CcStarlarkApiInfo) rule.get(CcStarlarkApiProvider.NAME);
}

@Before
public void setUp() throws Exception {
MockProtoSupport.setupWorkspace(scratch);
Expand Down Expand Up @@ -99,6 +104,7 @@ public void testDisableInCcProtoLibrary() throws Exception {

@Test
public void testTransitiveHeaders() throws Exception {
setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_binary");
useConfiguration("--noincompatible_disable_legacy_cc_provider");
scratch.file(
"pkg/BUILD",
Expand All @@ -111,14 +117,17 @@ public void testTransitiveHeaders() throws Exception {
" name = 'check_lib',",
" srcs = ['lib.cc', 'lib.h'],",
")");
assertThat(ActionsTestUtil.baseArtifactNames(getApi("//pkg:check").getTransitiveHeaders()))
assertThat(
ActionsTestUtil.baseArtifactNames(
getApiForCcBinary("//pkg:check").getTransitiveHeaders()))
.containsAtLeast("lib.h", "bin.h");
assertThat(ActionsTestUtil.baseArtifactNames(getApi("//pkg:check_lib").getTransitiveHeaders()))
.contains("lib.h");
}

@Test
public void testLinkFlags() throws Exception {
setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_binary");
useConfiguration("--noincompatible_disable_legacy_cc_provider");
scratch.file(
"pkg/BUILD",
Expand Down Expand Up @@ -148,14 +157,13 @@ public void testLinkFlags() throws Exception {
assertThat(getApi("//pkg:dependent_lib").getLinkopts())
.containsAtLeast("-lz", "-Wl,-M")
.inOrder();
assertThat(getApi("//pkg:check").getLinkopts())
.isEmpty();
assertThat(getApi("//pkg:check_no_srcs").getLinkopts())
.isEmpty();
assertThat(getApiForCcBinary("//pkg:check").getLinkopts()).isEmpty();
assertThat(getApiForCcBinary("//pkg:check_no_srcs").getLinkopts()).isEmpty();
}

@Test
public void testLibraries() throws Exception {
setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_binary");
useConfiguration("--noincompatible_disable_legacy_cc_provider");
scratch.file(
"pkg/BUILD",
Expand All @@ -174,14 +182,17 @@ public void testLibraries() throws Exception {
")");
assertThat(ActionsTestUtil.baseArtifactNames(getApi("//pkg:check_lib").getLibraries()))
.containsExactly("libcheck_lib.a");
assertThat(ActionsTestUtil.baseArtifactNames(getApi("//pkg:check").getLibraries()))
assertThat(ActionsTestUtil.baseArtifactNames(getApiForCcBinary("//pkg:check").getLibraries()))
.isEmpty();
assertThat(ActionsTestUtil.baseArtifactNames(getApi("//pkg:check_no_srcs").getLibraries()))
assertThat(
ActionsTestUtil.baseArtifactNames(
getApiForCcBinary("//pkg:check_no_srcs").getLibraries()))
.isEmpty();
}

@Test
public void testCcFlags() throws Exception {
setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_binary");
useConfiguration("--noincompatible_disable_legacy_cc_provider");
scratch.file(
"pkg/BUILD",
Expand All @@ -194,7 +205,7 @@ public void testCcFlags() throws Exception {
" name = 'check_lib',",
" defines = ['foo'],",
")");
assertThat(getApi("//pkg:check").getCcFlags()).contains("-Dfoo");
assertThat(getApiForCcBinary("//pkg:check").getCcFlags()).contains("-Dfoo");
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ private Rule getRuleForTarget(String targetName) throws Exception {

@Test
public void testMacroHasGeneratorAttributes() throws Exception {
setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_binary");
scratch.file(
"test/starlark/extension.bzl",
"def _impl(ctx):",
Expand Down Expand Up @@ -266,10 +267,11 @@ public void testMacroHasGeneratorAttributes() throws Exception {
assertThat(nativeMacro.getAttr("generator_function")).isEqualTo("native_macro");
assertThat(nativeMacro.getAttr("generator_location")).isEqualTo("test/starlark/BUILD:5:18");

// Starlark version of cc_binary is created by a wrapper macro.
Rule ccTarget = getRuleForTarget("cc_target");
assertThat(ccTarget.getAttr("generator_name")).isEqualTo("");
assertThat(ccTarget.getAttr("generator_function")).isEqualTo("");
assertThat(ccTarget.getAttr("generator_location")).isEqualTo("");
assertThat(ccTarget.getAttr("generator_name")).isEqualTo("cc_target");
assertThat(ccTarget.getAttr("generator_function")).isEqualTo("cc_binary");
assertThat(ccTarget.getAttr("generator_location")).isEqualTo("test/starlark/BUILD:6:10");
}

@Test
Expand Down

0 comments on commit 4f341a3

Please sign in to comment.