-
Notifications
You must be signed in to change notification settings - Fork 4.4k
Expose main_output on cc_common.link #23838
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
Expose main_output on cc_common.link #23838
Conversation
|
I didn't notice anything in the code about issues that need to be solved before this makes sense to be public, so I was hoping this PR could serve as the discussion point for that, cc @comius |
|
There's a lot of tech debt around this code, so I wouldn't expose it. It's mostly related to Apple and Android rules. There's a design doc about the ideal end state: https://docs.google.com/document/d/1zm1UOftT2xHQiNNxNO7XU_BOn2KrXjFlx5tl4QBVjV0/edit#heading=h.5mcn15i0e1ch I have an idea how to makes things nicer even without it, but I'll only be able to do it once Starlark cc_common.link lands. Currently link is feature complete, passing Hyrum depo-wide testing, but it has a large regression, which I just didn't have time to look into. |
|
thanks for the context. AFAICT in the open source world, rules_apple doesn't ever use This must have diverged internally? (even the Also worth noting that it looks like internally google has solved the python case that I am trying to solve using this API
bazel/src/main/starlark/builtins_bzl/common/python/py_executable.bzl Lines 472 to 473 in 99434b1
|
|
@comius since it's been a while has there been any movement on your end around this stuff? Looks to me like rules_python is still relying on these private APIs for the same reason, and now they're exposed through a new python specific shim for that reasons bazel-contrib/rules_python@6607a4a#diff-c93321fbbb76c6d72593e7c24e6b7d460e7a92dace1a7fd73123232e7a590bc0R525-R529 I still have this case internally but also have a new case where rules_mojo would like to provide a For other folks hitting this issue you can workaround the private API checks like this: def _foo(rctx):
rctx.file("BUILD.bazel", "")
rctx.file("foo.bzl", """\
def link(**kwargs):
return cc_common.link(**kwargs)
""")
foo = repository_rule(
implementation = _foo,
)# MODULE.bazel
foo = use_repo_rule("//:foo.bzl", "foo")
foo(name = "build_bazel_rules_android") |
03fc3c0 to
a74e26a
Compare
This field is useful for many situations, such as when you want to create a native python extension, which doesn't follow the standard library naming conventions.
a74e26a to
6741203
Compare
|
past 26760a0 you need a new hack, instead of using a separate repository you need the bzl file calling the private API to be at a specific path. doing |
I tried that approach in aherrmann/rules_zig@5b5dfc6. It does indeed seem to work for targets within the same module, but I'm still seeing failures in the e2e tests that load rules_zig as an external module. Am I missing something? @keith have you seen that new link-hack approach work for use-sites of rules_mojo, i.e. other modules that depend on rules_mojo, or only in rules_mojo itself? |
|
I poked around the new change for a while and couldn't find a workaround. I think our best bet is to try and get the rules_cc change merged and released. In rules_mojo the feature is somewhat optional so I put the rules_cc patch there and downstream users would have to apply it. |
|
It wasn't my intention, but I also don't see a workaround that applies past my commit. |
|
I've opened a PR to add rules_zig to the allowlist bazelbuild/rules_cc#589. |
This field is useful for many situations, such as when you want to
create a native python extension, which doesn't follow the standard
library naming conventions.