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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
38 changes: 25 additions & 13 deletions gradle/scripts/lib/common-info.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,32 @@ 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()
if (groupIdComponents.last() == artifactIdComponents.first()) {
return String.join('.', groupIdComponents + artifactIdComponents.drop(1))
} else {
return String.join('.', groupIdComponents + artifactIdComponents)
Copy link
Contributor

@ikhoon ikhoon Jan 23, 2025

Choose a reason for hiding this comment

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

When checking the specification, it seems like a identifier needs to start with a JavaLetter.
ref: docs.oracle.com/javase/specs/jls/se16/html/jls-7.html#jls-7.4

What do you think of normalizing Automatic-Module-Name by using _ instead of . if the first character of an artifactIdComponent starts with a number?
It seems that the generated names for Scala modules violate the specification.

Manifest-Version: 1.0
Automatic-Module-Name: com.linecorp.armeria.sangria_2.12

}
}
}.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
1 change: 1 addition & 0 deletions thrift/thrift0.12/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ tasks.processTestResources.from "${rootProject.projectDir}/thrift/thrift0.13/src
def thriftFullVersion = libs.thrift012.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'))
automaticModuleNameOverride = "com.linecorp.armeria.thrift012"
}

// Disable checkstyle because it's checked by ':thrift0.13'.
Expand Down
3 changes: 2 additions & 1 deletion thrift/thrift0.13/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ 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('.'))
automaticModuleNameOverride = "com.linecorp.armeria.thrift013"
}

// Keep the original Guava references in ThriftListenableFuture,
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.14/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ ext {
// Use the old compiler.
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift014"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.15/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ def thriftFullVersion = libs.thrift015.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift015"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.16/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ def thriftFullVersion = libs.thrift016.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift016"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.17/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def thriftFullVersion = libs.thrift017.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift017"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.18/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ def thriftFullVersion = libs.thrift018.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift", "$projectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift018"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.19/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift", "$projectDir/src/test/thrift",
"$thrift018ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift019"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.20/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'));
testThriftSrcDirs = ["$thrift013ProjectDir/src/test/thrift", "$projectDir/src/test/thrift",
"$thrift018ProjectDir/src/test/thrift"]
automaticModuleNameOverride = "com.linecorp.armeria.thrift020"
}

tasks.generateSources.dependsOn(generateTestSources)
Expand Down
1 change: 1 addition & 0 deletions thrift/thrift0.9/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ def thriftFullVersion = libs.thrift09.get().versionConstraint.requiredVersion
ext {
thriftVersion = thriftFullVersion.substring(0, thriftFullVersion.lastIndexOf('.'))
disableThriftJson()
automaticModuleNameOverride = "com.linecorp.armeria.thrift09"
}

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