Make keepSecrets task compatible with configuration cache #21
Conversation
- Update Kotlin to 2.2.21 - Update Android Gradle Plugin to 8.13.1 - Update Gradle to 8.14.3 - Set JVM target to 17 - Update sample app `compileSdk` and `targetSdk` to 36 - Update sample app `minSdk` to 23 - Update `androidx.activity:activity-ktx` to 1.12.0 - Add edge-to-edge support in the sample app
WalkthroughUpdated build toolchain (Gradle, AGP, Kotlin), added jvmTarget=17, migrated Java/Kotlin compilation to Gradle toolchains, refactored KeepSecretsTask to use constructor injection and explicit Gradle property inputs, changed SecretsVaultExtension file properties to RegularFileProperty and added CMake config, and adjusted sample app SDK and window insets handling. Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin as SecretsVaultPlugin
participant Ext as SecretsVaultExtension
participant Task as KeepSecretsTask
participant Layout as ProjectLayout
Plugin->>Ext: read extension values (secretsFile, cmake, obfuscationKey, appSignatures, makeInjectable, packageName?)
Note right of Plugin: Determine packageName fallback (Android namespace) and cmake defaults
Plugin->>Task: create/configure task (inject ProjectLayout)
Plugin->>Task: set pluginSourceFolder (via ProjectLayout)
Plugin->>Task: wire RegularFileProperty and other inputs
Task->>Layout: resolve paths via DirectoryProperty/RegularFileProperty
Task->>Task: decode secrets (json.decodeFromString) and generate outputs (Kotlin/CPP/CMake)
Task-->>Plugin: task outputs (files written)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
Fix new issues
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt (1)
69-73: Consider using Provider-based path resolution.Using
copyTask.destinationDir.pathextracts the path as a String during configuration. While the task output is wrapped in a Provider viamap, thedestinationDirproperty access pattern could be improved for better configuration cache hygiene.Consider using the Provider-based destination:
pluginSourceFolder.set( unzipTaskProvider.map { copyTask -> - project.layout.projectDirectory.dir(copyTask.destinationDir.path) + copyTask.destinationDir } )If
pluginSourceFolderis aDirectoryProperty, you can directly set it from theFilereturned bydestinationDir.secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/KeepSecretsTask.kt (1)
126-126: Optional: Remove unnecessaryencodeDefaultsconfiguration.The
encodeDefaults = trueconfiguration only affects encoding/serialization. Since thisjsoninstance is only used fordecodeFromStringoperations (lines 197, 223), this setting has no effect and can be removed for clarity.- private val json = Json { encodeDefaults = true } + private val json = Json
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
gradle/libs.versions.toml(3 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)sampleapp/build.gradle.kts(2 hunks)sampleapp/src/main/kotlin/com/commencis/secretsvaultplugin/sampleapp/MainActivity.kt(2 hunks)secretsvaultplugin/build.gradle.kts(1 hunks)secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/KeepSecretsTask.kt(13 hunks)secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt(3 hunks)secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/extensions/SecretsVaultExtension.kt(2 hunks)
🔇 Additional comments (15)
gradle/libs.versions.toml (4)
7-7: Good addition of centralized JVM target.Adding
jvmTarget = "17"to the version catalog enables consistent JVM target configuration across modules.
31-32: Bundle reference syntax updated correctly.The bundle now correctly references the library aliases using kebab-case (
detekt-formating,detekt-rules), matching the library definitions on lines 17-18.
2-3: Version numbers are legitimate — both Kotlin 2.2.21 and Android Gradle Plugin 8.13.1 are actual releases.Kotlin 2.2.21 was released on October 23, 2025. Android Gradle Plugin 8.13.1 was published on November 10, 2025. Both versions exist and are appropriate choices for the current timeframe. The version numbers are not unusually high; they represent normal patch releases within their respective major/minor version series.
Likely an incorrect or invalid review comment.
21-21: androidx.activity:activity-ktx 1.12.0 is valid and stable.Version 1.12.0 was released on November 19, 2025 as a stable release. The version bump from 1.7.2 to 1.12.0 is significant but represents a legitimate upstream release.
sampleapp/build.gradle.kts (3)
13-14: Breaking change: minSdk increased from 21 to 23.This drops support for Android 5.0-5.1 (Lollipop). This is a sample app, so the impact is limited, but worth noting.
78-82: Good migration to Java toolchain.Using
java { toolchain { ... } }instead ofcompileOptionsandkotlinOptionsis the recommended approach for configuration cache compatibility. The toolchain automatically configures both Java and Kotlin compilation targets.
9-9: Android SDK 36 is available and stable. SDK 36 (Android 16) was officially released to the stable channel in March 2025. ThecompileSdk = 36change is appropriate and supported.secretsvaultplugin/build.gradle.kts (1)
17-21: Clean toolchain configuration.Good migration from explicit
sourceCompatibility/targetCompatibilityandKotlinCompiletask configuration to the simpler toolchain DSL. This reduces boilerplate and is configuration cache friendly.secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/extensions/SecretsVaultExtension.kt (1)
28-29: Breaking API change: File properties migrated to RegularFileProperty.Migrating from
Property<File>toRegularFilePropertyis the correct approach for configuration cache compatibility. However, this is a breaking change for any consumers setting these properties.Previous usage:
secretsFile.set(file("secrets.json"))New usage:
secretsFile.set(layout.projectDirectory.file("secrets.json"))Consider documenting this migration in the changelog or PR description.
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt (2)
88-89: Good use of flatMap for lazy property chaining.Using
flatMapto chain into the nested CMakeExtension properties maintains lazy evaluation, which is important for configuration cache compatibility.
116-116: Correct migration to layout-based file convention.Using
project.layout.projectDirectory.file(...)instead ofproject.file(...)is the proper approach for configuration cache compatibility withRegularFileProperty.gradle/wrapper/gradle-wrapper.properties (1)
3-3: Gradle 8.14.3 is a valid, stable release.Gradle 8.14.3 was released on July 4, 2025 (4 months before the current date), includes bug fixes and Java 24 support, and the
-all.zipdistribution is confirmed available on Gradle's distributions server. The version upgrade from 8.1.1 is reasonable, and switching to the-alldistribution is appropriate for IDE integration.secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/KeepSecretsTask.kt (2)
61-61: Excellent: Constructor injection for configuration cache compatibility.The use of
@Injectconstructor withProjectLayoutis the correct approach for configuration cache compatibility.
165-165: LGTM: Correct property usage for configuration cache.All property accesses follow configuration cache best practices:
- Properties are accessed via
.get()only during task execution time- File properties are properly resolved using
.asFile- Optional properties are handled correctly with
.orNull- Path resolution uses
projectDirectory.file(...)instead of directFileaccessThis implementation correctly ensures that all property evaluation happens during task execution, not during configuration.
Also applies to: 180-180, 266-268, 281-282, 285-285, 299-299, 302-302, 307-307, 310-310, 334-334, 360-360, 363-364, 410-410, 415-415, 421-421, 512-512, 518-518
sampleapp/src/main/kotlin/com/commencis/secretsvaultplugin/sampleapp/MainActivity.kt (1)
9-11: LGTM!The imports are appropriate for implementing edge-to-edge window insets handling.
sampleapp/src/main/kotlin/com/commencis/secretsvaultplugin/sampleapp/MainActivity.kt
Show resolved
Hide resolved
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/KeepSecretsTask.kt
Show resolved
Hide resolved
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt
Show resolved
Hide resolved
- Use `orElse` with namespace provider to lazily get the package name - Simplify setting the `pluginSourceFolder` path.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt (2)
60-77: WirepluginSourceFolderdirectly from the unzip task’s output directoryRight now you convert
destinationDirto an absoluteStringand then back to aDirectoryviaprojectDirectory.dir(...). You can keep things simpler and better connected to the producing task by wiring theDirectoryPropertydirectly from theCopytask:- pluginSourceFolder.set( - project.layout.projectDirectory.dir( - unzipTaskProvider.map { task -> task.destinationDir.absolutePath } - ) - ) + pluginSourceFolder.set( + unzipTaskProvider.flatMap { task -> task.destinationDirectory } + )This avoids the string round‑trip and keeps the consumer input tied to the producer’s output property, which is more idiomatic and plays nicer with task dependencies and incremental behavior.
Please double‑check this against the Gradle 8.14
CopyAPI and your local build to ensuredestinationDirectoryis available and correctly annotated as an output directory.
79-90: Confirm behavior for explicitly blanksecretsVaultExtension.packageNameThe new provider chain:
packageName.set( secretsVaultExtension.packageName.orElse( project.provider { project.extensions.findByType(CommonExtension::class.java)?.namespace.orEmpty() } ) )changes semantics slightly compared to the earlier
getOrElse(EMPTY_STRING).ifEmpty { namespace }approach: if someone explicitly setspackageName = ""in the extension, it will now remain""instead of falling back to the Androidnamespace.If no one relies on “blank string means use namespace”, this is fine but might be worth a brief doc note. If that behavior is still desired for backward compatibility, you may want an extra mapping step so that blank values also route to the namespace provider.
Please verify how existing build scripts use
secretsVaultExtension.packageName(if at all) to decide whether preserving or restoring the old “blank means fallback” behavior is necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt(3 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). (1)
- GitHub Check: build
🔇 Additional comments (1)
secretsvaultplugin/src/main/kotlin/com/commencis/secretsvaultplugin/SecretsVaultPlugin.kt (1)
117-117: Good move to basesecretsFileonProjectLayoutUsing
secretsFile.convention(project.layout.projectDirectory.file(DEFAULT_SECRETS_FILE_NAME))aligns with Gradle’s file abstractions and is configuration‑cache‑friendly. No issues here.
Modernize gradle task inputs for configuration cache compatibility
Bump dependencies as below:
compileSdkandtargetSdkto 36minSdkto 23androidx.activity:activity-ktxto 1.12.0Fixes Issue: #19
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.