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.

Sign up for GitHub

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

Jump to bottom

cc_deps should use the new Starlark C++ API #31

Closed
siddharthab opened this issue Feb 2, 2019 · 5 comments
Closed

cc_deps should use the new Starlark C++ API #31

siddharthab opened this issue Feb 2, 2019 · 5 comments

Comments

@siddharthab
Copy link
Collaborator

See bazelbuild/bazel#4570 for progress on the API.

It is not quite ready yet, but this issue will track work on using the API to solve our long standing problem of obtaining PIC.
@Gawainus
Copy link

@siddharthab Hi Thanks for writing this repo. I am trying to use it to do a Rcpp wrapper around some c++ code. The cc_deps were specified and it was observed that the cc dependencies were compiled when I run bazel build. But there is no .so lib emitted in the final rpkg, just the R wrapper itself. Is this related to the "ong standing problem of obtaining PIC" you mentioned?

If not can you be a bit more explicit about how to use c++ dependencies in the doc? Many thanks.

@siddharthab
Copy link
Collaborator Author

@Gawainus Do you have a src directory in your package with some C/C++ code? You will need to have that much for R to actually compile a dynamically linked library for you and include it in your package.

@Gawainus
Copy link

@siddharthab No. To respect the existing code structure. I just did cc_deps = ["//some/path:lib"]. You mean I need to put all the C/C++ code in src? Or I just need some thing to trigger the compilation?

@siddharthab
Copy link
Collaborator Author

You just need something to trigger the compilation.

@siddharthab
Copy link
Collaborator Author

I think this is completely resolved as part of c143bca. I will open a new issue for future API changes when they happen.

grailbot pushed a commit to grailbio/recordio that referenced this issue Oct 29, 2019
bazel: delete -fPIC where manually specified
35870fc 
Summary:
We used to specify `-fPIC` in all cc builds which were dependencies of R
packages because that is a requirement for linking in shared libraries,
and bazel did not provide a way to specify a dependency on a PIC .a
library.

With the resolution of grailbio/rules_r#31,
R rules for bazel now explicitly state their preference for a PIC build,
and so we don't have to force all builds to be PIC.

This also allows us to use the default `cc_proto_library` rules instead
of the experimental ones from the protobuf repo.

Test Plan:
R package builds and tests pass. They used to fail before without
`-fPIC`.

Reviewers: ysaito

Reviewed By: ysaito

Maniphest Tasks: T12955

Differential Revision: https://phabricator.grailbio.com/D31313

fbshipit-source-id: caafb4a
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@siddharthab @Gawainus