Add additional_linker_inputs option to cc_library rule
#18952
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nicholasjng
changed the title
additional_linker_inputs option to cc_library ruleadditional_linker_inputs option to cc_library rule
github-actions
bot
added
the
awaiting-review
nicholasjng
commented
Jul 17, 2023
nicholasjng
added a commit
to nicholasjng/benchmark
that referenced
this pull request
Jul 20, 2023
This change needs bazelbuild/bazel#18952 to be merged first. Fixes macOS linkage of GBM's nanobind bindings on macOS by supplying a linker response file instead of `-undefined dynamic_lookup`. The latter has since been deprecated on macOS.
This is achieved by the following related changes: 1) Moving the `additional_linker_inputs` attribute up from the `cc_binary_base` rule to the `cc_rule`. The rationale here is that any rule that allows `linkopts` to be set should also support `additional_linker_inputs`. 2) Adding the `additional_linker_inputs` attr to the `cc_library` rule context. This was copied verbatim from the existing `cc_binary` attribute list. 3) Appending the `ctx.files.additional_linker_inputs` list to the list of linker scripts wherever they are passed as `additional_inputs` in the `cc_library.bzl` builtin file corresponding to the `cc_library` rule.
buildbreaker2021
approved these changes
Aug 16, 2023
buildbreaker2021
added
awaiting-PR-merge
github-actions
bot
removed
the
awaiting-PR-merge
Contributor
|
@bazel-io flag |
bazel-io
added
the
potential release blocker
Member
|
@brentleyjones I just want to make sure since google/benchmark#1636 was mentioned.. Do we want this to be part of release-6.4.0 or some other release? cc: @bazelbuild/triage |
Contributor
|
I believe this is an additive change and can go into 6.4.0. Do you agree @nicholasjng? |
Contributor
Author
Fully agreed, would appreciate to see this in 6.4.0 if possible |
Member
|
@bazel-io fork 6.4.0 |
bazel-io
removed
the
potential release blocker
iancha1992
pushed a commit
to iancha1992/bazel
that referenced
this pull request
Aug 16, 2023
This is achieved by the following related changes: 1) Moving the `additional_linker_inputs` attribute up from the `cc_binary_base` rule to the `cc_rule`. The rationale here is that any rule that allows `linkopts` to be set should also support `additional_linker_inputs`. 2) Adding the `additional_linker_inputs` attr to the `cc_library` rule context. This was copied verbatim from the existing `cc_binary` attribute list. 3) Appending the `ctx.files.additional_linker_inputs` list to the list of linker scripts wherever they are passed as `additional_inputs` in the `cc_library.bzl` builtin file corresponding to the `cc_library` rule. ---------------------- For context on this PR, see issue bazelbuild#17788 as well as the original [Google group discussion](https://groups.google.com/g/bazel-discuss/c/1VFvNBDqzVU/m/F5UmYO-RAAAJ), which inspired the feature request. Resolves bazelbuild#17788. Closes bazelbuild#18952. PiperOrigin-RevId: 557505297 Change-Id: I4811b60e102c6426697efcb202296fe77a3d5f25
iancha1992
added a commit
that referenced
this pull request
Aug 22, 2023
…19264) This is achieved by the following related changes: 1) Moving the `additional_linker_inputs` attribute up from the `cc_binary_base` rule to the `cc_rule`. The rationale here is that any rule that allows `linkopts` to be set should also support `additional_linker_inputs`. 2) Adding the `additional_linker_inputs` attr to the `cc_library` rule context. This was copied verbatim from the existing `cc_binary` attribute list. 3) Appending the `ctx.files.additional_linker_inputs` list to the list of linker scripts wherever they are passed as `additional_inputs` in the `cc_library.bzl` builtin file corresponding to the `cc_library` rule. ---------------------- For context on this PR, see issue #17788 as well as the original [Google group discussion](https://groups.google.com/g/bazel-discuss/c/1VFvNBDqzVU/m/F5UmYO-RAAAJ), which inspired the feature request. Resolves #17788. Closes #18952. Commit 0589995 PiperOrigin-RevId: 557505297 Change-Id: I4811b60e102c6426697efcb202296fe77a3d5f25 Co-authored-by: Nicholas Junge <[email protected]>
nicholasjng
added a commit
to nicholasjng/benchmark
that referenced
this pull request
Oct 23, 2023
This change needs bazelbuild/bazel#18952 to be merged first. Fixes macOS linkage of GBM's nanobind bindings on macOS by supplying a linker response file instead of `-undefined dynamic_lookup`. The latter has since been deprecated on macOS.
nicholasjng
added a commit
to nicholasjng/benchmark
that referenced
this pull request
Oct 23, 2023
This change needs bazelbuild/bazel#18952 to be merged first. Fixes macOS linkage of GBM's nanobind bindings on macOS by supplying a linker response file instead of `-undefined dynamic_lookup`. The latter has since been deprecated on macOS.
dmah42
pushed a commit
to google/benchmark
that referenced
this pull request
Oct 24, 2023
* Change nanobind linkage to response file approach This change needs bazelbuild/bazel#18952 to be merged first. Fixes macOS linkage of GBM's nanobind bindings on macOS by supplying a linker response file instead of `-undefined dynamic_lookup`. The latter has since been deprecated on macOS. * Fix bazel_skylib checksum, bump skylib version in MODULE.bazel * Bump Bazel to version 6.4.0 for linker response file support
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is achieved by the following related changes:
Moving the
additional_linker_inputsattribute up from thecc_binary_baserule to thecc_rule. The rationale here is that any rule that allowslinkoptsto be set should also supportadditional_linker_inputs.Adding the
additional_linker_inputsattr to thecc_libraryrule context. This was copied verbatim from the existingcc_binaryattribute list.Appending the
ctx.files.additional_linker_inputslist to the list of linker scripts wherever they are passed asadditional_inputsin thecc_library.bzlbuiltin file corresponding to thecc_libraryrule.For context on this PR, see issue #17788 as well as the original Google group discussion, which inspired the feature request.
Resolves #17788.