-
Notifications
You must be signed in to change notification settings - Fork 539
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
Unable to use python_interpreter_target when using bzlmod #1058
Comments
That's to be expected, and WAI. Return values from use_extension aren't usable directly by functions (bazelbuild/bazel#17048). Here's how I do it in our repo. It's not great because it can only work on a single platform. Ideally I'd like to be able to write
|
I am having the same problem. There is another argument that is named |
@matts1 Any ideas on how to get the toolchain to download the Python as we did before? It is not ... at least with windows. |
So I have figured out that |
In https://github.com/mvgijssel/setup/pull/180/files I've hacked something together that works for now. It creates a module extension which symlinks the correct Python interpreter in place and references that file inside the
load("@python3//:defs.bzl", "interpreter")
def _symlink_python_interpreter_impl(repository_ctx):
repository_ctx.file('BUILD.bazel', content='''
package(default_visibility = ["//visibility:public"])
exports_files(["python"])
''')
python_path = repository_ctx.path(Label(interpreter))
repository_ctx.symlink(python_path, 'python')
_symlink_python_interpreter = repository_rule(
implementation=_symlink_python_interpreter_impl,
local=True,
)
def _python_interpreter(module_ctx):
print(module_ctx)
_symlink_python_interpreter(
name = "python_interpreter",
)
python_interpreter = module_extension(
implementation = _python_interpreter,
)
# NOTE: custom extension so we can reference the hermetic python interpreter in the pip.parse rules
# Can be removed once https://github.com/bazelbuild/rules_python/issues/1058 is solved.
python_interpreter = use_extension(":extensions.bzl", "python_interpreter")
use_repo(python_interpreter, "python_interpreter")
# ...
pip.parse(
name = "pytest",
python_interpreter_target = "@python_interpreter//:python",
requirements_lock = "@//tools/pytest:requirements.lock",
)
use_repo(pip, "pytest") |
@mvgijssel do you have any recommendations for us to add this to the extension in the rules? |
So @mvgijssel way you did this does not work on Windows. Grumble. Why cannot windows support symlinks normally? |
Good question! The gist of the change is to use a generically named repo to symlink the platform specific interpreter. Not sure if this actually works with cross platform builds etc and apparently not with Windows 🙈. Maybe it’s possible to copy (instead of symlink) the interpreter and associated files into a generically named repo? Then at least it also works for windows. Now I’m curious btw how folks solve this in rules_nodejs as that also provides a hermetic nodejs version 🤔. |
@mvgijssel I have started to document how we are loading the interpreter in this issue: #1161 - we have differences from the other rules that we may run into. I have also added an extension base on @mvgijssel's extension in this PR #1155 @siddharthab; once that PR is in, I think you will have an example of how to configure the interpreter. |
@siddharthab can we close this? #1155 gives you this functionality now. When we release next you will will have it |
Thanks for all the consideration and work. The solution looks good to me. Closing this issue. |
🐞 bug report
Affected Rule
The issue is caused by the rule: pip.parse (in bzlmod)Is this a regression?
NoDescription
pip.parse supports the `python_intepreter_target` attribute but getting its value from `@python//:defs.bzl` is not currently possible because the string value is not available as an extension. This attribute is also not used in the bzlmod example in the repo.🔬 Minimal Reproduction
🔥 Exception or Error
🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_python version:
Anything else relevant?
The text was updated successfully, but these errors were encountered: