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

gRPC Java is not usable from Java 9 modules #3522

Closed
aseovic opened this issue Sep 29, 2017 · 44 comments · Fixed by #10313
Closed

gRPC Java is not usable from Java 9 modules #3522

aseovic opened this issue Sep 29, 2017 · 44 comments · Fixed by #10313
Labels
gRPC 2.0 Issue requires breaking stable APIs
Milestone

Comments

@aseovic
Copy link

aseovic commented Sep 29, 2017

Please answer these questions before submitting your issue.

What version of gRPC are you using?

1.6.1

What JVM are you using (java -version)?

java version "9"
Java(TM) SE Runtime Environment (build 9+181)
Java HotSpot(TM) 64-Bit Server VM (build 9+181, mixed mode)

What did you do?

Java 9 allows users to depend on older, non-modularized versions of the libraries by "converting" them to automatic modules. For example, when Maven dependencies on grpc are configured correctly, Java 9 allows me to do the following:

module myapp.foo {
    exports com.myapp.foo;
    requires grpc.core;
}

This allowed me to use classes from the grpc-core within my Java 9 module, but unfortunately it wouldn't compile:

ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.6.2:compile (default-compile) on project myapp.foo: Compilation failure: Compilation failure: 
[ERROR] the unnamed module reads package io.grpc from both grpc.core and grpc.context
[ERROR] module grpc.context reads package io.grpc from both grpc.core and grpc.context
[ERROR] module guava reads package io.grpc from both grpc.core and grpc.context
[ERROR] module error.prone.annotations reads package io.grpc from both grpc.core and grpc.context
[ERROR] module jsr305 reads package io.grpc from both grpc.core and grpc.context
[ERROR] module instrumentation.api reads package io.grpc from both grpc.core and grpc.context
[ERROR] module opencensus.api reads package io.grpc from both grpc.core and grpc.context
[ERROR] module grpc.core reads package io.grpc from both grpc.core and grpc.context

The issue is that Java 9 does not support split packages across modules and this is exactly what's happening here, as io.grpc package exists in both grpc-core and grpc-context, and to make things worse both grpc-core and grpc-stub have transitive dependency on grpc-context.

I've tried excluding grpc-context from both modules using Maven exclusions, which allowed me to compile successfully, as I don't have any direct dependencies on grpc-context. However, I was not able to run the test server, because of the missing Context class:

java.lang.NoClassDefFoundError: io/grpc/Context
        at io.grpc.internal.AbstractServerImplBuilder.build(AbstractServerImplBuilder.java:188)
        at io.grpc.testing.GrpcServerRule.before(GrpcServerRule.java:133)

There are several possible solutions, some better than the others:

  1. Merge classes from grpc-context into grpc-core and leave empty/dummy grpc-context module around for backwards compatibility (although most people probably do not depend on it directly).
  2. Do the same as above, but get rid of unnecessary grpc-context module.
  3. Rename the io.grpc package in grpc-context to io.grpc.context, which would eliminate split package issue, but would break existing code that uses classes from the current location.

In any case, I'm happy to help do the work, but someone will need to decide which approach to take.

@carl-mastrangelo
Copy link
Contributor

carl-mastrangelo commented Oct 1, 2017

The reason for the split is to keep the dependencies down for grpc-context.

  1. Merging won't work. It would add the dependencies back in.
  2. Same reason.
  3. This is probably the most reasonable path, but that would break backwards compatibility. That makes it not so reasonable.

A package per jar is my own preference, but that ship has sailed.

How is Java 9 supposed to work with generated classes, that live in a parallel package hierarchy but different jars?

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Oct 2, 2017 via email

@carl-mastrangelo
Copy link
Contributor

@adriancole the issue isn't that it's in the io.grpc realm, its that the same package is split over multiple jars. Making grpc-context contain io.grpc.context.Context would solve this.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Oct 2, 2017 via email

@carl-mastrangelo carl-mastrangelo added the gRPC 2.0 Issue requires breaking stable APIs label Oct 2, 2017
@aseovic
Copy link
Author

aseovic commented Oct 2, 2017

@carl-mastrangelo I can think of another option:

  1. Change the package name to io.grpc.context in grpc-context module. That will fix the split package issue going forward, but will break existing clients.

  2. Introduce io.grpc.Context and io.grpc.Deadline into grpc-core, which simply extend their io.grpc.context.* implementations in grpc-context. This would fix the clients that do not depend directly (and only) on grpc-context at the moment. It does imply, however, that io.grpc.context.Deadline could not be final, which it is at the moment.

Now, the bigger question is whether something like Context class even belongs in gRPC implementation, and I believe that's the 'top-leveling' @adriancole mentioned. It is a generally useful feature that most of us have written at least once, and would probably feel more 'at home' as java.util.concurrent.Context than anywhere else. The fact that the com.google.instrumentation:instrumentation-api depends on it also speaks volumes about where it does and does not belong...

Anyway, I've implemented a workaround that gets me unblocked for the time being by shading core, stub and context into a single JAR with a proper Java 9 module descriptor, but that's definitely not the right direction and I'm happy to help with both this issue and #3523 in any way I can (signed the CLA the other day, so we should be good to go from that perspective).

@zhangkun83
Copy link
Contributor

This was discussed in #2847

@ejona86
Copy link
Member

ejona86 commented Aug 13, 2018

Note: the generated code also can run afoul of Java 9 modules. If nothing else, the gRPC generated code is frequently in a separate JAR from the protobuf generated code, but the two exist in the same JAR.

@UkonnRa
Copy link

UkonnRa commented Oct 11, 2018

So now the problem still exists in Java 11. Anyone can provide a solution(even a temporary one)?
Module 'xxx' reads package 'io.grpc' from both 'grpc.core' and 'grpc.context'

@ejona86
Copy link
Member

ejona86 commented Oct 12, 2018

@CasterKKK, the only workaround I know is to repackage the two JARs into a single one as outlined in #2727 (comment)

@marx-freedom
Copy link

Same problem for our projects. Are there any plans to support JPMS?

mmichaelis added a commit to mmichaelis/poc-grpc that referenced this issue Feb 4, 2019
mmichaelis added a commit to mmichaelis/poc-grpc that referenced this issue Feb 4, 2019
In order to work around grpc/grpc-java#3522 JPMS
has been disabled within this project.
suadzh referenced this issue in wavesplatform/WavesJ May 24, 2019
* gRPC classes

* PB converters WIP

* PB converters WIP

* PB converters WIP

* PB converters WIP

* PB converters WIP

* Hash fixes

* Fixes

* Fixes

* Fixes

* Fixes

* Fixes

* Fixes
@lukecwik
Copy link

lukecwik commented Aug 8, 2019

Users have requested support for Java 11 in Apache Beam and since it uses grpc it would need for grpc libraries to be compatible with Java 11. Hopefully this request helps increase the priority for fixing this.

@lgajowy
Copy link

lgajowy commented Oct 16, 2019

Is there any progress or ETA for this issue maybe?

@ejona86
Copy link
Member

ejona86 commented Oct 16, 2019

We'll be looking into options this quarter. Main choices are to try to reduce dependencies of grpc-api and then combine grpc-context with grpc-api or to do a trick like com.google.guava:listenablefuture's empty version 9999.0.

@aseovic
Copy link
Author

aseovic commented Oct 17, 2019

In the meantime, we have repackaged grpc-java (version 1.22.1) as a Java 9+ module as part of Helidon, in order to support our gRPC framework implementation, and have made it available on Maven Central.

This module basically merges grpc-core, grpc-context and grpc-stub into a single JAR, and adds necessary module_info.java to it.

@ejona86 This is not to say that we are not looking forward to a proper Java 9+ module support in grpc-java, which will allow us to get rid of this "necessary evil". But we needed something now...

@rnayabed
Copy link

will this ever be fixed?

@dansiviter
Copy link

dansiviter commented Nov 10, 2020

Not sure if I'm missing something here, but isn't changing the major version the point of backwardly incompatible API changes?!

  • v1 branch: keep the status quo for the time being but deprecate,
  • v2 branch: refactor to io.grpc.context.* and implement module-info.java.

Progress on this sooner rather than later would have avoided potential impact to things like OpenTelemetry that is now bound to io.grpc.Context, however that's still in pre-release (0.10.0 at time of writing) so any package changes could be accommodated.
Update: Looks like I jumped the gun on OpenTelemetry, 0.10.0 has specifically moved away from gRPC context due to that exact reason.

@ejona86
Copy link
Member

ejona86 commented Nov 12, 2020

@dansiviter

Looks like I jumped the gun on OpenTelemetry, 0.10.0 has specifically moved away from gRPC context due to that exact reason.

No, that's not true. They had to make their own because their spec said they had to if there was not a commonly agreed upon one for the language. And even without this issue, there are many competing contexts in Java.

We cannot accept a v2 major version for this change. And if we did that would cascade downstream to all APIs that use it. If we introduced io.grpc.context.* it would just be within our current 1.x branch, since they have different names.

@dansiviter
Copy link

@ejona86 Although a breaking change is diametrically against keeping a major version number, if we can move from this impass with a package change in 1.x I think that will unblock a lot of developers. Any idea when this may happen?

@klaraward
Copy link

@ejona86 Any update on this, is this the best issue to follow?

@quimodotcom
Copy link

would still like support!

@dpratt
Copy link

dpratt commented Aug 31, 2021

Is there any particular reason why the various jars can't just have Automatic-Module-Name attributes added to their manifests? This doesn't preclude any future behavior or impose any other constraint on the library itself, but rather just makes it so downstream consumers can actually use the gRPC artifacts in their projects and refer to a stable module name.

Edit: Ignore me, I missed the bit about Context being in a split package.

@cjohnstoniv
Copy link

Following up on this a year later, is there any plans to actually make any required breaking changes to gRPC to fix this issue? I work for a large enterprise that has artifact on boarding requirements and gRPC sits at the core of our services. For many of these it is the lone dependencies that are not modularity compliant and we do not have the luxury of leveraging personal projects that hack around the issue.

It sounds like we have run out of ideas and have spent multiple years trying to figure a way out of this problem that doesn't involve breaking changes somewhere. It ultimately sounds like a path just needs to be picked and go with it, understanding no matter what is done its going to break someone.

IMO worrying about breaking people classpath's/dependency trees on upgrades should be the most minor of concerns. They are an absolute reality of Java and Maven dependency management. This already happens today across several grpc versions when guava is upgraded. The amount of times I've seen guava force multiple artifacts into a diamond dependency problem well surpasses the amount of gRPC projects that would be impacted by any modularity upgrades.

I'm willing to do the work, just need an SME and reviewer of gRPC to pick a path.

@dpratt
Copy link

dpratt commented Sep 1, 2021

I’d be willing to help out as well, but only if the maintainers bless the effort and can ensure that there’s at least a chance of it being merged. I’d really like to solve this problem as well, as it is for myself (and I suspect a gigantic number of others) the final bit preventing us from moving to use Java modules.

@AlexSolorio
Copy link

Checking in on this issue- is there any update on timeline for this? My organization is hoping to start using gRPC in our backend services, but our codebase is moving to Java 17 (and uses java modules), so it seems like this is not an option for us at this point.

I was looking into the Helidon workaround mentioned above, but it seems that there is no Helidon equivalent to the protoc-gen-grpc-java gradle plugin that we would need in order to generate Java classes from our .proto services.

Are there any other workarounds that I could look into for gRPC to be an option for us?

@aseovic
Copy link
Author

aseovic commented Dec 22, 2021

I was looking into the Helidon workaround mentioned above, but it seems that there is no Helidon equivalent to the protoc-gen-grpc-java gradle plugin that we would need in order to generate Java classes from our .proto services.

@AlexSolorio Helidon simply repackages existing grpc-java JARs into a single module, and adds module_info with necessary module configuration to it.

You can still use all standard tooling to generate your Java classes from the .proto files, such as protoc compiler, Maven and Gradle plugins, etc. We don't really change anything there, unless you are using Helidon gRPC framework, which is not required.

@AlexSolorio
Copy link

@aseovic Thanks for the quick reply 😃. From what I can tell, it seems there might be a conflict between the grpc protobuf plugin (io.grpc:protoc-gen-grpc-java:1.42.1) and the Helidon module (io.helidon.grpc:io.grpc:2.4.1). When I run a gradle build on my project (see attached build.gradle file contents), I run into errors about package io.grpc.stub does not exist and package io.grpc.protobuf does not exist.

Is there a Helidon version of io.grpc:protoc-gen-grpc-java:1.42.1 that I should be using instead?

build.gradle.txt

@aperrot42
Copy link

Did not find a simple a suitable way to import gRPC to our current java projects (we have decided to use modern java and modular builds).
Is there any chance that official gRPC client side libraries & reps get updated to work with java modules ? This is an important decision architecture-wise for us as it directs our adoption of gRPC.

@sanjaypujare
Copy link
Contributor

@aperrot42 you can take a look at https://github.com/grpc/proposal/blob/master/P5-jdk-version-support.md specifically https://github.com/grpc/proposal/blob/master/P5-jdk-version-support.md#rationale . As part of dropping support for Java8 may be Java9 modules will be supported?

@ejona86
Copy link
Member

ejona86 commented Feb 9, 2022

@sanjaypujare, dropping old version support doesn't magically fix the issues here. We need to manually resolve the grpc-context vs grpc-api issue which is the biggest issue. Then we can work on smaller problems like Automatic-Module-Name which may still have some fallout, e.g., inprocess is in grpc-core and maybe should be split out.

@sanjaypujare
Copy link
Contributor

@ejona86 thanks. I was thinking that the work done to drop support for Java8 will also include adding support for Java9 modules.

@pedrolamarao
Copy link

pedrolamarao commented Mar 30, 2023

I have just been bitten by this issue. If shading gRPC doesn't work, we will be forced to fall back to a raw HTTP interface for our project.

@ejona86
Copy link
Member

ejona86 commented Jun 26, 2023

This is being addressed in #10313 . It merges grpc-context into grpc-api. grpc-context will then be empty and will depend on grpc-api, but will exclude the unnecessary dependencies from grpc-api if you are just using io.grpc.Context. So the main difference for grpc-context-only users is the jar file increases in size, but at least no extra dependencies.

@Leejjon
Copy link

Leejjon commented Jun 29, 2023

I'm happy this has finally been resolved. I'll test it next month.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
gRPC 2.0 Issue requires breaking stable APIs
Projects
None yet
Development

Successfully merging a pull request may close this issue.