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

Enabling bzlmod causes workspace toolchains to no longer be registered #1675

Closed
rickeylev opened this issue Jan 8, 2024 · 6 comments · Fixed by #2289
Closed

Enabling bzlmod causes workspace toolchains to no longer be registered #1675

rickeylev opened this issue Jan 8, 2024 · 6 comments · Fixed by #2289
Labels

Comments

@rickeylev
Copy link
Contributor

rickeylev commented Jan 8, 2024

🐞 bug report

Affected Rule

python_register_toolchains in WORKSPACE files

Is this a regression?

Unclear

Description

Enabling bzlmod, but still using a workspace file (i.e. an empty module file), causes the python toolchains to no longer be registered.

🔬 Minimal Reproduction

Modify tests/cc/py_cc_toolchain_registered in the following ways:

  • Comment out everything in MODULE.bazel
  • Change the toolchain being looked up to @rules_python//python:toolchain_type

Run bazel test ... --enable_bzlmod

Actual behavior:

Toolchain debug output says:

Selected @@bazel_tools//tools/python:_autodetecting_py_runtime_pair

This means its using python from the environment's path (i.e. some system python) instead of the hermetic runtimes that were configured in WORKSPACE.

Expected behavior

Toolchain debug output should print

Selected @@python_3_11_x86_64-unknown-linux-gnu//:python_runtimes

(the name of the repo set in WORKSPACE).

Analysis

The cause of this is python/repositories.bzl detects if bzlmod is enabled, and, if so, disables toolchain registration. This was done because native.register_toolchains isn't normally available when bzlmod is enabled. However, when the code path is through a WORKSPACE file, it appears that symbol is available.

The immediate fix is to move the "if bzlmod enabled, don't do X" logic out of python/repositories.bzl itself and into the bzlmod extension. The extension logic calls that same python_register_toolchains function. The "X" logic here is two parts:

  1. Whether to call native.register_toolchains
  2. Whether to call toolchains_repo() (which creates the repo with all the toolchain() calls)

Now that we have integration tests again, it's feasible to construct a test for this.

The thing to double check is toolchain precedence. I can't remember offhand what the precedence of workspace-registered toolchains are when bzlmod is enabled. The main thing we're looking for is that the workspace-registered toolchains are after the module.bazel ones -- this is necessary so that the version-aware rules pick up their correct toolchain. The workspace python_register_toolchains() call will create version-unaware toolchains, which will match everything

Workarounds

The best workaround is to update MODULE.bazel to register the toolchain.

# MODULE.bazel
bazel_dep(name = "rules_python")

python = use_extension("@rules_python//python/extensions/python.bzl", "python")
python.toolchain(python_version="3.10") # Or whatever version

# No need to register anything; it'll happen automatically by the above

The logic that prevents the toolchain from being registered also prevents creating the repo that defines the toolchains, so modifying the WORKSPACE file to fix this isn't very easy. To go that route, you need to manually redefine the toolchains somehow (either manually in your own BUILD file, or possibly by loading the private toolchains_repo() rule that is being skipped and manually re-invoking it). I wouldn't suggest either of those; the couple lines of MODULE config are much easier.

@RobertClarke64
Copy link

Hi @rickeylev, is there any progress on this? For me, adding that workaround doesn't fix things for the pip_compile targets. The lock files are being generated with my system python version (3.11), even though I'm registering 3.10 as the default python version in the MODULE.bazel file. Or do you know any workarounds for this too?

Copy link

This issue has been automatically marked as stale because it has not had any activity for 180 days. It will be closed if no further activity occurs in 30 days.
Collaborators can add an assignee to keep this open indefinitely. Thanks for your contributions to rules_python!

@github-actions github-actions bot added the Can Close? Will close in 30 days if there is no new activity label Jul 30, 2024
@arwedus
Copy link

arwedus commented Jul 31, 2024

Workarounds

The best workaround is to update MODULE.bazel to register the toolchain.

# MODULE.bazel
bazel_dep(name = "rules_python")

