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

Add jlink-compatible Java9/Jigsaw module-info (not automatic modules) #125

Closed
xzel23 opened this issue Mar 24, 2018 · 39 comments · Fixed by #286
Closed

Add jlink-compatible Java9/Jigsaw module-info (not automatic modules) #125

xzel23 opened this issue Mar 24, 2018 · 39 comments · Fixed by #286

Comments

@xzel23
Copy link

xzel23 commented Mar 24, 2018

Latest version has automatic modules and can be used in Java 9 projects (#114). However, this is not compatible with the jlink tool.

Steps to reproduce the problem (provide example input):

  • create a Java 9 based project that uses modules
  • Use jlink

Expected behavior:

jlink links the prokect

Actual behavior:

jlink reports an error:

> Task :link FAILED
Error: automatic module cannot be used with jlink: org.commonmark.ext.gfm.strikethrough from file:///Users/axel/.gradle/caches/modules-2/files-2.1/com.atlassian.commonmark/commonmark-ext-gfm-strikethrough/0.11.0/2b3f085711511202c380f2540760d64302c6bb7e/commonmark-ext-gfm-strikethrough-0.11.0.jar

@xzel23
Copy link
Author

xzel23 commented Mar 24, 2018

I think module-info should be added in a Java 8 compatible way. I have done it for some libraries that use the gradle build tool, so it should be possible in Maven too. If noone is working on this, I will try to do this myself, but. since I have no experience in Maven, it might take some time.

@robinst
Copy link
Collaborator

robinst commented Mar 24, 2018

Apparently adding a module-info breaks Android usage, quoted from Stephen's blog post:

And just for kicks, your project can no longer be used by Android (as the team there seems to be very slow in adding a simple "ignore module-info" rule).

@xzel23
Copy link
Author

xzel23 commented Mar 24, 2018

Well, then I'd keep it in a separate branch until Android and other tools catch up. But Reducing download size by more than 100 MB when packaging your app is a big "positive benefit" of Java JPMS.

Would have been nice if Stephen had added the different bug reports for the issues he mentions so that we could begin hacking away those issues instead of just complaining that support is still not there.

@robinst
Copy link
Collaborator

robinst commented Mar 26, 2018

I'm not a fan of having a separate branch that needs to be kept up to date. Releasing a duplicate set of artifacts would be possible for Java 9+ (with a different group ID or artifact IDs), but that is also a lot of effort and would add confusion.

How would you feel about having a script that can process a set of jars to add the module-info? Would that work for you use case?

@xzel23
Copy link
Author

xzel23 commented Mar 26, 2018

I think that would not really help the situation. IMHO JPMS has huge benefits once it's there, but we will never get there unless libraries are updated to include module-info. And that can already be done in a Java 8 compatible way. Thats's what should be done and will be of benefit for us all.

I can however see your point about Android and tools not being ready. IMHO those tools should be fixed as soon as possible. I have already started adding module.info in my own fork of commonmark and maybe will even publish jars (with version "xxx-JPMS" on bintray) that are still compatible with Java 8, but not android and the tools you mentioned.

Once Java 11 (the next LTS) is out, we should look at this again and the issues with android and tools are solved, can merge my changes into master.

BTW, if anyone has links to the bug reports concerning the issues described in Stephen's blog, maybe they could be added here so that we can track progress and/or even help fixing.

@robinst
Copy link
Collaborator

robinst commented Mar 26, 2018

I feel you. At the same time I'm also being asked to support Android, e.g. see #124.

Once Java 11 (the next LTS) is out, we should look at this again and the issues with android and tools are solved, can merge my changes into master.

Yeah, sounds good.

This issue might be good to watch too: google/guava#2970

@xzel23
Copy link
Author

xzel23 commented Apr 14, 2018

Ok, I have a works-for-me solution now. I added module-info.java, but compile all other files with targetCompatibility Java 8. module-info.java is compiled in Java 9 mode and a multi-release jar is created. This should hopefully be a solution that works for Java 8 as well as Android. The problem is my knowledge with Maven is quite limited, and so I did it in gradle (using a plugin I created to help with modularising libraries).

I can use the library in my jlinked standalone application, but I don't use all plugins (just gfm-tables and fm-strikethrough), so there might still be issues with the other plugins. If anyone wants to try, just write to me. If there's interest and I find the time to set it up, I could provide binaries of my jpms-version via bintray.

I think the module-info files could be easily merged back, but someone clever would have to help with the Maven part:

  • remove all module-info.java files from the "normal" sorceset
  • compile all module-info.java with targetCompatibility Java 9 and move the resulting class files to META-INF/versions/9
  • set the Multi-Release to true in the POM

@xzel23
Copy link
Author

xzel23 commented May 2, 2018

If anyone else is interesteed: A modularized version with all 6 plugins usable with jlink is available here (binaries on bintray): https://github.com/xzel23/commonmark-java.

I will try to keep track of new releases of commonmark-java and update my fork and provide binaries for new releases.

@ice1000
Copy link

ice1000 commented Sep 8, 2021

If anyone else is interesteed: A modularized version with all 6 plugins usable with jlink is available here (binaries on bintray): https://github.com/xzel23/commonmark-java.

I will try to keep track of new releases of commonmark-java and update my fork and provide binaries for new releases.

Bintray is closed, do you have plan to publish to maven central?

@cichte
Copy link

cichte commented Dec 2, 2021

Hi, love the lib, but also get the error: automatic module cannot be used with jlink: org.commonmark. I am developing on JDK 16.0.2 and have to build the app with maven and jlink. The modularized version from @xzel23 doesnt do the job... Is there any chance to get the lib jlink compatibel in 2021? :-) Any help is appreciated...

