From a20797edecf36b4da7fe4669031cb667492870e0 Mon Sep 17 00:00:00 2001 From: plf Date: Wed, 23 Feb 2022 08:34:26 -0800 Subject: [PATCH] Apply changes from https://github.com/bazelbuild/bazel/commit/56409448dfd7507f551f65283b4214020754c25c to Starlark cc_library This CL inverts the experimental implementation_deps attribute to interface_deps. RELNOTES:none PiperOrigin-RevId: 430460463 --- .../build/lib/rules/cpp/CppConfiguration.java | 3 +- .../builtins_bzl/common/cc/cc_library.bzl | 29 +++++++++++-------- .../builtins_bzl/common/cc/semantics.bzl | 13 +++++++++ .../cpp/CcLibraryConfiguredTargetTest.java | 4 +++ .../lib/rules/cpp/StarlarkCcCommonTest.java | 2 +- 5 files changed, 36 insertions(+), 15 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java index eda371806c27ad..62668f7735739d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java @@ -859,9 +859,8 @@ public boolean objcShouldGenerateDotdFiles() { return cppOptions.objcGenerateDotdFiles; } - // TODO(plf): Change in separate CL where Starlark cc_library.implementation_deps is renamed. @StarlarkMethod( - name = "experimental_cc_implementation_deps", + name = "experimental_cc_interface_deps", documented = false, useStarlarkThread = true) public boolean experimentalCcInterfaceDepsForStarlark(StarlarkThread thread) diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl index 7ef0c2ec587e6e..185f9080bcaea7 100755 --- a/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/cc_library.bzl @@ -23,11 +23,6 @@ cc_internal = _builtins.internal.cc_internal def _cc_library_impl(ctx): cc_helper.check_srcs_extensions(ctx, ALLOWED_SRC_FILES, "cc_library") - cpp_config = ctx.fragments.cpp - - if (not cpp_config.experimental_cc_implementation_deps() and - len(ctx.attr.implementation_deps) > 0): - fail("requires --experimental_cc_implementation_deps", attr = "implementation_deps") common = cc_internal.create_common(ctx = ctx) common.report_invalid_options(ctx = ctx) @@ -47,10 +42,18 @@ def _cc_library_impl(ctx): semantics.validate_attributes(ctx = ctx) _check_no_repeated_srcs(ctx) - compilation_contexts = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps) + interface_deps = None + implementation_deps = [] + should_use_interface_deps_behavior = semantics.should_use_interface_deps_behavior(ctx) + if should_use_interface_deps_behavior: + interface_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.interface_deps) + implementation_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps) + else: + interface_deps = cc_helper.get_compilation_contexts_from_deps(ctx.attr.deps) + if not _is_stl(ctx.attr.tags) and ctx.attr._stl != None: - compilation_contexts.append(ctx.attr._stl[CcInfo].compilation_context) - implementation_compilation_contexts = cc_helper.get_compilation_contexts_from_deps(ctx.attr.implementation_deps) + interface_deps.append(ctx.attr._stl[CcInfo].compilation_context) + (compilation_context, srcs_compilation_outputs) = cc_common.compile( actions = ctx.actions, name = ctx.label.name, @@ -67,8 +70,8 @@ def _cc_library_impl(ctx): private_hdrs = common.private_hdrs, public_hdrs = common.public_hdrs, code_coverage_enabled = cc_helper.is_code_coverage_enabled(ctx), - compilation_contexts = compilation_contexts, - implementation_compilation_contexts = implementation_compilation_contexts, + compilation_contexts = interface_deps, + implementation_compilation_contexts = implementation_deps, hdrs_checking_mode = semantics.determine_headers_checking_mode(ctx), grep_includes = ctx.executable._grep_includes, textual_hdrs = ctx.files.textual_hdrs, @@ -108,7 +111,7 @@ def _cc_library_impl(ctx): is_google = True linking_contexts = cc_helper.get_linking_contexts_from_deps(ctx.attr.deps) - linking_contexts.extend(cc_helper.get_linking_contexts_from_deps(ctx.attr.implementation_deps)) + linking_contexts.extend(cc_helper.get_linking_contexts_from_deps(ctx.attr.interface_deps)) if ctx.file.linkstamp != None: linkstamps = [] linkstamps.append(cc_internal.create_linkstamp( @@ -584,7 +587,7 @@ attrs = { ), "alwayslink": attr.bool(default = False), "linkstatic": attr.bool(default = False), - "implementation_deps": attr.label_list(providers = [CcInfo], allow_files = False), + "interface_deps": attr.label_list(providers = [CcInfo], allow_files = False), "hdrs": attr.label_list( allow_files = True, flags = ["ORDER_INDEPENDENT", "DIRECT_COMPILE_TIME_INPUT"], @@ -628,6 +631,8 @@ attrs = { attrs.update(semantics.get_licenses_attr()) attrs.update(semantics.get_distribs_attr()) attrs.update(semantics.get_loose_mode_in_hdrs_check_allowed_attr()) +attrs.update(semantics.get_interface_deps_allowed_attr()) + cc_library = rule( implementation = _cc_library_impl, attrs = attrs, diff --git a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl index 1c8a70d0646453..a56ae19cfc81e2 100644 --- a/src/main/starlark/builtins_bzl/common/cc/semantics.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/semantics.bzl @@ -58,6 +58,17 @@ def _get_def_parser(): def _get_grep_includes(): return attr.label() +def _get_interface_deps_allowed_attr(): + return {} + +def _should_use_interface_deps_behavior(ctx): + experimental_cc_interface_deps = ctx.fragments.cpp.experimental_cc_interface_deps() + if (not experimental_cc_interface_deps and + len(ctx.attr.interface_deps) > 0): + fail("requires --experimental_cc_interface_deps", attr = "interface_deps") + + return experimental_cc_interface_deps + semantics = struct( ALLOWED_RULES_IN_DEPS = [ "cc_library", @@ -84,4 +95,6 @@ semantics = struct( get_stl = _get_stl, should_create_empty_archive = _should_create_empty_archive, get_grep_includes = _get_grep_includes, + get_interface_deps_allowed_attr = _get_interface_deps_allowed_attr, + should_use_interface_deps_behavior = _should_use_interface_deps_behavior, ) diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java index 83df4da35762f6..8dfa7b5e9bea18 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcLibraryConfiguredTargetTest.java @@ -1894,6 +1894,7 @@ public void testSameSymlinkedLibraryDoesNotGiveDuplicateError() throws Exception @Test public void testImplementationDepsCompilationContextIsNotPropagated() throws Exception { + setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_library"); useConfiguration("--experimental_cc_interface_deps"); scratch.file( "foo/BUILD", @@ -1959,6 +1960,7 @@ public void testImplementationDepsCompilationContextIsNotPropagated() throws Exc @Test public void testImplementationDepsLinkingContextIsPropagated() throws Exception { + setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_library"); useConfiguration("--experimental_cc_interface_deps"); scratch.file( "foo/BUILD", @@ -2030,6 +2032,7 @@ public void testImplementationDepsConfigurationHostSucceeds() throws Exception { @Test public void testInterfaceDepsFailsWithoutFlagOrTag() throws Exception { + setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_library"); scratch.file( "foo/BUILD", "cc_library(", @@ -2055,6 +2058,7 @@ public void testInterfaceDepsFailsWithoutFlagOrTag() throws Exception { @Test public void testInterfaceDepsNotInAllowlistThrowsError() throws Exception { + setBuildLanguageOptions("--experimental_builtins_injection_override=+cc_library"); if (analysisMock.isThisBazel()) { // In OSS usage is controlled only by a flag and not an allowlist. return; diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java index 90b215bbc08670..5c38525506cca5 100755 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/StarlarkCcCommonTest.java @@ -6978,7 +6978,7 @@ public void testExpandedCppConfigurationApiBlocked() throws Exception { "build_test_dwp()", "grte_top()", "enable_legacy_cc_provider()", - "experimental_cc_implementation_deps()", + "experimental_cc_interface_deps()", "share_native_deps()", "experimental_platform_cc_test()"); scratch.file(