Skip to content

Commit 544b816

Browse files
authored
[6.1.0] Fix cc_binary bug related to cc_shared_library on Windows and prepare for future removal of --experimental_cc_shared_library flag (bazelbuild#17445)
* Cherrypicks 9815b76, 68aad18 and 4ed6327 from HEAD * Add missing in comma in test BUILD file
1 parent 27bc896 commit 544b816

File tree

9 files changed

+271
-145
lines changed

9 files changed

+271
-145
lines changed

src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl

+2-2
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ def _create_transitive_linking_actions(
526526
win_def_file = win_def_file,
527527
)
528528
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)
529-
return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files)
529+
return (cc_linking_outputs, cc_launcher_info, rule_impl_debug_files, cc_linking_context)
530530

531531
def _use_pic(ctx, cc_toolchain, cpp_config, feature_configuration):
532532
if _is_link_shared(ctx):
@@ -735,7 +735,7 @@ def cc_binary_impl(ctx, additional_linkopts):
735735
if extra_link_time_libraries != None:
736736
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))
737737

738-
cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files = _create_transitive_linking_actions(
738+
cc_linking_outputs_binary, cc_launcher_info, rule_impl_debug_files, deps_cc_linking_context = _create_transitive_linking_actions(
739739
ctx,
740740
cc_toolchain,
741741
feature_configuration,

src/main/starlark/builtins_bzl/common/cc/experimental_cc_shared_library.bzl

+111-72
Large diffs are not rendered by default.

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test

+72-32
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,14 @@
1-
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")
1+
load(
2+
":starlark_tests.bzl",
3+
"additional_inputs_test",
4+
"build_failure_test",
5+
"debug_files_test",
6+
"interface_library_output_group_test",
7+
"linking_suffix_test",
8+
"paths_test",
9+
"runfiles_test",
10+
"no_exporting_static_lib_test",
11+
)
212

313
LINKABLE_MORE_THAN_ONCE = "LINKABLE_MORE_THAN_ONCE"
414

@@ -42,28 +52,28 @@ cc_binary(
4252
cc_shared_library(
4353
name = "python_module",
4454
features = ["windows_export_all_symbols"],
45-
roots = [":a_suffix"],
55+
deps = [":a_suffix"],
4656
shared_lib_name = "python_module.pyd",
4757
)
4858

4959
cc_shared_library(
5060
name = "a_so",
5161
features = ["windows_export_all_symbols"],
52-
roots = [":a_suffix"],
62+
deps = [":a_suffix"],
5363
)
5464

5565
cc_shared_library(
5666
name = "diamond_so",
5767
dynamic_deps = [":a_so"],
5868
features = ["windows_export_all_symbols"],
59-
roots = [":qux"],
69+
deps = [":qux"],
6070
)
6171

6272
cc_shared_library(
6373
name = "diamond2_so",
6474
dynamic_deps = [":a_so"],
6575
features = ["windows_export_all_symbols"],
66-
roots = [":qux2"],
76+
deps = [":qux2"],
6777
)
6878

6979
cc_binary(
@@ -88,19 +98,17 @@ cc_shared_library(
8898
],
8999
"//conditions:default": [],
90100
}),
91-
dynamic_deps = ["bar_so"],
101+
dynamic_deps = [
102+
"bar_so",
103+
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg_so"
104+
],
92105
features = ["windows_export_all_symbols"],
93106
preloaded_deps = ["preloaded_dep"],
94-
roots = [
107+
deps = [
95108
"baz",
96109
"foo",
97110
"a_suffix",
98111
],
99-
static_deps = [
100-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux",
101-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2",
102-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:prebuilt",
103-
],
104112
user_link_flags = select({
105113
"//src/conditions:linux": [
106114
"-Wl,-rpath,kittens",
@@ -139,6 +147,7 @@ cc_library(
139147
"qux",
140148
"qux2",
141149
"prebuilt",
150+
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:diff_pkg"
142151
],
143152
)
144153

@@ -190,18 +199,13 @@ cc_shared_library(
190199
permissions = [
191200
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:permissions",
192201
],
193-
roots = [
202+
deps = [
194203
"bar",
195204
"bar2",
196205
] + select({
197206
":is_bazel": ["@test_repo//:bar"],
198207
"//conditions:default": [],
199208
}),
200-
static_deps = [
201-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
202-
"@test_repo//:bar",
203-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:qux2",
204-
],
205209
user_link_flags = select({
206210
"//src/conditions:linux": [
207211
"-Wl,--version-script=$(location :bar.lds)",
@@ -346,7 +350,7 @@ filegroup(
346350
cc_shared_library(
347351
name = "direct_so_file",
348352
features = ["windows_export_all_symbols"],
349-
roots = [
353+
deps = [
350354
":direct_so_file_cc_lib",
351355
],
352356
)
@@ -367,7 +371,7 @@ filegroup(
367371
cc_shared_library(
368372
name = "renamed_so_file",
369373
features = ["windows_export_all_symbols"],
370-
roots = [
374+
deps = [
371375
":direct_so_file_cc_lib2",
372376
],
373377
shared_lib_name = "renamed_so_file.so",
@@ -398,6 +402,51 @@ cc_shared_library_permissions(
398402
],
399403
)
400404

405+
cc_library(
406+
name = "static_lib_no_exporting",
407+
srcs = [
408+
"bar.cc",
409+
"bar.h",
410+
],
411+
tags = ["NO_EXPORTING"],
412+
)
413+
414+
cc_library(
415+
name = "static_lib_exporting",
416+
srcs = [
417+
"bar2.cc",
418+
"bar2.h",
419+
],
420+
)
421+
422+
cc_shared_library(
423+
name = "lib_with_no_exporting_roots_1",
424+
deps = [":static_lib_no_exporting"],
425+
)
426+
427+
cc_shared_library(
428+
name = "lib_with_no_exporting_roots_2",
429+
deps = [":static_lib_no_exporting"],
430+
dynamic_deps = [":lib_with_no_exporting_roots_3"],
431+
)
432+
433+
cc_shared_library(
434+
name = "lib_with_no_exporting_roots_3",
435+
deps = [":static_lib_no_exporting"],
436+
)
437+
438+
cc_shared_library(
439+
name = "lib_with_no_exporting_roots",
440+
deps = [
441+
":static_lib_no_exporting",
442+
":static_lib_exporting",
443+
],
444+
dynamic_deps = [
445+
":lib_with_no_exporting_roots_1",
446+
":lib_with_no_exporting_roots_2",
447+
],
448+
)
449+
401450
build_failure_test(
402451
name = "two_dynamic_deps_same_export_in_so_test",
403452
message = "Two shared libraries in dependencies export the same symbols",
@@ -433,16 +482,7 @@ runfiles_test(
433482
target_under_test = ":python_test",
434483
)
435484

436-
build_failure_test(
437-
name = "static_deps_error_test",
438-
messages = select({
439-
":is_bazel": [
440-
"@//:__subpackages__",
441-
"@test_repo//:__subpackages__",
442-
],
443-
"//conditions:default": [
444-
"@//:__subpackages__",
445-
],
446-
}),
447-
target_under_test = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets:unaccounted_for_libs_so",
485+
no_exporting_static_lib_test(
486+
name = "no_exporting_static_lib_test",
487+
target_under_test = ":lib_with_no_exporting_roots",
448488
)

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test

+5-38
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,9 @@ cc_binary(
1919
cc_shared_library(
2020
name = "should_fail_shared_lib",
2121
dynamic_deps = ["//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:bar_so"],
22-
roots = [
22+
deps = [
2323
":intermediate",
2424
],
25-
static_deps = [
26-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:barX",
27-
],
2825
tags = TAGS,
2926
)
3027

@@ -37,7 +34,7 @@ cc_library(
3734

3835
cc_shared_library(
3936
name = "permissions_fail_so",
40-
roots = [
37+
deps = [
4138
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3:bar",
4239
],
4340
tags = TAGS,
@@ -72,7 +69,7 @@ cc_shared_library(
7269
":b_so",
7370
":b2_so",
7471
],
75-
roots = [
72+
deps = [
7673
":a",
7774
],
7875
tags = TAGS,
@@ -90,46 +87,16 @@ cc_binary(
9087

9188
cc_shared_library(
9289
name = "b_so",
93-
roots = [
90+
deps = [
9491
":b",
9592
],
9693
tags = TAGS,
9794
)
9895

9996
cc_shared_library(
10097
name = "b2_so",
101-
roots = [
98+
deps = [
10299
":b",
103100
],
104101
tags = TAGS,
105102
)
106-
107-
cc_shared_library(
108-
name = "unaccounted_for_libs_so",
109-
roots = [
110-
":d1",
111-
],
112-
tags = TAGS,
113-
)
114-
115-
cc_library(
116-
name = "d1",
117-
srcs = ["empty.cc"],
118-
deps = ["d2"],
119-
)
120-
121-
cc_library(
122-
name = "d2",
123-
srcs = ["empty.cc"],
124-
deps = [
125-
"d3",
126-
] + select({
127-
"//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:is_bazel": ["@test_repo//:bar"],
128-
"//conditions:default": [],
129-
}),
130-
)
131-
132-
cc_library(
133-
name = "d3",
134-
srcs = ["empty.cc"],
135-
)

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc

+2
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,10 @@
1717
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/direct_so_file_cc_lib2.h"
1818
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/preloaded_dep.h"
1919
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/qux.h"
20+
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h"
2021

2122
int foo() {
23+
diff_pkg();
2224
bar();
2325
baz();
2426
qux();

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl

+30-1
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,16 @@ def _debug_files_test_impl(ctx):
150150
actual_files = []
151151
for debug_file in target_under_test[OutputGroupInfo].rule_impl_debug_files.to_list():
152152
actual_files.append(debug_file.basename)
153-
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"]
153+
154+
expected_files = [
155+
"bar_so_exports.txt",
156+
"bar_so_link_once_static_libs.txt",
157+
"diff_pkg_so_exports.txt",
158+
"diff_pkg_so_link_once_static_libs.txt",
159+
"foo_so_exports.txt",
160+
"foo_so_link_once_static_libs.txt",
161+
"binary_link_once_static_libs.txt",
162+
]
154163
asserts.equals(env, expected_files, actual_files)
155164

156165
return analysistest.end(env)
@@ -166,8 +175,10 @@ def _runfiles_test_impl(ctx):
166175
expected_suffixes = [
167176
"libfoo_so.so",
168177
"libbar_so.so",
178+
"libdiff_pkg_so.so",
169179
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibfoo_Uso.so",
170180
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary_Slibbar_Uso.so",
181+
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary3_Slibdiff_Upkg_Uso.so",
171182
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/renamed_so_file_copy.so",
172183
"Smain_Sstarlark_Stests_Sbuiltins_Ubzl_Scc_Scc_Ushared_Ulibrary_Stest_Ucc_Ushared_Ulibrary/libdirect_so_file.so",
173184
]
@@ -211,3 +222,21 @@ interface_library_output_group_test = analysistest.make(
211222
"is_windows": attr.bool(),
212223
},
213224
)
225+
226+
def _no_exporting_static_lib_test_impl(ctx):
227+
env = analysistest.begin(ctx)
228+
229+
target_under_test = analysistest.target_under_test(env)
230+
231+
# There should be only one exported file
232+
actual_file = target_under_test[CcSharedLibraryInfo].exports[0]
233+
234+
# Sometimes "@" is prefixed in some test environments
235+
expected = "//src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library:static_lib_exporting"
236+
asserts.true(env, actual_file.endswith(expected))
237+
238+
return analysistest.end(env)
239+
240+
no_exporting_static_lib_test = analysistest.make(
241+
_no_exporting_static_lib_test_impl,
242+
)

src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test

+14
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,20 @@ cc_library(
88
hdrs = ["bar.h"],
99
)
1010

11+
cc_library(
12+
name = "diff_pkg",
13+
srcs = ["diff_pkg.cc"],
14+
hdrs = ["diff_pkg.h"],
15+
)
16+
17+
cc_shared_library(
18+
name = "diff_pkg_so",
19+
features = ["windows_export_all_symbols"],
20+
deps = [
21+
":diff_pkg",
22+
],
23+
)
24+
1125
cc_shared_library_permissions(
1226
name = "permissions",
1327
targets = [
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
// Copyright 2023 The Bazel Authors. All rights reserved.
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
#include "src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.h"
15+
16+
int diff_pkg() { return 42; }

0 commit comments

Comments
 (0)