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

py_binary doesn't automatically pick a Python runtime matching the requested version #4815

Open
afking opened this Issue Mar 9, 2018 · 8 comments

Comments

Projects
None yet
7 participants
@afking
Copy link

afking commented Mar 9, 2018

Description of the problem / feature request:

Running py_test() and py_binary() rules under bazel doesn't select python3 when only srcs_version="PY3", default_python_version="PY3" attributes are set on the rules. How do you specify python2/3 builds?

Bugs: what's the simplest, easiest way to reproduce this bug? Please provide a minimal example if possible.

I wrote an integration test: afking@5a6feef

bazel.log

What operating system are you running Bazel on?

Mac

What's the output of bazel info release?

release 0.10.0-homebrew

What's the output of git remote get-url origin ; git rev-parse master ; git rev-parse HEAD ?

git@github.com:afking/bazel.git
d3e102c9fb62f4416099dc9f097a5f207aa7d442
5a6feef7953d2fe664e7a3444a36c9a875416913

Have you found anything relevant by searching the web?

#3871
#3517
bazelbuild/rules_python#62

@buchgr

This comment has been minimized.

Copy link
Contributor

buchgr commented Mar 16, 2018

@lberki can you take this please?

@brandjon

This comment has been minimized.

Copy link
Member

brandjon commented Sep 7, 2018

A proper solution would probably involve allowing py_runtime to be used to specify both py2 and py3 interpreters, and allowing individual py_binary rules to declare their runtimes (probably by exposing the hidden py_interpreter attribute).

But in the meantime, a quicker and simpler solution would probably be to make the default behavior (where py_interpreter / --python_top is not given) simply choose shell commands "python2" or "python3" depending on the python mode.

@brandjon

This comment has been minimized.

Copy link
Member

brandjon commented Feb 5, 2019

The proper fix for this bug is to implement py_toolchain [edit: now py_runtime_pair] and to create a default toolchain that sensibly locates the system's installed Python 2 and Python 3 runtimes.

Until that's implemented, the workaround is to dispatch to the system interpreters in a py_runtime rule using select(), and point to that py_runtime in --python_top. The best way to do this from 0.23 onward is as follows:

py_runtime(
    name = "myruntime",
    interpreter_path = select({
        # Update paths as appropriate for your system.
        "@bazel_tools//tools/python:PY2": "/usr/bin/python2",
        "@bazel_tools//tools/python:PY3": "/usr/bin/python3",
    }),
    files = [],
)

Then run your build with the flag --python_top=//path/to/the:myruntime.

bazel-io pushed a commit that referenced this issue Feb 7, 2019

Add caveat to documentation warning of Python version bug
This puts users on notice that even when they set the Python version correctly at loading/analysis time, the default behavior will still incorrectly use the same Python runtime for both PY2 and PY3 at execution time (bug #4815).

This warning was requested by users and may save them some time and frustration until #4815 is fixed.

RELNOTES: None
PiperOrigin-RevId: 232880171
@Toxicable

This comment has been minimized.

Copy link

Toxicable commented Feb 17, 2019

I could be totally missing something about how Python works here, so feel free to correct me.
But is there a reason why we're relying on the system having python installed at all?
Couldn't you just have a python binary downloaded similar to how rules_nodejs does it.
https://github.com/bazelbuild/rules_nodejs#installation-with-a-manually-specified-version-of-nodejs-and-yarn
This will make it much easier to use the python rules across platforms or even the same systems
Also allowing you to specific exactly what python version is being used by the repository rather than just going with whatever the system has installed

@brandjon

This comment has been minimized.

Copy link
Member

brandjon commented Feb 19, 2019

At the moment we don't bundle a specific standard Python interpreter with Bazel. We would need an interpreter for each platform that Bazel supports, and we'd have to worry about coordinating upgrades to this version.

We don't tend to bundle tools with Bazel and its rule sets in other languages either, e.g. there's no gcc included in Bazel.

However, as a user you can include a Python interpreter in your repo and reference it via a py_runtime rule. Then you'd pass --python_top=//path/to/that:py_runtime to make the Python rules all use that interpreter instead of the system default. (This mechanism will soon change when toolchains are implemented, so that different runtimes can be used for different platforms.) In this way, you can have your py_binary target executing using a vendored (built-from-source or checked-in) Python interpreter, rather than the system one. However, a system interpreter is still needed to execute the Python stub script that launches your py_binary target.

Having a workspace rule fetch a Python interpreter sounds like a reasonable feature off the top of my head. Created #7467.

@Toxicable

This comment has been minimized.

Copy link

Toxicable commented Feb 20, 2019

Ah yes, I didn't mean that bazel should ship an interpreter itself.
But instead as you mentioned; a way to fetch it.

Your plan for fetching it should work perfect for us, we have a lot of systems that don't necessarily have Python on them and would prefer avoiding any transient issues due to version skews.

@depthwise

This comment has been minimized.

Copy link

depthwise commented Mar 1, 2019

This is especially bad when building Python programs (or Subpar *.par's) for e.g. RaspberryPi. You build and copy your program, and then it barfs because it uses Python2 instead of 3 even though everything in the build tells it to use Python3.

@brandjon brandjon changed the title Bazel run/test python 3 rules py_binary doesn't automatically pick a Python runtime matching the requested version Mar 14, 2019

@brandjon brandjon added P1 and removed P2 labels Mar 14, 2019

toastwaffle added a commit to toastwaffle/LiME that referenced this issue Mar 21, 2019

bazel-io pushed a commit that referenced this issue Mar 26, 2019

Implement autodetecting Python toolchain
This replaces the stub default Python toolchain with one that actually locates the target platform's Python interpreter at runtime. Try it out with

    bazel build //some_py_binary --experimental_use_python_toolchains

and note that, unlike before (#4815), the correct Python interpreter gets invoked by default regardless of whether you specify `--python_version=PY2` or `--python_version=PY3`.

This toolchain is only intended as a last resort, if the user doesn't define and register a better toolchain (that satisfies the target platform constraints).

Work toward #7375 and #4815. Follow-up work needed to add a test (#7843) and windows support (#7844).

RELNOTES: None
PiperOrigin-RevId: 240417315
@brandjon

This comment has been minimized.

Copy link
Member

brandjon commented Apr 1, 2019

A solution to this is implemented in Bazel 0.25 if --incompatible_use_python_toolchains is enabled. Windows support is tracked in #7844.

emusand added a commit to emusand/bazel that referenced this issue Apr 16, 2019

Implement autodetecting Python toolchain
This replaces the stub default Python toolchain with one that actually locates the target platform's Python interpreter at runtime. Try it out with

    bazel build //some_py_binary --experimental_use_python_toolchains

and note that, unlike before (bazelbuild#4815), the correct Python interpreter gets invoked by default regardless of whether you specify `--python_version=PY2` or `--python_version=PY3`.

This toolchain is only intended as a last resort, if the user doesn't define and register a better toolchain (that satisfies the target platform constraints).

Work toward bazelbuild#7375 and bazelbuild#4815. Follow-up work needed to add a test (bazelbuild#7843) and windows support (bazelbuild#7844).

RELNOTES: None
PiperOrigin-RevId: 240417315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.