ADFA-2508: Add PluginBuilder Gradle plugin for .cgp file generation#805
ADFA-2508: Add PluginBuilder Gradle plugin for .cgp file generation#805Daniel-ADFA merged 2 commits intostagefrom
Conversation
📝 WalkthroughPluginBuilder Gradle Plugin Release NotesFeatures
|
| Cohort / File(s) | Summary |
|---|---|
CI/CD Notification Enhancement .github/workflows/release.yml |
Extended Slack payload with commit author, Firebase console URL, and branch name; minor formatting adjustments in run blocks. |
Build Artifact Configuration .gitignore |
Refined build directory ignore pattern from **/build to **/build/ with negation rule !**/src/**/build/ to preserve source-path builds; added explicit ignores for plugin-api.jar and plugin-artifacts.zip. |
Gradle Plugin Infrastructure plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt, plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilderExtension.kt |
New PluginBuilder Gradle plugin class registering extension and defining two tasks (assemblePluginDebug, assemblePlugin) that locate APKs from build outputs, copy them to plugin/ directory with .cgp extension, delete originals, and log steps; accompanying PluginBuilderExtension abstract class with pluginName property. |
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
#783: Modifies plugin-builder module configuration and composite build inclusion, directly related to the PluginBuilder infrastructure added here.
Suggested reviewers
- jomen-adfa
Poem
🐰 A builder's task, so neat and tidy,
Plugin CGPs, assembled Friday!
APKs dance to .cgp delight,
Gradle workflows, burning bright! ✨
Ignores refined, notifications spread,
Build systems flourish, well-fed! 🌟
🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. | |
| Description check | ❓ Inconclusive | No pull request description was provided by the author, making it impossible to assess relevance to the changeset. | Add a description explaining the purpose of the PluginBuilder Gradle plugin, its functionality, and how it generates .cgp files from APKs. |
✅ Passed checks (1 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly summarizes the main change: adding a new PluginBuilder Gradle plugin for .cgp file generation, which aligns with the primary additions in the changeset. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing touches
- 📝 Generate docstrings
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In
@plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt:
- Around line 72-73: The code currently performs an unconditional destructive
delete of the source APK (apkFile.delete()) after copying; make this behavior
configurable instead of always deleting. Add a boolean option/property (e.g.,
deleteSourceApk or keepSourceApk flag) on PluginBuilder or the method that
performs the copy, default it to false (or follow the debug task's suggested
default), and wrap the apkFile.delete() call in a conditional that checks that
flag; update any constructors/factory methods to accept the flag and add a short
comment/docstring describing the flag’s effect so callers are aware of the
destructive behavior.
- Around line 43-44: The task currently deletes the source APK after copying
(apkFile.copyTo(...); apkFile.delete()) which is destructive; add a configurable
option on the extension (e.g., PluginBuilderExtension with abstract val
deleteSourceApk: Property<Boolean>, default false) and update the PluginBuilder
task to check that property before calling apkFile.delete(), or alternatively
remove the delete call to preserve the APK and document the behavior in the task
description; reference PluginBuilder.kt (the copyTo/delete call) and
PluginBuilderExtension (new deleteSourceApk) when making the change.
- Around line 59-60: In PluginBuilder.kt replace deprecated usages of
project.buildDir when creating apkDir and outputDir: instead of
File(project.buildDir, "outputs/apk/release") and File(project.buildDir,
"plugin"), use the Gradle Layout API via
project.layout.buildDirectory.dir("outputs/apk/release").get().asFile for apkDir
and project.layout.buildDirectory.dir("plugin").get().asFile for outputDir so
the code uses DirectoryProvider from project.layout.buildDirectory rather than
the deprecated project.buildDir.
- Around line 30-31: Replace deprecated project.buildDir usage in
PluginBuilder.kt: the declarations that create apkDir and outputDir using
"File(project.buildDir, ...)" should instead use the Gradle Property API via
project.layout.buildDirectory; update the code that defines apkDir and outputDir
(symbols: apkDir, outputDir) to resolve paths from project.layout.buildDirectory
(e.g., call get().dir("outputs/apk/debug") and get().dir("plugin") or
buildDirectory.asFile/dir equivalents) so you no longer reference the removed
project.buildDir property.
🧹 Nitpick comments (6)
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt (6)
36-40: Task should fail when APK is not found.The task currently logs a warning and returns silently when no APK is found, but still reports success. This could be confusing during CI/CD or debugging. Consider failing the task explicitly to provide clearer feedback.
💡 Proposed fix to fail the task explicitly
val apkFile = apkDir.listFiles()?.firstOrNull { it.extension == "apk" } if (apkFile == null) { - project.logger.warn("No APK found in ${apkDir.absolutePath}") - return + throw org.gradle.api.GradleException("No APK found in ${apkDir.absolutePath}") }
43-45: Add error handling for file operations.The file copy and delete operations can fail for various reasons (permissions, disk space, file locks). Without try-catch, the task will fail with an unclear error. Consider adding explicit error handling with meaningful messages.
🛡️ Proposed error handling
val outputFile = File(outputDir, "$pluginName-debug.cgp") - apkFile.copyTo(outputFile, overwrite = true) - apkFile.delete() - project.logger.lifecycle("Plugin assembled: ${outputFile.absolutePath}") + try { + apkFile.copyTo(outputFile, overwrite = true) + apkFile.delete() + project.logger.lifecycle("Plugin assembled: ${outputFile.absolutePath}") + } catch (e: Exception) { + throw org.gradle.api.GradleException("Failed to create plugin file: ${e.message}", e) + }
65-69: Task should fail when APK is not found.Same issue as the debug task - silent failure on missing APK is confusing. Consider throwing a
GradleExceptioninstead.💡 Proposed fix to fail the task explicitly
val apkFile = apkDir.listFiles()?.firstOrNull { it.extension == "apk" } if (apkFile == null) { - project.logger.warn("No APK found in ${apkDir.absolutePath}") - return + throw org.gradle.api.GradleException("No APK found in ${apkDir.absolutePath}") }
72-74: Add error handling for file operations.Same issue as the debug task - file operations need proper error handling.
🛡️ Proposed error handling
val outputFile = File(outputDir, "$pluginName.cgp") - apkFile.copyTo(outputFile, overwrite = true) - apkFile.delete() - project.logger.lifecycle("Plugin assembled: ${outputFile.absolutePath}") + try { + apkFile.copyTo(outputFile, overwrite = true) + apkFile.delete() + project.logger.lifecycle("Plugin assembled: ${outputFile.absolutePath}") + } catch (e: Exception) { + throw org.gradle.api.GradleException("Failed to create plugin file: ${e.message}", e) + }
27-46: Consider using Kotlin lambda syntax for cleaner code.The verbose
object : org.gradle.api.Action<org.gradle.api.Task>syntax can be simplified using Kotlin's lambda syntax withdoLast { }.♻️ Cleaner lambda syntax
- task.doLast(object : org.gradle.api.Action<org.gradle.api.Task> { - override fun execute(t: org.gradle.api.Task) { + task.doLast { val pluginName = extension.pluginName.getOrElse(project.name) // ... rest of the code - } - }) + }Also applies to: 56-75
21-48: Consider declaring task outputs for Gradle caching and up-to-date checks.The tasks don't declare their outputs, which prevents Gradle from properly tracking incremental builds and enabling build caching. Consider adding output declarations.
📦 Add task outputs
task.doLast { val pluginName = extension.pluginName.getOrElse(project.name) val buildDir = project.layout.buildDirectory.get().asFile val apkDir = File(buildDir, "outputs/apk/debug") val outputDir = File(buildDir, "plugin") outputDir.mkdirs() + + val outputFile = File(outputDir, "$pluginName-debug.cgp") + task.outputs.file(outputFile) project.logger.lifecycle("Looking for APK in: ${apkDir.absolutePath}") // ... rest of the codeNote: This would require restructuring to declare outputs before
doLast.Also applies to: 50-77
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/release.yml.gitignoreplugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.ktplugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilderExtension.kt
🔇 Additional comments (6)
.gitignore (2)
131-132: LGTM! Refined build directory ignore pattern.The change from
**/buildto**/build/with the negation rule!**/src/**/build/is a good refinement. This ensures that actual build output directories are ignored while preserving anybuilddirectories that might exist in source paths.
137-139: LGTM! Plugin artifact ignores align with the new plugin builder.The added ignore entries for
plugin-api.jarandplugin-artifacts.zipappropriately prevent tracking of generated plugin artifacts introduced by the PluginBuilder functionality..github/workflows/release.yml (1)
655-664: LGTM! Enhanced Slack notification with additional context.The extended payload now includes commit author and branch name, providing more comprehensive release notification information. The formatting improvements also enhance readability.
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilderExtension.kt (1)
1-7: LGTM! Clean extension design.The extension is minimal and follows Gradle best practices by using
Property<String>for lazy configuration.plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt (2)
7-19: LGTM! Proper plugin structure.The plugin correctly registers the extension and uses
afterEvaluateto ensure configuration values are available before creating tasks.
59-59: Verify hardcoded APK output path matches Android Gradle Plugin structure.Same concern as the debug task - the hardcoded
outputs/apk/releasepath may not match the actual Android Gradle Plugin output structure.Refer to the verification script in the debug task comment.
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt
Show resolved
Hide resolved
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt
Show resolved
Hide resolved
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt
Show resolved
Hide resolved
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt
Show resolved
Hide resolved
plugin-api/plugin-builder/src/main/kotlin/com/itsaky/androidide/plugins/build/PluginBuilder.kt
Show resolved
Hide resolved
2 participants
No description provided.