Skip to content

Add multi-release jars to enable Java Modules#8767

Merged
yschimke merged 37 commits intosquare:masterfrom
yschimke:mrjar
Sep 21, 2025
Merged

Add multi-release jars to enable Java Modules#8767
yschimke merged 37 commits intosquare:masterfrom
yschimke:mrjar

Conversation

@yschimke
Copy link
Collaborator

@yschimke yschimke commented May 11, 2025

Attempting to apply multi-release Jars and use it to add java9/module-info.java files.

This should run a HTTP request against square.com using a jlink image.

./gradle module-tests:imageRun

fixes #4184

Todo

  • Enable OkHttp modules in the test to flush out errors
  • Setup a CI job probably testing against Mockserver

@yschimke yschimke requested a review from JakeWharton May 11, 2025 14:53
@yschimke yschimke added android Relates to usage specifically on Android providers Non core security providers jdkversions JDK 8, 17, 19 etc. labels May 11, 2025
@yschimke yschimke marked this pull request as ready for review May 11, 2025 15:53
@yschimke
Copy link
Collaborator Author

yschimke commented May 11, 2025

Current status! resolved

Exception in thread "main" java.lang.IllegalAccessError: class okhttp3.internal.concurrent.TaskRunner (in module okhttp3) cannot access class java.util.logging.Logger (in module java.logging) because module okhttp3 does not read module java.logging
	at okhttp3/okhttp3.internal.concurrent.TaskRunner.<clinit>(TaskRunner.kt:362)

@yschimke yschimke requested a review from swankjesse May 26, 2025 10:56
@yschimke
Copy link
Collaborator Author

Is this post 5.0?

yschimke added 4 commits June 21, 2025 11:03
# Conflicts:
#	okhttp/build.gradle.kts
# Conflicts:
#	buildSrc/build.gradle.kts
#	okhttp/build.gradle.kts
yschimke added 2 commits July 12, 2025 16:30
# Conflicts:
#	gradle/libs.versions.toml
@yschimke
Copy link
Collaborator Author

yschimke commented Jul 12, 2025

@swankjesse appropriate for 5.2?

Copy link
Collaborator

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code looks fine but I don’t much appreciate what it buys us or our users?

@@ -0,0 +1,6 @@
module okhttp3.modules {
requires okhttp3;
requires okhttp3.logging;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what the heck is okhttp3.logging ?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3743 (comment)

I think I misread your comment.

image

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or maybe favoured Stephen's comment over yours?

Your module name should be the same as the package name in that jar file, unless you have a good reason otherwise. ie. you as a human are deciding that one package is the base, and assigning that as the module name.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep yep, it’s the package name. I missed that we gave that module two different names.

requires jdk.crypto.ec;
requires org.junit.jupiter.api;
requires okhttp3.modules;
opens okhttp3.modules.test to org.junit.platform.commons;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do any developers do stuff like this in the wild?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exports okhttp3;
exports okhttp3.internal to okhttp3.logging, okhttp3.sse, okhttp3.java.net.cookiejar, okhttp3.dnsoverhttps, mockwebserver3, okhttp3.mockwebserver, okhttp3.coroutines, okhttp3.tls;
exports okhttp3.internal.platform to okhttp3.logging, okhttp3.java.net.cookiejar, okhttp3.dnsoverhttps, mockwebserver3, okhttp3.mockwebserver, okhttp3.tls;
exports okhttp3.internal.http to okhttp3.logging, okhttp3.brotli, mockwebserver3;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m still curious about what okhttp3.logging is

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A mistake from 2018...

Comment on lines +224 to +228
error: package okhttp3.internal.platform is not visible
okhttp3.internal.platform.Platform.get();
^
(package okhttp3.internal.platform is declared in module okhttp3,
which does not export it to module i.am.bad.and.i.should.feel.bad)
Copy link
Collaborator Author

@yschimke yschimke Jul 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swankjesse I guess this is the main point?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also added a CI test that jlink works, see https://www.baeldung.com/jlink

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@swankjesse do you need more convincing?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good behavior. I wouldn’t use i.am.bad.and.i.should.feel.bad as the example; this is not a place for shame

@yschimke yschimke mentioned this pull request Jul 19, 2025
@yschimke
Copy link
Collaborator Author

@swankjesse requesting post review.

Seems like a net good to support something some users are adopting. Happy to revert if you are completely against it.

@yschimke yschimke merged commit b7290e4 into square:master Sep 21, 2025
26 checks passed
Copy link
Collaborator

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines +224 to +228
error: package okhttp3.internal.platform is not visible
okhttp3.internal.platform.Platform.get();
^
(package okhttp3.internal.platform is declared in module okhttp3,
which does not export it to module i.am.bad.and.i.should.feel.bad)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good behavior. I wouldn’t use i.am.bad.and.i.should.feel.bad as the example; this is not a place for shame

// not needed when compiling with recent JDKs, e.g. 17
options.compilerArgs.add("-Xlint:-requires-transitive-automatic")

// Patch the compileKotlinJvm output classes into the compilation so exporting packages works correctly.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very helpful comment!

@@ -0,0 +1,6 @@
module okhttp3.modules {
requires okhttp3;
requires okhttp3.logging;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep yep, it’s the package name. I missed that we gave that module two different names.

// Note that module checking only works on JDK 9+,
// because the JDK built-in base modules are not available in earlier versions.
val javaVersion = compileKotlinTask.kotlinJavaToolchain.javaVersion.getOrNull()
when {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t really understand what this when{} is supposed to be doing

}

// From https://github.com/Kotlin/kotlinx-atomicfu/blob/master/atomicfu/build.gradle.kts
val compileJavaModuleInfo by tasks.registering(JavaCompile::class) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refactor this so it’s the same applyJavaModules() call we have in all of our other modules? Having two copy-pastes of this configuration will confuse me for all time

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(You could definitely have a parameter to toggle the options that only the okhttp module needs? Why doesn’t this module use me.champeau.mrjar?!)

Java Modules
------------

OkHttp (5.3+) implements Java 9 Modules.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the heck is OkHttp 5.3 ?! We have to ship 5.2 first!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ooops, can fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

android Relates to usage specifically on Android jdkversions JDK 8, 17, 19 etc. providers Non core security providers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JPMS support

3 participants