Skip to content

Commit

Permalink
[6.1.0] Fix cc_binary bug related to cc_shared_library on Windows and…
Browse files Browse the repository at this point in the history
… prepare for future removal of --experimental_cc_shared_library flag (#17445)

* Cherrypicks 9815b76, 68aad18 and 4ed6327 from HEAD

* Add missing in comma in test BUILD file
  • Loading branch information
oquenchil authored Feb 8, 2023
1 parent 27bc896 commit 544b816
Show file tree
Hide file tree
Showing 9 changed files with 271 additions and 145 deletions.
4 changes: 2 additions & 2 deletions src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ def _create_transitive_linking_actions(
win_def_file = win_def_file,
)
cc_launcher_info = cc_internal.create_cc_launcher_info(cc_info = cc_info_without_extra_link_time_libraries, compilation_outputs = cc_compilation_outputs_with_only_objects)
return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files)
return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files, cc_linking_context)

def _use_pic(ctx, cc_toolchain, cpp_config, feature_configuration):
if _is_link_shared(ctx):
Expand Down Expand Up @@ -735,7 +735,7 @@ def cc_binary_impl(ctx, additional_linkopts):
if extra_link_time_libraries != None:
linker_inputs_extra, runtime_libraries_extra = extra_link_time_libraries.build_libraries(ctx = ctx, static_mode = linking_mode != _LINKING_DYNAMIC, for_dynamic_library = _is_link_shared(ctx))

cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files = _create_transitive_linking_actions(
cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files, deps_cc_linking_context = _create_transitive_linking_actions(
ctx,
cc_toolchain,
feature_configuration,
Expand Down

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -1,4 +1,14 @@
load(":starlark_tests.bzl", "additional_inputs_test", "build_failure_test", "debug_files_test", "interface_library_output_group_test", "linking_suffix_test", "paths_test", "runfiles_test")
load(
":starlark_tests.bzl",
"additional_inputs_test",
"build_failure_test",
"debug_files_test",
"interface_library_output_group_test",
"linking_suffix_test",
"paths_test",
"runfiles_test",
"no_exporting_static_lib_test",
)

LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE"

Expand Down Expand Up @@ -42,28 +52,28 @@ cc_binary(
cc_shared_library(
name = "python_module",
features = ["windows_export_all_symbols"],
roots = [":a_suffix"],
deps = [":a_suffix"],
shared_lib_name = "python_module.pyd",
)

cc_shared_library(
name = "a_so",
features = ["windows_export_all_symbols"],
roots = [":a_suffix"],
deps = [":a_suffix"],
)

cc_shared_library(
name = "diamond_so",
dynamic_deps = [":a_so"],
features = ["windows_export_all_symbols"],
roots = [":qux"],
deps = [":qux"],
)

cc_shared_library(
name = "diamond2_so",
dynamic_deps = [":a_so"],
features = ["windows_export_all_symbols"],
roots = [":qux2"],
deps = [":qux2"],
)

cc_binary(
Expand All @@ -88,19 +98,17 @@ cc_shared_library(
],
"//conditions:default": [],
}),
dynamic_deps = ["bar_so"],
dynamic_deps = [
"bar_so",
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so"
],
features = ["windows_export_all_symbols"],
preloaded_deps = ["preloaded_dep"],
roots = [
deps = [
"baz",
"foo",
"a_suffix",
],
static_deps = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux",
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2",
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:prebuilt",
],
user_link_flags = select({
"//src/conditions:linux": [
"-Wl,-rpath,kittens",
Expand Down Expand Up @@ -139,6 +147,7 @@ cc_library(
"qux",
"qux2",
"prebuilt",
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg"
],
)

Expand Down Expand Up @@ -190,18 +199,13 @@ cc_shared_library(
permissions = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:permissions",
],
roots = [
deps = [
"bar",
"bar2",
] + select({
":is_bazel": ["@test_repo//:bar"],
"//conditions:default": [],
}),
static_deps = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
"@test_repo//:bar",
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2",
],
user_link_flags = select({
"//src/conditions:linux": [
"-Wl,--version-script=$(location :bar.lds)",
Expand Down Expand Up @@ -346,7 +350,7 @@ filegroup(
cc_shared_library(
name = "direct_so_file",
features = ["windows_export_all_symbols"],
roots = [
deps = [
":direct_so_file_cc_lib",
],
)
Expand All @@ -367,7 +371,7 @@ filegroup(
cc_shared_library(
name = "renamed_so_file",
features = ["windows_export_all_symbols"],
roots = [
deps = [
":direct_so_file_cc_lib2",
],
shared_lib_name = "renamed_so_file.so",
Expand Down Expand Up @@ -398,6 +402,51 @@ cc_shared_library_permissions(
],
)

cc_library(
name = "static_lib_no_exporting",
srcs = [
"bar.cc",
"bar.h",
],
tags = ["NO_EXPORTING"],
)

cc_library(
name = "static_lib_exporting",
srcs = [
"bar2.cc",
"bar2.h",
],
)

cc_shared_library(
name = "lib_with_no_exporting_roots_1",
deps = [":static_lib_no_exporting"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots_2",
deps = [":static_lib_no_exporting"],
dynamic_deps = [":lib_with_no_exporting_roots_3"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots_3",
deps = [":static_lib_no_exporting"],
)

cc_shared_library(
name = "lib_with_no_exporting_roots",
deps = [
":static_lib_no_exporting",
":static_lib_exporting",
],
dynamic_deps = [
":lib_with_no_exporting_roots_1",
":lib_with_no_exporting_roots_2",
],
)

build_failure_test(
name = "two_dynamic_deps_same_export_in_so_test",
message = "Two shared libraries in dependencies export the same symbols",
Expand Down Expand Up @@ -433,16 +482,7 @@ runfiles_test(
target_under_test = ":python_test",
)

build_failure_test(
name = "static_deps_error_test",
messages = select({
":is_bazel": [
"@//:__subpackages__",
"@test_repo//:__subpackages__",
],
"//conditions:default": [
"@//:__subpackages__",
],
}),
target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:unaccounted_for_libs_so",
no_exporting_static_lib_test(
name = "no_exporting_static_lib_test",
target_under_test = ":lib_with_no_exporting_roots",
)
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,9 @@ cc_binary(
cc_shared_library(
name = "should_fail_shared_lib",
dynamic_deps = ["//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar_so"],
roots = [
deps = [
":intermediate",
],
static_deps = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
],
tags = TAGS,
)

Expand All @@ -37,7 +34,7 @@ cc_library(

cc_shared_library(
name = "permissions_fail_so",
roots = [
deps = [
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
],
tags = TAGS,
Expand Down Expand Up @@ -72,7 +69,7 @@ cc_shared_library(
":b_so",
":b2_so",
],
roots = [
deps = [
":a",
],
tags = TAGS,
Expand All @@ -90,46 +87,16 @@ cc_binary(

cc_shared_library(
name = "b_so",
roots = [
deps = [
":b",
],
tags = TAGS,
)

cc_shared_library(
name = "b2_so",
roots = [
deps = [
":b",
],
tags = TAGS,
)

cc_shared_library(
name = "unaccounted_for_libs_so",
roots = [
":d1",
],
tags = TAGS,
)

cc_library(
name = "d1",
srcs = ["empty.cc"],
deps = ["d2"],
)

cc_library(
name = "d2",
srcs = ["empty.cc"],
deps = [
"d3",
] + select({
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:is_bazel": ["@test_repo//:bar"],
"//conditions:default": [],
}),
)

cc_library(
name = "d3",
srcs = ["empty.cc"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,10 @@
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/direct_so_file_cc_lib2.h"
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/preloaded_dep.h"
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/qux.h"
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h"

int foo() {
diff_pkg();
bar();
baz();
qux();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,16 @@ def _debug_files_test_impl(ctx):
actual_files = []
for debug_file in target_under_test[OutputGroupInfo].rule_impl_debug_files.to_list():
actual_files.append(debug_file.basename)
expected_files = ["bar_so_exports.txt", "bar_so_link_once_static_libs.txt", "foo_so_exports.txt", "foo_so_link_once_static_libs.txt", "binary_link_once_static_libs.txt"]

expected_files = [
"bar_so_exports.txt",
"bar_so_link_once_static_libs.txt",
"diff_pkg_so_exports.txt",
"diff_pkg_so_link_once_static_libs.txt",
"foo_so_exports.txt",
"foo_so_link_once_static_libs.txt",
"binary_link_once_static_libs.txt",
]
asserts.equals(env, expected_files, actual_files)

return analysistest.end(env)
Expand All @@ -166,8 +175,10 @@ def _runfiles_test_impl(ctx):
expected_suffixes = [
"libfoo_so.so",
"libbar_so.so",
"libdiff_pkg_so.so",
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so",
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/renamed_so_file_copy.so",
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libdirect_so_file.so",
]
Expand Down Expand Up @@ -211,3 +222,21 @@ interface_library_output_group_test = analysistest.make(
"is_windows": attr.bool(),
},
)

def _no_exporting_static_lib_test_impl(ctx):
env = analysistest.begin(ctx)

target_under_test = analysistest.target_under_test(env)

# There should be only one exported file
actual_file = target_under_test[CcSharedLibraryInfo].exports[0]

# Sometimes "@" is prefixed in some test environments
expected = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:static_lib_exporting"
asserts.true(env, actual_file.endswith(expected))

return analysistest.end(env)

no_exporting_static_lib_test = analysistest.make(
_no_exporting_static_lib_test_impl,
)
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ cc_library(
hdrs = ["bar.h"],
)

cc_library(
name = "diff_pkg",
srcs = ["diff_pkg.cc"],
hdrs = ["diff_pkg.h"],
)

cc_shared_library(
name = "diff_pkg_so",
features = ["windows_export_all_symbols"],
deps = [
":diff_pkg",
],
)

cc_shared_library_permissions(
name = "permissions",
targets = [
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// 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.
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h"

int diff_pkg() { return 42; }
Loading

0 comments on commit 544b816

Please sign in to comment.