rules_cc
rules_cc copied to clipboard
Added cc_library_func(), a function for cc_library()-like semantics.
This is a high-level API that sits on top of the sandwich to provide cc_library()-like semantics.
The goal is for this API to stay stable even as the underlying Sandwich APIs evolve into their final form. We can add version detection as necessary if/when the old APIs are deprecated.
Ideally this would be whitelisted to use the sandwich APIs in the next release of Bazel.
The cc_library_func()
function in this PR seems to work, but I did notice once difference from a real cc_library()
(here libbar.a
is from my custom rule):
$ cat bazel-out/k8-opt/bin/examples/my_c_rule/main-2.params
bazel-out/k8-opt/bin/examples/my_c_rule/_objs/main/main.o
--start-lib
bazel-out/k8-opt/bin/examples/my_c_rule/_objs/baz/baz.o
--end-lib
bazel-out/k8-opt/bin/examples/my_c_rule/libbar.a
--start-lib
bazel-out/k8-opt/bin/examples/my_c_rule/_objs/foo/foo.o
--end-lib
-lstdc++
-lm
The real cc_library()
targets are linked by their .o files (surrounded in --start-lib
/--end-lib
) but my custom my_cc_library()
is linked by a .a
file. Did I do something wrong in my cc_library_func()
function? Or is this difference coming from the underlying API?
Why do you create your own static library action? Sandwich api will create one if needed.
Also, I could be convinced to have that function here in this repo, but with these prerequisites:
- We know it's good enough for cc_library. We don't want to maintain a separate function if we cannot use it or which we don't understand how our users are using.
- It's using the stable APIs. We don't have a backward compatibility story set for rules_cc yet, which some can interpret the wrong way which will lead to surprises when we break people. I.e. we don't want to provide a incremental migration story for this for 0.24.0 and 0.25, only for 0.26 and forward when the API will be stable.
- it's thoroughly tested.
And even after all these are fulfilled I'm thinking if adding a third layer to support is something we want to do and we understand implications of (we have rules as the first layer, then the Sandwich API as the second). There will eventually be a backwards compatibility policy (before we tell users to use this repo), which will protect users of the API from willy-nilly breaking changes and will offer migration instructions and a way to migrate incrementally.
Wdyt?
Why do you create your own static library action? Sandwich api will create one if needed.
I didn't know that. I don't understand the sandwich API very well, because I couldn't find any docs that explain things like this. I just copy+pasted snippets from other projects and iterated through trial and error util I got it working.
How do I fix this? I think I still need a linking context somehow? As a guess I tried using cc_common.create_linking_context_from_compilation_outputs
instead of doing all the archiver stuff, but that function doesn't appear to exist in 0.24.1, only in master.
It's using the stable APIs. [...] we don't want to provide a incremental migration story for this for 0.24.0 and 0.25, only for 0.26 and forward when the API will be stable.
My goal with this PR is to provide a stable API now, even though it is built on top of unstable APIs. I want the cc_library_func()
function to do version detection like this, if/when it is necessary for new Bazel versions:
if not hasattr(cc_common, create_linking_context_from_compilation_outputs):
# Bazel 0.24.1 code
elif <some predicate that detects Bazel 0.25>:
# Bazel 0.25 code.
# etc.
This would let users like me start using it now, without requiring any user migration for future Bazel versions (only this function would need to change).
Wdyt?
I need this API now. The main question is whether the function goes in my repo (https://github.com/protocolbuffers/upb) or yours.
If it goes in your repo, users besides me can take advantage of it. I understand that you might want to avoid offering another public API (which is your decision to make). If your other users are like me, they will prefer cc_library_func()
to cc_common.*
. It would also let users start using it now, even though the full sandwich API won't be available for several more months.
I unfortunately don't have much more time to spend on this right now. I'm happy to add some more tests or fix bugs, but I don't have the time to do a full-scale cc_library()
on top of this that supports all attributes and tests all the cases. I also can't wait for the full sandwich API to be available. If you're not able to accept this now, I understand, I'll just add it to my own repo instead.
I'm sorry you had to struggle through this without proper docs. Docs will be provided for the stable API, but because the experimental API was changing a lot it didn't make sense to write docs yet.
You can always create a linking context by hand, the constructor function is stable - https://docs.bazel.build/versions/master/skylark/lib/cc_common.html#create_linking_context. You can also ignore the Sandwich API completely and create your own compilation and linking actions. C++ toolchain API and C++ providers API are all you need and those are both stable. Some usage is linked from https://docs.bazel.build/versions/master/integrating-with-rules-cc.html.
I think the most responsible way forward is to keep cc_library_func
-like function in your repo in the meantime, and only move it to rules_cc when the code and test coverage are robust enough for users to use and for us to claim stability. I must say the idea of this helper function is growing on me, but only based on the stable APIs and coverage.
Thanks Josh for understanding!
You can always create a linking context by hand, the constructor function is stable
But how do I create the LibraryToLink
that I need for create_linking_context
? The create_library_to_link function wants a single File
for static_library
and pic_static_library
, which seems to imply that I have to compile all the .o
files into a single .a
file. How can I give it the list of .o
files instead?
You can also ignore the Sandwich API completely and create your own compilation and linking actions.
Earlier you mentioned that I would be missing a lot of functionality if I did this, so I'd prefer not to go this route.
I think the most responsible way forward is to keep cc_library_func-like function in your repo in the meantime
Ok, sounds good.
I must say the idea of this helper function is growing on me
I'm glad. :)
I'm not sure we need this, the API should stay fairly stable and I can imagine this function would just start adding more and more parameters until it matches the actual API. For example, it might be fairly common to need to pass a custom feature_configuration.
Adding my two cents, if you have an aspect that wants to make a cc_library type thing next to each target it runs on (like generating code for each proto_library), you can't use native.cc_library in an aspect
Closing stale PR. Starlark API is stable now.