Skip to content
This repository has been archived by the owner on Feb 6, 2024. It is now read-only.

Not working with bazel 0.25rc1 #305

Closed
Globegitter opened this issue Apr 5, 2019 · 15 comments
Closed

Not working with bazel 0.25rc1 #305

Globegitter opened this issue Apr 5, 2019 · 15 comments
Assignees

Comments

@Globegitter
Copy link
Contributor

The latest rc changed the default python version to py3 so now we are seeing:

Traceback (most recent call last):
  File "/home/markus/.cache/bazel/_bazel_markus/c83dd5dc66625baf386a59b750f50a10/execroot/__main__/bazel-out/k8-fastbuild/bin/search/hotels/dev-manifest.runfiles/io_bazel_rules_k8s/k8s/resolver.py", line 24, in <module>
    from containerregistry.client import docker_creds
  File "/home/markus/.cache/bazel/_bazel_markus/c83dd5dc66625baf386a59b750f50a10/execroot/__main__/bazel-out/k8-fastbuild/bin/search/hotels/dev-manifest.runfiles/containerregistry/client/__init__.py", line 23, in <module>
    from containerregistry.client import docker_creds_
  File "/home/markus/.cache/bazel/_bazel_markus/c83dd5dc66625baf386a59b750f50a10/execroot/__main__/bazel-out/k8-fastbuild/bin/search/hotels/dev-manifest.runfiles/containerregistry/client/docker_creds_.py", line 31, in <module>
    import httplib2
  File "/home/markus/.cache/bazel/_bazel_markus/c83dd5dc66625baf386a59b750f50a10/execroot/__main__/bazel-out/k8-fastbuild/bin/search/hotels/dev-manifest.runfiles/httplib2/__init__.py", line 988
    raise socket.error, msg
                      ^
SyntaxError: invalid syntax

Fixing this should be as easy as explicitly setting the python_version to PY2 on the binaries.

@nlopezgi nlopezgi self-assigned this Apr 5, 2019
@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

looking into this now

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

Hi @Globegitter ,
I'm trying to repro this issue but the error is not showing up.
I'm running with bazel 0.25.0rc1 from a xenial container that has both py2 and py3 installed.
I was able to get the error to show up only if I change /usr/bin/python to point to python3 (and the error is not solved by adding python_version to par_binary targets).
I even tried running bazel test --python2_path=/usr/bin/python2 --python3_path=/usr/bin/python3 //...
This command failed if /usr/bin/python pointed to py3 but succeeded if /usr/bin/python pointed to py2.
Could you provide detailed repro instructions?
fyi @brandjon for status of what should work wrt python with 0.25rc1

@brandjon
Copy link
Member

brandjon commented Apr 5, 2019

Nicolas, your repro attempt is being foiled by bazelbuild/bazel#4815. That bug means that regardless of what Python version is decided for a target, the actual Python interpreter invoked at execution time is, by default, the "python" command on PATH. You need to workaround it in your build using --python_top. Alternatively, in Bazel 0.25 you can enable --incompatible_use_python_toolchains (doesn't work on Windows yet).

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

thanks for the clarifications. I am able to reproduce this by passing a --python_top. Fixing will involve changes to both rules_docker and here. Preparing PRs.

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

thanks again for clarifications @brandjon,
I've been struggling all day to get 0.25.0rc1 to work with rules_docker due to its dependency on containerregistry and the restriction that containerregistry is only compatible with python2.
I was able to uncover the following:

  • I modified containerregistry targets to set python_version= "PY2"
  • If I build a target (say, //containerregistry:digester) for the target configuration, the produced file (bazel-out/k8-py2-fastbuild/bin/external/containerregistry/digester) points to the correct PY2 that I passed in my py_runtime target.
  • However, if I have a target that depends on //containerregistry:digester for the host configuration, the output produced bazel-out/host/bin/external/containerregistry/digester points to PY3 incorrectly.

I'm pretty sure this is a bug in bazel 0.25.1 (or I need to pass some other extra flag for host python outputs to be produced correctly?)

@brandjon
Copy link
Member

brandjon commented Apr 5, 2019

Unfortunately we don't have granular control over the host configuration, so all targets built in the host configuration have the same Python version, which now defaults to PY3. See bazelbuild/bazel#6443. A rather coarse-grained workaround is to override all host tools to PY2 with --host_force_python=PY2.

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

more context in response here: https://groups.google.com/forum/?utm_medium=email&utm_source=footer#!msg/bazel-sig-python/cZvjanFL1AM/ZpjOweVyBwAJ

@brandjon does that mean that all rules_docker and rules_k8s builds will start failing with 0.25.x or some upcoming version (i.e., is the switch to PY3 as default python version coming out before exec configuration is rolled out) unless we use --host_force_python=PY2?

@Globegitter as per @brandjon we wont be able to use this feature of 0.25.0 until the exec configuration is introduced.

@brandjon
Copy link
Member

brandjon commented Apr 5, 2019

Note that in 0.25, the default host Python version is already PY3. But without the new --incompatible_use_python_toolchains flag (which does not yet default to true as of 0.25), it may run in a Python 2 interpreter despite being "built in PY3 mode" due to the aforementioned bug.

It might be a few releases before the exec configuration is available and even then it'll be on a case-by-case basis as each rule type needs to adapt to use it. What rule do you have that relies on a Python target in host configuration? Is it a genrule?

@brandjon
Copy link
Member

brandjon commented Apr 5, 2019

+@katre, FYI.

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

thanks for the additional comments.
My rule that relies on a Python target is a starlark rule. It has the dependency declared here: https://github.com/bazelbuild/rules_docker/blob/master/container/image.bzl#L493. And it runs the py_binary here: https://github.com/bazelbuild/rules_docker/blob/master/container/image.bzl#L230. Let me know if you have suggestions as to how we can better prepare for these changes.

@nlopezgi
Copy link
Contributor

nlopezgi commented Apr 5, 2019

I also have several other Starlark rules that have similar dependencies on python targets

nlopezgi pushed a commit to nlopezgi/rules_k8s that referenced this issue May 7, 2019
@brandjon
Copy link
Member

FYI rules_k8s is among the projects that fail on buildkite in the incompatible change pipeline, and that should therefore have --host_force_python=PY2 added.

@nlopezgi
Copy link
Contributor

thanks @brandjon,
this should pass once #306 is in

@nlopezgi nlopezgi closed this as completed Jul 3, 2019
@mariusgrigoriu
Copy link
Contributor

Which versions does rules_k8s work with now? Can you go all the way to 0.27?

@nlopezgi
Copy link
Contributor

nlopezgi commented Jul 3, 2019

@mariusgrigoriu afaik, yes. Please do let us know if you see any issues.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants