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

fix for jpms by remove use of ResourceBundle.Control #1544

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

Conversation

aanno
Copy link

@aanno aanno commented Sep 5, 2023

I observed the following

  1. The use of ResourceBundle.Control with the Java Platform Module System (jpms) is not possible. In Java >=9 the following method is present in ResourceBundle:
     private static void checkNamedModule(Class<?> caller) {
         if (caller.getModule().isNamed()) {
             throw new UnsupportedOperationException("ResourceBundle.Control not supported in named modules");
         }
     }
  2. ResourceBundle *.properties files in epubcheck are UTF-8 encoded.

Hence this pull request will (a) eliminate the use of ResourceBundle.Control and unify the loading of *.properties files to enhance the compatibility of epubcheck with jpms.

Review und comments are appreciated.

Kind regards,

aanno

@rdeltour rdeltour self-assigned this Dec 28, 2024
@rdeltour rdeltour added this to the Next maintenance release milestone Dec 28, 2024
@rdeltour rdeltour added type: improvement The issue suggests an improvement of an existing feature status: ready for review labels Dec 28, 2024
Copy link
Member

@rdeltour rdeltour left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @aanno!

I wasn't aware of this change in Java 9 and the JPMS limitation about ResourceBundle.Control. +1 on the objective to improve the compatibility of EPUBCheck with JPMS.

The suggested workaround might be a bit naïve compared to the default logic implemented by ResourceBundle however.

Specifically:

Besides, as far as I understand (I'm not very familiar with JPMS) this would still not allow another named module to provide localized properties for EPUBCheck (or does it?).

In any case, it seems that using ResourceBundle.Control raises an UnsupportedOperationException only when passed as an argument to one of the ResourceBundle#getBundle(…) methods. But its API can be used directly, for instance to get the list of candidate locales and locale fallbacks.

Before merging anything, I'd like to explore the possibility of implementing something closer to the ResourceBundle logic, using ResourceBundle.Control#getCandidateLocales(…) and possibly using naïve map-based caching.

I'm removing this PR from the next maintenance milestone, and I'll try to propose another PR (as an alternative to this one) for the next after.

@rdeltour rdeltour added status: in discussion The issue is being discussed by the development team and removed status: ready for review labels Dec 30, 2024
@rdeltour rdeltour removed this from the Next maintenance release milestone Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: in discussion The issue is being discussed by the development team type: improvement The issue suggests an improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants