Skip to content
This repository has been archived by the owner on Oct 2, 2023. It is now read-only.

Fix builds with python3 as default python #490

Merged
merged 3 commits into from
Aug 16, 2018
Merged

Fix builds with python3 as default python #490

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b5e5d5e
adding py_runtime target to use with envs that have python pointing t…
Aug 16, 2018
c8a7e5c
add doc comment
Aug 16, 2018
97662e3
fix buildifier issue
Aug 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions python/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,17 @@
package(default_visibility = ["//visibility:public"])

licenses(["notice"]) # Apache 2.0


# Python2 is a dependency of rules_docker due to its dependency on
# containerregistry. To run builds on environments in which 'which python'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or if the user has set python_path to be something else in their bazelrc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this would work if the user has set python_path to something else as i think this flag would override it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant just to change the documentation. This runtime will override the python_path flag, which by default is set to "python", but if users have set it to something else, then that will also be overridden.

# points to python3, you can use the py_runtime target below.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also verify that this works with the build_python_zip bazel flag.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not too familiar with the build_python_zip flag. I ran the tests in the repo using these flags:
--python_top=//python:py_runtime_2 --build_python_zip
and the tests worked as expected (some of our tests seem to not be compatible with build_python_zip flag, but for unrelated reasons). Please let me know if you think we need to perform other checks to verify this works as you expect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing with the flag. I think this is sufficient.

# To use this target, simply add the following flag to your build commands:
# --python_top=//python:py_runtime_2
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you are building a python3 binary and packaging in a docker image in the same bazel command?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, Bazel does not quite support running builds with python2 and python3 at the same time (as far as I know). This is one of the outstanding problems for python support with Bazel that has been around for quite some time. I recommend you open an issue to ask about support for this use case (these are some of the old issues I saw related to this topic: https://github.com/bazelbuild/bazel/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+python2+python3)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which actually is the main problem. For repos with python3 code, they have already set their python_path flag to point to python3 which makes building python2 code impossible. If it's just a question of "python" on people's machine pointing to python3, but all their monorepo code is python2, then maybe they are already setting the python_path or python_top.

# Note you need to have a symlink to a valid python2 version in
# '/usr/bin/python2' for this to work.
py_runtime(
name = "py_runtime_2",
files = [],
interpreter_path = "/usr/bin/python2",
)