bazelbuild / rules_jvm_external

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

In BzlMod, overrides are not repo-specific #1180

Open Jonpez2 opened 1 week ago

Jonpez2 commented 1 week ago

When using bzlmod, if you invoke

maven.override( name = "repo1", coordinates = "foo.bar:baz", target = "//foo/bar:baz", )

and then invoke maven.install twice:

maven.install( name = "repo1", ... )

maven.install( name = "repo2", ... )

Then the override is applied to both repo1 and repo2.

It is unclear to me whether this is intended behaviour. I think it isn't, as the parameter 'name' (which I had assumed was intended to point to a 'name' in maven.install(...)) appears to be dropped in the override accumulation logic here https://github.com/bazelbuild/rules_jvm_external/blob/5393477ae7371b21ed0d97fe8202963e206efab8/private/extensions/maven.bzl#L232

My usecase is that I need support a configuration with an overridden dependency, as well as a configuration with the un-overridden dependency. Pre-bzlmod, I did that with two repos, which did not interfere with each other.

Thank you!

Jonpez2 commented 6 days ago

Any thoughts on this please? Thank you!

pcj commented 2 days ago

I also got bitten by this.

TL;DR: it appears intentional, not a bug per-se.

Similar story. I have the following:

maven.install(
    name = "maven_scala",
    artifacts = [
        "com.thesamet.scalapb:compilerplugin_2.12:0.11.17",
    ],
    repositories = ["https://repo1.maven.org/maven2"],
)

Here com.thesamet.scalapb:compilerplugin_2.12:0.11.17 has a dependency on com.google.protobuf:protobuf-java:3.19.6, but the following is generated:

jvm_import(
    name = "com_thesamet_scalapb_compilerplugin_2_12",
    visibility = ["//visibility:public"],
    tags = [
        "maven_coordinates=com.thesamet.scalapb:compilerplugin_2.12:0.11.17",
        "maven_repository=https://repo1.maven.org/maven2",
        "maven_sha256=a9dc6cc0dbe6ff53a7c914433d5a19711018217b432b385c97778cd4050210d0",
        "maven_url=https://repo1.maven.org/maven2/com/thesamet/scalapb/compilerplugin_2.12/0.11.17/compilerplugin_2.12-0.11.17.jar",
    ],
    jars = [
        "@maven_scala//:v1/https/repo1.maven.org/maven2/com/thesamet/scalapb/compilerplugin_2.12/0.11.17/compilerplugin_2.12-0.11.17.jar",
    ],
    srcjar = "@maven_scala//:v1/https/repo1.maven.org/maven2/com/thesamet/scalapb/compilerplugin_2.12/0.11.17/compilerplugin_2.12-0.11.17-sources.jar",
    deps = [
        "@com_google_protobuf//:protobuf_java",
        "@maven_scala//:com_thesamet_scalapb_protoc_gen_2_12",
        "@maven_scala//:org_scala_lang_modules_scala_collection_compat_2_12",
        "@maven_scala//:org_scala_lang_scala_library",
    ],
)

I could not for the life of me figure out how @com_google_protobuf//:protobuf_java was showing up here, since I did not ask for any overrides. To figure it out, I had to debug rules_jvm_external itself using a local_repo_override.

It turns out I also had in my MODULE.bazel:

 bazel_dep(name = "grpc-java", version = "1.64.0", repo_name = "io_grpc_grpc_java")

And that module invokes a maven.override which forcefully does maven.override(coordinates="com.google.protobuf:protobuf-java", target="@com_google_protobuf//:protobuf_java")

can be seen by:

$ bazel mod show_extension @rules_jvm_external//:extensions.bzl%maven

This was quite surprising that the maven.overrides declared in a dependency effectively determine it for all maven.install rules.

Bzlmod was supposed to eliminate "spooky action at a distance" subtleties from the WORKSPACE, but now we have all kinds of new surprises because the module_extension implementation functions can and will choose various different version selection behaviors like this one.

pcj commented 2 days ago

Perhaps the implementation function in maven.bzl should be modified such that overrides (and other configuration) are only shared across maven_install instances of they share the same name.

For example, grpc-java uses this:

use_repo(
  maven,
  "maven",
)

If I wanted to participate in the definition of @maven//... declared in grpc-java, I could also choose that name (and hope and pray), otherwise I'd be insulated from it. In this case since I chose the name maven_scala, they would remain independent.

@shs96c WDYT?

Jonpez2 commented 2 days ago

I think we need a bit more - at the moment there’s no real way to avoid the (and hope and pray) bit here. Concretely, if we request a version of a dep in the root module, and someone else requests one in some dependency (under the same repo name), we have no way to explicitly choose which one to use.

I think we may need a root-only api to set up resolution rules for a specific repo. We could make it very explicit saying ‘I expect to have this exact set of ambiguous resolved versions for this artifact, and the answer should be this’; and it could fail if the expected set was incorrect. This would introduce toil (since the ambiguities would change as dependencies tick along) but would help people understand and document the decisions they have made as things change over time.

I also think that even if the override behaviour described in this issue was intended, it’s not desirable. I’d of course be super interested to be reeducated by a maintainer please!

(This module stuff is definitely moving in the right direction, so thank you so much for the deep thought and great code you guys are putting in here)

On Sun, 30 Jun 2024 at 23:23, Paul Cody Johnston @.***> wrote:

Perhaps the implementation function in maven.bzl should be modified such that overrides (and other configuration) are only shared across maven_install instances of they share the same name.

For example, grpc-java uses this:

use_repo( maven, "maven", )

If I wanted to participate in the definition of @maven//... declared in grpc-java, I could also choose that name (and hope and pray), otherwise I'd be insulated from it.

— Reply to this email directly, view it on GitHub https://github.com/bazelbuild/rules_jvm_external/issues/1180#issuecomment-2198782428, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABN425J5A6V4WQ4IK6ZDL3DZKCAN5AVCNFSM6AAAAABJXHEAL6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJYG44DENBSHA . You are receiving this because you authored the thread.Message ID: @.***>