Rework the Bazel workaround's relationship with linkstatic
Due to a longstanding Bazel flaw
(https://github.com/bazelbuild/bazel/issues/22041), we need to split all
our mixed C/C++ targets in two. Ideally this split would behave as if
the Bazel flaw were fixed, with the split-out library statically linked
with the other source files. Accordingly, the helper macro sets
linkstatic = True.
It turns out linkstatic = True does not work this way. Bazel interprets
linkstatic such that, if A(test, linkshared) -> B(library) -> C(library,
linkstatic), C will be statically linked into A, not B. This is probably
to avoid diamond dependency problems but means it is not possible for a
cc_library split to be transparent, only cc_binary and cc_test.
So what is happening is that every cc_test that links libcrypto is
getting mlkem.cc statically linked into it, separate from the rest of
libcrypto! That means we're getting the worst of both worlds: worse
cache hit rate for tests that link libcrypto AND our C/C++ bits are
still not contained in the same library.
linkstatic = True on the helper is still valuable in cc_test and
cc_binary, but otherwise inherit the outer value.
Change-Id: I1089c58c6ddaa90c89efd8cdcebd88169b0236c8
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/71508
Reviewed-by: Bob Beck <bbe@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/util/util.bzl b/util/util.bzl
index 756957a..5028af6 100644
--- a/util/util.bzl
+++ b/util/util.bzl
@@ -83,6 +83,16 @@
"//conditions:default": ["-std=c11"] + gcc_copts_c,
})
+def linkstatic_kwargs(linkstatic):
+ # Although Bazel's documentation says linkstatic defaults to True or False
+ # for the various target types, this is not true. The defaults differ by
+ # platform non-Windows and True on Windows. There is now way to request the
+ # default except to omit the parameter, so we must use kwargs.
+ kwargs = {}
+ if linkstatic != None:
+ kwargs["linkstatic"] = linkstatic
+ return kwargs
+
def handle_mixed_c_cxx(
name,
copts,
@@ -90,6 +100,7 @@
internal_hdrs,
includes,
linkopts,
+ linkstatic,
srcs,
testonly,
alwayslink):
@@ -116,13 +127,11 @@
srcs = srcs_cxx + internal_hdrs,
copts = copts + boringssl_copts_cxx,
includes = includes,
- # This target only exists to be linked into the main library, so
- # always link it statically.
- linkstatic = True,
linkopts = linkopts,
deps = deps,
testonly = testonly,
alwayslink = alwayslink,
+ **linkstatic_kwargs(linkstatic),
)
# Build the remainder as a C-only target.
@@ -168,16 +177,6 @@
"//conditions:default": asm_srcs,
})
-def linkstatic_kwargs(linkstatic):
- # Although Bazel's documentation says linkstatic defaults to True or False
- # for the various target types, this is not true. The defaults differ by
- # platform non-Windows and True on Windows. There is now way to request the
- # default except to omit the parameter, so we must use kwargs.
- kwargs = {}
- if linkstatic != None:
- kwargs["linkstatic"] = linkstatic
- return kwargs
-
def bssl_cc_library(
name,
asm_srcs = [],
@@ -199,6 +198,14 @@
internal_hdrs = hdrs + internal_hdrs,
includes = includes,
linkopts = linkopts,
+ # Ideally we would set linkstatic = True to statically link the helper
+ # library into main cc_library. But Bazel interprets linkstatic such
+ # that, if A(test, linkshared) -> B(library) -> C(library, linkstatic),
+ # C will be statically linked into A, not B. This is probably to avoid
+ # diamond dependency problems but means linkstatic does not help us make
+ # this function transparent. Instead, just pass along the linkstatic
+ # nature of the main library.
+ linkstatic = linkstatic,
srcs = srcs,
testonly = testonly,
alwayslink = alwayslink,
@@ -250,6 +257,10 @@
deps = deps,
internal_hdrs = [],
includes = includes,
+ # If it weren't for https://github.com/bazelbuild/bazel/issues/22041,
+ # the split library be part of `srcs` and linked statically. Set
+ # linkstatic to match.
+ linkstatic = True,
linkopts = linkopts,
srcs = srcs,
testonly = testonly,
@@ -288,6 +299,10 @@
deps = deps,
internal_hdrs = [],
includes = includes,
+ # If it weren't for https://github.com/bazelbuild/bazel/issues/22041,
+ # the split library be part of `srcs` and linked statically. Set
+ # linkstatic to match.
+ linkstatic = True,
linkopts = linkopts,
srcs = srcs,
testonly = True,
