grpc / grpc-java

The Java gRPC implementation. HTTP/2 based RPC
https://grpc.io/docs/languages/java/
Apache License 2.0
11.37k stars 3.82k forks source link

When importing GRPC Java on bzlmod, we cannot control which version of guava is imported #11102

Closed hlawrence-cn closed 5 months ago

hlawrence-cn commented 5 months ago

What version of gRPC-Java are you using?

1.62.2 on bzlmod

What is your environment?

Mac in a bazel repo. Also reproducible in Linux.

What did you expect to see?

My application using the version of guava specified in our app at runtime.

What did you see instead?

My application being compiled against our specified version of guava... and run against guava-android from grpc-java at runtime.

Steps to reproduce the bug

Import guava-jre from rules_jvm Use a non-android API, like com.google.common.collect.ImmutableSet.toImmutableSet This works as expected

Import grpc-java via bzlmod Use in app com.google.common.collect.ImmutableSet.toImmutableSet calls fail at runtime with a java.lang.NoSuchMethodError

I can attempt to create a repo to repro if needed.

Perhaps separate targets could be used for android vs jre... though even in that case, we would be "stuck" with whatever version grpc-java uses. Something like netty-shaded could be used, but I'm sure you'd rather avoid shading. Some way to override dependencies in other repos within bzlmod could resolve... but I suspect that's complexity they'd rather not add. Not really sure what a sane solution is.

ejona86 commented 5 months ago

I am a bit worried about you seeing different runtime vs compile time behavior. That shouldn't really be happening with Bazel how this are configured. I think that may be caused by you having duplicate Guava's on the class path. That's a really bad idea as which one you get is fickle; you need a single version.

Some way to override dependencies in other repos within bzlmod could resolve

That's a bit stronger than you need. What I'd expect you to do is depend on -jre with at least the version used by gRPC for -android. So right now gRPC is using Guava 32.1.3-android. You should add a dependency on 32.1.3-jre or later. The "largest version wins" selection will then choose the JRE version instead of the Android version. That's the same approach used for Gradle.

So something like the below might work, in your own project that depends on gRPC.

maven.install(
    artifacts = ["com.google.guava:guava:32.1.3-jre"],
    repositories = ["https://repo.maven.apache.org/maven2/"],
    strict_visibility = True,
)
hlawrence-cn commented 5 months ago

I have a direct dependency on guava 33. When I was installed grpc-java dependencies via maven.install

    "io.grpc:grpc-services:" + IO_GRPC_JAVA_EXTERNAL_TAG,
    "io.grpc:grpc-netty-shaded:" + IO_GRPC_JAVA_EXTERNAL_TAG,
    "io.grpc:grpc-inprocess:" + IO_GRPC_JAVA_EXTERNAL_TAG,
    "io.grpc:grpc-testing:" + IO_GRPC_JAVA_EXTERNAL_TAG,

the correct guava was resolved, as we have another dependency "com.google.guava:guava:33.0.0-jre",

Issues came when I attempted to move to your bzlmod repo for the java targets ("@grpc-java//core", "@grpc-java//api" etc), as it seems to be using separate maven repos. Perhaps the solution here is that I should stick to maven for the java targets, and just use bzlmod for the grpc_services target?

ejona86 commented 5 months ago

Sounds like the issue is you are using a separate maven bzlmod repo than "maven".

Right now grpc_java_library doesn't let you use Maven-provided jars; it always uses Bazel-built gRPC. That should maybe change, but seems a separate discussion from bzlmod vs WORKSPACE.

shs96c commented 5 months ago

You should be able to control the guava version by setting it in the root project's MODULE.bazel's install tag. If that's not working, then please file an issue with rules_jvm_external

hlawrence-cn commented 5 months ago

https://github.com/hlawrence-cn/grpc-java-bazel-repro -- this should clarify what I'm seeing.

I am loading guava via jvm_external

I am loading grpc-java via bzlmod

I am loading grpc-java api, stubs, etc via the load from bzlmod

Amusingly in this repro, which version of guava is actually loaded is dependent on order of the deps array of my //code target. If guava is first in the array, guava-jre is loaded. If it's last, your version is loaded.

hlawrence-cn commented 5 months ago
bazel build //code
INFO: Analyzed target //code:code (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //code:code up-to-date:
  bazel-bin/code/code
  bazel-bin/code/code.jar
INFO: Elapsed time: 0.108s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

this works

bazel run //code
INFO: Analyzed target //code:code (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //code:code up-to-date:
  bazel-bin/code/code
  bazel-bin/code/code.jar
INFO: Elapsed time: 0.237s, Critical Path: 0.00s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action
INFO: Running command line: bazel-bin/code/code
Exception in thread "main" java.lang.NoSuchMethodError: 'java.util.stream.Collector com.google.common.collect.ImmutableSet.toImmutableSet()'
    at com.example.helloworld.HelloWorld.main(HelloWorld.java:8)

this does not

bazel_dep(name = "grpc-java", version = "1.62.2")
bazel_dep(name = "rules_jvm_external", version = "6.0")

maven = use_extension("@rules_jvm_external//:extensions.bzl", "maven")
maven.install(
    artifacts = [
        "com.google.guava:guava:33.0.0-jre",
    ],
)
use_repo(maven, "maven")

MODULE.bazel

java_binary(
    name = "code",
    srcs = glob(["src/main/java/**/*.java"]),
    main_class="com.example.helloworld.HelloWorld",
    deps = [
        "@grpc-java//services:health", "@grpc-java//api", "@grpc-java//core",
        "@grpc-java//stub", "@maven//:com_google_guava_guava", 
    ]
)

build target

hlawrence-cn commented 5 months ago

Note that if I don't import any grpc-java stuff, it works as expected, or if I change the order of deps. Unfortunately in a larger project there's not really a reasonable way to handle that... Excluding in maven.install did nothing since your jars are being built froom the grpc-java dep, not pulled from maven.

ejona86 commented 5 months ago

bazel_dep(name = "grpc-java", version = "1.62.2")

I see now. We had nothing to do wit this version in bazel-central-repository. This was a community change (https://github.com/bazelbuild/bazel-central-registry/pull/1699). When the author made the similar changes to this repo I had the repo name changed so you could add your own deps with maven.install (https://github.com/grpc/grpc-java/pull/11046#pullrequestreview-1968993446). So, yeah, I would expect 1.62.2 from bcr to be broken as you describe.

Using the support in master, like this, it seems to work:

git_override(
    commit = "1d6f1f1b4251191bddb9d6605fc8f8152275b6b7",
    module_name = "grpc-java",
    remote = "https://github.com/grpc/grpc-java.git",
)

There's not yet been a release with it, so you're probably best off avoiding bzlmod for grpc until the next release. Or someone could fix up the patches in bazel-central-registry to mirror the approach we recently merged.

hlawrence-cn commented 5 months ago

This does seem to work for me too! Look forward to the next release!