diff --git a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl index b06d341a606709..f5f54bd97b403d 100644 --- a/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl +++ b/src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl @@ -193,6 +193,47 @@ def _check_if_target_should_be_exported_with_filter(target, current_label, expor return False +# Checks if the linker_input has code to link statically, i.e. either +# archives or object files, ignores library.dynamic_library. +def _contains_code_to_link(linker_input): + for library in linker_input.libraries: + if (library.static_library != None or + library.pic_static_library != None or + len(library.objects) or len(library.pic_objects)): + return True + + return False + +def _find_top_level_linker_input_labels( + nodes, + linker_inputs_to_be_linked_statically_map, + targets_to_be_linked_dynamically_set): + top_level_linker_input_labels_set = {} + nodes_to_check = list(nodes) + + for i in range(2147483647): + if i == len(nodes_to_check): + break + + node = nodes_to_check[i] + node_label = str(node.label) + if node_label in linker_inputs_to_be_linked_statically_map: + has_code_to_link = False + for linker_input in linker_inputs_to_be_linked_statically_map[node_label]: + if _contains_code_to_link(linker_input): + top_level_linker_input_labels_set[node_label] = True + has_code_to_link = True + break + + if not has_code_to_link: + nodes_to_check.extend(node.children) + elif node_label not in targets_to_be_linked_dynamically_set: + # This can happen when there was a target in the graph that exported other libraries' + # linker_inputs but didn't contribute any linker_input of its own. + nodes_to_check.extend(node.children) + + return top_level_linker_input_labels_set + def _filter_inputs( ctx, feature_configuration, @@ -205,11 +246,11 @@ def _filter_inputs( graph_structure_aspect_nodes = [] dependency_linker_inputs = [] - direct_exports = {} - for export in deps: - direct_exports[str(export.label)] = True - dependency_linker_inputs.extend(export[CcInfo].linking_context.linker_inputs.to_list()) - graph_structure_aspect_nodes.append(export[GraphNodeInfo]) + direct_deps_set = {} + for dep in deps: + direct_deps_set[str(dep.label)] = True + dependency_linker_inputs.extend(dep[CcInfo].linking_context.linker_inputs.to_list()) + graph_structure_aspect_nodes.append(dep[GraphNodeInfo]) can_be_linked_dynamically = {} for linker_input in dependency_linker_inputs: @@ -224,6 +265,18 @@ def _filter_inputs( can_be_linked_dynamically, ) + linker_inputs_to_be_linked_statically_map = {} + for linker_input in dependency_linker_inputs: + owner = str(linker_input.owner) + if owner in targets_to_be_linked_statically_map: + linker_inputs_to_be_linked_statically_map.setdefault(owner, []).append(linker_input) + + top_level_linker_input_labels_set = _find_top_level_linker_input_labels( + graph_structure_aspect_nodes, + linker_inputs_to_be_linked_statically_map, + targets_to_be_linked_dynamically_set, + ) + # We keep track of precompiled_only_dynamic_libraries, so that we can add # them to runfiles. precompiled_only_dynamic_libraries = [] @@ -260,7 +313,6 @@ def _filter_inputs( # link_once_static_libs_map[owner] but is not being exported linked_statically_but_not_exported.setdefault(link_once_static_libs_map[owner], []).append(owner) - is_direct_export = owner in direct_exports dynamic_only_libraries = [] static_libraries = [] for library in linker_input.libraries: @@ -272,7 +324,7 @@ def _filter_inputs( if len(dynamic_only_libraries): precompiled_only_dynamic_libraries.extend(dynamic_only_libraries) if not len(static_libraries): - if is_direct_export: + if owner in direct_deps_set: dynamic_only_roots[owner] = True linker_inputs.append(linker_input) continue @@ -280,7 +332,7 @@ def _filter_inputs( dynamic_only_roots.pop(owner) linker_input_to_be_linked_statically = linker_input - if is_direct_export: + if owner in top_level_linker_input_labels_set: linker_input_to_be_linked_statically = _wrap_static_library_with_alwayslink( ctx, feature_configuration, diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test index 4dd8bc4c266aa3..343fee87acb42d 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test @@ -7,7 +7,9 @@ load( "linking_suffix_test", "paths_test", "runfiles_test", - "check_already_linked_inputs_are_not_passed_to_linking_action_test", + "check_linking_action_lib_parameters_test", + "forwarding_cc_lib", + "nocode_cc_lib", ) LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE" @@ -117,6 +119,8 @@ cc_shared_library( deps = [ "baz", "foo", + "cc_lib_with_no_srcs", + "nocode_cc_lib", "a_suffix", ], user_link_flags = select({ @@ -156,6 +160,33 @@ cc_library( }), ) +forwarding_cc_lib( + name = "cc_lib_with_no_srcs", + deps = [ + "indirect_dep", + ], +) + +nocode_cc_lib( + name = "nocode_cc_lib", + additional_inputs = [ + ":additional_script.txt", + ], + deps = [ + "indirect_dep2", + ], +) + +cc_library( + name = "indirect_dep", + srcs = ["indirect_dep.cc"], +) + +cc_library( + name = "indirect_dep2", + srcs = ["indirect_dep2.cc"], +) + cc_library( name = "a_suffix", srcs = ["a_suffix.cc"], @@ -420,13 +451,13 @@ runfiles_test( target_under_test = ":python_test", ) -check_already_linked_inputs_are_not_passed_to_linking_action_test( +check_linking_action_lib_parameters_test( name = "check_binary_doesnt_take_already_linked_in_libs", target_under_test = ":binary", libs_that_shouldnt_be_present = ["foo", "bar"], ) -check_already_linked_inputs_are_not_passed_to_linking_action_test( +check_linking_action_lib_parameters_test( name = "check_shared_lib_doesnt_take_already_linked_in_libs", target_under_test = ":foo_so", libs_that_shouldnt_be_present = ["bar"], @@ -437,3 +468,9 @@ build_failure_test( message = "'cc_shared_library' must have at least one dependency in 'deps' (or 'roots')", target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:failing_with_no_deps_so", ) + +build_failure_test( + name = "direct_dep_with_only_shared_lib_file", + message = "Do not place libraries which only contain a precompiled dynamic library", + target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:failing_only_dynamic_lib", +) diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh index 192815ef9964fd..61d36400e29c50 100755 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/cc_shared_library_integration_test.sh @@ -38,14 +38,15 @@ function check_symbol_absent() { function test_shared_library_symbols() { foo_so=$(find . -name libfoo_so.so) - symbols=$(nm -D $foo_so) + symbols=$(nm $foo_so) check_symbol_present "$symbols" "U _Z3barv" check_symbol_present "$symbols" "T _Z3bazv" check_symbol_present "$symbols" "T _Z3foov" - - check_symbol_absent "$symbols" "_Z3quxv" + check_symbol_present "$symbols" "T _Z3foov" + check_symbol_present "$symbols" "t _Z3quxv" + check_symbol_present "$symbols" "t _Z12indirect_depv" + check_symbol_present "$symbols" "t _Z13indirect_dep2v" check_symbol_absent "$symbols" "_Z4bar3v" - check_symbol_absent "$symbols" "_Z4bar4v" } function test_shared_library_user_link_flags() { diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test index d4406d52583cc5..5b0b495c8ed70f 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test @@ -89,3 +89,27 @@ cc_shared_library( name = "failing_with_no_deps_so", tags = TAGS, ) + + +cc_shared_library( + name = "failing_only_dynamic_lib", + deps = [ + "dynamic_only", + ], + tags = TAGS, +) + +cc_library( + name = "dynamic_only", + srcs = [ + "libabc.so", + ], +) + +genrule( + name = "abc_lib", + srcs = [], + outs = ["libabc.so"], + cmd = "touch \"$@\"", +) + diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep.cc new file mode 100644 index 00000000000000..e1b446f22ececb --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep.cc @@ -0,0 +1,15 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +int indirect_dep() { return 0; } diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep2.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep2.cc new file mode 100644 index 00000000000000..a21e7d854c7669 --- /dev/null +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/indirect_dep2.cc @@ -0,0 +1,15 @@ +// Copyright 2023 The Bazel Authors. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +int indirect_dep2() { return 0; } diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl index 8732cea67f1932..2426a3f272e87c 100644 --- a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl +++ b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl @@ -223,7 +223,7 @@ interface_library_output_group_test = analysistest.make( }, ) -def _check_already_linked_inputs_are_not_passed_to_linking_action_test_impl(ctx): +def _check_linking_action_lib_parameters_test_impl(ctx): env = analysistest.begin(ctx) actions = analysistest.target_actions(env) @@ -240,9 +240,37 @@ def _check_already_linked_inputs_are_not_passed_to_linking_action_test_impl(ctx) return analysistest.end(env) -check_already_linked_inputs_are_not_passed_to_linking_action_test = analysistest.make( - _check_already_linked_inputs_are_not_passed_to_linking_action_test_impl, +check_linking_action_lib_parameters_test = analysistest.make( + _check_linking_action_lib_parameters_test_impl, attrs = { "libs_that_shouldnt_be_present": attr.string_list(), }, ) + +def _forwarding_cc_lib_impl(ctx): + return [ctx.attr.deps[0][CcInfo]] + +forwarding_cc_lib = rule( + implementation = _forwarding_cc_lib_impl, + attrs = { + "deps": attr.label_list(), + }, + provides = [CcInfo], +) + +def _nocode_cc_lib_impl(ctx): + linker_input = cc_common.create_linker_input( + owner = ctx.label, + additional_inputs = depset([ctx.files.additional_inputs[0]]), + ) + cc_info = CcInfo(linking_context = cc_common.create_linking_context(linker_inputs = depset([linker_input]))) + return [cc_common.merge_cc_infos(cc_infos = [cc_info, ctx.attr.deps[0][CcInfo]])] + +nocode_cc_lib = rule( + implementation = _nocode_cc_lib_impl, + attrs = { + "additional_inputs": attr.label_list(allow_files = True), + "deps": attr.label_list(), + }, + provides = [CcInfo], +)