Skip to content

Commit

Permalink
Support java modules for thrift (#6076)
Browse files Browse the repository at this point in the history
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:

- Closes #6075

<!--
Visit this URL to learn more about how to write a pull request
description:

https://armeria.dev/community/developer-guide#how-to-write-pull-request-description
-->
  • Loading branch information
jrhee17 authored Feb 19, 2025
1 parent 38e45fa commit d30a11c
Show file tree
Hide file tree
Showing 5 changed files with 47 additions and 20 deletions.
12 changes: 10 additions & 2 deletions gradle/scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -695,8 +695,16 @@ and artifact ID. For example:

If enabled, each project with `java` flag will have the `automaticModuleName` property.

You can override the automatic module name of a certain project via the `automaticModuleNameOverrides`
extension property:
You can override the automatic module name of a certain project via `automaticModuleNameOverride`:

```groovy
ext {
// Change the automatic module name of a project to 'com.example.fubar'.
automaticModuleNameOverride = 'com.example.fubar'
}
```

Alternatively, you can also specify a mapping via the `automaticModuleNameOverrides` extension property:

```groovy
ext {
Expand Down
41 changes: 28 additions & 13 deletions gradle/scripts/lib/common-info.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,35 @@ allprojects {
return null
}

// Use the overridden one if available.
def overriddenAutomaticModuleName = findOverridden('automaticModuleNameOverrides', project)
if (overriddenAutomaticModuleName != null) {
return overriddenAutomaticModuleName
}
return project.provider {
// per-project override
if (project.ext.has('automaticModuleNameOverride')) {
def override = project.ext.get('automaticModuleNameOverride')
if (!(override instanceof String)) {
throw new IllegalStateException("project.ext.automaticModuleNameOverride must be a String: ${override}")
}
return override
}

// Use the overridden one if available.

// Generate from the groupId and artifactId otherwise.
def groupIdComponents = String.valueOf(rootProject.group).split("\\.").toList()
def artifactIdComponents =
String.valueOf(project.ext.artifactId).replace('-', '.').split("\\.").toList()
if (groupIdComponents.last() == artifactIdComponents.first()) {
return String.join('.', groupIdComponents + artifactIdComponents.drop(1))
} else {
return String.join('.', groupIdComponents + artifactIdComponents)
def overriddenAutomaticModuleName = findOverridden('automaticModuleNameOverrides', project)
if (overriddenAutomaticModuleName != null) {
return overriddenAutomaticModuleName
}

// Generate from the groupId and artifactId otherwise.
def groupIdComponents = String.valueOf(rootProject.group).split("\\.").toList()
def artifactIdComponents =
String.valueOf(project.ext.artifactId).replace('-', '.').split("\\.").toList()
def generatedName
if (groupIdComponents.last() == artifactIdComponents.first()) {
generatedName = String.join('.', groupIdComponents + artifactIdComponents.drop(1))
} else {
generatedName = String.join('.', groupIdComponents + artifactIdComponents)
}
generatedName = generatedName.replaceAll("\\.(\\d)", "_\$1")
return generatedName
}
}.call()
}
Expand Down
6 changes: 4 additions & 2 deletions gradle/scripts/lib/java-shade.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ configure(relocatedProjects) {

// Set the 'Automatic-Module-Name' property in MANIFEST.MF.
if (project.ext.automaticModuleName != null) {
manifest {
attributes('Automatic-Module-Name': project.ext.automaticModuleName)
doFirst {
manifest {
attributes('Automatic-Module-Name': project.ext.automaticModuleName.get())
}
}
}
}
Expand Down
6 changes: 4 additions & 2 deletions gradle/scripts/lib/java.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -166,8 +166,10 @@ configure(projectsWithFlags('java')) {
// Set the 'Automatic-Module-Name' property in 'MANIFEST.MF' if `automaticModuleName` is not null.
if (project.ext.automaticModuleName != null) {
tasks.named('jar') {
manifest {
attributes('Automatic-Module-Name': project.ext.automaticModuleName)
doFirst {
manifest {
attributes('Automatic-Module-Name': project.ext.automaticModuleName.get())
}
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion thrift/thrift0.13/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ dependencies {
// Use the old compiler.
def thriftFullVersion = libs.thrift013.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'))
}

// Keep the original Guava references in ThriftListenableFuture,
Expand Down

0 comments on commit d30a11c

Please sign in to comment.