rules_python 0.26 changes behavior for sequence of py_repositories call

[mikex-oss]

🐞 bug report

Affected Rule

The issue is caused by the rule:

It seems to be a new behavior introduced in rules_python 0.26 that forces the py_repositories call to take place before loading other rules_python loads:

load("@<python_register_toolchains_name>//:defs.bzl", "interpreter")
load("@rules_python//python:pip.bzl", "pip_parse")

Seems to be related to #1428.

Is this a regression?

Yes, the previous version in which this bug was not present was: 0.25.0

Yes, the py_repositories() call can take place later within a helper function of a .bzl file with other rules_python loads (see below).

Description

A clear and concise description of the problem...

In rules_python 0.25, using Bazel workspaces, the following works:

# WORKSPACE
load("@some_repo//:load_deps.bzl", "load_external_deps")

load_external_deps()

load("@some_repo//:init_toolchains.bzl", "initialize_python_toolchain")

initialize_python_toolchain(version = "3.9")

load("@some_repo//:init_deps.bzl", "initialize_external_deps")

initialize_external_deps()

# load_deps.bzl
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

def load_external_deps():
    ...
    http_archive(
        name = "rules_python",
        ...
    )
    ...

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

def initialize_python_toolchain(version):
    python_register_toolchains(
      name = "project_python",
      version = version,
    )

# init_deps.bzl
load("@bazel_skylib//:workspace.bzl", "bazel_skylib_workspace")
load("@com_google_protobuf//:protobuf_deps.bzl", "protobuf_deps")
load("@project_python//:defs.bzl", "interpreter")
load("@rules_python//python:pip.bzl", "pip_parse")
load("@rules_python//python:repositories.bzl", "py_repositories")

def initialize_external_deps():
    bazel_skylib_workspace()
    protobuf_deps()
    py_repositories()  # moving this to first line of function does not matter

    pip_parse(
        ...
        python_interpreter_target = interpreter,
    )

This same WORKSPACE sequence fails in rules_python 0.26. To avoid updating every repo WORKSPACE we have, the only workaround seems to be to move the py_repositories call to anywhere within the initialize_python_toolchain helper function. It does not seem to matter whether that call happens before or after the call to python_register_toolchains. It just must be before the interpreter and pip_parse loads (moving py_repositories before any other external dep helpers inside initialize_external_deps does not matter).

🔬 Minimal Reproduction

See above.

🔥 Exception or Error

ERROR: Failed to load Starlark extension '@rules_python_internal//:rules_python_config.bzl'.
Cycle in the workspace file detected. This indicates that a repository is used prior to being defined.
The following chain of repository dependencies lead to the missing definition.
 - @rules_python_internal
This could either mean you have to add the '@rules_python_internal' repository with a statement like `http_archive` in your WORKSPACE file (note that transitive dependencies are not added automatically), or move an existing definition earlier in your WORKSPACE file.
ERROR: Error computing the main repository mapping: cycles detected during computation of main repo mapping

🌍 Your Environment

Operating System:

  
centos7
  

Output of bazel version:

  
bazel 6.4.0
  

Rules_python version:

  
0.26.0
  

Anything else relevant?

I couldn't figure out if there was a single load that mattered from:

load("@project_python//:defs.bzl", "interpreter")
load("@rules_python//python:pip.bzl", "pip_parse")

I tried selectively removing one or the other (stuff downstream should still fail due to missing references) but they still failed early at the missing rules_python_internal. Only when I remove both does it generate a different error (about missing pip_parse reference).

[rickeylev]

Not sure, but my guess here is this is a side effect of having repo rules and regular rules in the same bzl file. i.e. pip.bzl. This has both pip_parse (repo rule) and compile_pip_requirements (regular rule that loads py_binary). We should split pip.bzl up to better separate the repo-rules from regular-rules.

Or maybe put a call to py_repositories in register_python_toolchains() ? I'm not sure that would fix it. Not sure we should do that, either.

The core issue, though is before using anything from rules_python, py_repositories() should be called to give rules_python a chance to setup any state subsequent things might need.

I think this also is because of an annoying behavior WORKSPACE has? I don't fully remember/understand, but I think it's something like: repos created during a macro call aren't actually created/usable until execution returns back to the WORKSPACE file. Or something like that? This would explain why changing the py_repositories() call in initialize_external_deps doesn't fix things. I bet if you put it in WORKSPACE directly, just before the call to initialize_external_deps, that would also suffice.

[mikex-oss]

We should split pip.bzl up to better separate the repo-rules from regular-rules.

Is this something the Bazel team will consider?

Or maybe put a call to py_repositories in register_python_toolchains() ?

Yes (assuming you meant initialize_python_toolchain), this does work. The py_repository call can go anywhere in that helper macro, before or after python_register_toolchains. Mainly I dislike this workaround because it's not really related to the toolchain. It fits more naturally alongside the other third-party calls, like bazel_skylib_workspace() and protobuf_deps().

I bet if you put it in WORKSPACE directly, just before the call to initialize_external_deps, that would also suffice.

Agreed, though we are trying to avoid this because it needs to be proliferated to many repos. These helper macros were intended to simplify WORKSPACE boilerplate (actually, I got some pushback because folks wanted a single macro which wasn't possible).

[aignas]

Is this still relevant?

[mikex-oss]

Is this still relevant?

If there has been any suspected fix for this issue, happy to give it a try. We are still using rules_python-0.25.0 to avoid this problem.

[bluec0re]

I'm getting the same error, but my WORKSPACE file looks like this:

load(":repositories.bzl", "my_repositories")

my_repositories()

load("@rules_python//python:pip.bzl", "pip_parse")
load("@rules_python//python:repositories.bzl", "py_repositories", "python_register_toolchains")
[...]
py_repositories()

python_register_toolchains(
    name = "python311",
    python_version = "3.11",
    register_coverage_tool = True,
)

load("@python311//:defs.bzl", "interpreter")

pip_parse(
    name = "pip_dependencies",
    python_interpreter_target = interpreter,
    requirements_lock = "//:requirements_lock.txt",
)

load("@pip_dependencies//:requirements.bzl", "install_deps")

install_deps(
    # python_interpreter = interpreter,
)

with repositories.bzl:

load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")
load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe")

def my_repositories():
    [...]

    maybe(
        http_archive,
        name = "rules_python",
        sha256 = "4912ced70dc1a2a8e4b86cec233b192ca053e82bc72d877b98e126156e8f228d",
        strip_prefix = "rules_python-0.32.2",
        url = "https://github.com/bazelbuild/rules_python/releases/download/0.32.2/rules_python-0.32.2.tar.gz",
    )

    # from rules_python 0.32+
    http_archive(
        name = "rules_cc",
        sha256 = "2037875b9a4456dce4a79d112a8ae885bbc4aad968e6587dca6e64f3a0900cdf",
        strip_prefix = "rules_cc-0.0.9",
        urls = ["https://github.com/bazelbuild/rules_cc/releases/download/0.0.9/rules_cc-0.0.9.tar.gz"],
    )

To fix it, I had to move the load("@rules_python//python:pip.bzl", "pip_parse") line after the py_repositories call. I prefer to have all load commands near the top (where possible), similar to imports in python. As @rules_python//python:pip.bzl is from an already defined repository, this (rules_python), it isn't clear that it depends on py_repositories being called first. Maybe add a note here?

[groodt]

Closing. It appears there is a workaround and the maintainers won't be looking at this issue any time soon. Future work on the rules is focussed on bzlmod so anything that relates to WORKSPACE is unlikely to get attention. However, contributions are always welcome.