Skip to content

Add module-info.java#1486

Open
Sineaggi wants to merge 24 commits into
square:masterfrom
Sineaggi:add-module-info
Open

Add module-info.java#1486
Sineaggi wants to merge 24 commits into
square:masterfrom
Sineaggi:add-module-info

Conversation

@Sineaggi

@Sineaggi Sineaggi commented Jun 1, 2024

Copy link
Copy Markdown
Contributor

Adds a new java compile task (depends on jvm dependencies and kotlin class files) to compile a module-info.java file, and puts it into the jvm jar.

I've never done this for a kotlin-multiplatform project, so I'm sure there are improvements. I looked into using multi-platforms 'withJvm' feature, but that dropped the files that were compiled with java 8 directly into the resulting jar. At least with this custom sourceset approach, we can compile the file with java 9 and put the module-info.class file where we want (META-INF/versions/9).

@Sineaggi

Sineaggi commented Jun 1, 2024

Copy link
Copy Markdown
Contributor Author

The fixupmessage is necessary as okio is not yet #1363 on bnd 7.0.0 bndtools/bnd#2227.

EDIT: Fixed in #1487

@Sineaggi

Sineaggi commented Jun 3, 2024

Copy link
Copy Markdown
Contributor Author

We should be able to simplify the build once base-line increases to java > 8. We'd be able to remove the multi-release jar, and just use multiplatform's java source folder directly. We wouldn't need the META-INF/versions directory, nor the Multi-Release: true entry in the manifest.

@Sineaggi

Sineaggi commented Jun 3, 2024

Copy link
Copy Markdown
Contributor Author

I was able to simplify module compilation via the docs here https://kotlinlang.org/docs/gradle-configure-project.html#configure-with-java-modules-jpms-enabled.

@JakeWharton

Copy link
Copy Markdown
Collaborator

Unfortunately I suspect it will be a long while before we drop Java 8 support completely.

@Sineaggi

Sineaggi commented Jun 4, 2024

Copy link
Copy Markdown
Contributor Author

Unfortunately I suspect it will be a long while before we drop Java 8 support completely.

That's fine if you're alright with the mild additional complexity that an mr-jar brings along with it.

@JakeWharton

Copy link
Copy Markdown
Collaborator

It's probably worth it, yeah. I'll talk to Jesse about it soon and see if he has any concerns.

I have a similar setup (with some different complexities) in Retrofit. I should add a module descriptor there, too, although I have about 20 modules to add it to there...

@Sineaggi

Sineaggi commented Jun 4, 2024

Copy link
Copy Markdown
Contributor Author

It might be possible to remove the dependency on java.logging too, if on versions > 8 we use System.Logger instead. Afaik it's only used for one utility class, and even then only for warning.

@JakeWharton

Copy link
Copy Markdown
Collaborator

Yes, that would be great, too!

@Sineaggi

Sineaggi commented Jun 4, 2024

Copy link
Copy Markdown
Contributor Author

The logger classes have been added. I converted them to kotlin and made them internal to not trigger the api validator. Speaking of, I don't think the api-validator currently checks that the api is identical between two classes in an mr-jar.

@Sineaggi Sineaggi force-pushed the add-module-info branch 3 times, most recently from d99dc0e to 7913508 Compare June 5, 2024 01:03
@JakeWharton

Copy link
Copy Markdown
Collaborator

Yeah I saw your issue on BCV. Thanks for thinking of it. I'm not sure whether it's BCV's problem or not. From what I remember MR jars only replace classes at runtime and have no effect on compile-time. So even if you were to expose new API in one, it cannot be linked against in code (except through reflection at runtime).

@Sineaggi

Sineaggi commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

Code should be ready for review.

@Sineaggi

Sineaggi commented Jun 5, 2024

Copy link
Copy Markdown
Contributor Author

There's an issue where the new Logger class isn't being included in the jar.

@Sineaggi

Sineaggi commented Jun 6, 2024

Copy link
Copy Markdown
Contributor Author

Ok so if you don't exclude the META-INF folder that the java9 kotlin compile task generated, you'll get strange errors on java 11

Error occurred during initialization of boot layer
java.lang.NullPointerException

@Sineaggi

Copy link
Copy Markdown
Contributor Author

Is there anything else I've missed and should add? Any further simplifications to the build process?

@MukjepScarlet

Copy link
Copy Markdown

Conflicts...

@Sineaggi

Copy link
Copy Markdown
Contributor Author

Resolved conflicts.

As there are no more java.util.Logger instances in okio, I've removed the internal logger class.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants