diff --git a/.changeset/deprecate-gradle-plugin-release-mode.md b/.changeset/deprecate-gradle-plugin-release-mode.md new file mode 100644 index 000000000..163506afb --- /dev/null +++ b/.changeset/deprecate-gradle-plugin-release-mode.md @@ -0,0 +1,7 @@ +--- +'posthog-android-gradle-plugin': minor +--- + +Deprecate the `posthog.releaseMode` gradle property and the upload task's `releaseMode` input. Both are ignored now, with a build-log warning: the plugin uploads the proguard mapping bound to the release it creates, which is what it did before the property existed. Remove the property to silence the warning. The `POSTHOG_RELEASE_MODE` environment variable no longer affects this upload either: the task warns when an inherited value used to select event mode, and pins `symbol-set` into the posthog-cli environment so the value cannot leave the mapping release-independent on an older CLI. + +Event mode only helps when two releases ship a byte-identical mapping. The map id is already a content hash, so an ordinary release that changes code gets its own symbol set and never collides with an earlier one. diff --git a/posthog-android-gradle-plugin/src/functionalTest/kotlin/com/posthog/android/NativeSymbolsUploadFunctionalTest.kt b/posthog-android-gradle-plugin/src/functionalTest/kotlin/com/posthog/android/NativeSymbolsUploadFunctionalTest.kt index 3fe19ec1f..a9486963e 100644 --- a/posthog-android-gradle-plugin/src/functionalTest/kotlin/com/posthog/android/NativeSymbolsUploadFunctionalTest.kt +++ b/posthog-android-gradle-plugin/src/functionalTest/kotlin/com/posthog/android/NativeSymbolsUploadFunctionalTest.kt @@ -60,6 +60,7 @@ internal class NativeSymbolsUploadFunctionalTest( private fun setUpProject( uploadNativeSymbols: Boolean, includeNativeSymbolSources: Boolean = false, + minifyEnabled: Boolean = false, ): File { val fakeCliLog = File(projectDir.root, "fake-cli-args.txt") val fakeCli = File(projectDir.root, "fake-posthog-cli") @@ -102,6 +103,36 @@ internal class NativeSymbolsUploadFunctionalTest( File(app, "src/main/jniLibs/arm64-v8a").mkdirs() File(app, "src/main/jniLibs/arm64-v8a/libfake.so").writeBytes(byteArrayOf(0x7f, 0x45, 0x4c, 0x46)) + var minifyBlock = "" + if (minifyEnabled) { + // A kept-but-obfuscated class makes R8 write a non-empty mapping, so the + // mapping upload task has something to run on. + File(app, "src/main/java/com/posthog/test").mkdirs() + File(app, "src/main/java/com/posthog/test/Placeholder.java").writeText( + """ + package com.posthog.test; + + public class Placeholder { + public static String tag() { + return "placeholder"; + } + } + """.trimIndent(), + ) + File(app, "proguard-rules.pro").writeText( + "-keep,allowobfuscation class com.posthog.test.** { *; }\n", + ) + minifyBlock = + """ + | buildTypes { + | release { + | minifyEnabled true + | proguardFiles 'proguard-rules.pro' + | } + | } + """.trimMargin() + } + File(app, "build.gradle").writeText( """ apply plugin: 'com.android.application' @@ -115,6 +146,7 @@ internal class NativeSymbolsUploadFunctionalTest( versionCode 1 versionName '1.0' } + $minifyBlock } posthog { @@ -235,4 +267,34 @@ internal class NativeSymbolsUploadFunctionalTest( val scheduled = runner(":app:assembleDebug", "--dry-run").build() assertFalse(scheduled.output.contains(":app:uploadPostHogNativeSymbolsDebug"), scheduled.output) } + + @Test + fun `warns about the deprecated release mode on configuration-cache-reused builds`() { + setUpProject(uploadNativeSymbols = false, minifyEnabled = true) + // a non-cache warm-up stabilizes AGP's TestKit android.lock creation, + // which can otherwise invalidate the first stored entry + runner(":app:tasks").build() + + // --rerun-tasks on both runs keeps the cache key identical while forcing the upload task + // to execute on the reused entry, where a configuration-time warning cannot fire. + val arguments = + arrayOf( + ":app:uploadPostHogProguardMappingsRelease", + "--rerun-tasks", + "--configuration-cache", + "--configuration-cache-problems=fail", + "-Pposthog.releaseMode=event", + ) + val warning = "is deprecated and ignored" + + val first = runner(*arguments).build() + assertTrue(first.output.contains("Configuration cache entry stored"), first.output) + assertEquals(TaskOutcome.SUCCESS, first.task(":app:uploadPostHogProguardMappingsRelease")?.outcome) + assertTrue(first.output.contains(warning), first.output) + + val second = runner(*arguments).build() + assertTrue(second.output.contains("Reusing configuration cache"), second.output) + assertEquals(TaskOutcome.SUCCESS, second.task(":app:uploadPostHogProguardMappingsRelease")?.outcome) + assertTrue(second.output.contains(warning), second.output) + } } diff --git a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogAndroidGradlePlugin.kt b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogAndroidGradlePlugin.kt index e74d9e761..5adf6c654 100644 --- a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogAndroidGradlePlugin.kt +++ b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogAndroidGradlePlugin.kt @@ -38,6 +38,8 @@ internal abstract class PostHogAndroidGradlePlugin extension.uploadNativeSymbols.convention(false) extension.includeNativeSymbolSources.convention(false) + warnIfDeprecatedReleaseModeSet(project) + val failureTracker = project.gradle.sharedServices.registerIfAbsent( PostHogTaskFailureTracker.NAME, @@ -45,8 +47,6 @@ internal abstract class PostHogAndroidGradlePlugin ) {} buildEventsListenerRegistry.onTaskCompletion(failureTracker) - val releaseMode = resolvePostHogReleaseMode(project) - project.pluginManager.withPlugin("com.android.application") { val androidComponentsExt = project.extensions.getByType(ApplicationAndroidComponentsExtension::class.java) @@ -66,7 +66,7 @@ internal abstract class PostHogAndroidGradlePlugin // TODO: skip variants, skip autoUpload, release info, allow failure, debug mode val paths = OutputPaths(project, variant.name) - val generateMapIdTask = generateMapIdTask(project, variant, paths, failureTracker, releaseMode) + val generateMapIdTask = generateMapIdTask(project, variant, paths, failureTracker) tasksGeneratingProperties.add(generateMapIdTask) variant.apply { @@ -158,7 +158,6 @@ internal abstract class PostHogAndroidGradlePlugin variant: ApplicationVariant, paths: OutputPaths, failureTracker: Provider, - releaseMode: PostHogReleaseMode, ): TaskProvider { val generateMapIdTask = PostHogGenerateMapIdTask.register( @@ -174,7 +173,6 @@ internal abstract class PostHogAndroidGradlePlugin generateMapIdTask = generateMapIdTask, variant = variant, mappingFiles = variant.mappingFileProvider(project), - releaseMode = releaseMode, ) generateMapIdTask.hookWithMinifyTasks(project, variant.name, generateMapIdTask) @@ -189,7 +187,6 @@ internal abstract class PostHogAndroidGradlePlugin generateMapIdTask: Provider, variant: ApplicationVariant, mappingFiles: Provider, - releaseMode: PostHogReleaseMode, ): TaskProvider { val primaryOutput = variant.outputs.firstOrNull() val uploadMapIdTask = @@ -201,7 +198,6 @@ internal abstract class PostHogAndroidGradlePlugin releaseName = variant.applicationId, releaseVersion = primaryOutput?.versionName?.map { it.orEmpty() }, build = primaryOutput?.versionCode?.map { it ?: 0 }, - releaseMode = releaseMode, ) return uploadMapIdTask } diff --git a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogUploadProguardMappingsTask.kt b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogUploadProguardMappingsTask.kt index 4fd14f3c7..e0cfd8ca2 100644 --- a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogUploadProguardMappingsTask.kt +++ b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/PostHogUploadProguardMappingsTask.kt @@ -53,7 +53,7 @@ public abstract class PostHogUploadProguardMappingsTask : PostHogCliExecTask() { @get:Optional public abstract val build: Property - /** [PostHogReleaseMode.cliValue], as the enum itself is internal to the plugin. */ + @Deprecated("The value is ignored. The mapping always binds to the release the build creates.") @get:Input @get:Optional public abstract val releaseMode: Property @@ -62,12 +62,37 @@ public abstract class PostHogUploadProguardMappingsTask : PostHogCliExecTask() { if (!mappingsFiles.isPresent || mappingsFiles.get().isEmpty) { error("[PostHog] Mapping files are missing!") } - // posthog-cli reads POSTHOG_RELEASE_MODE itself when --release-mode is absent, and the - // Gradle daemon's environment may carry one that contradicts the mode resolved here — - // where the gradle property is supposed to win. Pin the resolved mode in the child's - // environment so it cannot be overridden by inheritance. A posthog-cli predating the flag - // has no such argument and ignores the variable, so this stays safe for old CLIs. - releaseMode.orNull?.let { environment(POSTHOG_RELEASE_MODE_ENV, it) } + + // Warns at execution time on purpose: a configuration-time warning goes silent on a reused + // configuration cache entry, and the plugin wires the posthog.releaseMode gradle property + // into this input so it lands here on every executed upload. + @Suppress("DEPRECATION") + val deprecatedReleaseMode = releaseMode + if (deprecatedReleaseMode.isPresent) { + logger.warn( + "[PostHog] releaseMode is deprecated and ignored. The mapping uploads bound to " + + "the release this build creates. Remove posthog.releaseMode, or the " + + "releaseMode assignment on this task.", + ) + } + + // POSTHOG_RELEASE_MODE=event used to select event mode here too, so a build that still + // carries it gets told that the mapping binds again. The variable keeps steering the + // PostHog sourcemap and hermes uploads, so the message does not ask to remove it. + val inheritedReleaseMode = System.getenv("POSTHOG_RELEASE_MODE")?.trim() + if (!inheritedReleaseMode.isNullOrEmpty() && inheritedReleaseMode != "symbol-set") { + logger.warn( + "[PostHog] POSTHOG_RELEASE_MODE no longer affects the proguard mapping upload. " + + "The mapping uploads bound to the release this build creates.", + ) + } + + // The mapping always binds to the release this build creates. posthog-cli 0.13.0 up to + // the release that carries PostHog/posthog#92401 reads POSTHOG_RELEASE_MODE on `proguard + // upload`, and an Exec task inherits the daemon environment, so a value set for another + // tool would leave the mapping release-independent on those versions. Pin it. Older and + // newer CLIs ignore the variable, so the pin can go once that range has aged out. + environment("POSTHOG_RELEASE_MODE", "symbol-set") super.exec() } @@ -105,12 +130,6 @@ public abstract class PostHogUploadProguardMappingsTask : PostHogCliExecTask() { args.add("--build") args.add(it.toString()) } - // Passed only outside the default mode, so a symbol-set build keeps working against a - // posthog-cli predating the flag. - releaseMode.orNull?.takeIf { it != PostHogReleaseMode.SYMBOL_SET.cliValue }?.let { - args.add("--release-mode") - args.add(it) - } } internal companion object { @@ -122,7 +141,6 @@ public abstract class PostHogUploadProguardMappingsTask : PostHogCliExecTask() { releaseName: Provider? = null, releaseVersion: Provider? = null, build: Provider? = null, - releaseMode: PostHogReleaseMode = PostHogReleaseMode.SYMBOL_SET, ): TaskProvider { val uploadPostHogProguardMappingsTask = project.tasks.register( @@ -136,7 +154,10 @@ public abstract class PostHogUploadProguardMappingsTask : PostHogCliExecTask() { releaseName?.let { this.releaseName.set(it) } releaseVersion?.let { this.releaseVersion.set(it) } build?.let { this.build.set(it) } - this.releaseMode.set(releaseMode.cliValue) + // Feeds the deprecated property into the execution-time warning above. + // gradleProperty is the configuration-cache-safe read. + @Suppress("DEPRECATION") + this.releaseMode.set(project.providers.gradleProperty(POSTHOG_RELEASE_MODE_PROPERTY)) resolvePostHogDotenvFile(project)?.let { this.postHogDotenvFile.set(it) } } return uploadPostHogProguardMappingsTask diff --git a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/Utils.kt b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/Utils.kt index a322d8233..748aa24e6 100644 --- a/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/Utils.kt +++ b/posthog-android-gradle-plugin/src/main/kotlin/com/posthog/android/Utils.kt @@ -98,6 +98,25 @@ internal fun DirectoryProperty.getAndDelete(): File { internal const val POSTHOG_CLI_DEFAULT_EXECUTABLE = "posthog-cli" +internal const val POSTHOG_RELEASE_MODE_PROPERTY = "posthog.releaseMode" + +/** + * The property no longer does anything: the mapping always binds to the release the build creates. + * The POSTHOG_RELEASE_MODE environment variable stays silent here on purpose, because it still + * steers the sourcemap and hermes uploads of other PostHog tools, so a warning about it would not + * be actionable. + */ +internal fun warnIfDeprecatedReleaseModeSet(project: Project) { + val value = project.findProperty(POSTHOG_RELEASE_MODE_PROPERTY)?.toString()?.trim() + if (value.isNullOrEmpty()) { + return + } + project.logger.warn( + "[PostHog] $POSTHOG_RELEASE_MODE_PROPERTY is deprecated and ignored. The proguard " + + "mapping uploads bound to the release this build creates. Remove the property.", + ) +} + internal const val POSTHOG_DOTENV_FILE_PROPERTY = "posthog.dotenvFile" /** @@ -116,56 +135,6 @@ internal fun resolvePostHogDotenvFile(project: Project): String? { return if (file.isAbsolute) file.path else File(project.rootDir, value).path } -internal const val POSTHOG_RELEASE_MODE_PROPERTY = "posthog.releaseMode" - -internal const val POSTHOG_RELEASE_MODE_ENV = "POSTHOG_RELEASE_MODE" - -/** How the release a build belongs to gets associated with the exceptions it reports. */ -internal enum class PostHogReleaseMode(val cliValue: String) { - /** - * posthog-cli stamps the release onto the uploaded mapping file, and exceptions inherit it - * from the mapping their frames resolved against. The behavior before event mode existed. - */ - SYMBOL_SET("symbol-set"), - - /** - * The mapping is uploaded release-independent, and each event resolves its own release from - * the `$app_namespace` / `$app_version` / `$app_build` the SDK already sends. Nothing is - * injected into the app. A map id is derived from the mapping's content, so two releases - * sharing a mapping would otherwise both report whichever release uploaded it first. - */ - EVENT("event"), - ; - - internal companion object { - fun from(value: String): PostHogReleaseMode? = values().firstOrNull { it.cliValue == value } - } -} - -/** - * Release mode for this build: the `posthog.releaseMode` gradle property, then the - * `POSTHOG_RELEASE_MODE` environment variable posthog-cli and the bundler plugins already read, - * then [PostHogReleaseMode.SYMBOL_SET]. - * - * An unrecognized value fails the build rather than falling back, so a typo can't silently leave - * a build binding its mapping to a release it meant to keep independent. - */ -internal fun resolvePostHogReleaseMode( - project: Project, - environment: Map = System.getenv(), -): PostHogReleaseMode { - val value = - project.findProperty(POSTHOG_RELEASE_MODE_PROPERTY)?.toString()?.trim()?.takeIf { it.isNotEmpty() } - ?: environment[POSTHOG_RELEASE_MODE_ENV]?.trim()?.takeIf { it.isNotEmpty() } - ?: return PostHogReleaseMode.SYMBOL_SET - - return PostHogReleaseMode.from(value) - ?: error( - "$POSTHOG_RELEASE_MODE_PROPERTY must be one of " + - "${PostHogReleaseMode.values().joinToString { it.cliValue }}, was '$value'", - ) -} - /** * Locates posthog-cli for builds whose environment lacks the shell PATH — * IDE-launched Gradle daemons don't source shell profiles, so a CLI installed