Shadow kotlinx.serialization to the compiler plugin #54
Conversation
WalkthroughAdds the Shadow plugin and config to produce a fat compiler JAR (including kotlinx.serialization), bumps kotlinx.serialization to 1.9.0, enables Kotlin explicit API mode in the compiler build, and updates related plugin comments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Dev as Developer / CI
participant Gradle as Gradle
participant Shadow as Shadow Plugin (shadowJar)
participant Jar as jar task
note right of Shadow `#f0f0f0`: Embeds runtime deps\n(including kotlinx.serialization)
Dev->>Gradle: run build
Gradle->>Shadow: execute shadowJar (collect runtimeClasspath, create *-all.jar)
Shadow-->>Gradle: produces shadow JAR
Gradle->>Jar: run jar (dependsOn shadowJar)
Jar->>Gradle: copy shadow JAR contents -> standard jar (replace)
Gradle-->>Dev: final compiler JAR includes embedded serialization
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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: 2
🧹 Nitpick comments (2)
stability-compiler/build.gradle.kts (2)
59-59: Classifier mismatch: archiveClassifier set to "all" but jar replaces standard artifact.The shadowJar task has
archiveClassifier.set("all"), which would normally produce astability-compiler-all.jarartifact. However, the jar task replaces the standard jar with this content, so downstream consumers expecting either the shadowed "all" classifier or the standard jar may be confused.Consider clarifying the artifact strategy: Is this module intended to always produce a fat jar, or should shadowJar be an optional secondary artifact?
57-73: Relocation comment could be clearer.Lines 65-66 comment that relocation is intentionally disabled because "K/Native compiler needs original package names," but there is no actual relocation code present (the line is commented out). This comment is somewhat misleading.
Consider updating to clarify that relocation is intentionally NOT configured, or remove the misleading comment entirely.
- // Don't relocate - K/Native compiler needs original package names - // relocate("kotlinx.serialization", "...") + // Note: Do NOT configure relocation. The K/Native compiler requires original + // package names for kotlinx.serialization to function correctly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
gradle/libs.versions.toml(3 hunks)stability-compiler/build.gradle.kts(2 hunks)stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Gradle Plugin Tests
- GitHub Check: Compiler Tests (FIR/IR Dumps)
- GitHub Check: Runtime Module Tests
- GitHub Check: Compiler Plugin Tests
- GitHub Check: Lint Module Tests
- GitHub Check: Spotless check
- GitHub Check: API check
- GitHub Check: Build and Tests
🔇 Additional comments (3)
stability-gradle/src/main/kotlin/com/skydoves/compose/stability/gradle/StabilityAnalyzerGradlePlugin.kt (1)
176-177: Comment update aligns with shadow-based packaging.The clarification that kotlinx.serialization is embedded in the compiler JAR via the shadow plugin is accurate and helpful context for maintainers. This correctly reflects the new build strategy introduced in this PR.
stability-compiler/build.gradle.kts (1)
28-28: VerifyexplicitApi()doesn't break existing public APIs.Enabling explicit API mode requires all public declarations to have explicit visibility modifiers (public, private, internal, etc.). If existing public APIs in this compiler module lack these modifiers, the build will fail.
Ensure all public APIs in the stability-compiler module have explicit visibility modifiers by running a build and verifying no visibility-related errors occur.
gradle/libs.versions.toml (1)
8-8: Version catalog entries are correctly formatted and compatible.The kotlinxSerialization 1.9.0 version bump is compatible with Kotlin 2.2.21 (as confirmed—kotlinx.serialization 1.9.0 targets Kotlin 2.2 and requires Kotlin 2.2.x). All version catalog entries, including the new shadow plugin entries, follow conventions correctly.
Also applies to: 18-18, 53-53
| tasks.named<Jar>("jar") { | ||
| dependsOn(tasks.shadowJar) | ||
| // Copy shadowJar content after jar is built | ||
| doLast { | ||
| val shadowTask = tasks.shadowJar.get() | ||
| val shadowJarFile = shadowTask.archiveFile.get().asFile | ||
| val jarFile = archiveFile.get().asFile | ||
|
|
||
| if (shadowJarFile.exists()) { | ||
| shadowJarFile.copyTo(jarFile, overwrite = true) | ||
| logger.lifecycle("Replaced ${jarFile.name} with shadowJar content") | ||
| } | ||
| } |
There was a problem hiding this comment.
Jar replacement logic is fragile and non-standard.
Using doLast { ... } to overwrite the jar file with shadowJar content bypasses Gradle's task model and caching mechanisms, which can lead to non-deterministic builds and cache invalidation issues. This approach is not idiomatic for Gradle plugin development.
Consider these alternatives:
- Configure shadowJar to output to the expected jar location instead.
- Use Gradle's artifact management (publishArtifact) to declare shadowJar as the primary artifact.
- Adjust the Maven publication configuration to reference shadowJar directly if used in publications.
Do you have a specific reason for the current approach, or would you prefer to refactor this to use standard Gradle patterns?
🤖 Prompt for AI Agents
In stability-compiler/build.gradle.kts around lines 76 to 88, the current doLast
block copies the shadowJar over the produced jar, which bypasses Gradle's task
model and caching; remove the doLast override and instead produce the shadow
artifact via standard Gradle semantics by one of: configure the shadowJar task
to write to the same archive location/name as the jar (set
archiveFileName/archiveClassifier appropriately and disable the default jar if
needed), or mark the shadowJar as the published/primary artifact (use
publications { ... artifact(tasks.shadowJar) { builtBy(tasks.shadowJar) } } or
publishArtifact=true) so Maven publications reference the shadow output
directly; after switching, delete the file-copying logic and ensure task
dependencies are set (e.g., jar.dependsOn shadowJar or builtBy) so builds remain
cacheable and deterministic.
Shadow kotlinx.serialization to the compiler plugin. (#48)
Summary by CodeRabbit
Dependencies
Build Improvements
Documentation