bazelbuild / bazel

a fast, scalable, multi-language and extensible build system
https://bazel.build
Apache License 2.0
23.01k stars 4.03k forks source link

0.5.3 no longer allows to enforce Python3 #3517

Closed abergmeier-dsfishlabs closed 5 years ago

abergmeier-dsfishlabs commented 7 years ago

Description of the problem / feature request / question:

I have a python script, that worked fine in 0.5.2.

Switching to 0.5.3 running

import concurrent.futures

gives me:

ImportError: No module named concurrent.futures

Environment info

abergmeier-dsfishlabs commented 7 years ago

Printing paths before the error I get:

/<project>/fusion/python/gof3m/collect
/<bazelroot>/execroot/com_dsfishlabs_tools/bazel-out/local-py3-fastbuild/bin/fusion/python/collect.runfiles
/<bazelroot>/execroot/com_dsfishlabs_tools/bazel-out/local-py3-fastbuild/bin/fusion/python/collect.runfiles/com_dsfishlabs_tools
/usr/lib/python2.7
/usr/lib/python2.7/plat-x86_64-linux-gnu
/usr/lib/python2.7/lib-tk
/usr/lib/python2.7/lib-old
/usr/lib/python2.7/lib-dynload
/home/andreas/.local/lib/python2.7/site-packages
/usr/local/lib/python2.7/dist-packages
/usr/lib/python2.7/dist-packages
/usr/lib/python2.7/dist-packages/PILcompat
/usr/lib/python2.7/dist-packages/gtk-2.0
/usr/lib/python2.7/dist-packages/ubuntu-sso-client

which is obviously wrong since I enforced Python 3:

