Ensure sentry-cli works well with configuration cache (#675)

* make cli cache path predictable

* Cleanup

* Add tests

* Use stable AGP 8.3.0 version

* (temp): Print build output

* (temp) try with assembleDebug

* Add changelog

* Ignore configuration cache below Gradle 8.0

* Exclude older AGP versions as well

* Remove sentryCliTempFolder flag in favor of static path calculation

* ensure cli is recovered in case it gets deleted between configuration cache enabled runs

* Update tests

.github/workflows/test-matrix-agp-gradle.yaml

@@ -38,7 +38,7 @@ jobs:
           - agp: "8.2.2"
             gradle: "8.2"
             java: "17"
-          - agp: "8.3.0-rc02"
+          - agp: "8.3.0"
             gradle: "8.4"
             java: "17"
           - agp: "8.4.0-alpha11"

CHANGELOG.md

@@ -3,8 +3,8 @@
 ## Unreleased
 
 ### Fixes
-
 - Do not pollute build classpath with groovy dependencies ([#677](https://github.com/getsentry/sentry-android-gradle-plugin/pull/677))
+- Ensure sentry-cli works well with configuration cache ([#675](https://github.com/getsentry/sentry-android-gradle-plugin/pull/675))
 
 ### Dependencies
 

plugin-build/src/main/kotlin/io/sentry/android/gradle/SentryCliProvider.kt

@@ -2,6 +2,7 @@
 
 package io.sentry.android.gradle
 
+import io.sentry.BuildConfig
 import io.sentry.android.gradle.SentryCliValueSource.Params
 import io.sentry.android.gradle.SentryPlugin.Companion.logger
 import io.sentry.android.gradle.util.GradleVersions
@@ -10,7 +11,6 @@ import io.sentry.android.gradle.util.info
 import java.io.File
 import java.io.FileInputStream
 import java.io.FileOutputStream
-import java.nio.file.Files
 import java.util.Locale
 import java.util.Properties
 import org.gradle.api.Project
@@ -32,7 +32,7 @@ internal object SentryCliProvider {
      */
     @JvmStatic
     @Synchronized
-    fun getSentryCliPath(projectDir: File, rootDir: File): String {
+    fun getSentryCliPath(projectDir: File, projectBuildDir: File, rootDir: File): String {
         val cliPath = memoizedCliPath
         if (!cliPath.isNullOrEmpty() && File(cliPath).exists()) {
             logger.info { "Using memoized cli path: $cliPath" }
@@ -66,7 +66,7 @@ internal object SentryCliProvider {
             // otherwise we need to unpack into a file
             logger.info { "Trying to load cli from $resourcePath in a temp file..." }
 
-            loadCliFromResourcesToTemp(resourcePath)?.let {
+            extractCliFromResources(projectBuildDir, resourcePath)?.let {
                 logger.info { "cli extracted from resources into: $it" }
                 memoizedCliPath = it
                 return@getSentryCliPath it
@@ -103,32 +103,35 @@ internal object SentryCliProvider {
         }
     }
 
-    internal fun loadCliFromResourcesToTemp(resourcePath: String): String? {
-        val resourceStream = javaClass.getResourceAsStream(resourcePath)
-        val tmpDirPrefix = try {
-            // only specific for some tests for deterministic behavior when executing in parallel
-            System.getProperty("sentryCliTempFolder")
-        } catch (e: Throwable) {
-            null
-        }
-        val tempFile = File.createTempFile(
-            ".sentry-cli",
-            ".exe",
-            tmpDirPrefix?.let { Files.createTempDirectory(it).toFile() }
-        ).apply {
-            deleteOnExit()
-            setExecutable(true)
-        }
+    fun getCliTargetPathForResources(projectBuildDir: File): File {
+        // usually <project>/build/tmp/
+        return File(File(projectBuildDir, "tmp"), "sentry-cli-${BuildConfig.CliVersion}.exe")
+    }
 
+    internal fun extractCliFromResources(projectBuildDir: File, resourcePath: String): String? {
+        val resourceStream = javaClass.getResourceAsStream(resourcePath)
         return if (resourceStream != null) {
-            FileOutputStream(tempFile).use { output ->
+            val cliFilePath = getCliTargetPathForResources(projectBuildDir)
+
+            val baseFolder = cliFilePath.parentFile
+            logger.info { "sentry-cli base folder: ${baseFolder.absolutePath}" }
+
+            if (!baseFolder.exists() && !baseFolder.mkdirs()) {
+                logger.error { "sentry-cli base folder could not be created!" }
+                return null
+            }
+
+            FileOutputStream(cliFilePath).use { output ->
                 resourceStream.use { input ->
                     input.copyTo(output)
                 }
             }
-            tempFile.absolutePath
+            cliFilePath.setExecutable(true)
+            cliFilePath.deleteOnExit()
+
+            cliFilePath.absolutePath
         } else {
-            null
+            return null
         }
     }
 
@@ -150,13 +153,17 @@ abstract class SentryCliValueSource : ValueSource<String, Params> {
         @get:Input
         val projectDir: Property<File>
 
+        @get:Input
+        val projectBuildDir: Property<File>
+
         @get:Input
         val rootProjDir: Property<File>
     }
 
     override fun obtain(): String? {
         return SentryCliProvider.getSentryCliPath(
             parameters.projectDir.get(),
+            parameters.projectBuildDir.get(),
             parameters.rootProjDir.get()
         )
     }
@@ -168,12 +175,14 @@ fun Project.cliExecutableProvider(): Provider<String> {
         // e.g. switching branches
         providers.of(SentryCliValueSource::class.java) {
             it.parameters.projectDir.set(project.projectDir)
+            it.parameters.projectBuildDir.set(project.layout.buildDirectory.asFile.get())
             it.parameters.rootProjDir.set(project.rootDir)
         }
     } else {
         return provider {
             SentryCliProvider.getSentryCliPath(
                 project.projectDir,
+                project.layout.buildDirectory.asFile.get(),
                 project.rootDir
             )
         }

plugin-build/src/test/kotlin/io/sentry/android/gradle/SentryCliProviderTest.kt

@@ -1,8 +1,8 @@
 package io.sentry.android.gradle
 
+import io.sentry.android.gradle.SentryCliProvider.extractCliFromResources
 import io.sentry.android.gradle.SentryCliProvider.getCliSuffix
 import io.sentry.android.gradle.SentryCliProvider.getSentryPropertiesPath
-import io.sentry.android.gradle.SentryCliProvider.loadCliFromResourcesToTemp
 import io.sentry.android.gradle.SentryCliProvider.searchCliInPropertiesFile
 import io.sentry.android.gradle.SentryCliProvider.searchCliInResources
 import io.sentry.android.gradle.util.SystemPropertyRule
@@ -157,7 +157,7 @@ class SentryCliProviderTest {
                 writeText("echo \"This is just a dummy script\"")
             }
 
-        val loadedPath = loadCliFromResourcesToTemp(resourcePath)
+        val loadedPath = extractCliFromResources(File("."), resourcePath)
         assertNotNull(loadedPath)
 
         val binContent = File(loadedPath).readText()
@@ -170,7 +170,7 @@ class SentryCliProviderTest {
     fun `loadCliFromResourcesToTemp returns null if file does not exist`() {
         val resourcePath = "./dummy-bin/i-dont-exist"
 
-        assertNull(loadCliFromResourcesToTemp(resourcePath))
+        assertNull(extractCliFromResources(File("."), resourcePath))
     }
 
     @Test

plugin-build/src/test/kotlin/io/sentry/android/gradle/integration/SentryPluginConfigurationCacheTest.kt

@@ -1,7 +1,10 @@
 package io.sentry.android.gradle.integration
 
 import io.sentry.BuildConfig
+import io.sentry.android.gradle.SentryCliProvider
+import io.sentry.android.gradle.util.AgpVersions
 import io.sentry.android.gradle.util.GradleVersions
+import io.sentry.android.gradle.util.SemVer
 import io.sentry.android.gradle.verifyDependenciesReportAndroid
 import java.io.File
 import kotlin.test.assertEquals
@@ -121,59 +124,87 @@ class SentryPluginConfigurationCacheTest :
     }
 
     @Test
-    fun `sentry-cli path calculation respects configuration cache`() {
+    fun `works well with configuration cache`() {
+        // configuration cache doesn't seem to work well on older Gradle/AGP combinations
+        // producing the following output:
+        //
+        // 0 problems were found storing the configuration cache.
+        // Configuration cache entry discarded
+        //
+        // so we only run this test on supported versions
         assumeThat(
-            "SentryCliProvider only supports " +
-                "configuration cache from Gradle 7.5 onwards and Gradle 8.1 uses cli " +
-                "from resources, so there's no extracting from .jar to a /tmp folder involved",
-            GradleVersions.CURRENT >= GradleVersions.VERSION_7_5 &&
-                GradleVersions.CURRENT <= GradleVersions.VERSION_8_0,
+            "We only support configuration cache from AGP 7.4.0 and Gradle 8.0.0 onwards",
+            SemVer.parse(BuildConfig.AgpVersion) >= AgpVersions.VERSION_7_4_0 &&
+                GradleVersions.CURRENT >= GradleVersions.VERSION_8_0,
             `is`(true)
         )
-        appBuildFile.writeText(
-            // language=Groovy
-            """
-            plugins {
-              id "com.android.application"
-              id "io.sentry.android.gradle"
-            }
 
-            android {
-              namespace 'com.example'
-            }
+        val runner = runner.withArguments(
+            "--configuration-cache",
+            "--build-cache",
+            ":app:assembleDebug"
+        )
 
-            dependencies {
-              implementation 'io.sentry:sentry-android-core:7.0.0'
-            }
+        val run0 = runner.build()
+        assertFalse(
+            "Reusing configuration cache." in run0.output ||
+                "Configuration cache entry reused." in run0.output,
+            run0.output
+        )
 
-            sentry {
-              autoUploadProguardMapping = false
-              autoInstallation.enabled = false
-              includeDependenciesReport = false
-              telemetry = true
-            }
-            """.trimIndent()
+        val run1 = runner.build()
+        assertTrue(
+            "Reusing configuration cache." in run1.output ||
+                "Configuration cache entry reused." in run1.output,
+            run1.output
         )
+    }
 
-        runner.appendArguments(":app:assembleDebug")
-            .appendArguments("--configuration-cache")
-            .appendArguments("--info")
-            .appendArguments("-DsentryCliTempFolder=configCacheTest")
-        val output = runner.build().output
-        val cliPath = output
-            .lines()
-            .find {
-                it.startsWith("[sentry] cli extracted from resources into:") ||
-                    it.startsWith("[sentry] Using memoized cli path:")
-            }
-            ?.substringAfter("[sentry] cli extracted from resources into:")
-            ?.substringAfter("[sentry] Using memoized cli path:")
-            ?.trim()
+    @Test
+    fun `sentry-cli is recovered when deleted during runs and configuration cache is active`() {
+        // configuration cache doesn't seem to work well on older Gradle/AGP combinations
+        // producing the following output:
+        //
+        // 0 problems were found storing the configuration cache.
+        // Configuration cache entry discarded
+        //
+        // so we only run this test on supported versions
+        assumeThat(
+            "We only support configuration cache from AGP 7.4.0 and Gradle 8.0.0 onwards",
+            SemVer.parse(BuildConfig.AgpVersion) >= AgpVersions.VERSION_7_4_0 &&
+                GradleVersions.CURRENT >= GradleVersions.VERSION_8_0,
+            `is`(true)
+        )
 
-        val cli = File(cliPath!!).also { it.delete() }
-        assertFalse { cli.exists() }
+        val runner = runner.withArguments(
+            "--configuration-cache",
+            "--build-cache",
+            ":app:assembleDebug"
+        )
 
-        val outputWithConfigCache = runner.build().output
-        assertFalse { "BUILD FAILED" in outputWithConfigCache }
+        val run0 = runner.build()
+        assertFalse(
+            "Reusing configuration cache." in run0.output ||
+                "Configuration cache entry reused." in run0.output,
+            run0.output
+        )
+
+        val cliPath = SentryCliProvider.getCliTargetPathForResources(
+            File(runner.projectDir, "build")
+        )
+
+        // On Gradle >= 8, the whole build folder is wiped anyway
+        if (cliPath.exists()) {
+            cliPath.delete()
+        }
+
+        // then it should be recovered on the next run
+        val run1 = runner.build()
+        assertTrue(
+            "Reusing configuration cache." in run1.output ||
+                "Configuration cache entry reused." in run1.output,
+            run1.output
+        )
+        assertTrue(run1.output) { "BUILD SUCCESSFUL" in run1.output }
     }
 }

plugin-build/src/test/kotlin/io/sentry/android/gradle/integration/SentryPluginSourceContextTest.kt

@@ -296,7 +296,11 @@ class SentryPluginSourceContextTest :
 
     @Test
     fun `uploadSourceBundle task is up-to-date on subsequent builds`() {
-        val sentryCli = SentryCliProvider.getSentryCliPath(File(""), File(""))
+        val sentryCli = SentryCliProvider.getSentryCliPath(
+            File(""),
+            File("build"),
+            File("")
+        )
         sentryPropertiesFile.writeText("cli.executable=$sentryCli")
 
         runner.appendArguments("app:assembleRelease")
@@ -346,7 +350,11 @@ class SentryPluginSourceContextTest :
 
     @Test
     fun `uploadSourceBundle task is not up-to-date on subsequent builds if cli path changes`() {
-        val sentryCli = SentryCliProvider.getSentryCliPath(File(""), File(""))
+        val sentryCli = SentryCliProvider.getSentryCliPath(
+            File(""),
+            File("build"),
+            File("")
+        )
         sentryPropertiesFile.writeText("cli.executable=$sentryCli")
 
         runner.appendArguments("app:assembleRelease")