Skip to content
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

cyclic dependency issue in 1.58.0 (Bazel) #10576

Closed
patelprateek opened this issue Sep 27, 2023 · 20 comments
Closed

cyclic dependency issue in 1.58.0 (Bazel) #10576

patelprateek opened this issue Sep 27, 2023 · 20 comments
Milestone

Comments

@patelprateek
Copy link

patelprateek commented Sep 27, 2023

What version of gRPC-Java are you using?

1.58.0

What is your environment?

jdk 11, bazel

What did you expect to see?

succesful compilation

What did you see instead?

compilation error

Steps to reproduce the bug

cloud storage 2.27.0

using gcs cloud storage 2.27.0 and grpc-java 1.58.0

 @maven//:com_google_cloud_google_cloud_storage
    @maven//:io_grpc_grpc_inprocess
.-> @maven//:io_grpc_grpc_core
|   @maven//:io_grpc_grpc_util
`-- @maven//:io_grpc_grpc_core

OR

@maven//:com_google_cloud_google_cloud_storage
  @maven//:io_grpc_grpc_netty_shaded
.-> @maven//:io_grpc_grpc_core
|  @maven//:io_grpc_grpc_util
`-- @maven//:io_grpc_grpc_core

cyclic depdendency issue

If i use an older version of gfprc-java 1.56.0 or older then
noSuchMethodError
"java.lang.NoSuchMethodError: 'void io.grpc.internal.AbstractManagedChannelImplBuilder.(java.lang.String)'"

=> java.lang.NoSuchMethodError: 'void io.grpc.internal.AbstractManagedChannelImplBuilder.(java.lang.String)'

io.grpc.netty.NettyChannelBuilder.<init>(NettyChannelBuilder.java:147)

io.grpc.netty.NettyChannelBuilder.<init>(NettyChannelBuilder.java:142)

I saw that the particular method has been removed in 1.58.0

@ejona86
Copy link
Member

ejona86 commented Sep 27, 2023

I think inherently this error is a bug in https://github.com/bazelbuild/rules_jvm_external/ .

What Bazel version are you using? I used this snippet in a WORKSPACE, and it seemed to work fine for me in Bazel 6.3.2:

maven_install(
    artifacts = [
        "com.google.cloud:google-cloud-storage:2.27.0",
        "io.grpc:grpc-okhttp:1.58.0",
    ],
    generate_compat_repositories = True,
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)

We do generally prefer/encourage building from source, like in our examples/WORKSPACE. I know that since #10217 the README has mentioned using artifacts from Maven.

@ejona86
Copy link
Member

ejona86 commented Sep 27, 2023

Doh, rules_jvm_version is what really matters. This is what we had in the WORKSPACE I tested in:

http_archive(
    name = "rules_jvm_external",
    sha256 = "c21ce8b8c4ccac87c809c317def87644cdc3a9dd650c74f41698d761c95175f3",
    strip_prefix = "rules_jvm_external-1498ac6ccd3ea9cdb84afed65aa257c57abf3e0a",
    url = "https://github.com/bazelbuild/rules_jvm_external/archive/1498ac6ccd3ea9cdb84afed65aa257c57abf3e0a.zip",
)

@patelprateek
Copy link
Author

@patelprateek
Copy link
Author

If the POM itself are showing cyclic dependency , i am quite unsure how did work for you ?
in your example why io.grpc:grpc-okhttp ?

for example i have the following depdendencies in my bazel dependencies yaml config

   # Cloud services
      - artifact: "com.amazonaws:aws-java-sdk-s3:{{ versions.aws }}"
      - artifact: "com.amazonaws:aws-java-sdk-sts:{{ versions.aws }}"
      - artifact: "com.google.cloud:google-cloud-storage:2.1.6"
      - artifact: "com.google.cloud:google-cloud-core:2.1.6"

      # Google
      - artifact: "com.google.auth:google-auth-library-oauth2-http:0.26.0"
        force: True

      # Grpc
      - artifact: "io.grpc:grpc-services:{{ versions.grpc }}"
      - artifact: "io.grpc:grpc-netty:{{ versions.grpc }}"
      - artifact: "io.grpc:grpc-core:{{ versions.grpc }}"
      - artifact: "io.grpc:grpc-stub:{{ versions.grpc }}"
      - artifact: "io.grpc:grpc-api:{{ versions.grpc }}"
      - artifact: "io.grpc:grpc-auth:{{ versions.grpc }}"