py_binary(
    ...
    default_python_version = "PY3",
    srcs_version = "PY3ONLY",

Regression is in 0.5.4rc2, too 👎

buchgr commented 7 years ago

There was some breaking change in 0.5.3, that unfortunately didn't make it to the release notes. I think you need to specify --python3_path in the .bazelrc file now? It might be a different issue that I am thinking of.

@meteorcloudy might know more?

abergmeier-dsfishlabs commented 7 years ago

@buchgr To quote the python3_path docs:

Local path to the Python3 executable. Deprecated, please use python_path or python_top instead.

So I guess you mean python_path?

buchgr commented 7 years ago

So apparently, those attributes don't work anymore and you now need to specify --python_path to point to your python executable.

I am in contact with relevant people... sorry :(

buchgr commented 7 years ago

It was an intentional breakage, that unfortunately did not make it into the release notes. We will provide a blog post and updated documentation soon.

abergmeier-dsfishlabs commented 7 years ago

Can you perhaps write a bit more here. I now tried to use py_runtime in combination with PY3ONLY and PY3 and it seems like it does not properly link the library into the sandbox!?

buchgr commented 7 years ago

My understanding is that starting with 0.5.3, bazel no longer knows or cares about the python version. You just point it to a python runtime and it will use it. The default_python_version argument and PY3ONLY and PY3 don't have any effect.

abergmeier-dsfishlabs commented 7 years ago

My understanding is that starting with 0.5.3, bazel no longer knows or cares about the python version

Sadly that is wrong.

The default_python_version argument and PY3ONLY and PY3 don't have any effect.

That may be correct. Please either remove them or add tests to validate every permutation. Still there is --force_python=PY3 and --host_force_python=PY3, which still trigger 2to3 and fun in 0.5.3.

Also it seems like py_runtime is not getting properly added to the sandbox.

Can you please make the dev in charge of Python write up how it is "supposed" to work now and I will file bugs then.

abergmeier-dsfishlabs commented 7 years ago

My understanding is that starting with 0.5.3, bazel no longer knows or cares about the python version

Is it then no longer possible to have both Python2 and Python3 in one repo?

duggelz commented 7 years ago

Any movement on this?

AustinSchuh commented 6 years ago

Is it then no longer possible to have both Python2 and Python3 in one repo?

I think I just ran into that when attempting to upgrade from 0.5.0 to 0.6.0 yesterday. We've got mixed python2 and python3 binaries in our build. I'd love to upgrade to python3 entirely, but not all of the libraries we use support python3 yet.

buchgr commented 6 years ago

@lberki can tell you more about plans for python maybe?

joshburkart commented 6 years ago

At the very minimum, I need to be able to have a single codebase with both Python 2 and Python 3 code, and then manually specify in the individual test/binary which interpreter to use. Is this possible somehow? A temporary workaround would be fine. A global flag is unsuitable, since I need to be able to do e.g. bazel test //....

(This is in particular necessitated by Google-developed Python 2-only libraries like Apache Beam.)

buchgr commented 6 years ago

@lberki can you take this please?

abergmeier commented 6 years ago

Sooo soonish, we will be a year later and still no explanation?

buchgr commented 6 years ago

friendly ping @lberki :)

AdamDorwart commented 6 years ago

I just wanted to offer a temporary workaround that we're using to allow multiple python runtimes in a single workspace. You can set your py_runtime to a decorator script that inspects the shebang on the first line of the script to invoke a particular interpreter. You could make this a simple shell script which someone had suggested in another thread I can't seem to find.

If you're using distroless images (py_image or py3_image) using a shell script might be an issue so I wrote a statically linked go application. This is pretty heavy handed but not so bad if you're already using go in your workspace. Hopefully either one of these options is viable until someone gets this sorted out.

.bazelrc

build --python_top=//:python-2-or-3

BUILD

load(
    "@io_bazel_rules_go//go:def.bzl",
    "go_binary",
)

go_binary(
    name = "python_decorator",
    srcs = ["python.go"],
    gc_linkopts = [
        "-linkmode",
        "external",
        "-extldflags",
        "-static",
    ],
)

py_runtime(
    name = "python-2-or-3",
    files = [],
    interpreter = ":python_decorator",
)

python.go

package main

import (
    "bufio"
    "log"
    "os"
    "strings"
    "syscall"
)

func python2Binary() string {
    return "/usr/bin/python"
}

func python3Binary() string {
    return "/usr/bin/python3"
}

func main() {
    if len(os.Args) < 2 {
        log.Fatal("No python script provided!")
    }
    file, err := os.Open(os.Args[1])
    if err != nil {
        log.Fatal(err)
    }
    defer file.Close()

    scanner := bufio.NewScanner(file)
    scanner.Scan()
    firstLine := scanner.Text()
    if err := scanner.Err(); err != nil {
        log.Fatal(err)
    }

    switch {
    case strings.Contains(firstLine, "python3"):
        if err := syscall.Exec(python3Binary(), append([]string{python3Binary()}, os.Args[1:]...), os.Environ()); err != nil {
            log.Fatal(err)
        }
    default:
        if err := syscall.Exec(python2Binary(), append([]string{python2Binary()}, os.Args[1:]...), os.Environ()); err != nil {
            log.Fatal(err)
        }
    }

}
buchgr commented 6 years ago

Thanks @AdamDorwart !

rickeylev commented 6 years ago

Another way of doing it is using a select statement on the py_runtime rule. (the below using interpreter_path, but the same should be possible with interpreter) (I gave this a quick try, seems to work)

It still requires a custom arg to bazel; imho bazel should have it configured like this by default. Maybe an easy fix is to just add the config_setting() and select() I mention below directly to whatever Bazel's default py_runtime definition is.

Also, some caveats:

  1. I think host mode can only use 1 python version. i.e., if you have two genrules, one that needs py2 and one that needs py3, it won't work.
  2. I think the transition to py3 is a bit "one way", i.e., once its in py3 mode, bazel can't switch back to py2 mode. So if you do something complicated like a py2 binary that puts a py3 binary in data, and then that py3 binary puts a py2 binary in data, then it'll probably break in some way.
  3. Since its a custom py_runtime, you have to pass the extra flag to all your builds.
config_setting(
    name = "py3config",
    values = {"force_python": "PY3"}
)

py_runtime(
    name = "myruntime",
    interpreter_path = select({
        ":py3config": "/usr/bin/python3",
         "//conditions:default": "/usr/bin/python"
         }),
    files = []
    )

Also, just for posterity and to reconfirm what someone said up thread: Reading through the Bazel source, it looks like only --python_path and py_runtime are used to figure out the binary to use to run the python target (i started with how it populates the template script and went backwards from there). I'm guessing this is because Bazel expects people to use the select() way to select different values depending on the configuration.

mfarrugi commented 6 years ago

@AdamDorwart I believe this is the bash script you were referencing:

if head -n 1 "$1" | grep -q python3; then
  exec "$BASE_PATH"/usr/bin/python3 "$@"
else
  exec "$BASE_PATH"/usr/bin/python2 "$@"
fi

from https://groups.google.com/forum/#!topic/bazel-discuss/nVQ48R94S_8

brandjon commented 6 years ago

Note that the shebang-parsing way will choose the version based on the actual source file contents, while the select() way (and the eventual fix to Bazel's Python rules) will choose based on the version selected in the build (default_python_version attr / --force_python).

@rickeylev's right about the transition being one way, but I can imagine changing it so that it's only one way through deps, and gets reset when it crosses a data attribute.

The host mode issue is also a problem because it means you can't use Python 3 tools in your genrules (or if you invert it with --host_force_python=PY3, then you can't use Python 2 tools). That one's harder to fix, so if you really need multiple python versions in your host config you may want to stick with indirecting through a script.

brandjon commented 5 years ago

I'm closing this issue and opening several more directed ones to cover the points raised in this thread.

For a more detailed index of Python 2/3 issues, see tracking bug #6444, most of which is targeted for this quarter.

brandjon commented 5 years ago

Note that select()-ing on "force_python" is no longer supported / will be disallowed. As of release 0.23, the workaround in https://github.com/bazelbuild/bazel/issues/3517#issuecomment-417382123 should be replaced by the modified one given in https://github.com/bazelbuild/bazel/issues/4815#issuecomment-460777113.