@ice1000
Copy link

ice1000 commented Dec 2, 2021

Hi, love the lib, but also get the error: automatic module cannot be used with jlink: org.commonmark. I am developing on JDK 16.0.2 and have to build the app with maven and jlink. The modularized version from @xzel23 doesnt do the job... Is there any chance to get the lib jlink compatibel in 2021? :-) Any help is appreciated...

How doesn't the modularized version do the job? What did you try and what did you get v

@cichte
Copy link

cichte commented Dec 2, 2021

The artifact com.atlassian.commonmark from here ->https://github.com/xzel23/commonmark-java
caused an error because was moved as describe here

https://mvnrepository.com/artifact/com.atlassian.commonmark/commonmark

to here

https://mvnrepository.com/artifact/org.commonmark/commonmark/0.18.1

This was the artifact i used:

        <dependency>
            <groupId>org.commonmark</groupId>
            <artifactId>commonmark</artifactId>
            <version>0.18.1</version>
        </dependency>`

And i get this Message:

Error: automatic module cannot be used with jlink: org.commonmark from file:///C:/Users/developer/.m2/repository/org/commonmark/commonmark/0.18.1/commonmark-0.18.1.jar
[ERROR] Command execution failed.

This was the point i get lost :-)

@ice1000
Copy link

ice1000 commented Dec 2, 2021

I think you should checkout this branch: https://github.com/xzel23/commonmark-java/tree/0.14.0-jigsaw

I think the xzel23 didn't update the README to his artifacts, so you're actually using the artifact of this repo.

@cichte
Copy link

cichte commented Dec 2, 2021

Thank you for the hint. I tried now all Versions from 0.13.1 up to 0.16.1 ... all with the same result:

Error: automatic module cannot be used with jlink: org.commonmark from file:///C:/Users/developer/.m2/repository/com/atlassian/commonmark/commonmark/0.13.1/commonmark-0.13.1.jar
[ERROR] Command execution failed.

There is an automatic-module-name declaration in the jars manifest:

Automatic-Module-Name: org.commonmark

There ist also no module-info.java in the jar, so i think jlink isnt wrong with its suggestion.

@ice1000
Copy link

ice1000 commented Dec 2, 2021

Yes there aren't. Why are you using the ones without them? Just use the ones with them.

@cichte
Copy link

cichte commented Dec 2, 2021

Yes there aren't. Why are you using the ones without them? Just use the ones with them.

Seems the maven-repos are outdated, or wrong?

I cloned the git-repo now and try my own build. Thanks :-)

@ice1000
Copy link

ice1000 commented Dec 2, 2021

Seems the maven-repos are outdated, or wrong?

Wrong.

I cloned the git-repo now and try my own build. Thanks :-)

Yes, that's (supposed to be) the correct way to do it :-)

Maybe I'll try to maintain a fork with Jigsaw things and publish it to another maven repository, with the appropriate licenses.

@ice1000
Copy link

ice1000 commented Dec 2, 2021

As long as I am still using commonmark

@ice1000
Copy link

ice1000 commented Dec 3, 2021

Folks I've created a fork https://github.com/aya-prover/commonmark-java based on @xzel23's work. I've released it on maven central, but it may take a few days to be usable (there's a staging process). Lmk if there is any problem (some subprojects are not published because I don't need them. If you need me to publish them please lmk too)

@miasma
Copy link

miasma commented Jun 13, 2022

Folks I've created a fork https://github.com/aya-prover/commonmark-java based on @xzel23's work. I've released it on maven central, but it may take a few days to be usable (there's a staging process). Lmk if there is any problem (some subprojects are not published because I don't need them. If you need me to publish them please lmk too)

Hi - I tried using your fork, but unfortunately run into issues. I noticed the 0.19.0 version of the org.aya-prover:commonmark seems to be missing. Can't find it via the published Maven repo. The bytecode of the older 0.18.1 version also triggers this issue when compiling a simple project

$ mvn clean compile jlink:jlink
...
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.10.1:compile (default-compile) on project test-project: Compilation failure
[ERROR] /home/foo/test-project/src/main/java/module-info.java: class file for /home/foo/.m2/repository/org/aya-prover/commonmark/0.18.1/commonmark-0.18.1.jar(/module-info.class) uses preview features of Java SE 11.
[ERROR]   (use --enable-preview to allow loading of class files which contain preview features)

The bytecode decoder in IDEA mentions that your library is using bytecode version 55.65,535 (Java 11-preview). The normal Java 11 bytecode version is 55.0.

Are there still major issues with multi-release jars? Can't the official project support JPMS already? Maven's official documentation shows examples of building MR jars for JPMS projects that are backwards compatible.

@ice1000
Copy link

ice1000 commented Jun 13, 2022

Hi - I tried using your fork, but unfortunately run into issues. I noticed the 0.19.0 version of the org.aya-prover:commonmark seems to be missing. Can't find it via the published Maven repo.

Nice catch, I've fixed that.

The bytecode of the older 0.18.1 version also triggers this issue when compiling a simple project
The bytecode decoder in IDEA mentions that your library is using bytecode version 55.65,535 (Java 11-preview). The normal Java 11 bytecode version is 55.0.

I've fixed it too.

See 0.19.1 version, it may take some time to upload

@redcatbear
Copy link

I ran into problems with the missing module-info too and created #275 until I realized that this is basically a duplicate of this ticket here, so I closed #275 again. In my case Maven's surefire tests did not run because of missing exports. Still hoping that this ticket gets solved.

@ice1000
Copy link

ice1000 commented Nov 8, 2022

@redcatbear What do you think about https://github.com/aya-prover/commonmark-java?

@redcatbear
Copy link

@ice1000 , I will give that a try. Thanks.

@robinst
Copy link
Collaborator

robinst commented Nov 11, 2022

Hey everyone. I think it's time we modularized commonmark-java itself. I don't think Android tooling has a problem with module-info files anymore (that was the original blocker).

I'm gonna look at @ice1000's work as a starting point. One tricky part is going to be that until now, the classes in the internal package were accessible and I know some people have used them to extend inline parsing. If we no longer export those it's going to be harder. But maybe this is a good point in time to actually make those things public. There's some work that happened as part of #113 already.

I can't commit to a time frame at this point though.

@miasma
Copy link

miasma commented Feb 23, 2023

This project also depends on https://github.com/robinst/autolink-java, which doesn't provide any module-info file yet.

@aalmiray
Copy link

I'd suggest looking at ModiTect. It can generate/append module-info to a JAR while keeping the codebase compatible with Java 8. The generated module-info.class will be placed inside /META-INF/versions/x (turning the JAR into a Multi Resource JAR) which should avoid any further problems with Android.

If interested I can send a PR to get the ball rolling. I've done it successfully with other projects that also require Java 8 compatibility.

Here's how the current setup with automatic module name looks like:

aalmiray2:lib aalmiray$ jarviz module name --file commonmark-0.21.0.jar 
subject: commonmark-0.21.0.jar
name: org.commonmark
source: manifest
automatic: true
valid: true

$ jarviz module descriptor --file commonmark-0.21.0.jar 
subject: commonmark-0.21.0.jar
name: org.commonmark
version: 0.21.0
open: false
automatic: true
requires:
  java.base mandated
contains:
  org.commonmark
  org.commonmark.internal
  org.commonmark.internal.inline
  org.commonmark.internal.renderer
  org.commonmark.internal.renderer.text
  org.commonmark.internal.util
  org.commonmark.node
  org.commonmark.parser
  org.commonmark.parser.block
  org.commonmark.parser.delimiter
  org.commonmark.renderer
  org.commonmark.renderer.html
  org.commonmark.renderer.text

robinst added a commit that referenced this issue Mar 1, 2023
We've waited long enough for the ecosystem and tools such as Android to
catch up. Making it proper modular means people can run jlink on it.
Fixes #125.
@robinst
Copy link
Collaborator

robinst commented Mar 1, 2023

@aalmiray Thanks for the offer, but I've already been working on the change to just make it modular. It's in a PR now: #286

@aalmiray
Copy link

aalmiray commented Mar 1, 2023

I see. Unfortunately the changes in that PR bump the whole codebase to Java9. I was hoping that commonmark remained Java8 compatible for a bit longer. That’s what the ModiTect plugin enables. The module-info class may be auto generated or taken from an existing source file. It’ll be compiled using ASM and placed under META-INF/versions, turning JARs into multi resource JARs. This satisfies consumers on Java 8 and greater.

@robinst
Copy link
Collaborator

robinst commented Mar 2, 2023

I was hoping that commonmark remained Java8 compatible for a bit longer.

@aalmiray Why? Still have projects stuck on Java 8? I could consider maintaining 0.21.x versions with bug fixes for Java 8 (probably not many necessary), but I really want to move on from an almost 10 year old Java version and not have to make the build more complicated.

@aalmiray
Copy link

aalmiray commented Mar 2, 2023

Because I use commonmark in JReleaser's Maven and Gradle plugins whose codebase remains Java8 as that is the minimum supported Java version. I'm waiting for Maven 4 to come out as that likely move its baseline to Java 11, prompting JReleaser to move as well.

ModiTect is just one more plugin that doesn't take much configuration. There's no need for toolchains nor additional JDK configurations. The build must be run with Java 9+.

@aalmiray
Copy link

aalmiray commented Mar 2, 2023

For reference this is how the build could look like with ModiTect failsafe-lib/failsafe#359

@aalmiray
Copy link

aalmiray commented Feb 9, 2024

Hi @robinst, would you reconsider keeping artifacts with Java8 compatibility while adding full module descriptors (the ModiTect way)?

@robinst
Copy link
Collaborator

robinst commented Feb 10, 2024

@aalmiray I'm sorry, but no, it's time to move to Java 11+. Looks like Maven 4 is also likely to require 11 or even 17: https://lists.apache.org/thread/145fmwcfrt0qo1qwfpop7rozhsrv60wk

For your plugins, I'm happy to keep commonmark-java 0.21.x on Java 8 and accept bug fixes and make releases if required.

@aalmiray
Copy link

Understood. As much as I want the world to move past the Java9 bump and embrace modules we are still ways to go.

Maven4 is currently in alpha phase (first release posted Dec 2020) and we have no clue when it'll go GA.

@ice1000
Copy link

ice1000 commented Feb 10, 2024

That's tooooooo good 💯💯💯 modern versions Java motto motto

robinst added a commit that referenced this issue Mar 2, 2024
We've waited long enough for the ecosystem and tools such as Android to
catch up. Making it proper modular means people can run jlink on it.
Fixes #125.
@xzel23
Copy link
Author

xzel23 commented Mar 9, 2024

Thank you!

@ice1000
Copy link

ice1000 commented Mar 9, 2024

😍 I need a release after this

@robinst
Copy link
Collaborator

robinst commented Mar 15, 2024

Hi everyone! This change has now been released with 0.22.0 🎉: https://github.com/commonmark/commonmark-java/blob/main/CHANGELOG.md#0220---2024-03-15

Happy building!

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 a pull request may close this issue.

7 participants