search

bazel-contrib / rules_jvm_external

Bazel rules to resolve, fetch and export Maven artifacts
Apache License 2.0
337 stars 256 forks source link

Allow root module's override tags to take precedence over the overridees from transitive deps. #1278

Open jin opened 2 weeks ago

jin commented 2 weeks ago

Occasionally, a bazel_dep can introduce a maven.overrides tag that aliases an artifact to another label. This may not be what the root module desires, and so we should allow the root module's overrides to take precedence over any transitive overrides, but at the caution of the root module owner.

Concretely, this is a problem encountered by the Bazel project itself, where a grpc-java transitive dep overrides @maven//:com_google_protobuf_protobuf_java as an alias to @@protobuf+//:protobuf_java, which was not desirable.

This PR is validated by a test that contains a root override from okhttp to javapoet, which takes precedence over a dependency's override from okhttp to a poison pill label.

meteorcloudy commented 2 weeks ago

Thanks! Is it possible for the root module to just remove the override instead of adding a new one? Not sure how to do it properly.

/cc @shs96c

jin commented 2 weeks ago

It's possible, but removing it instead of aliasing it to something else might cause the transitive bazel_dep to break?

meteorcloudy commented 2 weeks ago

It's possible, but removing it instead of aliasing it to something else might cause the transitive bazel_dep to break?

So I guess, if the root module wants to revert it back to the jar from maven, they can still do that with an override, right?

jin commented 2 weeks ago

It's possible, but removing it instead of aliasing it to something else might cause the transitive bazel_dep to break?

So I guess, if the root module wants to revert it back to the jar from maven, they can still do that with an override, right?

Yep! That's the idea. The root owner can override it back to the original (or any other target) if they are sure that the transitive dep is ok with it. But outright removing it will probably break the transitive dep.