Add pip3_import to run pip with python 3. #82
Conversation
This is a stop gap change until bazelbuild#82 is landed.
python/pip.bzl
Outdated
| } | ||
|
|
||
| def _pip_import_impl(repository_ctx): | ||
| _shared_pip_import_impl("python2", repository_ctx) |
There was a problem hiding this comment.
The default pip_import() should just use the system python. ie. s/python2/python/. A pip2_import() could be added if needed that used python2, I suppose.
|
This change is incredibly important to have any type of meaningful Python3 support. Let me know if I can help in any way to get this merged in, spent quite a few hours debugging an issue that arose from missing this. |
|
Sorry, completely forgot about this. Rebased and addressed the review. |
|
Is this PR still under review process? |
|
No idea, I've been using it since I put it up, I'm happy to rebase it again if necessary. |
|
@brandjon FYI |
|
We were facing a number of issues with native libraries like in The pip3_import from this PR works wonderfully. Please get this merged. |
|
👍 from me. But I'm just a random person who wants this too and reviewed the PR. :) |
|
yes, I still consider the best way to solve this until we get rid of this binary building mess (I like the approach in #81) |
|
Bump .. |
|
I'm no longer with Google, and I'm taking a (long) break from programming, so it'll have to be someone else, sorry. |
|
FYI anything that you'd send to Douglas you can send to me instead. I know it's been radio silence for a while in this repo, but I'll be taking some time in the next few days to merge some PRs and do some triaging. |
|
@brandjon Any updates here? |
|
+1 vote for getting this PR approved. have need for py3 support in bazel. |
| @@ -24,7 +24,8 @@ def _pip_import_impl(repository_ctx): | |||
|
|
|||
| # To see the output, pass: quiet=False | |||
| result = repository_ctx.execute([ | |||
| "python", repository_ctx.path(repository_ctx.attr._script), | |||
| python_interpreter, repository_ctx.path(repository_ctx.attr._script), | |||
There was a problem hiding this comment.
How about make python_interpreter as a string attribute instead? I was happily using your branch, until I suddenly want to upgrade to python3.6/python3.7 and don't want to overwrite the system's python3 which is pointing to python3.5. I think this approach is better than adding extra pip36_import() and pip37_import(), etc. One original pip_import with python_interpreter attribute is enough.
There was a problem hiding this comment.
That's a good idea. I won't have time to update this in the near future though, but if you send me a PR on top of this I'll merge it into this PR.
There was a problem hiding this comment.
Done. Please take a look at #158 . Thanks!
There was a problem hiding this comment.
Nice! I like it, it's much simpler than this PR!
|
Superseded by #158. |
While it's possible to select which interpreter to run python rules with, pip will always be run with
pythonfrom the system path.This uses the interpreter names as described in PEP 394 to be able to run pip in both python 2 and python 3 as an interim solution until
py_toolchainis finalised, since the values for the current mechanism in Bazel to configure the python interpreter at the invocation level (python_path,python_top) are not forwarded to the skylark context.See discussion in #62.