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

Create MRJAR with module-info.class #263

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

webracer999
Copy link

@webracer999 webracer999 commented Aug 7, 2023

PR to issue #262

This creates a "Multi-Release JAR" (MRJAR). The generated jar works with Java 1.8 and Java >= 9.

For Java >= 9 a module-info.class is provided (META-INF/versions/9/module-info.class). This allows the usage of jlink-tool to generate custom java runtime image.

exports org.ocpsoft.prettytime.i18n;
exports org.ocpsoft.prettytime.impl;
exports org.ocpsoft.prettytime.units;
provides org.ocpsoft.prettytime.PrettyTime with org.ocpsoft.prettytime.PrettyTime;

Choose a reason for hiding this comment

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

What is this provides declaration for? At first glance it doesn't seem like that class is relevant to the service loader?

Copy link
Author

Choose a reason for hiding this comment

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

Thank you @bowbahdoe for your feedback!

Sorry, I forgot to mention how I created the module-info.java.
As a workaround for my project I found out that moditect can generate and inject module-info to dependencies.

I used in my project the following configuration in my pom.xml under <project> -> <build> -> <plugins>:

<plugin>
    <groupId>org.moditect</groupId>
    <artifactId>moditect-maven-plugin</artifactId>
    <version>1.0.0.Final</version>
    <executions>
        <execution>
            <id>add-module-infos</id>
            <phase>generate-resources</phase>
            <goals>
                <goal>add-module-info</goal>
            </goals>
            <configuration>
                <overwriteExistingFiles>true</overwriteExistingFiles>
                <modules>
                    <module>
                        <artifact>
                            <groupId>org.ocpsoft.prettytime</groupId>
                            <artifactId>prettytime</artifactId>
                            <version>5.0.7.Final</version>
                        </artifact>
                        <moduleInfo>
                            <name>prettytime</name>
                        </moduleInfo>
                    </module>
                </modules>
            </configuration>
        </execution>
    </executions>
</plugin>

In the PR I used the generated module-info.java. To be honest, I was and I am not sure what the provides really does.
At least the BasicJavaApp.java from https://www.ocpsoft.org/prettytime/ would also work without provides.

Do you think it is safe to delete it?

Choose a reason for hiding this comment

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

Nope, I found where in the code it came from

https://github.com/ocpsoft/prettytime/blob/master/core/src/main/resources/META-INF/services/org.ocpsoft.prettytime.PrettyTime

So it does seem like that is legitimate (if a bit strange)

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I can't remember exactly why that service file was added. It may be simply so that PrettyTime can be loaded via a ServiceLoader, but... I can't honestly recall.

Copy link
Author

@webracer999 webracer999 Aug 10, 2023

Choose a reason for hiding this comment

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

As long as there is a defined service I think it is correct to add it to the module-info.java.

@webracer999
Copy link
Author

@bowbahdoe
Copy link

I think there is nothing stopping this from being merged

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