-
Notifications
You must be signed in to change notification settings - Fork 314
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 multi registries #8955
Bazel multi registries #8955
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8955 +/- ##
=========================================
Coverage 67.64% 67.64%
Complexity 1168 1168
=========================================
Files 244 244
Lines 7783 7783
Branches 867 867
=========================================
Hits 5265 5265
Misses 2161 2161
Partials 357 357
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the minor nit the changes look good.
clients/bazel-module-registry/src/main/kotlin/LocalBazelModuleRegistryService.kt
Show resolved
Hide resolved
ad35a39
to
fb25bb5
Compare
There were multiple places that checked and transformed URLs to create either local or remote Bazel module registry service instances. Centralize this logic, so that redundancy can be reduced. Signed-off-by: Oliver Heger <[email protected]>
This is a special `BazelModuleRegistryService` implementation that wraps an arbitrary number of other registry services. On receiving a request, it delegates to the other registries and returns the first successful result. This implementation is going to be used to support multiple registries declared in a `.bazelrc` file. Signed-off-by: Oliver Heger <[email protected]>
For Bazel < 7.2.0, if the `.bazelrc` file lists multiple registries, use all of them to query package metadata. Fixes oss-review-toolkit#8801. Signed-off-by: Oliver Heger <[email protected]>
fb25bb5
to
e25b787
Compare
I don't understand all of the code but wonder why we need both the Is this due to the differences between Bazel <7.2 and >=7.2 lock files? Would it be possible to have those differences dealt with before |
You are mostly right; the different implementations deal with differences in the lock file formats. However, they apply different logic; therefore, I think they cannot be merged that easily:
There is a TODO comment in I have to admit that the names of the registry service implementations are probably not ideal. So, proposals for better names would be welcome. |
Thanks for the explanation! That makes sense.
Yeah, I also thought the names are confusingly generic and similar, but putting the Bazel version number in them seems worse and I couldn't come up with anything else either. |
This PR adds support for multiple registry services defined in the
.bazelrc
file.Taking over from @nnobelis during his vacation.