Add initial support for rules_python. #65
Conversation
|
Hmm. GitHub won't let me assign @rickeylev as well. |
| bazel_dep(name = "rules_python", version = "0.27.0") | ||
|
|
||
| python = use_extension("@rules_python//python/extensions:python.bzl", "python") | ||
| use_repo(python, "pythons_hub") |
There was a problem hiding this comment.
The @pythons_hub is an internal detail of rules_python. It's basically just a registry so that it can auto-register the toolchains and make pip integration easier.
There was a problem hiding this comment.
I feared as much. Hence why I really wanted you to review. :)
| @@ -167,6 +168,23 @@ def _get_python_bin(repository_ctx): | |||
| python_bin = _norm_path(python_bin) | |||
| return python_bin | |||
|
|
|||
| if ( | |||
There was a problem hiding this comment.
The way this is trying to work is fundamentally flawed. The core issue is the repository phase can't know what interpreter a target will use, and it can't know the full set of toolchains registered. The @pythons_hub only keeps track of python.toolchain() calls, but that doesn't include any custom user toolchains.
I'm not really sure this will work, either. IIRC, the label isn't a canonical one (one that starts with '@@`, so it won't be able to resolve (you would have to use_repo() the apparent name)
From what I remember, these labels are also host specific. This is because they're just used to drive pip.parse() (which does various repository phase things, so has to have a host-capable interpreter available).
What you really want is to get this information from the toolchain directly. e.g., in the regular rule code, do something like
def _pybindthing_impl(ctx):
py_lib_path = ctx.toolchains["pything"].lib_path
ctx.actions.run(...stuff using py_lib_path...)
This should be pretty easy. If you can give a run down of the information needed, we can figure out how to make it available from the toolchains.
There was a problem hiding this comment.
Oh snap. I just realised that I still owe you a reply on bazelbuild/rules_python#824 (comment). :(
|
I'm going to close this PR since it isn't going to fly. |
Fixes #64.