bazelbuild / rules_python

Bazel Python Rules
https://rules-python.readthedocs.io
Apache License 2.0
521 stars 536 forks source link

Unable to use python_interpreter_target when using bzlmod #1058

Closed siddharthab closed 1 year ago

siddharthab commented 1 year ago

🐞 bug report

Affected Rule

The issue is caused by the rule: pip.parse (in bzlmod) ### Is this a regression? No ### Description 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 ``` # MODULE.bazel bazel_dep(name = "rules_python", version = "0.18.0") python = use_extension("@rules_python//python:extensions.bzl", "python") python.toolchain( name = "python", python_version = "3.10", ) use_repo(python, "python") use_repo(python, "python_toolchains") register_toolchains( "@python_toolchains//:all", ) pip = use_extension("@rules_python//python:extensions.bzl", "pip") interpreter = use_extension("@python//:defs.bzl", "interpreter") pip.parse( name = "pip", python_interpreter_target = interpreter, requirements_lock = "//alife:requirements.txt", ) use_repo(pip, "pip") ``` ## 🔥 Exception or Error ``` ERROR: Target parsing failed due to unexpected exception: in tag at /MODULE.bazel:39:10, error converting value for attribute python_interpreter_target: expected value of type 'string' for python_interpreter_target, but got (module_extension_proxy) ``` ## 🌍 Your Environment **Operating System:**
  
Debian GNU/Linux rodete
  
**Output of `bazel version`:**
  
Build label: 6.0.0
Build target: bazel-out/k8-opt/bin/src/main/java/com/google/devtools/build/lib/bazel/BazelServer_deploy.jar
Build time: Mon Dec 19 15:52:35 2022 (1671465155)
Build timestamp: 1671465155
Build timestamp as int: 1671465155
  
**Rules_python version:**
  
0.18.0
  
**Anything else relevant?**
matts1 commented 1 year ago

That's to be expected, and WAI. Return values from use_extension aren't usable directly by functions (https://github.com/bazelbuild/bazel/issues/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 use_repo(python, "@python3.10.8_host"), which could be done via this mechanism https://github.com/bazelbuild/bazel/issues/17493, but for now the best approach would probably be for rules_python to always expose the _host suffix which is a duplicate of the interpreter defined for that specific platform.

# Python support
PY_VERSION = "3.10.8"
bazel_dep(name = "rules_python", version = "0.18.0")

python = use_extension("@rules_python//python:extensions.bzl", "python")
python.toolchain(
    name = "python%s" % PY_VERSION,
    python_version = PY_VERSION,
)

PY_INTERPRETER_REPO = "python%s_x86_64-unknown-linux-gnu" % PY_VERSION
use_repo(
    python,
    "python%s_toolchains" % PY_VERSION,
    PY_INTERPRETER_REPO,
)
register_toolchains("@python%s_toolchains//:all" % PY_VERSION)

pip = use_extension("@rules_python//python:extensions.bzl", "pip")
pip.parse(
    name = "pip",
    requirements_lock = "//rules_cros/toolchains/python:requirements_lock.txt",
    python_interpreter_target="@%s//:python" % PY_INTERPRETER_REPO
)
use_repo(pip, "pip")
chrislovecnm commented 1 year ago

I am having the same problem. There is another argument that is named python_interpreter and I am uncertain how to use that flag as well.

chrislovecnm commented 1 year ago

@matts1 Any ideas on how to get the toolchain to download the Python as we did before? It is not ... at least with windows.

chrislovecnm commented 1 year ago

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 bzlmod does not support downloading a hermetic toolchain in the MODULES.bzl file. I am working on figuring out a solution.

mvgijssel commented 1 year ago

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 python_interpreter_target attribute:

extensions.bzl

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,
)

MODULE.bazel

# 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")
chrislovecnm commented 1 year ago

@mvgijssel do you have any recommendations for us to add this to the extension in the rules?

chrislovecnm commented 1 year ago

So @mvgijssel way you did this does not work on Windows. Grumble. Why cannot windows support symlinks normally?

mvgijssel commented 1 year ago

@mvgijssel do you have any recommendations for us to add this to the extension in the rules?

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 🤔.

chrislovecnm commented 1 year ago

@mvgijssel I have started to document how we are loading the interpreter in this issue: https://github.com/bazelbuild/rules_python/issues/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 https://github.com/bazelbuild/rules_python/pull/1155 @siddharthab; once that PR is in, I think you will have an example of how to configure the interpreter.

chrislovecnm commented 1 year ago

@siddharthab can we close this? #1155 gives you this functionality now. When we release next you will will have it

siddharthab commented 1 year ago

Thanks for all the consideration and work. The solution looks good to me. Closing this issue.