refactor(android): Convert sentry.gradle to Kotlin DSL (sentry.gradle.kts)#6119
refactor(android): Convert sentry.gradle to Kotlin DSL (sentry.gradle.kts)#6119antonis wants to merge 10 commits into
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
iOS (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d038a14+dirty | 3845.71 ms | 1228.11 ms | -2617.59 ms |
| 5569641+dirty | 3839.22 ms | 1231.30 ms | -2607.91 ms |
| 5a21b51+dirty | 3823.11 ms | 1214.46 ms | -2608.65 ms |
| 4b87b12+dirty | 1212.90 ms | 1222.09 ms | 9.19 ms |
| f3215d3+dirty | 3842.73 ms | 1219.33 ms | -2623.40 ms |
| 7d6fd3a+dirty | 1223.29 ms | 1229.57 ms | 6.28 ms |
| 3d377b5+dirty | 1218.48 ms | 1219.51 ms | 1.03 ms |
| 23598c3+dirty | 1207.00 ms | 1209.90 ms | 2.90 ms |
| 4953e94+dirty | 1212.06 ms | 1214.83 ms | 2.77 ms |
| 04207c4+dirty | 1191.27 ms | 1189.78 ms | -1.48 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d038a14+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 5569641+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 5a21b51+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 4b87b12+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| f3215d3+dirty | 5.15 MiB | 6.67 MiB | 1.52 MiB |
| 7d6fd3a+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 3d377b5+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 23598c3+dirty | 3.38 MiB | 4.80 MiB | 1.42 MiB |
| 4953e94+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 04207c4+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
iOS (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d038a14+dirty | 3831.11 ms | 1216.30 ms | -2614.81 ms |
| 5569641+dirty | 3824.35 ms | 1210.78 ms | -2613.57 ms |
| 5a21b51+dirty | 3837.87 ms | 1223.47 ms | -2614.40 ms |
| 4b87b12+dirty | 1199.49 ms | 1199.78 ms | 0.29 ms |
| f3215d3+dirty | 3846.08 ms | 1231.85 ms | -2614.23 ms |
| 7d6fd3a+dirty | 1210.89 ms | 1217.63 ms | 6.74 ms |
| 3d377b5+dirty | 1201.55 ms | 1201.80 ms | 0.25 ms |
| 23598c3+dirty | 1223.59 ms | 1229.13 ms | 5.53 ms |
| 4953e94+dirty | 1217.41 ms | 1223.53 ms | 6.12 ms |
| 04207c4+dirty | 1228.55 ms | 1226.04 ms | -2.51 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d038a14+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 5569641+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 5a21b51+dirty | 5.15 MiB | 6.67 MiB | 1.51 MiB |
| 4b87b12+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| f3215d3+dirty | 5.15 MiB | 6.67 MiB | 1.52 MiB |
| 7d6fd3a+dirty | 3.38 MiB | 4.77 MiB | 1.39 MiB |
| 3d377b5+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
| 23598c3+dirty | 3.38 MiB | 4.80 MiB | 1.42 MiB |
| 4953e94+dirty | 3.38 MiB | 4.73 MiB | 1.35 MiB |
| 04207c4+dirty | 3.38 MiB | 4.76 MiB | 1.38 MiB |
antonis
force-pushed
the
antonis/convert-sentry-gradle-to-kts
branch
from
64495fd to
b1ed616
Compare
Android (legacy) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d377b5+dirty | 406.18 ms | 453.52 ms | 47.34 ms |
| 8929511+dirty | 405.33 ms | 452.16 ms | 46.83 ms |
| 100ce80+dirty | 463.66 ms | 539.56 ms | 75.90 ms |
| 3817909+dirty | 406.67 ms | 416.58 ms | 9.91 ms |
| 3b6e9f9+dirty | 442.70 ms | 486.44 ms | 43.74 ms |
| 4e0ba9c+dirty | 452.84 ms | 473.36 ms | 20.52 ms |
| 5fe1c6c+dirty | 401.62 ms | 445.28 ms | 43.66 ms |
| 44c8b3f+dirty | 414.20 ms | 457.28 ms | 43.08 ms |
| 890d145+dirty | 504.54 ms | 491.55 ms | -12.99 ms |
| 3ce5254+dirty | 410.57 ms | 448.48 ms | 37.91 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 3d377b5+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
| 8929511+dirty | 43.75 MiB | 48.16 MiB | 4.41 MiB |
| 100ce80+dirty | 48.30 MiB | 53.46 MiB | 5.15 MiB |
| 3817909+dirty | 43.75 MiB | 48.08 MiB | 4.33 MiB |
| 3b6e9f9+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 4e0ba9c+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| 5fe1c6c+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
| 44c8b3f+dirty | 48.30 MiB | 53.46 MiB | 5.15 MiB |
| 890d145+dirty | 43.75 MiB | 48.14 MiB | 4.39 MiB |
| 3ce5254+dirty | 43.75 MiB | 48.12 MiB | 4.37 MiB |
📲 Install BuildsAndroid
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 1118790. Configure here.
antonis
force-pushed
the
antonis/convert-sentry-gradle-to-kts
branch
from
7f5736c to
e850959
Compare
….kts) Convert the Groovy-based sentry.gradle script to Kotlin DSL for better type safety and IDE support. The original sentry.gradle is kept as a one-line shim that forwards to sentry.gradle.kts for backward compatibility with existing app/build.gradle references. Key changes: - Full Kotlin DSL rewrite with proper type annotations - Groovy interop via DefaultGroovyMethods and groovy.lang.Closure wrappers - AGP reflection wrapped in try/catch for resilience - Cross-version Kotlin compat using Java Character API - ProcessBuilder deadlock prevention (read stdout before waitFor) - Handle both File and Directory types for root property - Expo plugin auto-migrates old sentry.gradle refs to sentry.gradle.kts Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
antonis
force-pushed
the
antonis/convert-sentry-gradle-to-kts
branch
from
e850959 to
4735111
Compare
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4735111. Configure here.
Android (new) Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a736b76+dirty | 405.78 ms | 458.74 ms | 52.96 ms |
| 890d145+dirty | 486.42 ms | 514.85 ms | 28.43 ms |
| 5fe1c6c+dirty | 365.84 ms | 408.62 ms | 42.78 ms |
| 44c8b3f+dirty | 492.13 ms | 563.47 ms | 71.34 ms |
| 04207c4+dirty | 395.40 ms | 456.55 ms | 61.15 ms |
| 3b6e9f9+dirty | 442.39 ms | 486.44 ms | 44.05 ms |
| 4e0ba9c+dirty | 421.39 ms | 455.80 ms | 34.41 ms |
| 3817909+dirty | 357.52 ms | 391.52 ms | 34.00 ms |
| bc0d8cf+dirty | 407.66 ms | 461.35 ms | 53.69 ms |
| 5569641+dirty | 465.92 ms | 532.22 ms | 66.30 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| a736b76+dirty | 48.30 MiB | 53.48 MiB | 5.18 MiB |
| 890d145+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 5fe1c6c+dirty | 43.94 MiB | 49.00 MiB | 5.06 MiB |
| 44c8b3f+dirty | 48.30 MiB | 53.46 MiB | 5.15 MiB |
| 04207c4+dirty | 43.94 MiB | 48.98 MiB | 5.04 MiB |
| 3b6e9f9+dirty | 48.30 MiB | 53.54 MiB | 5.23 MiB |
| 4e0ba9c+dirty | 48.30 MiB | 53.49 MiB | 5.19 MiB |
| 3817909+dirty | 43.94 MiB | 48.94 MiB | 5.00 MiB |
| bc0d8cf+dirty | 48.30 MiB | 53.48 MiB | 5.18 MiB |
| 5569641+dirty | 48.30 MiB | 53.48 MiB | 5.18 MiB |
|
Might be worth adding a changelog for it. Also I am wondering if we shouldn't do this on v9 instead of main |
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Good point @lucas-zimerman 👍 I've added a note with 94b1795
I was thinking that this is an internal change that should not affect any public api or convention. Do you have a specific scenario in mind that may cause a breakage? |
Node stderr output (deprecation warnings etc.) would corrupt the resolved path string when redirectErrorStream(true) was used. Read only stdout and close the error stream instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: sentry-warden[bot] <258096371+sentry-warden[bot]@users.noreply.github.com>
Groovy string interpolation produces GStringImpl objects, not java.lang.String. The as? String cast silently returns null for GStrings, causing user-configured values (scripts, properties, etc.) to be ignored. Using ?.toString() handles both types correctly. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The sentryProperties config value can be a Groovy GString when set with string interpolation. Use toString() fallback instead of strict is String check to handle both types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ask properties Return null result instead of shouldCleanUp=true when cmd/cmdArgs properties are unavailable, preventing upload attempts against a non-existent sourcemap file. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Migration regex incorrectly rewrites io.sentry.android.gradle plugin declaration to io.sentry.android.gradle.kts
The migration guard buildGradle.includes('sentry.gradle') in modifyAppBuildGradle matches the substring in io.sentry.android.gradle, causing the subsequent regex to rewrite apply plugin: "io.sentry.android.gradle" → apply plugin: "io.sentry.android.gradle.kts", breaking the build for projects using the Sentry Android Gradle Plugin.
Verification
Checked packages/core/plugin/src/withSentryAndroid.ts lines 42-51. The function checks buildGradle.includes('sentry.gradle') to detect old references to migrate, but io.sentry.android.gradle contains the substring sentry.gradle. When a build.gradle has apply plugin: "io.sentry.android.gradle" but no apply from: ... "sentry.gradle" line: step 1 includes('sentry.gradle.kts') returns false; step 2 includes('sentry.gradle') returns true (false positive); step 3 the regex /sentry\.gradle(?!\.kts)/g matches sentry.gradle inside io.sentry.android.gradle (not followed by .kts) and replaces it, producing apply plugin: "io.sentry.android.gradle.kts". Reviewed packages/core/test/expo-plugin/modifyAppBuildGradle.test.ts — there is no test case that calls modifyAppBuildGradle with content containing only io.sentry.android.gradle, confirming the path is untested. The constant BUILD_GRADLE_WITH_ANDROID_GRADLE_PLUGIN exists only in sentryExpoNativeCheck.test.ts and is never passed to modifyAppBuildGradle.
Identified by Warden find-bugs
…radle plugin The lookbehind ensures the regex only matches sentry.gradle preceded by a quote or slash character, avoiding false positives on package names. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Good catch by Warden — fixed in c3effa6. The migration regex now uses a lookbehind |
… smart-cast - Re-add extractBundleTaskArgumentsLegacy for projects on RN 0.65-0.70 that lack Provider-based bundle task properties - Assign reactRoot to immutable val after null check for clean Kotlin smart-casting in task lambdas Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Extract args.bundleOutput into local val to avoid smart-cast failure on older Kotlin compilers (Gradle 7.5.1 / RN 0.71). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3d46737. Configure here.
|
So far the PR looks good! but I'd like an extra review from @romtsn before merging it. |
2 participants
📢 Type of change
📜 Description
Converts
sentry.gradle(Groovy) tosentry.gradle.kts(Kotlin DSL).BundleTaskArgs,ForceSourceMapResult,VariantInfo) instead of Groovy dynamic listssentry.gradleas a one-line shim forwarding tosentry.gradle.ktsfor backward compatibility with existingbuild.gradlereferencesgroovy.lang.Closure<Boolean>withdoCall()forextra[...]properties to ensure proper Groovy interop from consumingbuild.gradlefilesjava.lang.reflect.Proxy) wrapped in try/catch for resilience across AGP versionsDefaultGroovyMethods.getProperties()for task property access (Kotlin DSL can't resolve.propertiesdirectly on Gradle tasks)List.execute()withProcessBuilder, withredirectErrorStream(true)and stdout read beforewaitFor()to prevent deadlocksFileandDirectorytypes for therootproperty (varies across RN/AGP versions)Character.toLowerCase()/Character.toUpperCase()andString.contains(ignoreCase)for cross-version Kotlin compat (works on both Gradle 7.5/RN 0.71 and Gradle 9.3/RN 0.85+)--sourcemap-outputisn't explicitly configuredextractBundleTaskArgumentsLegacy(RN < 0.71 dead code — minimum supported is 0.71+)sentry.gradlereferences tosentry.gradle.ktson upgradebuild.gradlereferences across samples, e2e tests, perf tests, and patch scripts💡 Motivation and Context
Closes #2327
Gradle's Kotlin DSL provides type safety, better IDE support, and is the recommended script language going forward. The Groovy
.gradleformat is in maintenance mode. This aligns with the sentry-java/android SDK which already uses Kotlin DSL.💚 How did you test it?
modifyAppBuildGradle, including new migration test)sentryExpoNativeCheck)📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
sentry.gradle.kts