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

Bump required version to 3.9 and add it to bazel #137

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 9 additions & 1 deletion WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ http_archive(
url = "https://github.com/bazelbuild/rules_python/releases/download/0.31.0/rules_python-0.31.0.tar.gz",
)

load("@rules_python//python:repositories.bzl", "py_repositories")
load("@rules_python//python:repositories.bzl", "py_repositories", "python_register_toolchains")

py_repositories()

# Use Python 3.9 for bazel Python rules.
python_register_toolchains(
name = "python3",
python_version = "3.9",
Copy link
Member

Choose a reason for hiding this comment

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

Without this change which version of Python is used? Would it default to current (i.e. 3.11 today)?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's arbitrary, it'll use whatever the user has on their system. I happen to have 3.11.8 on my dev machine.

Copy link
Member

Choose a reason for hiding this comment

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

Is there a way for users to get that functionality again after this change? Can it be overridden with a flag to bazel, for example? I'd assume that 3.11 would generally have better performance so a user might rather keep using that. If there's a flag or some way to override then a comment would be sufficient.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmmm good question, while testing things out I used the --action_env=PYENV_VERSION=3.8 param to bazel so presumably something like that might work. For my current use case we're using a cusom integration with gn and cmake so they'll just use whatever we've specified in our build system. For bazel I'm not sure which toolchain would "win.", I'll do some research here, but I suspect the toolchain we specify in our project would be used.

Really my intention was to just use this for tests and not the exported emboss rules, I'll see if I can scope this to just that use case.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If we want to keep the current behavior (use the system python), but still ensure that things aren't broken on the oldest supported Python release, I could add setup-python to the GitHub actions, add a .python-version file to the repo. That would be the "source of truth" so to speak of the minimum supported version for the repo and we would just bump that whenever we update the supported version of python.

)

load("@python3//:defs.bzl", "interpreter")
2 changes: 1 addition & 1 deletion doc/guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ emails.

#### Running the Emboss Compiler

The Emboss compiler requires Python 3.8 or later -- the minimum supported
The Emboss compiler requires Python 3.9 or later -- the minimum supported
version tracks the support timeline of the Python project. On a Linux-like
system with Python 3 installed in the usual place (`/usr/bin/python3`), you
can run the embossc script at the top level on an `.emb` file to generate
Expand Down