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

Support Java9 Platform Module System(JPMS) #9406

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

Conversation

miurahr
Copy link
Contributor

@miurahr miurahr commented Sep 29, 2023

Motivation

languagetool does not support Java Platform Module System (JPMS).
The project only not support it, also break downstream applications/libraries to migrate Java 11, 17 and 21 and adopt JPMS .

It is because languagetool share same package among artifacts jar files, such as o.l.language package.
It is well known as split package problem.

What issue is fixed

What changes

  • languagetool-core

    • Escalate Contributor(s), TokenExpressionFactory class public.
      These classes are referenced from o.l.language.(Language) classes. I move these classes packages, so referenced class should be public.
  • language-* submodules

    • Move modules classes to resolve a split package problem. (see. reference for details)

      • o.l.launguage to o.l.l.(lang)
      • o.l.chunking to o.l.c.(lang)
      • o.l.synthesis to o.l.s.(lang)
    • Gather MessageBundles_*.properties into language-core to resolve split package problem.
      when languagetool project want to keep a way to split bundles to submodules, Java9 provide a new way.
      There is no way to keep compatibility with both Java8 and JPMS without resolving split package problem by file moves.

  • all submodules

    • Update imports to adjust for the language-* modules package changes

    • Add Automatic-Module-Name manifest entry
      manifest property Automatic-Module-Name was introduced in Java 9 platform module system, to allow migration from Java8 code to JPMS without breaking Java8 compatibility. Library users who already running on >Java9 receive benefit from the configuration.

Reference

A downside

Users who use LT with following style become compile error as symbol not found.
You should rewrite it as recommended way.

bad

import org.languagetool.Language;
import org.languagetool.language.Japanese;

class Main {
static void main() {
Language language = new Japanese();
}}

good

import org.languagetool.Language;
import org.languagetool.Languages;

class Main {
static void main() {
Language language;
language = Languages.getLanguageForShortCode("ja"); // since 3.6
language = Languages.getLanguageForLocale(new Locale("en", "US"));
language = Languages.getLanguageForName("German");
}}

Status

ready for review.

Further tasks

languagetool-org/languagetool-org.github.io#19

  • Provide compatibility layer for the user who use older language classes and want to continue their usage without JPMS support.

Other information

@miurahr miurahr force-pushed the topic/java9-module-system branch 2 times, most recently from d92bec6 to 055181e Compare September 29, 2023 10:41
@miurahr miurahr marked this pull request as ready for review September 29, 2023 10:41
@miurahr miurahr changed the title WIP: Support Java9 Platform Module System(JPMS) Support Java9 Platform Module System(JPMS) Sep 29, 2023
@miurahr
Copy link
Contributor Author

miurahr commented Oct 3, 2023

Does LanguageTool project want to provide a Java8 compatibility option for users who stick in java8 world, and don't want to go forward to future?

@miurahr
Copy link
Contributor Author

miurahr commented Oct 3, 2023

Does LanguageTool project want to provide a Java8 compatibility option for users who stick in java8 world, and don't want to go forward to future?

I'd like to raise it as another PR because of resolving a dependency issue with #9406 changes. After release it as SNAPSHOT version, the java8 compat module can be introduced and tested.

@miurahr

This comment was marked as resolved.

@danielnaber
Copy link
Member

When the change for Language class methods visibility, I'd like just to do it?

Yes, I see no issue with that change.

@miurahr miurahr marked this pull request as draft October 4, 2023 13:17
@miurahr
Copy link
Contributor Author

miurahr commented Oct 4, 2023

Partial provide of compatible classes, now work in progress.

@miurahr miurahr marked this pull request as ready for review October 5, 2023 00:54
@miurahr
Copy link
Contributor Author

miurahr commented Oct 5, 2023

I've updated LanguageSpecificTest class to respect onlyRunCode on Disambiguation Rule Tests.

see b50d10d

It is why java8compat module test environment has all known language modules, and current test run all the languages loaded. It lead test executions too long duration, because of redundant cases.

@miurahr
Copy link
Contributor Author

miurahr commented Oct 5, 2023

Status

Ready for review and merge.

Explanation of compatibility module

I've introduced project in languagetool-language-modules/language-java8compat that provide org.languagetool.language.(LANGUAGE) classes.

When user specified the module language-java8comat that become split package status which is incompatible with JPMS. It is why I give a name java8compat means as java9 incompatible.
Alternative naming idea is language-compat-nojpms

Core class of the compatibility module

org.languagetool.language.AbstractLanguageProxy class implements "Proxy pattern" in GoF design pattern.
All the individual language classes are inherited from it, which only have a default constructor with new language class path as argument.

Tests

Most of compatibility language classes are tested with simple test case.

@miurahr
Copy link
Contributor Author

miurahr commented Oct 9, 2023

The change here moves all the language file package, so easily conflict other changes.
It is not pragmatic to track all the changes in master branch.

- escalate Contributor(s), TokenExpressionFactory public
- move modules classes
   - o.l.launguage to o.l.l.(lang)
   - o.l.chunking to o.l.c.(lang)
   - o.l.synthesis to o.l.s.(lang)
- Add Automatic-Module-Name manifest entry
- Update imports to adjust for the above changes
- resolve split package by MessagesBundle files
- add automatic-module-name
    - languagetool-server and languagetool-tools
- Update package path for o.l.l.en.English class
- Improve LanguageSpecificTest
    - DisambiguationRuleTest respects onlyRunCode

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
Signed-off-by: Hiroshi Miura <miurahr@linux.com>
@miurahr
Copy link
Contributor Author

miurahr commented Jan 2, 2024

rebased on current master.

Signed-off-by: Hiroshi Miura <miurahr@linux.com>
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.

2 participants