Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CcSkylarkApiProvider does not expose alwayslink libraries #7033

Closed
steeve opened this issue Jan 3, 2019 · 6 comments
Closed

CcSkylarkApiProvider does not expose alwayslink libraries #7033

steeve opened this issue Jan 3, 2019 · 6 comments
Assignees
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: feature request

Comments

@steeve
Copy link
Contributor

steeve commented Jan 3, 2019

Description of the problem / feature request:

CcSkylarkApiProvider should expose cc_libraryies with alwayslink = True, so that it is possible to properly add -Wl,--whole-source when we need to build the link flag ourselves, such as in rules_go.

What's the output of bazel info release?

release 0.21.0

Any other information, logs, or outputs that you want to share?

Ongoing dicussion at bazel-contrib/rules_go#1879

@jayconrod
Copy link
Contributor

cc @hlopko

Just to comment on why this would be useful, Go rules like go_library allow cc_library dependencies through the cdeps attribute. The libraries are eventually passed to the linker through go_binary and go_test rules.

For some libraries, it's necessary to pass -Wl,--whole-archive and -Wl,--no-whole-archive before and after the library file. This is needed if a library contains an unreferenced object file with a constructor that has side effects.

Setting alwayslink = 1 on cc_library communicates the need for these flags to the build system, but Starlark rules can't see whether this flag is set, so rules like go_binary don't know whether to pass the flags above. We're considering passing -Wl,--whole-archive for all libraries, but we're concerned this will bloat binary size.

@aiuto aiuto added team-Rules-CPP Issues for C++ rules untriaged labels Jan 7, 2019
@hlopko
Copy link
Member

hlopko commented Jan 9, 2019

With bazel 0.22 we'll introduce a new Starlark API that contains library_to_link which contains alwayslink field. Docs are being written. Relevant is also the issue for incompatible flag removing the CcSkylarkApiProvider: #7036.

Pedro, pls close once docs are there and there are no more comments. Thanks!

@hlopko hlopko added type: feature request P2 We'll consider working on this in future. (Assignee optional) and removed untriaged labels Jan 9, 2019
@hlopko
Copy link
Member

hlopko commented Jan 10, 2019

I apologize, it will only be in the Bazel 0.23, the commit was submitted today: 94e5016. Thanks!

@hlopko hlopko closed this as completed Jan 10, 2019
@jayconrod
Copy link
Contributor

@hlopko Sorry to resurrect this old issue, but is this flag exposed anywhere in the CcInfo provider? cc.linking_context.user_link_flags sounded promising, but it appears to be empty in a simple example.

The problem is that go_library and other rules may depend on cc_libraries via a cdeps attribute. We keep track of all the libraries in the dependency tree, and we pass them to the Go linker, which invokes the C/C++ linker within go_binary. For the libraries with alwayslink = 1, we need to wrap the linker arguments with -Wl,--whole-archive.

I'm not sure how to tell if a cc_library has alwayslink = 1 set from its CcInfo provider though. Is there a way? Let me know if I should open a new issue.

@oquenchil
Copy link
Contributor

You would get the libraries_to_link from the linking_context. Each library_to_link has an alwayslink field:https://docs.bazel.build/versions/master/skylark/lib/LibraryToLink.html

@jayconrod
Copy link
Contributor

@oquenchil Thanks, this should work. I initially skipped over that field because I thought it was deprecated, but after reading #8118, it looks like it's only the list return value that's deprecated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 We'll consider working on this in future. (Assignee optional) team-Rules-CPP Issues for C++ rules type: feature request
Projects
None yet
Development

No branches or pull requests

5 participants