bazel icon indicating copy to clipboard operation
bazel copied to clipboard

Avoid deduplicating linkopts of cc_shared_library dependencies.

Open katzdm opened this issue 1 month ago • 6 comments

Linker inputs for dependencies of cc_shared_library are currently passing their linkopts as a depset, which leads to deduplication of the flags thereby passed. Consequently, a sequence of well-formed flags like -framework Security -framework IOKit gets collapsed to -framework Security IOKit; the linker then interprets IOKit as an object file that it's unable to locate, which yields a malformed link line.

Passing linkopts directly rather than wrapping it in depset appears to resolve the issue. I've added a test that replicates the original issue; since -framework isn't meaningful on all platforms, I've only enabled it for MacOS.

Note that the tests in this repository utilize the rules_cc repository rather than the built-in rules; therefore, I don't think the new test can be expected to pass until the change has been propagated there, and the controlling MODULE.bazel.lock entry has been updated. I've tested it locally by cating a local_path_override directive to the MODULE.bazel file that gets synthesized for these tests, in order to point the tests towards a local fork of rules_cc with this same change applied; under these conditions, the test passes.

Closes #27735.

katzdm avatar Nov 21 '25 13:11 katzdm

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Nov 21 '25 13:11 google-cla[bot]

@bazel-io fork 9.0.0

fmeum avatar Nov 21 '25 18:11 fmeum

@katzdm Could you please take a look at the failing checks?

bharadwaj08-one avatar Nov 24 '25 04:11 bharadwaj08-one

@bharadwaj08-one Please see the explanation in the PR description:

Note that the tests in this repository utilize the rules_cc repository rather than the built-in rules; therefore, I don't think the new test can be expected to pass until the change has been propagated there, and the controlling MODULE.bazel.lock entry has been updated. I've tested it locally by cating a local_path_override directive to the MODULE.bazel file that gets synthesized for these tests, in order to point the tests towards a local fork of rules_cc with this same change applied; under these conditions, the test passes.

katzdm avatar Nov 24 '25 04:11 katzdm

@bharadwaj08-one friendly ping here, just checking that there's no work needed on my end.

katzdm avatar Dec 02 '25 20:12 katzdm

Thanks for doing this. Not only does it fix an error (that I wasn't aware of), it also cleans up tech debt.

From https://bazel.build/rules/lib/toplevel/cc_common#create_linker_input:

user_link_flags None; or depset of strings; or sequence of strings; default is None User link flags passed as strings. Accepts either [String], [[String]] or depset(String). The latter is discouraged as it's only kept for compatibility purposes, the depset is flattened. If you want to propagate user_link_flags via unflattened depsets() wrap them in a LinkerInput so that they are not flattened till the end.

pzembrod avatar Dec 08 '25 10:12 pzembrod

@katzdm @pzembrod Could you please take a look at the presubmit failure? Thanks!

iancha1992 avatar Dec 17 '25 20:12 iancha1992

@iancha1992 Please see the explanation in the PR description:

Note that the tests in this repository utilize the rules_cc repository rather than the built-in rules; therefore, I don't think the new test can be expected to pass until the change has been propagated there, and the controlling MODULE.bazel.lock entry has been updated. I've tested it locally by cating a local_path_override directive to the MODULE.bazel file that gets synthesized for these tests, in order to point the tests towards a local fork of rules_cc with this same change applied; under these conditions, the test passes.

katzdm avatar Dec 17 '25 20:12 katzdm