python = use_extension("@rules_python//python/extensions/python.bzl", "python")
python.toolchain(python_version="3.8") # Or whatever version

# No need to register anything; it'll happen automatically by the above

I've done that, but there is no documentation on how to use that toolchain, if it is not the default one (we have two python toolchains defined in our repo), in a BUILD.bazel file.

Our MODULE.bazel looks similar:

bazel_dep(name = "rules_python", version = "0.31.0")
python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(is_default = True, python_version = "3.8")
python.toolchain(python_version = "3.10")
use_repo(python, "python_3_10", "python_3_8", "python_versions")

The documentation is missing for this use case.

The only thing documented is to use the current_py_toolchain alias in targets or rules, e.g.:

genrule(
    name = "test_py_toolchain",
    srcs = [],
    outs = ["out.txt"],
    cmd = "$(PYTHON3) --version > $(location out.txt)",
    toolchains = ["@rules_python//python:current_py_toolchain"],
)

but something like this doesn't work:

genrule(
    name = "test_py_3_10_toolchain",
    srcs = [],
    outs = ["out.txt"],
    cmd = "$(PYTHON3) --version > $(location out.txt)",
    toolchains = ["@python_3_10//:python3"],
)

By which means is it possible with rules_python to use a non-default python toolchain registered via python.toolchain?

@github-actions github-actions bot removed the Can Close? Will close in 30 days if there is no new activity label Jul 31, 2024
@s0l0ist
Copy link

s0l0ist commented Oct 2, 2024

I have this use case as well. I want to register python toolchains and be able to specify that toolchain when building python wheels (py_wheel).

We are building multiple python wheels that use a cpp module and would love to use the registered toolchain so we could do something like this:

load("@python_versions//3.10:defs.bzl", py_binary_3_10 = "py_binary", py_test_3_10 = "py_test", )

py_wheel(
    name = "wheel_3_10",
    abi = "cp310",
    toolchains = ["@python_3_10//:toolchain"],
    ...
)

Instead, we use GHA to install the python version which builds a single wheel for that particular runtime.

@s0l0ist
Copy link

s0l0ist commented Oct 2, 2024

I ended up with a workaround:

python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
    python_version = "3.8",
)
python.toolchain(
    python_version = "3.9",
)
python.toolchain(
    python_version = "3.10",
)
python.toolchain(
    python_version = "3.11",
)
python.toolchain(
    python_version = "3.12",
)

use_repo(
    python,
    # register toolchains
    "python_3_8",
    "python_3_9",
    "python_3_10",
    "python_3_11",
    "python_3_12",
    # Handy helper for all
    "python_versions",
)

Ex: running the 3.12 toolchain. Replace the string with one of the registered toolchains in the MODULE.bazel file:

bazel run -c opt //my_lib/python:wheel.publish --@rules_python//python/config_settings:python_version=3.12 -- --verbose --skip-existing

@jfirebaugh
Copy link

Discussion on Bazel Slack about this issue: https://bazelbuild.slack.com/archives/C014RARENH0/p1728595467884609

github-merge-queue bot pushed a commit that referenced this issue Oct 11, 2024
…ster toolchains (#2289)

While migrating to bzlmod, users may have a hybrid build where WORKSPACE
contains
python_register_toolchain() calls. Because these calls originate from
WORKSPACE files,
the `native.register_toolchain` APIs are still available. At the same
time, we still
detect that bzlmod is enabled, and thus disable calling those functions.
The net
effect is, users aren't able to register toolchains in such a hybrid
setup. At the
same time, because the code path is shared, we can't have the bzlmod
toolchain code
try to call them, as it'll fail.

To accomodate both cases, have the bzlmod toolchain code pass a special
arg so that
`python_register_toolchains` knows to skip parts that don't apply to the
bzlmod toolchain
invocation.

This was all unwound by some users and pointed out in a Slack thread. A
few people are
manually carrying an equivalent patch for a working hybrid mode.

Fixes #1675
@aignas aignas unpinned this issue Oct 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants