Conversation
📝 WalkthroughWalkthroughThis PR modifies the Gradle build configuration to improve handling of Thrift-generated Java sources. Changes include registering a dedicated "javaThrift" directory for generated sources in the Spring build utilities, updating the Thrift compilation output path, and implementing logic to exclude generated Thrift/gRPC/Proto classes from the shading process with debug logging. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@gradle/scripts/lib/java-shade.gradle`:
- Around line 374-381: The relocate block currently uses a closure-based exclude
(task.relocate(...) { exclude { ... } }) which ShadowJar does not support;
remove the closure-based exclude and instead implement the generated-source
check via a custom Relocator and register it with task.relocate, or convert to
string/regex excludes if generated classes follow a predictable package/path.
Specifically, remove the exclude closure from the task.relocate call and create
a class implementing Shadow's Relocator interface (override canRelocatePath or
canRelocateClass to call isGenerated(project, path, isMain)), then add that
Relocator instance to task.relocate so generated files are skipped during
relocation.
🧹 Nitpick comments (1)
gradle/scripts/lib/java-shade.gradle (1)
418-421: Edge case: classes with$in the package path.Line 421 splits on
$to extract the top-level class name, which correctly handles inner classes likecom/foo/Outer$Inner. However, if any generated code produces file paths containing$in directory names (rare, but possible with some code generators), this could incorrectly truncate the path. This is an unlikely edge case given standard Java conventions, just noting it.
| logger.debug("[${project.name}] Relocating ${props['name']} from ${props['from']} to ${props['to']} " + | ||
| "in ${isMain ? 'main' : 'test'}") | ||
| task.relocate(props['from'], props['to']) { | ||
| exclude { | ||
| return isGenerated(project, it.path, isMain) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find Shadow plugin version
rg "com.gradleup.shadow|com.github.jengelman.gradle.plugins.shadow" --type=gradle,toml -C2Repository: line/armeria
Length of output: 91
🏁 Script executed:
# Check how relocate is used elsewhere in the codebase
rg "\.relocate\(" --type=groovy -C5Repository: line/armeria
Length of output: 859
🌐 Web query:
Shadow Gradle plugin relocate exclude closure predicate API
💡 Result:
In the ShadowJar relocate(from, to) { … } block, exclude/include are path-pattern filters, not predicate closures. You pass Ant-style path match patterns (and optionally %regex[...] patterns), e.g.:
tasks.shadowJar {
relocate('junit.textui', 'a') {
exclude 'junit.textui.TestRunner' // Ant-path style
}
relocate('org.foo', 'a') {
include '%regex[org/foo/.*Factory[0-9].*]' // regex form
}
}[1]
If what you want is a closure/predicate-style exclude, that API exists for dependency filtering (i.e., which dependencies get merged), via tasks.shadowJar { dependencies { exclude { dep -> … } } }, e.g.:
tasks.shadowJar {
dependencies {
exclude { it.moduleGroup == 'org.apache.logging.log4j' }
}
}[2]
Remove the closure-based exclude from relocate — Shadow plugin does not support predicate closures in relocate blocks.
The exclude/include methods in ShadowJar's relocate configuration accept only Ant-style path patterns or regex patterns (e.g., exclude 'com/foo/Bar'), not closure-based predicates. The code at lines 376–379 will be silently ignored or cause a runtime error.
To exclude generated sources during relocation, either:
- Use string patterns if all generated sources follow a predictable path pattern, or
- Implement a custom
Relocatorthat performs the generated-source check incanRelocatePathorcanRelocateClassmethods.
The isGenerated function (lines 411–437) is correctly implemented but needs to be applied via the correct API.
🤖 Prompt for AI Agents
In `@gradle/scripts/lib/java-shade.gradle` around lines 374 - 381, The relocate
block currently uses a closure-based exclude (task.relocate(...) { exclude { ...
} }) which ShadowJar does not support; remove the closure-based exclude and
instead implement the generated-source check via a custom Relocator and register
it with task.relocate, or convert to string/regex excludes if generated classes
follow a predictable package/path. Specifically, remove the exclude closure from
the task.relocate call and create a class implementing Shadow's Relocator
interface (override canRelocatePath or canRelocateClass to call
isGenerated(project, path, isMain)), then add that Relocator instance to
task.relocate so generated files are skipped during relocation.
🔍 Build Scan® (commit: e25ce78) |
2 participants
Motivation:
Explain why you're making this change and what problem you're trying to solve.
Modifications:
Result: