diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b9d86253..ea0f87ed 100644 --- 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 index 17ce9abb..3976247c 100644 --- 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 index e83c310c..937fc097 100644 --- 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 index c1962a79..033e24c4 100644 Binary files a/gradle/wrapper/gradle-wrapper.jar and b/gradle/wrapper/gradle-wrapper.jar differ diff --git a/gradle/wrapper/gradle-wrapper.properties b/gradle/wrapper/gradle-wrapper.properties index 0c85a1f7..9f4197d5 100644 --- 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 index aeb74cbb..fcb6fca1 100755 --- 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 index 031fb1e2..933617db 100644 --- 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().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 index c1962a79..033e24c4 100644 Binary files a/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.jar and b/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.jar differ diff --git a/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.properties b/keeper-gradle-plugin/gradle/wrapper/gradle-wrapper.properties index 0c85a1f7..9f4197d5 100644 --- 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 index aeb74cbb..fcb6fca1 100755 --- 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 index 7cfddb11..e224e494 100644 --- 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.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 { * 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 { 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(interpolateL8TaskName(appVariant.name)) { l8Task -> - // First merge the L8 rules into the app's L8 task - project.namedLazy(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(interpolateL8TaskName(testVariant.name)) { testL8Task + -> + project.namedLazy(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 { .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 index d2969573..0703ad55 100644 --- 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().configureEach { doLast { println("Checking expected rules") @@ -136,27 +135,19 @@ if (isCi) { } } - if (verifyL8) { - tasks - .withType() - .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