@ejona86
Copy link
Member

ejona86 commented Sep 27, 2023

grpc-core has a runtime dependency on grpc-util. That's how it works. I'll note that neither Gradle nor Maven have a problem with it.

Build label: 4.2.2

Unclear if that is Bazel version or rules_jvm_external. But it looks like rules_jvm_external didn't have a 4.2.2, so that must be the Bazel version. Bazel 4 isn't supported by anyone, to my knowledge.

Because you are using an older Bazel, you are probably using an older rules_jvm_external. I suggest upgrading rules_jvm_external and reporting if that fixes the problem. I provided what I did to try and reproduce, and it seemed to be working, so it seems this is possibly as simple as "upgrade your tools and the problem is fixed."

@patelprateek
Copy link
Author

upgrading bazel is non trivial effort and requires coordination with another repo as well
i upgraded the rules_jvm_external

using storage 2.27.0 and grpc-java 1.56.1 works
but storage 2.27.1 and grpc-java 1.58.0 is still giving cyclci dependency .

Can you please elaborate on why the rules_jvm_external could have been the issue , doesnt it generate the dependency list based on the POM ? it is unclear to me how we differentiate in the bazel depdendency list what is runtime vs compile time , they are just all included in dependency list

@ejona86
Copy link
Member

ejona86 commented Sep 28, 2023

i upgraded the rules_jvm_external

What version did you upgrade from and to? If it was a version from the past year, I think you need to provide a reproduction example, as it doesn't reproduce for me.

rules_jvm_external is what powers maven_install. It is the thing producing the error... (actually coursier is the component, but I think rules_jvm_external packages that dependency.)

it is unclear to me how we differentiate in the bazel depdendency list what is runtime vs compile time , they are just all included in dependency list

Sure, but that shouldn't matter. You aren't specifying dependencies between those in the list.

When you use a particular dependency, you'd use java_library(runtime_deps=":the_dep") for a runtime-only dependency vs the normal java_library(deps=":the_dep") compile dependency.

@patelprateek
Copy link
Author

we internally use a forked Repo from rules_jvm_external
Rebased from upstream rules_jvm_external master at commit: 93386f

@ejona86
Copy link
Member

ejona86 commented Sep 29, 2023

I've reproduced it. The error wasn't in coursier but in Bazel itself. And in my earlier test I think I didn't trigger the right target during a build.

With WORKSPACE:

# rules_jvm_external boilerplate
load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive")

RULES_JVM_EXTERNAL_TAG = "5.3"

RULES_JVM_EXTERNAL_SHA = "d31e369b854322ca5098ea12c69d7175ded971435e55c18dd9dd5f29cc5249ac"

http_archive(
    name = "rules_jvm_external",
    sha256 = RULES_JVM_EXTERNAL_SHA,
    strip_prefix = "rules_jvm_external-%s" % RULES_JVM_EXTERNAL_TAG,
    url = "https://github.com/bazelbuild/rules_jvm_external/releases/download/%s/rules_jvm_external-%s.tar.gz" % (RULES_JVM_EXTERNAL_TAG, RULES_JVM_EXTERNAL_TAG),
)

load("@rules_jvm_external//:repositories.bzl", "rules_jvm_external_deps")

rules_jvm_external_deps()

load("@rules_jvm_external//:setup.bzl", "rules_jvm_external_setup")

rules_jvm_external_setup()
# END rules_jvm_external boilerplate

load("@rules_jvm_external//:defs.bzl", "maven_install")

