bazelbuild / bazelbuild/rules_cc

Add binary link deps attribute to cc_library

Open
#194 2 comments 0 reactions 0 assignees View on GitHub
P3
Dominant language
Starlark
Stars
247
Forks
196
PR merge metrics
No merged PRs in 30d

Description

### Description of the problem / feature request:

I would like Bazel to provide a solution to the "linktime transitive dependency" problem. This is a problem commonly encountered in implementing low-level libraries like logging or assertions as part of larger middleware projects like [Pigweed](http://pigweed.dev). It's described in detail in the next section.

The proposed solution (described in more detail below, too) is to introduce a label-valued `binary_link_deps` attribute on `cc_library`, with the following semantics: Whenever a `cc_binary` transitively depends on a library with `binary_link_deps = [":sometarget"]`, add `":sometarget"` to the `link_extra_lib` of that `cc_binary`.

### Feature requests: what underlying problem are you trying to solve with this feature?

#### Problem description

Low-level libraries like Pigweed or Abseil define assertion and logging primitives. This naturally leads to dependency cycles: you would like to use string utilities, IO abstractions, and other libraries in _implementing_ logging and assertions; but the string utilities, IO abstractions, etc, themselves invoke logging and assertions in their implementation. How to have your cake (use assertions/logging throughout your project) and have it, too (use your project's libraries in implementing assertions/logging)?

Specific example of a dependency cycle from Pigweed (the names are target labels and arrows `deps` relations):

```
pw_assert -> pw_assert_basic -> pw_assert_basic_impl -> pw_assert_basic_handler -> //pw_string:builder -+
^ |
| |
+-------------------------------------------------------------------------------------------------+
```

One solution to this problem is to split up the assertion API (headers) and assertion implementation (source files) into separate build targets, and delete the dependency edge between them:

```
pw_assert -> pw_assert_basic pw_assert_basic_impl -> pw_assert_basic_handler -> //pw_string:builder -+
^ |
| |
+-------------------------------------------------------------------------------------------------+
```

There is now no cycle, but you have a different problem: when building a `cc_binary` that transitively depends on `pw_assert`, you will be missing the definitions in the source files unless you add a dep on `pw_assert_basic_impl` somewhere. So, your binary will compile, but will fail to link.

#### Workarounds

There are some workarounds for this problem, but they're not satisfactory:

1. You can add deps on `pw_assert_basic_impl` manually to any `cc_binary` or `cc_test` targets. This is not great: the linker failure is fairly obscure, and this is certain to regularly trip up developers, especially ones new to the project.
2. You can add `pw_assert_basic_impl` to [link_extra_lib](https://bazel.build/reference/be/c-cpp#cc_binary.link_extra_lib). In practice you'd probably set `@bazel_tools//tools/cpp:link_extra_lib` in `bazelrc`. This is better, but you'll end up linking the implementation library even when building targets that don't need it (have no dep on the assertion library at all). This is especially debilitating in a large monorepo.

#### Impact

Solving this problem would be very useful to Pigweed; https://issues.pigweed.dev/234877642 is the tracking issue on our side.

But my understanding is that the same problem affects [Abseil](https://github.com/abseil/abseil-cpp). Abseil has a similar problem to Pigweed: they define a logging library, but that library has dependencies on other parts of Abseil that themselves want to do logging. They use a separate macro, [ABSL_INTERNAL_LOG](https://chromium.googlesource.com/external/github.com/abseil/abseil-cpp/+/HEAD/absl/base/internal/raw_logging.h#65), in other Abseil modules to avoid a cyclic dependency. The function actually called by that macro is registered by setting [a pointer](https://chromium.googlesource.com/external/github.com/abseil/abseil-cpp/+/HEAD/absl/base/internal/raw_logging.h#169); by default it points to the minimal `RawLog` implementation, but that can be overridden if a richer log implementation is available at link time. _But how does one ensure that the richer implementation is, in fact, available?_ This is left to the user to ensure, by calling [RegisterInternalLogFunction](https://github.com/abseil/abseil-cpp/blob/a74b796ab3f114f6991479c9ad9e4c1a0dad3a4b/absl/base/internal/raw_logging.h#L209C6-L209C33) in a dependency of their binary and test targets.

More broadly, this could be useful in any collection of libraries that implements low-level utility routines (like logging and assertions) to be used in other parts of the collection.

#### Proposed solution

Introduce a label-valued `binary_link_deps` attribute on `cc_library`, with the following semantics: Whenever a `cc_binary` transitively depends on a library with `binary_link_deps = [":sometarget"]`, add `":sometarget"` to the `link_extra_lib` of that `cc_binary`.

This resolves the problem: instead of just removing the dependency edge between `pw_assert_basic` and `pw_assert_basic_impl` (the header target and the source target), we replace it with the `binary_link_deps` relation. This does not make `pw_assert_basic_impl` a dependency of `pw_assert_basic`, and so does not produce a cycle. But it does ensure that any binary that depends on `pw_assert_basic` will be linked with `pw_assert_basic_impl`.

This proposal is for illustrative purposes only. I've not prototyped it and am certainly open to other solutions!

Contributor guide

Open the contributing guide

Research direction

Start by studying the proposed binary_link_deps semantics for cc_library and the existing link_extra_lib behavior for cc_binary. Review how rules_cc models transitive C++ dependencies and identify the tests covering linking behavior. Done means an agreed design and implementation that adds the requested binary-only linking behavior without introducing dependency cycles.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp
Domain
build-system
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.