-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
bazel_dep
should allow specifying a maximum supported compatibility_level
#17378
Comments
cc @Wyverald |
I actually like this idea after thinking about it ;) It could make bumping But we need to think through how this would affect our version selection algorithm. Currently, we group all modules appeared in the dependency graph by module name and compatibility level, then we select the newest version for each group. We fail the resolution if different compatibility levels are required after resolution and pruning unless multiple_version_override is specified. To allow With multiple_version_override, it's a bit more complicated, but I think it'll still work after adding the Also, I don't think this is going to be an incompatible change. Because what used to work should also work in the new resolution process. Therefore, we can even try to cherry-pick this back to 6.0. /cc @Wyverald WDYT? |
It's hard for me to predict the impact of such a feature, but I don't have a better idea for now so I won't block it. It's worth noting that using the |
Awesome. Thanks. Does the P2 mean no one on the Bazel team is going to work on it? I would like to get this into 6.1. |
Unfortunately I don't think we'll have time for it, given 6.1 cut is like ~a week or two away. There are some higher-priority things we need to address. I'm happy to review PRs, though, if anyone wants a go. |
I'm working on this. It probably won't make the 6.1 rc cut, and I'll leave it up to your discretion once you start reviewing it if we could get it into 6.1. 6.2 isn't the worst place for it to land though. |
The class Module currently holds data that's no longer needed after resolution finishes (such as the compatibility level, bazel compatibility, the registry where it comes from, etc). Conversely, the repo spec field is not computed until the end of resolution. To remove runtime checks and reduce cognitive overhead, this CL splits the Module class into two; one only used after resolution finishes (Module), and one only used before (InterimModule). This allows us to introduce max_compatibility_level in a follow CL. Work towards #17378 Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 525780111 Change-Id: I2df8d78d324b3c8744ba0a4eda405d162e9bbb8c
@bazel-io fork 6.2.0 |
The class Module currently holds data that's no longer needed after resolution finishes (such as the compatibility level, bazel compatibility, the registry where it comes from, etc). Conversely, the repo spec field is not computed until the end of resolution. To remove runtime checks and reduce cognitive overhead, this CL splits the Module class into two; one only used after resolution finishes (Module), and one only used before (InterimModule). This allows us to introduce max_compatibility_level in a follow CL. Work towards #17378 Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 525780111 Change-Id: I2df8d78d324b3c8744ba0a4eda405d162e9bbb8c
See code comments for more details. tl;dr: we use the new `bazel_dep(max_compatibility_level=)` attribute to influence version selection. Fixes #17378 RELNOTES: Added a new `max_compatibility_level` attribute to the `bazel_dep` directive, which allows version selection to upgrade a dependency up to the specified compatibility level. Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 526118928 Change-Id: I332eb3761e0dee0cb7f318cb5d8d1780fca91be8
* Add InterimModule to represent a Module before resolution finishes The class Module currently holds data that's no longer needed after resolution finishes (such as the compatibility level, bazel compatibility, the registry where it comes from, etc). Conversely, the repo spec field is not computed until the end of resolution. To remove runtime checks and reduce cognitive overhead, this CL splits the Module class into two; one only used after resolution finishes (Module), and one only used before (InterimModule). This allows us to introduce max_compatibility_level in a follow CL. Work towards #17378 Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 525780111 Change-Id: I2df8d78d324b3c8744ba0a4eda405d162e9bbb8c * Selection with max_compatibility_level See code comments for more details. tl;dr: we use the new `bazel_dep(max_compatibility_level=)` attribute to influence version selection. Fixes #17378 RELNOTES: Added a new `max_compatibility_level` attribute to the `bazel_dep` directive, which allows version selection to upgrade a dependency up to the specified compatibility level. Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 526118928 Change-Id: I332eb3761e0dee0cb7f318cb5d8d1780fca91be8 --------- Co-authored-by: Brentley Jones <[email protected]>
The class Module currently holds data that's no longer needed after resolution finishes (such as the compatibility level, bazel compatibility, the registry where it comes from, etc). Conversely, the repo spec field is not computed until the end of resolution. To remove runtime checks and reduce cognitive overhead, this CL splits the Module class into two; one only used after resolution finishes (Module), and one only used before (InterimModule). This allows us to introduce max_compatibility_level in a follow CL. Work towards bazelbuild#17378 Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 525780111 Change-Id: I2df8d78d324b3c8744ba0a4eda405d162e9bbb8c
See code comments for more details. tl;dr: we use the new `bazel_dep(max_compatibility_level=)` attribute to influence version selection. Fixes bazelbuild#17378 RELNOTES: Added a new `max_compatibility_level` attribute to the `bazel_dep` directive, which allows version selection to upgrade a dependency up to the specified compatibility level. Co-authored-by: Brentley Jones <[email protected]> PiperOrigin-RevId: 526118928 Change-Id: I332eb3761e0dee0cb7f318cb5d8d1780fca91be8
Description of the feature request:
bazel_dep
should gain something like amax_compatibility_level
which would allow specifying an upper bound for a dependency. Theversion
would imply the lowercompatibility_level
.What underlying problem are you trying to solve with this feature?
Image that rules_apple depends on another module, rules_swift, at
version=1.0.0/compatibility_level=1
. Then rules_swift makes an API breaking change and bumps toversion=2.0.0/compatibility_level=2
. In subsequent change to rules_apple it accommodates the new API in a way that it can use either version/compatibility_level of rules_swift. Currently there is no way to declare that inbazel_dep
, becausebazel_dep(name = "rules_swift", version = "1.0.0")
will cause a resolution issue if another dep or the root module declares a dependency on rules_swift 2.0.0, andbazel_dep(name = "rules_swift", version = "2.0.0")
will do the same the other way. Ideally it could do this instead:bazel_dep(name = "rules_swift", version = "1.0.0", max_compatibility_level = 2)
to say that it supports both.This is a real problem that I see happening very soon, just with different example rulesets (rules_xcodeproj and rules_apple).
Which operating system are you running Bazel on?
No response
What is the output of
bazel info release
?No response
If
bazel info release
returnsdevelopment version
or(@non-git)
, tell us how you built Bazel.No response
What's the output of
git remote get-url origin; git rev-parse master; git rev-parse HEAD
?No response
Have you found anything relevant by searching the web?
No response
Any other information, logs, or outputs that you want to share?
https://bazelbuild.slack.com/archives/C014RARENH0/p1675176300407969
The text was updated successfully, but these errors were encountered: