Add opt-out from merging cc_lib object files in bindgen (#2959)

This is a ~rollback of
#2590, as it turns out the
current behavior is incomatible with
https://bazel.build/docs/user-manual#dynamic-mode (which is very hard to
use in the OSS because of
bazelbuild/bazel#4135).

In short, the dynamic mode works as follows:

* Each cc_library produces object files that are used to produce
* a static library (often not materialized but objects get passed as
`--start-lib`/`--end-lib` to the linker),
* a shared library (only containing the objects, not transitive
dependencies),
* and an interface library (produced from .so by "stripping function
bodies").
* When linking binaries, by default we use the static linking.
* When linking tests, by default we use interface libraries for linking,
and shared libraries for test execution.

The motivation for this is to avoid lengthy linking actions when all
that has changed between previous build is the method body (therefore
Bazel produces the same interface library, so we don't need to reexecute
linking action).

We do not support dynamic mode in rules_rust yet.

The current bindgen behavior works by taking object from `cc_lib`, and
merging them into the `rlib` output. When dynamic_mode is on, we now
have a linking action that has the rlib early on the command line, and
then interface libraries of dependencies of the `cc_lib`, and then the
interface library of the `cc_lib`. For reasons, lld prefers statically
defined symbols, it sees that the `rlib` defines the same symbols as
`cc_lib` but statically, and errors out with `backward reference
detected`, even though `cc_lib` is positioned correctly.

The fix for us is to not merge objects.

bindgen/private/bindgen.bzl

@@ -78,6 +78,9 @@ def rust_bindgen_library(
     ):
         if shared in kwargs:
             bindgen_kwargs.update({shared: kwargs[shared]})
+    if "merge_cc_lib_objects_into_rlib" in kwargs:
+        bindgen_kwargs.update({"merge_cc_lib_objects_into_rlib": kwargs["merge_cc_lib_objects_into_rlib"]})
+        kwargs.pop("merge_cc_lib_objects_into_rlib")
 
     rust_bindgen(
         name = name + "__bindgen",
@@ -334,22 +337,28 @@ def _rust_bindgen_impl(ctx):
         toolchain = None,
     )
 
-    return [
-        _generate_cc_link_build_info(ctx, cc_lib),
-        # As in https://github.com/bazelbuild/rules_rust/pull/2361, we want
-        # to link cc_lib to the direct parent (rlib) using `-lstatic=<cc_lib>`
-        # rustc flag. Hence, we do not need to provide the whole CcInfo of
-        # cc_lib because it will cause the downstream binary to link the cc_lib
-        # again. The CcInfo here only contains the custom link flags (i.e.
-        # linkopts attribute) specified by users in cc_lib.
-        CcInfo(
-            linking_context = cc_common.create_linking_context(
-                linker_inputs = depset([cc_common.create_linker_input(
-                    owner = ctx.label,
-                    user_link_flags = _get_user_link_flags(cc_lib),
-                )]),
+    if ctx.attr.merge_cc_lib_objects_into_rlib:
+        providers = [
+            _generate_cc_link_build_info(ctx, cc_lib),
+            # As in https://github.com/bazelbuild/rules_rust/pull/2361, we want
+            # to link cc_lib to the direct parent (rlib) using `-lstatic=<cc_lib>`
+            # rustc flag. Hence, we do not need to provide the whole CcInfo of
+            # cc_lib because it will cause the downstream binary to link the cc_lib
+            # again. The CcInfo here only contains the custom link flags (i.e.
+            # linkopts attribute) specified by users in cc_lib.
+            CcInfo(
+                linking_context = cc_common.create_linking_context(
+                    linker_inputs = depset([cc_common.create_linker_input(
+                        owner = ctx.label,
+                        user_link_flags = _get_user_link_flags(cc_lib),
+                    )]),
+                ),
             ),
-        ),
+        ]
+    else:
+        providers = [cc_lib[CcInfo]]
+
+    return providers + [
         OutputGroupInfo(
             bindgen_bindings = depset([output]),
             bindgen_c_thunks = depset(([c_output] if wrap_static_fns else [])),
@@ -376,6 +385,11 @@ rust_bindgen = rule(
             allow_single_file = True,
             mandatory = True,
         ),
+        "merge_cc_lib_objects_into_rlib": attr.bool(
+            doc = ("When True, objects from `cc_lib` will be copied into the `rlib` archive produced by " +
+                   "the rust_library that depends on this `rust_bindgen` rule (using `BuildInfo` provider)"),
+            default = True,
+        ),
         "wrap_static_fns": attr.bool(
             doc = "Whether to create a separate .c file for static fns. Requires nightly toolchain, and a header that actually needs this feature (otherwise bindgen won't generate the file and Bazel complains).",
             default = False,

docs/src/flatten.md

@@ -329,7 +329,8 @@ is available under the key `dsym_folder` in `OutputGroupInfo`.
 ## rust_bindgen
 
 <pre>
-rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-wrap_static_fns">wrap_static_fns</a>)
+rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-merge_cc_lib_objects_into_rlib">merge_cc_lib_objects_into_rlib</a>,
+             <a href="#rust_bindgen-wrap_static_fns">wrap_static_fns</a>)
 </pre>
 
 Generates a rust source file from a cc_library and a header.
@@ -344,6 +345,7 @@ Generates a rust source file from a cc_library and a header.
 | <a id="rust_bindgen-cc_lib"></a>cc_lib |  The cc_library that contains the `.h` file. This is used to find the transitive includes.   | <a href="https://bazel.build/concepts/labels">Label</a> | required |  |
 | <a id="rust_bindgen-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   | List of strings | optional |  `[]`  |
 | <a id="rust_bindgen-header"></a>header |  The `.h` file to generate bindings for.   | <a href="https://bazel.build/concepts/labels">Label</a> | required |  |
+| <a id="rust_bindgen-merge_cc_lib_objects_into_rlib"></a>merge_cc_lib_objects_into_rlib |  When True, objects from `cc_lib` will be copied into the `rlib` archive produced by the rust_library that depends on this `rust_bindgen` rule (using `BuildInfo` provider)   | Boolean | optional |  `True`  |
 | <a id="rust_bindgen-wrap_static_fns"></a>wrap_static_fns |  Whether to create a separate .c file for static fns. Requires nightly toolchain, and a header that actually needs this feature (otherwise bindgen won't generate the file and Bazel complains).   | Boolean | optional |  `False`  |
 
 

docs/src/rust_bindgen.md

@@ -53,7 +53,8 @@ toolchains following the instructions for [rust_bindgen_toolchain](#rust_bindgen
 ## rust_bindgen
 
 <pre>
-rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-wrap_static_fns">wrap_static_fns</a>)
+rust_bindgen(<a href="#rust_bindgen-name">name</a>, <a href="#rust_bindgen-bindgen_flags">bindgen_flags</a>, <a href="#rust_bindgen-cc_lib">cc_lib</a>, <a href="#rust_bindgen-clang_flags">clang_flags</a>, <a href="#rust_bindgen-header">header</a>, <a href="#rust_bindgen-merge_cc_lib_objects_into_rlib">merge_cc_lib_objects_into_rlib</a>,
+             <a href="#rust_bindgen-wrap_static_fns">wrap_static_fns</a>)
 </pre>
 
 Generates a rust source file from a cc_library and a header.
@@ -68,6 +69,7 @@ Generates a rust source file from a cc_library and a header.
 | <a id="rust_bindgen-cc_lib"></a>cc_lib |  The cc_library that contains the `.h` file. This is used to find the transitive includes.   | <a href="https://bazel.build/concepts/labels">Label</a> | required |  |
 | <a id="rust_bindgen-clang_flags"></a>clang_flags |  Flags to pass directly to the clang executable.   | List of strings | optional |  `[]`  |
 | <a id="rust_bindgen-header"></a>header |  The `.h` file to generate bindings for.   | <a href="https://bazel.build/concepts/labels">Label</a> | required |  |
+| <a id="rust_bindgen-merge_cc_lib_objects_into_rlib"></a>merge_cc_lib_objects_into_rlib |  When True, objects from `cc_lib` will be copied into the `rlib` archive produced by the rust_library that depends on this `rust_bindgen` rule (using `BuildInfo` provider)   | Boolean | optional |  `True`  |
 | <a id="rust_bindgen-wrap_static_fns"></a>wrap_static_fns |  Whether to create a separate .c file for static fns. Requires nightly toolchain, and a header that actually needs this feature (otherwise bindgen won't generate the file and Bazel complains).   | Boolean | optional |  `False`  |
 
 

test/bindgen/bindgen_test.bzl

@@ -44,10 +44,60 @@ def _test_cc_linkopt(name):
         impl = _test_cc_linkopt_impl,
     )
 
+def _test_cc_lib_object_merging_impl(env, target):
+    env.expect.that_int(len(target.actions)).is_greater_than(2)
+    env.expect.that_action(target.actions[0]).mnemonic().contains("RustBindgen")
+    env.expect.that_action(target.actions[1]).mnemonic().contains("FileWrite")
+    env.expect.that_action(target.actions[1]).content().contains("-lstatic=test_cc_lib_object_merging_cc")
+    env.expect.that_action(target.actions[2]).mnemonic().contains("FileWrite")
+    env.expect.that_action(target.actions[2]).content().contains("-Lnative=")
+
+def _test_cc_lib_object_merging_disabled_impl(env, target):
+    # no FileWrite actions writing arg files registered
+    env.expect.that_int(len(target.actions)).is_greater_than(0)
+    env.expect.that_action(target.actions[0]).mnemonic().contains("RustBindgen")
+
+def _test_cc_lib_object_merging(name):
+    cc_library(name = name + "_cc", hdrs = ["simple.h"], srcs = ["simple.cc"])
+
+    rust_bindgen_library(
+        name = name + "_rust_bindgen",
+        cc_lib = name + "_cc",
+        header = "simple.h",
+        tags = ["manual"],
+        edition = "2021",
+    )
+
+    analysis_test(
+        name = name,
+        target = name + "_rust_bindgen__bindgen",
+        impl = _test_cc_lib_object_merging_impl,
+    )
+
+def _test_cc_lib_object_merging_disabled(name):
+    cc_library(name = name + "_cc", hdrs = ["simple.h"], srcs = ["simple.cc"])
+
+    rust_bindgen_library(
+        name = name + "_rust_bindgen",
+        cc_lib = name + "_cc",
+        header = "simple.h",
+        tags = ["manual"],
+        merge_cc_lib_objects_into_rlib = False,
+        edition = "2021",
+    )
+
+    analysis_test(
+        name = name,
+        target = name + "_rust_bindgen__bindgen",
+        impl = _test_cc_lib_object_merging_disabled_impl,
+    )
+
 def bindgen_test_suite(name):
     test_suite(
         name = name,
         tests = [
             _test_cc_linkopt,
+            _test_cc_lib_object_merging,
+            _test_cc_lib_object_merging_disabled,
         ],
     )