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.

Sign up for GitHub

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

Jump to bottom

Support environment/file-based default Python version in python.toolchain #2587

Open
vonschultz opened this issue Jan 28, 2025 · 5 comments · May be fixed by #2588
Open

Support environment/file-based default Python version in python.toolchain #2587

vonschultz opened this issue Jan 28, 2025 · 5 comments · May be fixed by #2588

Comments

@vonschultz
Copy link
Contributor

🚀 feature request

Relevant Rules

python.toolchain
where python = use_extension("@rules_python//python/extensions:python.bzl", "python")

Description

It should be possible to set the default Python version using a file or an environment variable.

In python.toolchain you need to set the is_default attribute. You can have several Python versions defined, and for one of them you need to set is_default = True. Since a MODULE.bazel file cannot read an environment variable or read a file, you need to set the default in MODULE.bazel. Before Bzlmod, you could simply load a file created by a repo rule and have access to the value of an environment variable or the contents of a .python-version file in the WORKSPACE.bazel file, and set the default_version in a call to python_register_multi_toolchains accordingly.

This is important for interoperability in projects where not everything is run through Bazel; pyenv for example uses the PYENV_VERSION environment variable, falling back on the .python-version file if the environment variable is not set.

Describe the solution you'd like

A new attribute for python.toolchain called e.g. default_python_version_file, which can be the label of a file generated by a repository rule, or a file in the current repo. If the python_version attribute of python.toolchain matches the contents of the file, treat it as the default (as if is_default = True).

Describe alternatives you've considered

One could imagine having both an attribute for setting the default Python version with a file and another attribute for setting the default Python version with an environment variable, which would lower the threshold for any user wanting to react to an environment variable, at the cost of a more complex implementation in rules_python. Since the same functionality can be achieved with a repository rule, I'm leaning towards telling users to write a repository rule.

One could also imagine having the default Python version be defined in MODULE.bazel, as the source of truth, and have something like write_source_file update a .python-version file in the repo for compatibility with other tools. But done this way, there's no way to support environment variables as a way of setting the default Python version.
vonschultz added a commit to vonschultz/rules_python that referenced this issue Jan 28, 2025
@vonschultz
feat(python.toolchain): support file-based default Python version
6813055 
This change adds a new `default_version_file` attribute to
`python.toolchain`. If set, the toolchain compares the file's contents
to its `python_version`, and if they match, treats that toolchain as
default (ignoring `is_default`). This allows Bazel to synchronize the
default Python version with external tools (e.g., pyenv) that use
a `.python-version` file or environment variables.

Fixes bazel-contrib#2587.
@vonschultz vonschultz linked a pull request Jan 28, 2025 that will close this issue
Open
@aignas
Copy link
Collaborator

aignas commented Feb 12, 2025 •
edited
Loading

I have seen this issue but I did not have enough time to come up with good pros/cons list for the long-term maintaining of such a thing. I would love to do this through https://github.com/buildbuddy-io/bazel_env.bzl since this has proven to have the right API for user settings like this.

This does require direnv support, but it does hook into the toolchains. Maybe we need to provide an additional file in the toolchains to do this? Something like $(PYTHON_VERSION_FILE)?

EDIT: This preference would lean towards writing the .python-version file using the bazel_env.bzl ruleset. This in theory could be much easier to maintain than expanding the python module extension surface API.

@vonschultz
Copy link
Contributor Author

I don't really follow. What is bazel_env.bzl helping us with? Writing the .python-version file based on the environment variable? But then, how do we get that information to the default toolchain? Or setting the environment variable based on the .python-version file? Then the information flows in the wrong direction. The environment variable is supposed to override the .python-version file in the repo.

This is one of the reasons we're still using WORKSPACE.bazel now. Different CI jobs have different values set for the environment variable specifying the default Python to use, and that needs to propagate to Bazel. It's not actually pyenv, but the logic is the same: the preferred Python version is set through an environment variable, falling back to the .python-version file if the environment variable is undefined.

Now, if rules_python could read a user-provided .python-version file I don't actually think we'll need to implement separate logic for the environment variable in rules_python: the user could do that in their own repo rules and write a file with the result of the selection process.

@aignas
Copy link
Collaborator

aignas commented Feb 12, 2025

Ah sorry, my breain went and jumped into conclusion that you want to have a python version file for pyenv based on python toolchain configuration in bzlmod.

Out of interest, would an include file work in your case? module.bazel files can load/include other files that have fragments of the module.bazel itself.

Another way is to use .bazelrc to set the default value of the python version - it can be done by adding --@rules_python//python/config_settings:python_version=3.9.5.

Could you add a little more context why those ways would not be sufficient?

@vonschultz
Copy link
Contributor Author

include() can only include files from the main repo, so I can't have a repo rule read the environment variable and write a file that can be included, since that file would go in a new repo. Similarly, there's no way to read an environment variable in a .bazelrc file. Given that the source of truth is an environment variable, how can we get Bazel to respect it? As far as I can see, the cleanest solution is for the rules and module extensions themselves to read it, either directly or by supporting a file I can create dynamically. This is essentially the same reasoning that motivated #1673.

Part of the problem is that python.toochain insists on a default at all. One could imagine having several Python toolchains, all with is_default = False, and insisting that the user always needs to set --@rules_python//python/config_settings:python_version explicitly. That still doesn't solve how to get the info from an environment variable and falling back on a file, pyenv style, but at least there wouldn't be an incorrect default value lurking around as a potential source of errors.

Given that .python-version seems to be something of a standard solution in the Python ecosystem (see e.g. pyenv, Github actions/setup-python, Heroku), I think the nicest experience for the user would be not having to specify an is_default in MODULE.bazel at all, and read it from .python-version like the other tools would (with an option to read it from somewhere else, which, if it could be a generated file, also solves my use case with reading from environment variables).

Of course, I could just make a wrapper for Bazel with whatever logic I want to determine the default version and then go in and physically change the MODULE.bazel file, but that doesn't really feel like a clean solution.

@aignas
Copy link
Collaborator

aignas commented Feb 20, 2025

Thanks for the explanation. I agree about the default being settable by .python-version file in the root module if it exists, if not, we fallback to the current logic. The root module could also provide an alternative label to the file that has the version.

That said, having this logic or an env var doing the switching is probably also fine. I'll look at your PR once again sometime next week then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(python.toolchain): support file-based default Python version vonschultz/rules_python
2 participants
@aignas @vonschultz