maven_install(
    artifacts = [
        "io.grpc:grpc-okhttp:1.58.0",
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)
$ bazel build @maven//:io_grpc_grpc_okhttp
ERROR: CACHE/3b4b3cfdd7a4382a6a990a3244c39990/external/maven/BUILD:243:11: in jvm_import rule @maven//:io_grpc_grpc_util: cycle in dependency graph:
    @maven//:io_grpc_grpc_okhttp (442d8db79c046018027f86fb6ba2e9e3560c56c0504c8b6423d08c7d06207c4d)
.-> @maven//:io_grpc_grpc_util (442d8db79c046018027f86fb6ba2e9e3560c56c0504c8b6423d08c7d06207c4d)
|   @maven//:io_grpc_grpc_core (442d8db79c046018027f86fb6ba2e9e3560c56c0504c8b6423d08c7d06207c4d)
`-- @maven//:io_grpc_grpc_util (442d8db79c046018027f86fb6ba2e9e3560c56c0504c8b6423d08c7d06207c4d)
ERROR: Analysis of target '@maven//:io_grpc_grpc_okhttp' failed; build aborted
INFO: Elapsed time: 0.339s
INFO: 0 processes.
FAILED: Build did NOT complete successfully (45 packages loaded, 391 targets configured)

Looking at the generated BUILD file, it doesn't use runtime_deps for the runtime-scoped:

jvm_import(
        name = "io_grpc_grpc_core",
        jars = ["v1/https/repo.maven.apache.org/maven2/io/grpc/grpc-core/1.58.0/grpc-core-1.58.0.jar"],
        deps = [
                ":com_google_android_annotations",
                ":com_google_code_gson_gson",
                ":com_google_errorprone_error_prone_annotations",
                ":com_google_guava_guava",
                ":io_grpc_grpc_api",
                ":io_grpc_grpc_context",
                ":io_grpc_grpc_util",
                ":io_perfmark_perfmark_api",
                ":org_codehaus_mojo_animal_sniffer_annotations",
        ],
        tags = [
                "maven_coordinates=io.grpc:grpc-core:1.58.0",
                "maven_url=https://repo.maven.apache.org/maven2/io/grpc/grpc-core/1.58.0/grpc-core-1.58.0.jar",
        ],
        visibility = ["//visibility:public"],
)

And that's because jvm_import doesn't support runtime_deps, unlike java_import. So this is simply "rules_jvm_external is broken."

A workaround looks to be to add an exclusion from grpc-core to grpc-util:

load("@rules_jvm_external//:specs.bzl", "maven")

maven_install(
    artifacts = [
        "io.grpc:grpc-okhttp:1.58.0",
        maven.artifact(
            artifact = "grpc-core",
            exclusions = [
                "io.grpc:grpc-util",
            ],
            group = "io.grpc",
            version = "1.58.0",
        ),
    ],
    repositories = [
        "https://repo.maven.apache.org/maven2/",
    ],
)

Note that grpc-util contains the round_robin load balancer, so you may need to depend on grpc-util manually. For "com.google.cloud:google-cloud-storage:2.27.0" in particular, this isn't a problem, because it depends on grpc-rls and grpc-services, both of which depend on grpc-util.

dependencies {
    implementation(platform("io.grpc:grpc-bom:1.58.0")) {
        exclude group: "io.grpc", module: "grpc-util"
    }
    implementation "com.google.cloud:google-cloud-storage:2.27.0"
}
$ gradle dependencies --configuration runtimeClasspath

> Task :dependencies

------------------------------------------------------------
Root project 'examples'
------------------------------------------------------------

runtimeClasspath - Runtime classpath of source set 'main'.
+--- io.grpc:grpc-bom:1.58.0
|    +--- io.grpc:grpc-alts:1.58.0 (c)
|    +--- io.grpc:grpc-grpclb:1.58.0 (c)
|    +--- io.grpc:grpc-auth:1.58.0 (c)
|    +--- io.grpc:grpc-protobuf:1.58.0 (c)
|    +--- io.grpc:grpc-protobuf-lite:1.58.0 (c)
|    +--- io.grpc:grpc-context:1.58.0 (c)
|    +--- io.grpc:grpc-api:1.58.0 (c)
|    +--- io.grpc:grpc-netty-shaded:1.58.0 (c)
|    +--- io.grpc:grpc-core:1.58.0 (c)
|    +--- io.grpc:grpc-stub:1.58.0 (c)
|    +--- io.grpc:grpc-googleapis:1.58.0 (c)
|    +--- io.grpc:grpc-xds:1.58.0 (c)
|    +--- io.grpc:grpc-services:1.58.0 (c)
|    \--- io.grpc:grpc-rls:1.58.0 (c)
\--- com.google.cloud:google-cloud-storage:2.27.0
     +--- com.google.guava:guava:32.1.2-jre
     |    \--- com.google.guava:guava-parent:32.1.2-jre
     +--- com.google.guava:failureaccess:1.0.1
     +--- com.google.guava:listenablefuture:9999.0-empty-to-avoid-conflict-with-guava
     +--- com.google.errorprone:error_prone_annotations:2.18.0 -> 2.20.0
     +--- com.google.j2objc:j2objc-annotations:2.8
     +--- com.google.http-client:google-http-client:1.43.3
     +--- io.opencensus:opencensus-contrib-http-util:0.31.1
     +--- com.google.http-client:google-http-client-jackson2:1.43.3
     +--- com.google.http-client:google-http-client-gson:1.43.3
     +--- com.google.api-client:google-api-client:2.2.0
     +--- commons-codec:commons-codec:1.15
     +--- com.google.oauth-client:google-oauth-client:1.34.1
     +--- com.google.http-client:google-http-client-apache-v2:1.43.3
     +--- com.google.apis:google-api-services-storage:v1-rev20230907-2.0.0
     +--- com.google.code.gson:gson:2.10.1
     +--- com.google.cloud:google-cloud-core:2.23.0
     +--- com.google.auto.value:auto-value-annotations:1.10.2
     +--- com.google.cloud:google-cloud-core-http:2.23.0
     +--- com.google.http-client:google-http-client-appengine:1.43.3
     +--- com.google.api:gax-httpjson:2.33.0
     +--- com.google.cloud:google-cloud-core-grpc:2.23.0
     +--- com.google.api:gax:2.33.0
     +--- com.google.api:gax-grpc:2.33.0
     +--- io.grpc:grpc-alts:1.56.1 -> 1.58.0
     +--- io.grpc:grpc-grpclb:1.56.1 -> 1.58.0
     +--- org.conscrypt:conscrypt-openjdk-uber:2.5.2
     +--- io.grpc:grpc-auth:1.56.1 -> 1.58.0
     +--- io.grpc:grpc-protobuf:1.56.1 -> 1.58.0
     +--- io.grpc:grpc-protobuf-lite:1.56.1 -> 1.58.0
     +--- com.google.auth:google-auth-library-credentials:1.19.0
     +--- com.google.auth:google-auth-library-oauth2-http:1.19.0
     +--- com.google.api:api-common:2.16.0
     +--- javax.annotation:javax.annotation-api:1.3.2
     +--- io.opencensus:opencensus-api:0.31.1
     +--- io.grpc:grpc-context:1.56.1 -> 1.58.0
     |    \--- io.grpc:grpc-api:1.58.0
     |         +--- com.google.code.findbugs:jsr305:3.0.2
     |         \--- com.google.errorprone:error_prone_annotations:2.20.0
     +--- com.google.api.grpc:proto-google-iam-v1:1.19.0
     +--- com.google.protobuf:protobuf-java:3.23.2
     +--- com.google.protobuf:protobuf-java-util:3.23.2
     +--- com.google.api.grpc:proto-google-common-protos:2.24.0
     +--- org.threeten:threetenbp:1.6.8
     +--- com.google.api.grpc:proto-google-cloud-storage-v2:2.27.0-alpha
     +--- com.google.api.grpc:grpc-google-cloud-storage-v2:2.27.0-alpha
     +--- com.google.api.grpc:gapic-google-cloud-storage-v2:2.27.0-alpha
     +--- com.fasterxml.jackson.core:jackson-core:2.15.2
     |    \--- com.fasterxml.jackson:jackson-bom:2.15.2
     |         \--- com.fasterxml.jackson.core:jackson-core:2.15.2 (c)
     +--- com.google.code.findbugs:jsr305:3.0.2
     +--- io.grpc:grpc-api:1.56.1 -> 1.58.0 (*)
     +--- io.grpc:grpc-netty-shaded:1.56.1 -> 1.58.0
     +--- io.perfmark:perfmark-api:0.26.0
     +--- io.grpc:grpc-core:1.56.1 -> 1.58.0
     |    +--- io.grpc:grpc-api:1.58.0 (*)
     |    +--- com.google.code.gson:gson:2.10.1
     |    +--- com.google.android:annotations:4.1.1.4
     |    +--- org.codehaus.mojo:animal-sniffer-annotations:1.23
     |    +--- com.google.errorprone:error_prone_annotations:2.20.0
     |    +--- io.perfmark:perfmark-api:0.26.0
     |    +--- io.grpc:grpc-context:1.58.0 (*)
     |    \--- io.grpc:grpc-util:1.58.0
     |         +--- io.grpc:grpc-core:1.58.0 (*)
     |         \--- org.codehaus.mojo:animal-sniffer-annotations:1.23
     +--- com.google.android:annotations:4.1.1.4
     +--- org.codehaus.mojo:animal-sniffer-annotations:1.23
     +--- io.grpc:grpc-stub:1.56.1 -> 1.58.0
     +--- io.grpc:grpc-googleapis:1.56.1 -> 1.58.0
     +--- org.checkerframework:checker-qual:3.33.0
     +--- io.grpc:grpc-xds:1.56.1 -> 1.58.0
     +--- io.opencensus:opencensus-proto:0.2.0
     +--- io.grpc:grpc-services:1.56.1 -> 1.58.0
     |    +--- io.grpc:grpc-util:1.58.0 (*)
     |    +--- com.google.j2objc:j2objc-annotations:2.8
     |    \--- com.google.code.gson:gson:2.10.1
     +--- com.google.re2j:re2j:1.7
     \--- io.grpc:grpc-rls:1.56.1 -> 1.58.0
          \--- io.grpc:grpc-util:1.58.0 (*)

@vorburger
Copy link
Contributor

I have just run into this as well while trying to bump gRPC from 1.54.1 to 1.58.0 in https://github.com/enola-dev/enola.

Copy/pasting the suggested workaround above verbatim is giving me the following error; I'm looking into it:

ERROR: in tag at <root>/MODULE.bazel:22:14, error converting value for attribute artifacts: expected value of type 'string' for element 9 of artifacts, but got None (NoneType)

@vorburger
Copy link
Contributor

vorburger commented Sep 29, 2023

Copy/pasting the suggested workaround above verbatim is giving me the following error; I'm looking into it:

Oh, this might hav something to do with https://github.com/bazelbuild/rules_jvm_external#artifact-helper-macro? Except I have that maven_install in my MODULE.bazel, not a BUILD, so I can't load("@rules_jvm_external//:defs.bzl", "artifact") ...

vorburger added a commit to vorburger/enola that referenced this issue Sep 29, 2023
vorburger added a commit to vorburger/rules_jvm_external that referenced this issue Sep 29, 2023
I initially struggled with how apply the workaround
from grpc/grpc-java#10576
for my enola-dev/enola#295;
hopefully this docs enhancement helps others later.
shs96c pushed a commit to bazelbuild/rules_jvm_external that referenced this issue Oct 11, 2023
I initially struggled with how apply the workaround
from grpc/grpc-java#10576
for my enola-dev/enola#295;
hopefully this docs enhancement helps others later.
@larry-safran
Copy link
Contributor

It appears that you were able to resolve the problems and upgrade to v1.58.0

@pkoenig10
Copy link
Contributor

I'll note that neither Gradle nor Maven have a problem with it.

Although basic usage of Gradle does not cause problems, there are Gradle APIs that don't work when there are cyclical dependencies. For example, ResolvedDependency#getAllModuleArtifacts:

gradle/gradle#22850

This API is used not infrequently in Gradle plugins and this prevents consumers from upgrading their gRPC dependencies.

I don't disagree that, ideally, build tools would handle cyclical dependencies appropriately. But this is often not the case and it feels preferable to avoid cyclical dependencies here if at all possible.

@vorburger
Copy link
Contributor

vorburger commented Jan 17, 2024

@ejona86 as per this comment I'm still seeing this, reproducible, over in enola-dev/enola#390 as of right now, at ec355b2.

@larry-safran would it be possible to re-open this issue?

@vorburger
Copy link
Contributor

Because you are using an older Bazel, you are probably using an older rules_jvm_external. I suggest upgrading rules_jvm_external and reporting if that fixes the problem. I provided what I did to try and reproduce, and it seemed to be working, so it seems this is possibly as simple as "upgrade your tools and the problem is fixed."

In enola-dev/enola#390 as of right now, at 5b62332 I'm now on bazel_dep(name = "rules_java", version = "7.3.2") and bazel_dep(name = "rules_jvm_external", version = "5.3") which is the latest of both of these, and still seeing this, reproducible.

So this is simply "rules_jvm_external is broken."

Might this be worth for you to report on rules_jvm_external?

A workaround looks to be to add an exclusion from grpc-core to grpc-util:

I'm trying to figure out how to do such an exclusion in MODULE.bazel instead of in WORKSPACE.bazel...

vorburger added a commit to vorburger/enola that referenced this issue Jan 17, 2024
@vorburger
Copy link
Contributor

vorburger commented Jan 17, 2024

I'm trying to figure out how to do such an exclusion in MODULE.bazel instead of in WORKSPACE.bazel...

Done in enola-dev/enola@964e8a0, and seems to work! 🥇

Interestingly, it now still works after removing the exclusion with enola-dev/enola@2ec63f6... hm, curious. It may be because I "unified" those 2 separate Maven Installs? Anyway. Moving on!

@larry-safran
Copy link
Contributor

@vorburger Since it is working for you now, do you still wish this issue to be reopened?

@vorburger
Copy link
Contributor

@larry-safran no, all good as-is; thanks for asking!

@ekohlwey
Copy link

@larry-safran may I suggest reopening this issue? The workaround in the patch mentioned above is basically not to use bazel modules, which is not really the Right Thing given that WORKSPACE files are deprecated in Bazel 7.0.0+

@ejona86
Copy link
Member

ejona86 commented Jan 29, 2024

@ekohlwey, we aren't aware of any incompatibility with bzlmod, except you may need to upgrade rules_jvm_external because of a fixed bug: bazelbuild/rules_jvm_external#966 (comment) . It looks like you got a response from your rules_jvm_external issue for the exclusion syntax.

ejona86 added a commit to ejona86/grpc-java that referenced this issue Feb 15, 2024
Add the 'fake' dependency to grpc-netty instead of grpc-core.
grpc-okhttp already depends on grpc-util and probably would be fine
without round_robin on Android.

There's not actually a circular dependency, but some tools can't handle
the compile vs runtime distinction. Such tools are broken, but fixes
have been slow and this approach works with no real downfalls.

Works around grpc#10576 grpc#10701
ejona86 added a commit that referenced this issue Feb 16, 2024
Add the 'fake' dependency to grpc-netty instead of grpc-core.
grpc-okhttp already depends on grpc-util and probably would be fine
without round_robin on Android.

There's not actually a circular dependency, but some tools can't handle
the compile vs runtime distinction. Such tools are broken, but fixes
have been slow and this approach works with no real downfalls.

Works around #10576 #10701
ejona86 added a commit to ejona86/grpc-java that referenced this issue Feb 16, 2024
Add the 'fake' dependency to grpc-netty instead of grpc-core.
grpc-okhttp already depends on grpc-util and probably would be fine
without round_robin on Android.

There's not actually a circular dependency, but some tools can't handle
the compile vs runtime distinction. Such tools are broken, but fixes
have been slow and this approach works with no real downfalls.

Works around grpc#10576 grpc#10701
@ejona86 ejona86 added this to the 1.63 milestone Feb 16, 2024
ejona86 added a commit that referenced this issue Feb 16, 2024
Add the 'fake' dependency to grpc-netty instead of grpc-core.
grpc-okhttp already depends on grpc-util and probably would be fine
without round_robin on Android.

There's not actually a circular dependency, but some tools can't handle
the compile vs runtime distinction. Such tools are broken, but fixes
have been slow and this approach works with no real downfalls.

Works around #10576 #10701
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants