[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
fweikert · Feb 8, 2023 · 544b816 · 544b816
1 parent 27bc896
commit 544b816
Show file tree

Hide file tree

Showing 9 changed files with 271 additions and 145 deletions.
diff --git a/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl b/src/main/starlark/builtins_bzl/common/cc/cc_binary.bzl
@@ -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):
@@ -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,

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
diff --git a/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test b/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/BUILD.builtin_test
@@ -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"
 
@@ -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(
@@ -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",
@@ -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"
     ],
 )
 
@@ -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)",
@@ -346,7 +350,7 @@ filegroup(
 cc_shared_library(
     name = "direct_so_file",
     features = ["windows_export_all_symbols"],
-    roots = [
+    deps = [
         ":direct_so_file_cc_lib",
     ],
 )
@@ -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",
@@ -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",
@@ -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",
 )
diff --git a/...iltins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test b/...iltins_bzl/cc/cc_shared_library/test_cc_shared_library/failing_targets/BUILD.builtin_test
@@ -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,
 )
 
@@ -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,
@@ -72,7 +69,7 @@ cc_shared_library(
         ":b_so",
         ":b2_so",
     ],
-    roots = [
+    deps = [
         ":a",
     ],
     tags = TAGS,
@@ -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"],
-)
diff --git a/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc b/src/main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/foo.cc
@@ -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();

diff --git a/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl b/...tarlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library/starlark_tests.bzl
@@ -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)
@@ -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",
     ]
@@ -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,
+)
diff --git a/...arlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test b/...arlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/BUILD.builtin_test
@@ -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 = [

diff --git a/...main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc b/...main/starlark/tests/builtins_bzl/cc/cc_shared_library/test_cc_shared_library3/diff_pkg.cc
@@ -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; }