Update to AGP 8.1 (#123)

* Gradle 8.1.1 * Update dokka * Update CI * Update deps and AGP 8.1 * Remove L8 patching * Add back a new implementation of L8 patching * Fix configuration cache bug * Update more deps * Update CI * Fix
slackhq · Jul 25, 2023 · f6d9be3 · f6d9be3
1 parent f39acfc
commit f6d9be3
Show file tree

Hide file tree

Showing 12 changed files with 101 additions and 91 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -25,8 +25,7 @@ jobs:
     strategy:
       fail-fast: false # We want to see all results
       matrix:
-        # TODO add back '8.1.0-alpha04' once API changes around L8 keep rules are sorted out
-        agp: ['8.0.0']
+        agp: ['8.1.0', '8.2.0-alpha13']
         job: ['instrumentation', 'plugin']
     env:
       DEP_OVERRIDE_agp: ${{ matrix.agp }}
@@ -58,7 +57,7 @@ jobs:
         if: matrix.job == 'instrumentation'
         uses: gradle/gradle-build-action@v2
         with:
-          arguments: :sample:minifyExternalStagingWithR8 --stacktrace -Pkeeper.verifyL8=true
+          arguments: :sample:minifyExternalStagingWithR8 validateL8 --stacktrace
 
       # TODO AVD caching disabled due to https://github.com/ReactiveCircus/android-emulator-runner/issues/278
 #      - name: AVD cache

diff --git a/docs/index.md b/docs/index.md
@@ -91,11 +91,11 @@ classpath of the test APK may not have the right `j$` classes available on its c
 code it is invoking. To work around this, Keeper does two things:
 
 1. Keeper merges generated L8 rules from both the androidTest and target app to ensure they cover all
-used APIs. These merged rules are given to the target app `L8DexDesugarLibTask`.
+   used APIs. These merged rules are given to the target app `L8DexDesugarLibTask`.
 2. L8 will still, by default, generate a dex file of backported APIs into both the test app and target
-app, which can cause confusing runtime classpath issues due to L8 generating different implementations
-in each app. Keeper works around this by forcing the use of a single dex file in the target app and
-preventing the inclusion of a backport dex file in the test app.
+   app, which can cause confusing runtime classpath issues due to L8 generating different implementations
+   in each app. Keeper works around this by forcing the use of a single dex file in the target app and
+   preventing the inclusion of a backport dex file in the test app.
 
 This L8 support is automatically enabled if `android.compileOptions.coreLibraryDesugaringEnabled` is
 true in AGP.

diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml
@@ -1,16 +1,16 @@
 [versions]
-agp = "8.0.0"
+agp = "8.1.0"
 androidx-test = "1.5.2"
-kotlin = "1.8.20"
-gjf = "1.16.0"
-ktfmt = "0.43"
+kotlin = "1.8.22"
+gjf = "1.17.0"
+ktfmt = "0.44"
 
 [plugins]
 agp-library = { id = "com.android.library", version.ref = "agp" }
-binaryCompatibilityValidator = { id = "org.jetbrains.kotlinx.binary-compatibility-validator", version = "0.13.0" }
+binaryCompatibilityValidator = { id = "org.jetbrains.kotlinx.binary-compatibility-validator", version = "0.13.2" }
 kotlin-jvm = { id = "org.jetbrains.kotlin.jvm", version.ref = "kotlin" }
-mavenPublish = { id = "com.vanniktech.maven.publish", version = "0.25.1" }
-spotless = { id = "com.diffplug.spotless", version = "6.18.0" }
+mavenPublish = { id = "com.vanniktech.maven.publish", version = "0.25.3" }
+spotless = { id = "com.diffplug.spotless", version = "6.20.0" }
 
 [libraries]
 androidx-annotation = "androidx.annotation:annotation:1.6.0"
@@ -24,9 +24,9 @@ javapoet = "com.squareup:javapoet:1.13.0"
 junit = "junit:junit:4.13.2"
 kgp = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin", version.ref = "kotlin" }
 kgp-api = { module = "org.jetbrains.kotlin:kotlin-gradle-plugin-api", version.ref = "kotlin" }
-kotlinpoet = "com.squareup:kotlinpoet:1.13.0"
-okio = "com.squareup.okio:okio:3.3.0"
-truth = "com.google.truth:truth:1.1.3"
+kotlinpoet = "com.squareup:kotlinpoet:1.14.2"
+okio = "com.squareup.okio:okio:3.4.0"
+truth = "com.google.truth:truth:1.1.5"
 zipflinger = { module = "com.android:zipflinger", version.ref = "agp" }
 
 renovateTrigger-gjf = { module = "com.google.googlejavaformat:google-java-format", version.ref = "gjf" }

diff --git a/gradle/wrapper/gradle-wrapper.jar b/gradle/wrapper/gradle-wrapper.jar
diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties
@@ -1,6 +1,7 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.2.1-bin.zip
 networkTimeout=10000
+validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
diff --git a/gradlew b/gradlew
@@ -130,10 +130,13 @@ location of your Java installation."
     fi
 else
     JAVACMD=java
-    which java >/dev/null 2>&1 || die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
+    if ! command -v java >/dev/null 2>&1
+    then
+        die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
 
 Please set the JAVA_HOME variable in your environment to match the
 location of your Java installation."
+    fi
 fi
 
 # Increase the maximum file descriptors if we can.

diff --git a/keeper-gradle-plugin/build.gradle.kts b/keeper-gradle-plugin/build.gradle.kts
@@ -13,7 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import java.net.URL
+import java.net.URI
 import org.jetbrains.dokka.gradle.DokkaTask
 import org.jetbrains.kotlin.gradle.dsl.JvmTarget
 import org.jetbrains.kotlin.gradle.dsl.KotlinVersion
@@ -22,7 +22,7 @@ import org.jetbrains.kotlin.gradle.tasks.KotlinCompile
 plugins {
   kotlin("jvm") version libs.versions.kotlin.get()
   `java-gradle-plugin`
-  id("org.jetbrains.dokka") version "1.8.10"
+  id("org.jetbrains.dokka") version "1.8.20"
   alias(libs.plugins.mavenPublish)
   alias(libs.plugins.binaryCompatibilityValidator)
   id("org.jetbrains.kotlin.plugin.sam.with.receiver") version libs.versions.kotlin.get()
@@ -86,13 +86,16 @@ tasks.withType<DokkaTask>().configureEach {
     skipDeprecated.set(true)
     suppressInheritedMembers.set(true)
     externalDocumentationLink {
-      url.set(URL("https://docs.gradle.org/${gradle.gradleVersion}/javadoc/allpackages-index.html"))
+      url.set(
+        URI("https://docs.gradle.org/${gradle.gradleVersion}/javadoc/allpackages-index.html")
+          .toURL()
+      )
     }
     externalDocumentationLink {
       packageListUrl.set(
-        URL("https://developer.android.com/reference/tools/gradle-api/7.3/package-list")
+        URI("https://developer.android.com/reference/tools/gradle-api/7.3/package-list").toURL()
       )
-      url.set(URL("https://developer.android.com/reference/tools/gradle-api/7.3/classes"))
+      url.set(URI("https://developer.android.com/reference/tools/gradle-api/7.3/classes").toURL())
     }
   }
 }

diff --git a/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.jar b/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.jar
diff --git a/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.properties b/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.properties
@@ -1,6 +1,7 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.1-bin.zip
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.2.1-bin.zip
 networkTimeout=10000
+validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME
 zipStorePath=wrapper/dists
diff --git a/keeper-gradle-plugin/gradlew b/keeper-gradle-plugin/gradlew
@@ -130,10 +130,13 @@ location of your Java installation."
     fi
 else
     JAVACMD=java
-    which java >/dev/null 2>&1 || die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
+    if ! command -v java >/dev/null 2>&1
+    then
+        die "ERROR: JAVA_HOME is not set and no 'java' command could be found in your PATH.
 
 Please set the JAVA_HOME variable in your environment to match the
 location of your Java installation."
+    fi
 fi
 
 # Increase the maximum file descriptors if we can.

diff --git a/keeper-gradle-plugin/src/main/java/com/slack/keeper/KeeperPlugin.kt b/keeper-gradle-plugin/src/main/java/com/slack/keeper/KeeperPlugin.kt
@@ -116,7 +116,7 @@ public class KeeperPlugin : Plugin<Project> {
         project.extensions.getByType(ApplicationAndroidComponentsExtension::class.java)
       val extension = project.extensions.create("keeper", KeeperExtension::class.java)
       project.configureKeepRulesGeneration(appExtension, appComponentsExtension, extension)
-      project.configureL8(appExtension, appComponentsExtension, extension)
+      configureL8(project, appExtension, appComponentsExtension, extension)
     }
   }
 
@@ -145,7 +145,8 @@ public class KeeperPlugin : Plugin<Project> {
    * the source of truth. To force this, we simply clear all the generated output dex files from the
    * androidTest [L8DexDesugarLibTask] task.
    */
-  private fun Project.configureL8(
+  private fun configureL8(
+    project: Project,
     appExtension: AppExtension,
     appComponentsExtension: ApplicationAndroidComponentsExtension,
     extension: KeeperExtension
@@ -155,48 +156,55 @@ public class KeeperPlugin : Plugin<Project> {
       appVariant ->
       // TODO ideally move to components entirely https://issuetracker.google.com/issues/199411020
       if (appExtension.compileOptions.isCoreLibraryDesugaringEnabled) {
-        // namedLazy nesting here is unfortunate but necessary because these R8/L8 tasks don't
-        // exist yet during this callback. https://issuetracker.google.com/issues/199509581
-        project.namedLazy<L8DexDesugarLibTask>(interpolateL8TaskName(appVariant.name)) { l8Task ->
-          // First merge the L8 rules into the app's L8 task
-          project.namedLazy<R8Task>(interpolateR8TaskName(testVariant.name)) { provider ->
-            l8Task.configure { keepRulesFiles.from(provider.flatMap { it.projectOutputKeepRules }) }
-          }
 
-          l8Task.configure {
-            val taskName = name
-            keepRulesConfigurations.set(listOf("-dontobfuscate"))
-            val diagnosticOutputDir =
-              layout.buildDirectory.dir("$INTERMEDIATES_DIR/l8-diagnostics/$taskName").get().asFile
-
-            // We can't actually declare this because AGP's NonIncrementalTask will clear it
-            // during the task action
-            //                  outputs.dir(diagnosticOutputDir)
-            //                      .withPropertyName("diagnosticsDir")
-
-            if (extension.emitDebugInformation.getOrElse(false)) {
-              doFirst {
-                val mergedFilesContent =
-                  keepRulesFiles.files
-                    .asSequence()
-                    .flatMap { it.walkTopDown() }
-                    .filterNot { it.isDirectory }
-                    .joinToString("\n") { "# Source: ${it.absolutePath}\n${it.readText()}" }
-
-                val configurations =
-                  keepRulesConfigurations.orNull
-                    .orEmpty()
-                    .joinToString("\n", prefix = "# Source: extra configurations\n")
-
-                File(diagnosticOutputDir, "patchedL8Rules.pro")
-                  .apply {
-                    if (exists()) {
-                      delete()
+        // To support this, we need to:
+        // - Get the androidTest L8DexDesugarLibTask
+        // - Pipe its output keep rules into an intermediate mapped provider
+        // - Pipe those rules into app L8DexDesugarLibTask's keepRulesConfigurations
+
+        // namedLazy nesting here is unfortunate but necessary because these L8 tasks don't
+        // exist yet during this callback. https://issuetracker.google.com/issues/199509581
+        project.namedLazy<L8DexDesugarLibTask>(interpolateL8TaskName(testVariant.name)) { testL8Task
+          ->
+          project.namedLazy<L8DexDesugarLibTask>(interpolateL8TaskName(appVariant.name)) { appL8Task
+            ->
+            appL8Task.configure {
+              keepRulesConfigurations.addAll(
+                testL8Task.flatMap { it.keepRules }.map { it.asFile.readLines() }
+              )
+
+              // Diagnostics
+              if (extension.emitDebugInformation.getOrElse(false)) {
+                val taskName = name
+                val diagnosticOutputDir =
+                  project.layout.buildDirectory
+                    .dir("$INTERMEDIATES_DIR/l8-diagnostics/$taskName")
+                    .get()
+                    .asFile
+                doLast {
+                  // We can't actually declare this because AGP's NonIncrementalTask will clear it
+                  // during the task action
+                  // outputs.dir(diagnosticOutputDir)
+                  //     .withPropertyName("diagnosticsDir")
+                  val outputFile = keepRules.asFile.get()
+                  val mergedFilesContent =
+                    "# Source: ${outputFile.absolutePath}\n${outputFile.readText()}"
+
+                  val configurations =
+                    keepRulesConfigurations.orNull
+                      .orEmpty()
+                      .joinToString("\n", prefix = "# Source: extra configurations\n")
+
+                  File(diagnosticOutputDir, "patchedL8Rules.pro")
+                    .apply {
+                      if (exists()) {
+                        delete()
+                      }
+                      parentFile.mkdirs()
+                      createNewFile()
                     }
-                    parentFile.mkdirs()
-                    createNewFile()
-                  }
-                  .writeText("$mergedFilesContent\n$configurations")
+                    .writeText("$mergedFilesContent\n$configurations")
+                }
               }
             }
           }
@@ -362,7 +370,8 @@ public class KeeperPlugin : Plugin<Project> {
       .configureEach {
         logger.debug("$TAG: Patching task '$name' with inferred androidTest proguard rules")
         configurationFiles.from(prop)
-        // We offer an option to disable this because the FILTERED_PROGUARD_RULES doesn't propagate
+        // We offer an option to disable this because the FILTERED_PROGUARD_RULES doesn't
+        // propagate
         // task dependencies and breaks in Gradle 8.
         if (!disableTestProguardFiles) {
           configurationFiles.from(testProguardFiles)

diff --git a/sample/build.gradle.kts b/sample/build.gradle.kts
@@ -13,7 +13,6 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import com.android.build.gradle.internal.tasks.L8DexDesugarLibTask
 import com.android.build.gradle.internal.tasks.R8Task
 import com.google.common.truth.Truth.assertThat
 import com.slack.keeper.InferAndroidTestKeepRules
@@ -119,9 +118,9 @@ keeper {
 // To speed up testing, we can also eagerly check that the generated rules match what we expect.
 // This is _solely_ for CI use with Keeper!
 val isCi = providers.environmentVariable("CI").orElse("false").get() == "true"
-val verifyL8 = providers.gradleProperty("keeper.verifyL8").orElse("false").get() == "true"
 
 if (isCi) {
+  // TODO create a dependent lifecycle task
   tasks.withType<InferAndroidTestKeepRules>().configureEach {
     doLast {
       println("Checking expected rules")
@@ -136,27 +135,19 @@ if (isCi) {
     }
   }
 
-  if (verifyL8) {
-    tasks
-      .withType<L8DexDesugarLibTask>()
-      .matching { it.name == "l8DexDesugarLibExternalStagingAndroidTest" }
-      .configureEach {
-        doLast {
-          println("Checking expected input rules from diagnostics output")
-          val diagnosticFilePath =
-            "build/intermediates/keeper/l8-diagnostics/l8DexDesugarLibExternalStagingAndroidTest/patchedL8Rules.pro"
-          val diagnostics = file(diagnosticFilePath).readText()
-          if ("-dontobfuscate" !in diagnostics) {
-            throw IllegalStateException(
-              "L8 diagnostic rules don't have Keeper's configured '-dontobfuscate', see $diagnosticFilePath"
-            )
-          } else if ("-keep class j\$.time.Instant" !in diagnostics) {
-            throw IllegalStateException(
-              "L8 diagnostic rules include the main variant's R8-generated rules, see $diagnosticFilePath"
-            )
-          }
-        }
+  tasks.register("validateL8") {
+    dependsOn("l8DexDesugarLibExternalStaging")
+    doFirst {
+      println("Checking expected input rules from diagnostics output")
+      val diagnosticFilePath =
+        "build/intermediates/keeper/l8-diagnostics/l8DexDesugarLibExternalStaging/patchedL8Rules.pro"
+      val diagnostics = file(diagnosticFilePath).readText()
+      if ("-keep class j\$.time.Instant" !in diagnostics) {
+        throw IllegalStateException(
+          "L8 diagnostic rules include the main variant's R8-generated rules, see $diagnosticFilePath"
+        )
       }
+    }
   }
 
   tasks