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 java modules for thrift #6076

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jrhee17
Copy link
Contributor

@jrhee17 jrhee17 commented Jan 22, 2025

Motivation:

We received a report that thrift modules do not support java 9 modules.
When checking the specification, it seems like a identifier needs to start with a JavaLetter.
ref: https://docs.oracle.com/javase/specs/jls/se16/html/jls-7.html#jls-7.4

Unfortunately, our thrift modules contain a dot followed by a digit, which isn't a JavaLetter.
i.e. thrift0.9 where 9 isn't a digit.

I propose that for thrift modules, the last dot is omitted to avoid this issue.
i.e. thrift0.9 -> thrift09

Modifications:

  • Introduced a automaticModuleNameOverride so that the override configuration can live inside each module
  • automaticModuleName is now null if not configured, or a provider if configured. This change is necessary so that the actual value of automaticModuleName is not determined at configuration time, giving each module a chance to configure its own ext property.
  • Each Jar task now configures Automatic-Module-Name at execution time as opposed to configuration time. This is also necessary so that each module can configure its own ext property prior to setting the name.
  • Each thrift module now sets its own automaticModuleNameOverride

Result:

@jrhee17 jrhee17 added the defect label Jan 22, 2025
@jrhee17 jrhee17 added this to the 1.32.0 milestone Jan 22, 2025
@jrhee17 jrhee17 marked this pull request as ready for review January 22, 2025 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid Automatic-Module-Name for armeria-thrift0.9
1 participant