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.

Sign up for GitHub

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

Jump to bottom

Defining Java Jigsaw modules (JPMS) #785

Open
Djaytan opened this issue Jan 8, 2025 · 3 comments · May be fixed by #787
Open

Defining Java Jigsaw modules (JPMS) #785

Djaytan opened this issue Jan 8, 2025 · 3 comments · May be fixed by #787

Comments

@Djaytan
Copy link

Djaytan commented Jan 8, 2025 •
edited
Loading

The issue

The Jar files of the framework are not Jigsaw modules. Therefore, they are considered as "automatic" modules if put in the module path, or "unnamed" ones if put in the classpath instead.

When relying on Gradle, the Jar files are considered as "unnamed" ones, thus leading to troubles when trying to define our own modules depending on the Cloud framework (i.e. by describing the module through a module-info.java file). Here is a quote from the Gradle's documentation:

You probably want to use external libraries, like OSS libraries from Maven Central, in your modular Java project. Some libraries, in their newer versions, are already full modules with a module descriptor. For example, com.google.code.gson:gson:2.8.9 that has the module name com.google.gson.

Others, like org.apache.commons:commons-lang3:3.10, may not offer a full module descriptor but will at least contain an Automatic-Module-Name entry in their manifest file to define the module’s name (org.apache.commons.lang3 in the example). Such modules, that only have a name as module description, are called automatic module that export all their packages and can read all modules on the module path.

A third case are traditional libraries that provide no module information at all — for example commons-cli:commons-cli:1.4. Gradle puts such libraries on the classpath instead of the module path. The classpath is then treated as one module (the so called unnamed module) by Java.

It is worth to mention the fact an "application" module (i.e. a Jar file containing a module descriptor) can't depend on "unnamed" ones. This means that when defining a dependency to the Cloud framework in a module-info.java like that:

module my.module.name {
  requires cloud.annotations;
}

We end up with the following Java error when trying to build the project with Gradle:

module-info.java:2: error: module not found: cloud.annotations
requires cloud.annotations;

Even if an IDE like IntelliJ would not comply on its side since handling things differently. Thus, this proves that the framework's Jar files have not been included in the module path by Gradle.

Alternatives considered

One possibility is to adjust the Gradle's behavior at compilation time by adding the classpath in the module path, but that's not something that trivial to do nor convenient.

Relying on a Gradle plugin like extra-java-module-info may be the way to go even if not ideal. That's even more true in my case where I only have the issue with the Cloud framework.

Proposed solutions

Defining an automatic module name

One way to easily solve the issue with Gradle may be by simply defining the Automatic-Module-Name Jar manifest attribute:

jar {
  manifest {
    attributes("Automatic-Module-Name" to "cloud.annotations")
  }
}

Which should lead to the following content in the MANIFEST.MF Jar's file:

Manifest-Version: 1.0
Automatic-Module-Name: cloud.annotations

Instead of the following content as we have for the version 2.0.0:

Manifest-Version: 1.0

However, it would be important to understand why the Gradle team decided to not automatically convert unnamed modules to automatic ones (reference). Especially:

but at most such that declare an automatic module name in the manifest which should be a sign that the library author at least thought about it and at least made it JPMS compatible.

Even if it seems we don't have risks regarding split-package issues:

loicd@DESKTOP-781OBR8 MINGW64 ~/Downloads
$ jar --file=cloud-core-2.0.0.jar --describe-module
No module descriptor found. Derived automatic module.

[email protected] automatic
requires java.base mandated
contains org.incendo.cloud
contains org.incendo.cloud.annotation.specifier
contains org.incendo.cloud.bean
contains org.incendo.cloud.caption
contains org.incendo.cloud.component
contains org.incendo.cloud.component.preprocessor
contains org.incendo.cloud.context
contains org.incendo.cloud.description
contains org.incendo.cloud.exception
contains org.incendo.cloud.exception.handling
contains org.incendo.cloud.exception.parsing
contains org.incendo.cloud.execution
contains org.incendo.cloud.execution.postprocessor
contains org.incendo.cloud.execution.preprocessor
contains org.incendo.cloud.help
contains org.incendo.cloud.help.result
contains org.incendo.cloud.injection
contains org.incendo.cloud.internal
contains org.incendo.cloud.key
contains org.incendo.cloud.meta
contains org.incendo.cloud.parser
contains org.incendo.cloud.parser.aggregate
contains org.incendo.cloud.parser.flag
contains org.incendo.cloud.parser.standard
contains org.incendo.cloud.permission
contains org.incendo.cloud.setting
contains org.incendo.cloud.state
contains org.incendo.cloud.suggestion
contains org.incendo.cloud.syntax
contains org.incendo.cloud.type
contains org.incendo.cloud.type.range
contains org.incendo.cloud.type.tuple
contains org.incendo.cloud.util
contains org.incendo.cloud.util.annotation


loicd@DESKTOP-781OBR8 MINGW64 ~/Downloads
$ jar --file=cloud-annotations-2.0.0.jar --describe-module
No module descriptor found. Derived automatic module.

[email protected] automatic
requires java.base mandated
provides javax.annotation.processing.Processor with org.incendo.cloud.annotations.processing.CommandContainerProcessor org.incendo.cloud.annotations.processing.CommandMethodProcessor
contains org.incendo.cloud.annotations
contains org.incendo.cloud.annotations.assembler
contains org.incendo.cloud.annotations.descriptor
contains org.incendo.cloud.annotations.exception
contains org.incendo.cloud.annotations.extractor
contains org.incendo.cloud.annotations.injection
contains org.incendo.cloud.annotations.method
contains org.incendo.cloud.annotations.parser
contains org.incendo.cloud.annotations.processing
contains org.incendo.cloud.annotations.string
contains org.incendo.cloud.annotations.suggestion

It may still preferable to define module descriptors (module-info.java files).

Defining module descriptors

This task may be less trivial and require more efforts.

Learning more about JPMS is required. A great guide on this front may be the Baeldung one.
@Djaytan
Copy link
Author

Djaytan commented Jan 9, 2025

I'm ok to open a pull request. I'm just waiting feedback before going further.

Djaytan added a commit to Djaytan/cloud that referenced this issue Jan 12, 2025
@Djaytan
Incendo#785 - define JPMS module names
b060e88 
## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<jarBaseName>` (e.g. `org.incendo.cloud-core`).

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud-core;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud-core;
  [...]
}
```

Closes Incendo#785
Djaytan added a commit to Djaytan/cloud that referenced this issue Jan 12, 2025
@Djaytan
Incendo#785 - define JPMS module names
fabfff2 
## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<jarBaseName>` (e.g. `org.incendo.cloud-core`).

Details about this change can be found here: Incendo#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud-core;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud-core;
  [...]
}
```

Closes Incendo#785
@Djaytan Djaytan linked a pull request Jan 12, 2025 that will close this issue
Open
Djaytan added a commit to Djaytan/cloud-minecraft that referenced this issue Jan 12, 2025
@Djaytan
define JPMS module names
59bb4fa 
_This change is similar to https://github.com/Incendo/cloud/pull/787_

## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<jarBaseName>` (e.g. `org.incendo.cloud-paper`).

Details about this change can be found here: Incendo/cloud#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud-paper;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud-paper;
  [...]
}
```
@Djaytan Djaytan mentioned this issue Jan 12, 2025
Open
@Djaytan
Copy link
Author

Djaytan commented Jan 12, 2025

After a discussion with Citymonstret on Discord, he said being open for such change as long as it doesn't impact negatively the project's maintainability.
The Java Jigsaw project has the goal of improving software maintainability by providing better encapsulation capabilities. Therefore, going toward this direction would be beneficial for the project, thus is aligned with what Citymonstret want.

For the time being, I opened two PRs proposing the easier and less risky solution to implement:

Basically, we just add a manifest attribute in Jar files: the Automatic-Module-Name one. Details are provided in PRs' descriptions.

Can you have a look on these two PRs please?

Djaytan added a commit to Djaytan/cloud that referenced this issue Jan 12, 2025
@Djaytan
Incendo#785 - define JPMS module names
090ed76 
## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<jarBaseName>` (e.g. `org.incendo.cloud-core`).

Details about this change can be found here: Incendo#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud-core;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud-core;
  [...]
}
```

Closes Incendo#785
Djaytan added a commit to Djaytan/cloud-minecraft that referenced this issue Jan 12, 2025
@Djaytan
define JPMS module names
c8eae6a 
_This change is similar to https://github.com/Incendo/cloud/pull/787_

## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<jarBaseName>` (e.g. `org.incendo.cloud-paper`).

Details about this change can be found here: Incendo/cloud#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud-paper;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud-paper;
  [...]
}
```
Djaytan added a commit to Djaytan/cloud that referenced this issue Jan 12, 2025
@Djaytan
Incendo#785 - define JPMS module names
878a12f 
## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<moduleName>` (e.g. `org.incendo.cloud.core`).

Details about this change can be found here: Incendo#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud.core;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud.core;
  [...]
}
```

Closes Incendo#785
Djaytan added a commit to Djaytan/cloud-minecraft that referenced this issue Jan 12, 2025
@Djaytan
define JPMS module names
63ad869 
_This change is similar to https://github.com/Incendo/cloud/pull/787_

## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<moduleName>` (e.g. `org.incendo.cloud.paper`).

Details about this change can be found here: Incendo/cloud#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud.paper;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud.paper;
  [...]
}
```
Djaytan added a commit to Djaytan/cloud-minecraft-modded that referenced this issue Jan 12, 2025
@Djaytan
define JPMS module names
75b5320 
_This change is similar to https://github.com/Incendo/cloud/pull/787_

## Summary

The goal of this change is to encourage Gradle to put the framework's Jar files in the module path instead of the classpath. For that, we need to explicitly define the `Automatic-Module-Name` attribute in the `MANIFEST.MF` files in framework's Jar files (source: https://docs.gradle.org/current/userguide/java_library_plugin.html#using_libraries_that_are_not_modules).

The newly added attribute follows this format: `<groupId>.<moduleName>` (e.g. `org.incendo.cloud.paper`).

Details about this change can be found here: Incendo/cloud#785.

## Impact

There is no impact nor on maintainer side nor or consumer one not relying on JPMS.

For the ones already relying on JPMS (e.g. Maven users), they will just have to adapt the references in their `module-info.java` files as follow:

From:
```java
module my.module {
  [...]
  requires cloud.neoforge;
  [...]
}
```

To:
```java
module my.module {
  [...]
  requires org.incendo.cloud.neoforge;
  [...]
}
```
@Djaytan Djaytan mentioned this issue Jan 12, 2025
Closed
@Djaytan
Copy link
Author

Djaytan commented Jan 14, 2025

Created PRs:

Please, any chance to get these PRs merged and to publish new versions of Cloud modules?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

#785 - define JPMS module names Djaytan/cloud
1 participant
@Djaytan