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

Missed incrementing module compatibility_level after changing pip_parse module extension repo name tag to hub_name #2459

Closed
mikn opened this issue Dec 1, 2024 · 3 comments · Fixed by #2470
Labels
Can Close? Will close in 30 days if there is no new activity

Comments

@mikn
Copy link

mikn commented Dec 1, 2024

Affected Rule

https://registry.bazel.build/modules/grpc (probably more)

The issue appears in the pip module extension (@rules_python//python/extensions:pip.bzl), specifically renaming the name tag to hub_name without incrementing the rules_python module compatibility level from 1 to 2.

Is this a regression?

Yes - rules_python didn't increment its compatibility_level when introducing a breaking change from name to hub_name in the pip extension, causing issues with transitive dependencies that use the older API.

Description

The core issue is that rules_python made a breaking API change (changing name to hub_name in the pip extension) without incrementing the compatibility_level parameter from 1 (set in version 0.22) to 2. This causes problems in dependency chains where:

  1. A root module uses rules_python 1.0.0-rc2 (which requires hub_name)
  2. A transitive dependency (in this case, protoc-gen-validate via grpc) uses an older version with name
  3. Because the compatibility_level wasn't incremented, Bazel doesn't recognize this as a breaking change and attempts to resolve both versions together

This manifests as an error when running bazel mod tidy:

ERROR: in tag at https://bcr.bazel.build/modules/protoc-gen-validate/1.0.4/MODULE.bazel:64:10, unknown attribute name provided. Type 'bazel help mod' for syntax and help.

🔬 Minimal Reproduction

Here's a minimal reproduction case demonstrating the compatibility level issue:

  1. MODULE.bazel:
bazel_dep(name = "rules_python", version = "1.0.0-rc2")
bazel_dep(name = "protobuf", version = "29.0")
bazel_dep(name = "grpc", version = "1.68.0")

pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
pip.parse(
    hub_name = "pypi",  # New API
    python_version = "3.11",
    requirements_lock = "//python:requirements.txt",
)
use_repo(pip, "pypi")

The issue occurs because protoc-gen-validate (introduced as a transitive dependency by grpc) uses the older name parameter, but the compatibility_level wasn't incremented to prevent this version mixing.

The complete reproduction case includes additional files (BUILD.bazel, proto definitions, and Python code) which can be found in the linked BCR issue: bazelbuild/bazel-central-registry#3291

🔥 Exception or Error

bazel mod tidy
ERROR: in tag at https://bcr.bazel.build/modules/protoc-gen-validate/1.0.4/MODULE.bazel:64:10, unknown attribute name provided. Type 'bazel help mod' for syntax and help.

🌍 Your Environment

Operating System:

Linux (amd64)

Output of bazel version:

bazel 7.4.1

Rules_python version:

1.0.0-rc2

Anything else relevant?

Current workaround is to use multiple_version_override:

multiple_version_override(
    module_name = "rules_python",
    versions = [
        "1.0.0-rc2",
        "0.22.0",
    ],
)

The proper fix would be to increment the compatibility_level parameter in rules_python. This would prevent Bazel from attempting to mix incompatible versions of the rules in a single build.

@aignas aignas added this to the v1.0.0 milestone Dec 1, 2024
@aignas
Copy link
Collaborator

aignas commented Dec 1, 2024

I do remember a discussion somewhere in the early days of bzlmod that the compatibility_level bumping is super painful but I definitely don't have a reference anymore. @Wyverald, do you have any recommendation how rules_python should resolve this?

e.g. is a retroactive backfill of the bumped compatibility_level after we release a new version with a bumped level would be something that cannot be done? I assume we might be able to pull it off by applying extra patches to the MODULE.bazel metadata in the registry.

@aignas aignas self-assigned this Dec 1, 2024
@rickeylev
Copy link
Collaborator

The reason we didn't increment the compatibility level then (this would 0.24.0 - 2023-07-11) was because doing so would cause Bazel to produce an error, even if you didn't use the functionality that changed. So the choice we were faced with was: cause it to always fail even if it wasn't necessary (increment compat level), or cause it only fail if it was necessary (don't increment compat level). Hence we went with the latter.

Honestly, I still don't understand how compatibility_level is functional or supposed to work in practice. Not to be glib, but it seems more like a "break everyone" setting. Lets say rules_python version 22 increases to compatibility_level=2. An end user wants to use it. Unfortunately, one of their dependencies, X, specified rules_python v21 (CL=1), so they can't use it. The user updates X to rules_python v22. The user still can't use it, though, because another dependency, Y, also specifies rules_python v21. Worse, it turns out X and Y are commonly used together, so now every user of X has to wait for Y to be updated to rules_python v22 before they can use the next release of X. Meanwhile, some modules may have been broken, and some maybe not; it depends on if pip.parse(name) was being used or not. If Y wasn't using pip.parse(name), then the upgrade becomes unnecessarily required.

backfill versions

We'd have to patch about 20 versions in BCR 😬 . I don't think that's going to be feasible.

The proper fix would be to increment the compatibility_level parameter in rules_python. This would prevent Bazel from attempting to mix incompatible versions of the rules in a single build.

I don't think so? It just makes Bazel fail early (when it resolves modules vs evaluating them). You'd still have to use multiple_versions_override to restrict what versions are used.

@aignas
Copy link
Collaborator

aignas commented Dec 5, 2024

Yeah, backfill is too risky. I would like to close this as won't do since this is the least bad solution to the current situation.

@aignas aignas removed this from the v1.0.0 milestone Dec 5, 2024
@aignas aignas removed their assignment Dec 5, 2024
@aignas aignas added the Can Close? Will close in 30 days if there is no new activity label Dec 5, 2024
aignas added a commit to aignas/rules_python that referenced this issue Dec 6, 2024
github-merge-queue bot pushed a commit that referenced this issue Dec 6, 2024
Fixes #1361
Closes #2459 as won't do

---------

Co-authored-by: Richard Levasseur <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Can Close? Will close in 30 days if there is no new activity
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants