JPMS support

[shs96c]

The JPMS has been part of the java platform since Java 9, and will be in an LTS release when Java 11 is released in September. Very slowly, projects are adopting the module system, but to do so the libraries that they depend on need to also support modules. It would be really helpful if okhttp had a module-info.class added somehow so we can make proper use of okhttp in a modularised build.

[shs96c]

The author of bytebuddy has created a maven plugin to generate this on a normal java 8 build: https://github.com/raphw/modulemaker-maven-plugin This links to: https://github.com/moditect/moditect which seems to be more fully-featured.

The selenium project has some magic called from their buck build that does something similar: https://github.com/SeleniumHQ/selenium/commit/e57914ab0550e7b755bfd664d61040ae7856b03d

It looks like okhttp uses maven, so the first option looks like a lower-stress way of adding this support without going having to move the entire build to Java 9+

[yschimke]

What is the feature you need here over just the defined names in the current jars? We set module names and even moved some packages around to have valid module jars already.

Just curious what isn’t possible currently.

[yschimke]

Reopen if there is something specific missing, over the current module support.

[Dragas]

JLink comes to mind. Automatic module usage prevents usage of jlink.

[yschimke]

Reopening temporarily for discussion, worth reviewing in the time since JDK 9.

What are other JDK libraries doing here? Is there established good practice.

[JakeWharton]

MR jars

[yschimke]

TIL: https://blog.gradle.org/mrjars

[JakeWharton]

Although maybe not the best introduction since Cedric is/was opposed to them. But yeah we can tuck a module descriptor into the jar without affecting Java 8 or Android so it seems like an obvious choice.

On Sat, Jun 20, 2020, at 1:51 AM, Yuri Schimke wrote:

TIL: https://blog.gradle.org/mrjars

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/square/okhttp/issues/4184#issuecomment-646946721, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAQIENXDO3VYGEOL44UMLLRXRE4XANCNFSM4FM25XBA.

[shs96c]

In Selenium we build for and target Java 8, but we add the module-info under META-INF/versions/9. This allows older tools that inspect bytecode in classes to continue to function as expected, since they don't know about the versioned directory, while allowing modern javas to use the module system properly.

I, too, am not a fan of using the MR jar approach to shadow classes, but for the case where the bytecode might be misinterpreted, I think that it's a reasonable solution.

[cowwoc]

@JakeWharton Can you please remove the needs info tag? Hopefully this will allow this feature request to move forward.

[cowwoc]

@yschimke A few months back I added multi-release JAR support to Hikari. It works great.

In Maven the change is as simple as https://github.com/apache/maven-compiler-plugin/blob/master/src/it/multirelease-patterns/singleproject-runtime/pom.xml#L102-L108. Hopefully Gradle makes it as easy.

[vatbub]

Sorry to bother, are there any updates on the issue?

[robertvazan]

Module support as currently implemented via Automatic-Module-Name breaks builds of JPMS projects, because several modules claim the same package. For example, both okhttp3 and okhttp3.urlconnection contain okhttp3 package. In JPMS, every package must be defined in only one module. You probably don't want to rename packages in 4.x, but would it be possible to fix this in 5.x? It's still alpha, so moving packages should be fine.

[swankjesse]

@robertvazan what happens if you don't use the okhttp3.urlconnection module? Does that work? I'm extremely reluctant to break binary compatibility anywhere ever.

[shs96c]

If there are two jars in the transitive classpath with the same Automatic-Module-Name the Java Module System will be unhappy. Since okhttp-urlconnection depends on okhttp, this means that you can't currently use okhttp-urlconnection using the module system (though you can place it on the classpath: it's just the modulepath that will cause problems)

Fortunately, the Automatic-Module-Name is only used by th JPMS, and since this issue suggests it's not working right now, the obvious fix (to change that value of that manifest entry in the okhttp-urlconnection jar) is safe to make without any concerns about binary compat.

The thing to watch out for are split packages, but this issue doesn't mention them, so I don't think that's an issue here.

[robertvazan]

@shs96c Actually, you got it the wrong way round. Module names are distinct, but package name is shared. Package renaming is required to fix this.

@swankjesse If I only use okhttp3 module, then it works of course, because there's no package ownership conflict. I wouldn't change it in 4.x if I were you as that would break a lot of applications. But 5.x must change it or it will be unusable in downstream apps and libraries that use modules.

If you don't want to rename packages at all, then there's another option. Just move urlconnection classes into okhttp3 module and leave urlconnection JAR empty. That will fix module-based apps without breaking any existing apps.

[yschimke]

@swankjesse is it an option in 5.0 to deprecate those locations, and have a new location that doesn't conflict? or a new module like we did for mockwebserver?

[swankjesse]

I think our best options is to move the URL connection code to a new package. The name as is doesn't make sense anymore anyway. Perhaps we do a okhttp-java-net module? Or two, okhttp-java-net-authenticator and okhttp-java-net-cookiejar.

[swankjesse]

Rant: JPMS is a bad module system because it doesn’t support splitting a package. It’s perfectly reasonable for a package to grow over time as its project grows. And it should also be possible to split a single module into multiple, just as OpenJDK did when they modularized. Requiring project owners to have perfect forsight on module boundaries is bad. (Can we achieve this with the compiled class-based configuration of JPMS?)

[yschimke]

+1 to the two modules. 5.0 is a good time for this. We may be able to retain compatibilty through in the existing location via creative indirection, but it's a good point to ask people to prefer the separate modules.

[Dragas]

Rant: JPMS is a bad module system because it doesn’t support splitting a package. It’s perfectly reasonable for a package to grow over time as its project grows. And it should also be possible to split a single module into multiple, just as OpenJDK did when they modularized. Requiring project owners to have perfect forsight on module boundaries is bad. (Can we achieve this with the compiled class-based configuration of JPMS?)

That's an awful take. Each archive packages groups of classes and should be the only archive to expose that particular group. How would you otherwise ensure that two fully qualified class names do not clash? Not to mention, how would you prevent a circular dependency between two such archives? Remember that you must be able to use JPMS without maven, gradle or what ever other dependency manager.

[yschimke]

I don't have as strong feelings as Jesse, but its definitely in the category (with osgi?) of opinionated package systems that for the median developer doesn't visibly improve anything in practice but causes a lot of pain to adopt. Median is a deliberate word, there are probably a lot of people who thrive because of its rules.

[robertvazan]

@yschimke As a median library developer, I get two immediate benefits from JPMS: Firstly, I can have private packages in my modules, which greatly simplifies design of larger libraries. Secondly, (sometimes extensive) dependencies of my libraries do not pollute downstream projects. Implementing JPMS is straightforward for most libraries. What we are discussing here is an exceptional situation.

[unverbraucht]

@swankjesse I'll open a PR where I refactor urlconnection into okhttp-java-net-authenticator and okhttp-java-net-cookiejar as proposed.

[unverbraucht]

If there is consensus that adding a versioned module-info through multi-MR is the way to go I can also look into that.

[cowwoc]

@swankjesse @unverbraucht What are the next steps now that Jesse's PR got merged? https://github.com/square/okhttp/pull/7996#issuecomment-1775360078

[unverbraucht]

My understanding is that the deprecation notices prepare for dropping the module that causes split packages with 5.0, for which we could also replace automatic module names with a module descriptor. Is there an agreement to use MR Jars just for the module-info.class?

Personally I think it's the best solution. The alternative (which I've used in some libs I ported over to JPMS) is to build the sources twice (once with Java 8, once with Java 9+) and then take all Java 8 classes plus the Java 9 module-info.class into a jar without hiding the module-info behind a version since Java 8 will ignore it anyway. It adds compile overhead and is generally a little fragile.

If there's consensus I can try my hand at a PR.

[JakeWharton]

MR jar is my vote.

Is there some reason to compile everything twice? We can make the compiled Java 8 classes available to the Java 9 source set if that's required. Or just have it be a disjoint compilation.

Do we need to do this in Okio first?

[cowwoc]

MR jar would be my vote too.

PS: The next step I would push for after this would be Virtual Thread support. Jetty has implemented this using reflection in order to maintain backwards compatibility. See https://github.com/jetty/jetty.project/blob/3a6ad49271a86f5b0842b2cef7628bd2d7e30198/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/VirtualThreads.java#L31 for an example.

Detection uses reflection, but once an Executor is returned you are running at full speed again. They are also adding capability for users to name virtual threads, though I haven't found the code for that yet.

[JakeWharton]

Virtual thread discussion is #8167

[cowwoc]

@JakeWharton That doesn't seem like a duplicate discussion. The issue you linked to was about making Virtual Threads the default. I am just interested in ensuring their use with OkHttp is possible. If that's already the case, there is nothing to do. But is it already the case? :)

[JakeWharton]

You have always been able to supply a custom executor, yes, so long as it meets your requirements for concurrency as configured on the dispatcher.

[unverbraucht]

MR jar is my vote.

Seconded.

Is there some reason to compile everything twice? We can make the compiled Java 8 classes available to the Java 9 source set if that's required. Or just have it be a disjoint compilation.

No, this is only for the use case where you don't want a MR module-info.classes (for Gradle or for Maven). I think the MR is cleaner. Do we need to do this in Okio first?

No, I think this can be done independently, although okio as automatic module defeats the purpose of the exercise since it inhibits jlink as well :)