Use the artifact macro for loading maven deps#9574
Conversation
The recommended way to load dependencies from `rules_jvm_external` is to make use of the `@maven` workspace, and the most readable way of doing that is to use the `artifact` macro provides. This removes the need to generate the "compat" namespaces, which `rules_jvm_external` provided for backwards compatibility with older releases. This change also sets things up for supporting `bzlmod`: this requires all workspaces accessed by a library to be named "up front" in the `MODULE.bazel` file. This way, the only repo that needs to be exported is `@maven`, rather than the current huge list. Finally, this PR introduces a lock file for `rules_jvm_external` which improves local build times by avoiding the need to do a local resolution. In order to avoid the common failure case of "add a dep, forget to regenerate the lock file", the `fail_if_repin_required` attribute is set: builds will fail if the deps have been updated but the lock file hasn't been.
|
We've been purposefully avoiding using |
|
I'm +1 for this PR. Thank you for double-checking @ejona86! I'm one of the maintainers of Until |
ejona86
left a comment
There was a problem hiding this comment.
Must we have maven_install.json? I looked into that earlier and it seems super-annoying; basically meaning any version bumps can't be backported without manual conflict resolution. And all dependency changes must be made serially. I know you lose some things without it, but it solves problems we don't have and creates problems we don't need.
|
TL;DR: it's fine not to use the lock file. There's no need to use it, but it makes builds more reproducible. Builds may also be faster, since they can skip the dependency resolution step: I'm not sure how much of a difference that makes for you, and Bazel should be caching the results until the bazel server shuts down. |
ejona86
left a comment
There was a problem hiding this comment.
1.33.0 removed the manual maven repos from repositories.bzl, so it seems pretty safe to do this. The only risk I see is someone is using a tool other than maven_install to manage these deps, but I think it is worth a try to do this and see if any issues crop up.
|
Hmm... I don't know how #9559 will impact this (or everything, really) |
|
it would be great if we could merge this, I have a local change that supports bzlmod for this repo but it either depends on this or breaks compat w/ the non-bzlmod setup |
Turned back off compat repositories to swapping new targets to artifact() in a less-noisy commit. Conflicts: BUILD.bazel WORKSPACE api/BUILD.bazel core/BUILD.bazel netty/BUILD.bazel okhttp/BUILD.bazel services/BUILD.bazel testing/BUILD.bazel xds/BUILD.bazel
The recommended way to load dependencies from
rules_jvm_externalis to make use of the@mavenworkspace, and the most readable way of doing that is to use theartifactmacro provides.This removes the need to generate the "compat" namespaces, which
rules_jvm_externalprovided for backwards compatibility with older releases. This change also sets things up for supportingbzlmod: this requires all workspaces accessed by a library to be named "up front" in theMODULE.bazelfile. This way, the only repo that needs to be exported is@maven, rather than the current huge list.Finally, this PR introduces a lock file for
rules_jvm_externalwhich improves local build times by avoiding the need to do a local resolution. In order to avoid the common failure case of "add a dep, forget to regenerate the lock file", thefail_if_repin_requiredattribute is set: builds will fail if the deps have been updated but the lock file hasn't been.