-
Notifications
You must be signed in to change notification settings - Fork 548
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
Comments
I do remember a discussion somewhere in the early days of e.g. is a retroactive backfill of the bumped |
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.
We'd have to patch about 20 versions in BCR 😬 . I don't think that's going to be feasible.
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. |
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. |
Fixes bazelbuild#1361 Closes bazelbuild#2459 as won't do
Fixes #1361 Closes #2459 as won't do --------- Co-authored-by: Richard Levasseur <[email protected]>
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 thename
tag tohub_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
tohub_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
tohub_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:hub_name
)name
This manifests as an error when running
bazel mod tidy
:🔬 Minimal Reproduction
Here's a minimal reproduction case demonstrating the compatibility level issue:
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
🌍 Your Environment
Operating System:
Output of
bazel version
:Rules_python version:
Anything else relevant?
Current workaround is to use
multiple_version_override
: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.
The text was updated successfully, but these errors were encountered: