Work around a workaround of a workaround of a Bazel bug
This fixes an issue where :ssl_test in the Bazel build ran no tests.
(The CMake build was fine.)
Bazel has no way to handle C and C++ sources need different flags
(https://github.com/bazelbuild/bazel/issues/22041), so our Bazel rules
transparently split mixed C/C++ targets in two.
Bazel silently turns all static libraries into shared libraries for
tests. This means that, even if we set linkstatic = True on the
split-out library, the two halves don't end up in the same shared
object. This is because if A(test) -> B(lib) -> C(lib) and C is
linkstatic, C is statically linked into A, not B. This is probably to
avoid diamond dependency issues. From what I can tell, there is no clean
way to say "B can be made into a shared library but please statically
link C into it, because C will never be referenced except by way of B".
(This means our use of linkshared is wrong. The next CL will try to redo
that.)
This Bazel behavior does not recognize that static and shared libraries
have very, very different symbol handling. In particular, our assembly
files needed to be in the same shared object as OPENSSL_ia32cap_P, so
all this required another workaround in
https://boringssl-review.googlesource.com/c/boringssl/+/70690
This, in turn, triggered this latest issue:
GoogleTest relies on static initializers of individual object files to
register tests. This does not work with linker dead code elimination and
needs --whole-archive, or alwayslink in Bazel parlance. The most recent
Bazel workaround required the C++ sources be the ones that were
extracted, so they lost the --whole-archive behavior. As a result,
:ssl_test silently ran no tests!
Work around this latest Bazel-induced problem by setting alwayslink on
cc_test helpers.
All this would go away if Bazel just fixed
https://github.com/bazelbuild/bazel/issues/22041
(We can probably revert
https://boringssl-review.googlesource.com/c/boringssl/+/70690 now, but
either way we should probably set alwayslink=True on the helpers so that
the build is not sensitive to which were extracted.)
Change-Id: I10745e1bcfe91ac929f154e66093b29723efc576
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71507
Reviewed-by: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Commit-Queue: Bob Beck <bbe@google.com>
diff --git a/util/util.bzl b/util/util.bzl
index 83ddff6..756957a 100644
--- a/util/util.bzl
+++ b/util/util.bzl
@@ -91,7 +91,8 @@
includes,
linkopts,
srcs,
- testonly):
+ testonly,
+ alwayslink):
"""
Works around https://github.com/bazelbuild/bazel/issues/22041. Determines
whether a target contains C, C++, or both. If the target is multi-language,
@@ -121,6 +122,7 @@
linkopts = linkopts,
deps = deps,
testonly = testonly,
+ alwayslink = alwayslink,
)
# Build the remainder as a C-only target.
@@ -188,6 +190,7 @@
linkstatic = None,
srcs = [],
testonly = False,
+ alwayslink = False,
visibility = []):
copts, deps, srcs = handle_mixed_c_cxx(
name = name,
@@ -198,6 +201,7 @@
linkopts = linkopts,
srcs = srcs,
testonly = testonly,
+ alwayslink = alwayslink,
)
# BoringSSL's notion of internal headers are slightly different from
@@ -217,6 +221,7 @@
linkopts = linkopts,
deps = deps,
testonly = testonly,
+ alwayslink = alwayslink,
**linkstatic_kwargs(linkstatic)
)
@@ -248,6 +253,9 @@
linkopts = linkopts,
srcs = srcs,
testonly = testonly,
+ # TODO(davidben): Should this be alwayslink = True? How does Bazel treat
+ # the real cc_binary.srcs?
+ alwayslink = False,
)
cc_binary(
@@ -268,7 +276,6 @@
asm_srcs = [],
data = [],
size = "medium",
- internal_hdrs = [],
copts = [],
includes = [],
linkopts = [],
@@ -284,6 +291,9 @@
linkopts = linkopts,
srcs = srcs,
testonly = True,
+ # If any sources get extracted, they must always be linked, otherwise
+ # tests will be dropped.
+ alwayslink = True,
)
cc_test(
