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.

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

Python 3 support #1580

Closed
abergmeier-dsfishlabs opened this issue Jul 28, 2016 · 22 comments
Closed

Python 3 support #1580

abergmeier-dsfishlabs opened this issue Jul 28, 2016 · 22 comments
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Python Native rules for Python type: process

Comments

@abergmeier-dsfishlabs
Copy link
Contributor

abergmeier-dsfishlabs commented Jul 28, 2016

I would like for this issue to collect all issues concerning Python 3.

As far as I know there are multiple problems:

  1. The supported values for srcs_version are PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY.
    • PY3 is documented wrong. It actually handles similar to PY2, which is fine.
    • PY3ONLY is not even documented, although it mostly is what a lot of Python 3 users want.
  2. By default Bazel always executes Python 2. Which might be an ok choice for now for PY2, PY3, PY2AND3 and PY2ONLY. It is however totally wrong for PY3ONLY. (Python 3 support? #1406)
    It seems like checkSourceIsCompatible is used instead of a hypothetical selectPythonVersionForSource.
    • Question there would be why it is not using Python 3.
    • How big is Googles interest in Python 3 support on a scale of 0 (want it now) to 5 (who cares)?
    • Would anyone inside of Google be available for mentorship for extending Python 3 support?
  3. Is there a way for Bazel to use a Python version embedded into a repository?
  4. --force_python and --host_force_python are given to users, yet they are not documented.
  5. py_binary wrapper uses Python 2 although it may not even be present (python stub_template.txt shouldn't assume /usr/bin/python always exists #544)
  6. py_binary wrapper spawns a new process for executing python scripts. I do know that we are not talking about great performance with Python but that seems a bit excessive.
  7. pkg_tar seems to internally use Python2 only. Does not work with 3.
  8. Python cannot be used as a label (Make --python3_path accept a label #2244)
  9. pyd cannot be created straight-forward (#2497)
  10. Support embeddable Python for Windows (Support embeddable Python #2509)
@damienmg damienmg added category: rules > python type: bug P2 We'll consider working on this in future. (Assignee optional) labels Jul 28, 2016
@damienmg damienmg added this to the 1.0 milestone Jul 28, 2016
@abergmeier-dsfishlabs
Copy link
Contributor Author

abergmeier-dsfishlabs commented Jul 28, 2016

Hope I collected all input from #1406, #544 and #1446.
Updated main post.

@bsilver8192
Copy link
Contributor

Some updates on 6: 3bed4af changed to exec instead of a whole separate process, but 679e911 added a (not default yet) code path which brings it back.

@ahyangyi
Copy link

I noticed that many wrapper scripts in Bazel are python 2 only. Would be useful if I can bundle a minimal python implementation so that our Bazel repository will work on various distributions.

@ahyangyi
Copy link

As an example, PackageTar in Bazel (tools/build_defs/pkg) is Python 2 only.

@benley
Copy link
Contributor

benley commented Sep 12, 2016

Is bazel capable of using a python interpreter that comes from a label yet? That was the main blocker the last time I looked into doing something similar, iirc.

@ahyangyi
Copy link

ahyangyi commented Sep 13, 2016

@benley A possible workaround: use a sh_binary that depends on your python script and your python interpreter label, and a wrapper shell script that feeds the script into the interpreter.

If that works you can do make it into a general function or rule so you can reuse the code and keep things organized until the devs add the proper supports.

@abergmeier-dsfishlabs
Copy link
Contributor Author

Integrated info of #2352

@abergmeier-dsfishlabs
Copy link
Contributor Author

Is bazel capable of using a python interpreter that comes from a label yet? That was the main blocker the last time I looked into doing something similar, iirc.

Nope, still not working. Integrating info of #2244

@abergmeier-dsfishlabs
Copy link
Contributor Author

Integrated info of #2497

@abergmeier-dsfishlabs
Copy link
Contributor Author

Integrated info of #2509

@tux21b
Copy link

tux21b commented Mar 7, 2017

It looks like I have encountered the same problem as described in #2352, which had been merged into this ticket. But I think this ticket is sightly misleading. I am not interested into supporting Python 3 (which might be a long term goal), I am interested in getting bazel to run right now using Python 2. Editing the $PATH so that "python" also points to py2 doesn't work either. Any suggestions?

~ bazel build ttbackup:debian-data --force_python=py2only --host_force_python=py2 --ignore_unsupported_sandboxing --strategy=PackageTar=standalone --verbose_failures --python2_path=/usr/bin/python2 --python3_path=/usr/bin/python3
INFO: Found 1 target...
ERROR: /home/christoph/projects/gopath/src/mgit.at/xxx/ttbackup/BUILD:45:1: null failed: build_tar failed: error executing command
  (cd /home/christoph/.cache/bazel/_bazel_christoph/9f259bee499666daa5e76d94ef621852/execroot/xxx && \
  exec env - \
  bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/build_tar '--flagfile=bazel-out/local-fastbuild/bin/ttbackup/debian-data.args'): com.google.devtools.build.lib.shell.BadExitStatusException: Process exited with status 1.
Traceback (most recent call last):
  File "/home/christoph/.cache/bazel/_bazel_christoph/9f259bee499666daa5e76d94ef621852/execroot/xxx/bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/build_tar.runfiles/__main__/../bazel_tools/tools/build_defs/pkg/build_tar.py", line 22, in <module>
    from tools.build_defs.pkg import archive
  File "/home/christoph/.cache/bazel/_bazel_christoph/9f259bee499666daa5e76d94ef621852/execroot/xxx/bazel-out/host/bin/external/bazel_tools/tools/build_defs/pkg/build_tar.runfiles/bazel_tools/tools/build_defs/pkg/archive.py", line 17, in <module>
    from StringIO import StringIO
ModuleNotFoundError: No module named 'StringIO'
Target //ttbackup:debian-data failed to build
INFO: Elapsed time: 0.259s, Critical Path: 0.12s
command exited with code 1

@abergmeier-dsfishlabs
Copy link
Contributor Author

@tux21b I would then perhaps create a new Issue since this here is really intended for Python3 specifically.

@skycoop
Copy link

skycoop commented Mar 7, 2017

Is there any word yet on plans for this? This is breaking parts of Tensorflow, which explicitly supports Py3.

In some cases, simply switching to using six imports would solve the problem. For example, using from six import StringIO on this line would solve the problem and maintain compatibility with Python 2.

@MarkIennaco
Copy link

bazel-0.5.0-rc2 still has this behavior.
Any timeline for making Python 3 a first class target?

@MarkusTeufelberger
Copy link
Contributor

It might be enough in the bazel/tools/build_defs/pkg/archive.py case to simply import io.StringIO (which exists on Python2 as well and is not used much in the rules) or to also catch "ModuleNotFoundError"s for Python3.

@treuherz
Copy link
Contributor

I've made some changes so that everything in bazel/tools/build_defs/pkg/archive.py passes tests in python 2.7 and 3, here treuherz/bazel@d6d0889. Since this is pretty small should I go ahead and open a PR or should I run it by the mailing list first?

@abergmeier
Copy link
Contributor

@treuherz Everything in that direction is great. Be aware, though that it is best to also have tests, that ensure, that it continuous to work on both (Google internally does not seem to test Python 3).
Summoning @damienmg

@treuherz
Copy link
Contributor

treuherz commented Oct 2, 2017

Yeah, the need for that occurred to me as I was doing it. Don't know enough about how google manages testing behind the scenes, and also don't know the rest of Bazel's codebase well enough to know how much work it would take to get it to pass if the test suite was run against both interpreters, tox-style.

I'll submit the PR with that caveat so it's at least ready.

@gregestren
Copy link
Contributor

See 8b5bf1f for some relevant steps forward (particularly proper honoring of default_python_version).

bazel-io pushed a commit that referenced this issue Jan 26, 2018
Use BytesIO instead of StringIO, change strings to bytes throughout the
archiving code. Needed to import from Six in a couple of places.

As discussed in #1580

Closes #3850.

PiperOrigin-RevId: 183429066
@brandjon
Copy link
Member

I've created a list of PY2/3 issues that I'm prioritizing in #6444, some of which are mentioned here. I'll keep this thread open as a useful index of issues but mark it P3.

@brandjon brandjon added P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Python Native rules for Python and removed P2 We'll consider working on this in future. (Assignee optional) category: rules > python labels Oct 18, 2018
@brandjon brandjon removed their assignment Oct 18, 2018
@meisterT meisterT removed this from the 1.0 milestone May 12, 2020
@aiuto
Copy link
Contributor

aiuto commented Oct 4, 2021

Friendly ping: I think this is effectively a dup of #6444. We should close this and leave that.
Thoughts?

@brandjon
Copy link
Member

brandjon commented Oct 5, 2021

Sure, we can ref this from #6444.

@brandjon brandjon closed this as completed Oct 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 We're not considering working on this, but happy to review a PR. (No assignee) team-Rules-Python Native rules for Python type: process
Projects
None yet
Development

No branches or pull requests