Merge pull request #536 from cashapp/jrod/2022-08-06/native-platform-…

…extract-optimization

Configure native platform transformed path directly in test task

paparazzi/paparazzi-gradle-plugin/src/main/java/app/cash/paparazzi/gradle/PaparazziPlugin.kt

@@ -26,13 +26,13 @@ import org.gradle.api.DefaultTask
 import org.gradle.api.Plugin
 import org.gradle.api.Project
 import org.gradle.api.Task
-import org.gradle.api.artifacts.Configuration
 import org.gradle.api.artifacts.type.ArtifactTypeDefinition
 import org.gradle.api.attributes.Attribute
-import org.gradle.api.internal.artifacts.ArtifactAttributes
+import org.gradle.api.file.FileCollection
 import org.gradle.api.internal.artifacts.transform.UnzipTransform
 import org.gradle.api.logging.LogLevel.LIFECYCLE
 import org.gradle.api.plugins.JavaBasePlugin
+import org.gradle.api.tasks.PathSensitivity
 import org.gradle.api.tasks.TaskProvider
 import org.gradle.api.tasks.options.Option
 import org.gradle.api.tasks.testing.Test
@@ -63,7 +63,8 @@ class PaparazziPlugin : Plugin<Project> {
   }
 
   private fun setupPaparazzi(project: Project) {
-    val unzipConfiguration = project.setupPlatformDataTransform()
+    project.addTestDependency()
+    val nativePlatformFileCollection = project.setupNativePlatformDependency()
 
     // Create anchor tasks for all variants.
     val verifyVariants = project.tasks.register("verifyPaparazzi")
@@ -81,13 +82,11 @@ class PaparazziPlugin : Plugin<Project> {
       val reportOutputDir = project.layout.buildDirectory.dir("reports/paparazzi")
       val snapshotOutputDir = project.layout.projectDirectory.dir("src/test/snapshots")
 
-      val packageAwareArtifacts = project.configurations.getByName("${variant.name}RuntimeClasspath")
+      val packageAwareArtifacts = project.configurations
+        .getByName("${variant.name}RuntimeClasspath")
         .incoming
         .artifactView {
-          it.attributes.attribute(
-            Attribute.of("artifactType", String::class.java),
-            "android-symbol-with-package-name"
-          )
+          it.attributes.attribute(ARTIFACT_TYPE_ATTRIBUTE, "android-symbol-with-package-name")
         }
         .artifacts
 
@@ -106,7 +105,6 @@ class PaparazziPlugin : Plugin<Project> {
         task.targetSdkVersion.set(android.targetSdkVersion())
         task.compileSdkVersion.set(android.compileSdkVersion())
         task.mergeAssetsOutput.set(mergeAssetsOutputDir)
-        task.platformDataRoot.set(unzipConfiguration.singleFile)
         task.paparazziResources.set(project.layout.buildDirectory.file("intermediates/paparazzi/${variant.name}/resources.txt"))
       }
 
@@ -155,6 +153,10 @@ class PaparazziPlugin : Plugin<Project> {
 
         test.inputs.dir(mergeResourcesOutputDir)
         test.inputs.dir(mergeAssetsOutputDir)
+        test.inputs.files(nativePlatformFileCollection)
+          .withPropertyName("paparazzi.nativePlatform")
+          .withPathSensitivity(PathSensitivity.NONE)
+
         test.outputs.dir(reportOutputDir)
         test.outputs.dir(snapshotOutputDir)
 
@@ -164,6 +166,8 @@ class PaparazziPlugin : Plugin<Project> {
         // why not a lambda?  See: https://docs.gradle.org/7.2/userguide/validation_problems.html#implementation_unknown
         test.doFirst(object : Action<Task> {
           override fun execute(t: Task) {
+            test.systemProperties["paparazzi.platform.data.root"] =
+              nativePlatformFileCollection.singleFile.absolutePath
             test.systemProperties["paparazzi.test.record"] = isRecordRun.get()
             test.systemProperties["paparazzi.test.verify"] = isVerifyRun.get()
             test.systemProperties.putAll(paparazziProperties)
@@ -197,17 +201,9 @@ class PaparazziPlugin : Plugin<Project> {
     }
   }
 
-  private fun Project.setupPlatformDataTransform(): Configuration {
-    configurations.getByName("testImplementation").dependencies.add(
-      dependencies.create("app.cash.paparazzi:paparazzi:$VERSION")
-    )
-
-    val unzipConfiguration = configurations.create("unzip")
-    unzipConfiguration.attributes.attribute(
-      ArtifactAttributes.ARTIFACT_FORMAT,
-      ArtifactTypeDefinition.DIRECTORY_TYPE
-    )
-    configurations.add(unzipConfiguration)
+  private fun Project.setupNativePlatformDependency(): FileCollection {
+    val nativePlatformConfiguration = configurations.create("nativePlatform")
+    configurations.add(nativePlatformConfiguration)
 
     val operatingSystem = OperatingSystem.current()
     val nativeLibraryArtifactId = when {
@@ -218,18 +214,26 @@ class PaparazziPlugin : Plugin<Project> {
       operatingSystem.isWindows -> "win"
       else -> "linux"
     }
-    unzipConfiguration.dependencies.add(
+    nativePlatformConfiguration.dependencies.add(
       dependencies.create("app.cash.paparazzi:layoutlib-native-$nativeLibraryArtifactId:$NATIVE_LIB_VERSION")
     )
     dependencies.registerTransform(UnzipTransform::class.java) { transform ->
-      transform.from.attribute(ArtifactAttributes.ARTIFACT_FORMAT, ArtifactTypeDefinition.JAR_TYPE)
-      transform.to.attribute(
-        ArtifactAttributes.ARTIFACT_FORMAT,
-        ArtifactTypeDefinition.DIRECTORY_TYPE
-      )
+      transform.from.attribute(ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.JAR_TYPE)
+      transform.to.attribute(ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE)
     }
 
-    return unzipConfiguration
+    return nativePlatformConfiguration
+      .incoming
+      .artifactView {
+        it.attributes.attribute(ARTIFACT_TYPE_ATTRIBUTE, ArtifactTypeDefinition.DIRECTORY_TYPE)
+      }
+      .files
+  }
+
+  private fun Project.addTestDependency() {
+    configurations.getByName("testImplementation").dependencies.add(
+      dependencies.create("app.cash.paparazzi:paparazzi:$VERSION")
+    )
   }
 
   private fun BaseExtension.packageName(): String {
@@ -256,4 +260,7 @@ class PaparazziPlugin : Plugin<Project> {
   }
 }
 
+// TODO: Migrate to ArtifactTypeDefinition.ARTIFACT_TYPE_ATTRIBUTE when Gradle 7.3 is
+//  acceptable as the minimum supported version
+private val ARTIFACT_TYPE_ATTRIBUTE = Attribute.of("artifactType", String::class.java)
 private const val DEFAULT_COMPILE_SDK_VERSION = 31

paparazzi/paparazzi-gradle-plugin/src/main/java/app/cash/paparazzi/gradle/PrepareResourcesTask.kt

@@ -49,10 +49,6 @@ abstract class PrepareResourcesTask : DefaultTask() {
   @get:PathSensitive(PathSensitivity.RELATIVE)
   abstract val mergeAssetsOutput: DirectoryProperty
 
-  @get:InputDirectory
-  @get:PathSensitive(PathSensitivity.RELATIVE)
-  abstract val platformDataRoot: DirectoryProperty
-
   @get:Input
   abstract val nonTransitiveRClassEnabled: Property<Boolean>
 
@@ -97,8 +93,6 @@ abstract class PrepareResourcesTask : DefaultTask() {
         it.newLine()
         it.write(projectDirectory.relativize(mergeAssetsOutput.get()))
         it.newLine()
-        it.write(platformDataRoot.get().asFile.invariantSeparatorsPath)
-        it.newLine()
         it.write(resourcePackageNames)
         it.newLine()
       }

paparazzi/paparazzi-gradle-plugin/src/test/java/app/cash/paparazzi/gradle/PaparazziPluginTest.kt

@@ -570,7 +570,7 @@ class PaparazziPluginTest {
     assertThat(resourceFileContents[0]).isEqualTo("app.cash.paparazzi.plugin.test")
     assertThat(resourceFileContents[1]).isEqualTo("build/intermediates/merged_res/debug")
     assertThat(resourceFileContents[4]).isEqualTo("build/intermediates/assets/debug/mergeDebugAssets")
-    assertThat(resourceFileContents[6]).isEqualTo("app.cash.paparazzi.plugin.test")
+    assertThat(resourceFileContents[5]).isEqualTo("app.cash.paparazzi.plugin.test")
   }
 
   @Test
@@ -590,7 +590,7 @@ class PaparazziPluginTest {
     assertThat(resourceFileContents[0]).isEqualTo("app.cash.paparazzi.plugin.test")
     assertThat(resourceFileContents[1]).isEqualTo("build/intermediates/merged_res/debug")
     assertThat(resourceFileContents[4]).isEqualTo("build/intermediates/assets/debug/mergeDebugAssets")
-    assertThat(resourceFileContents[6]).isEqualTo("app.cash.paparazzi.plugin.test")
+    assertThat(resourceFileContents[5]).isEqualTo("app.cash.paparazzi.plugin.test")
   }
 
   @Test

paparazzi/paparazzi/build.gradle

@@ -98,7 +98,6 @@ def generateTestConfig = tasks.register("generateTestConfig") {
       writer.writeLine("31")
       writer.writeLine("platforms/android-31/")
       writer.writeLine(".")
-      writer.writeLine(configurations.unzip.singleFile.path)
       writer.writeLine("app.cash.paparazzi")
     }
   }
@@ -107,8 +106,12 @@ def generateTestConfig = tasks.register("generateTestConfig") {
 tasks.withType(Test).configureEach {
   dependsOn(generateTestConfig)
   systemProperty(
-      "paparazzi.test.resources",
-      generateTestConfig.map { it.outputs.files.singleFile }.get().path
+    "paparazzi.test.resources",
+    generateTestConfig.map { it.outputs.files.singleFile }.get().path
+  )
+  systemProperty(
+    "paparazzi.platform.data.root",
+    configurations.unzip.singleFile.absolutePath
   )
   // Uncomment to debug JNI issues in layoutlib
   // jvmArgs '-Xcheck:jni'

paparazzi/paparazzi/src/main/java/app/cash/paparazzi/Environment.kt

@@ -29,7 +29,6 @@ data class Environment(
   val assetsDir: String,
   val packageName: String,
   val compileSdkVersion: Int,
-  val platformDataDir: String,
   val resourcePackageNames: List<String>
 ) {
   init {
@@ -63,8 +62,7 @@ fun detectEnvironment(): Environment {
     assetsDir = appTestDir.resolve(configLines[4]).toString(),
     packageName = configLines[0],
     compileSdkVersion = configLines[2].toInt(),
-    platformDataDir = configLines[5],
-    resourcePackageNames = configLines[6].split(",")
+    resourcePackageNames = configLines[5].split(",")
   )
 }
 

paparazzi/paparazzi/src/main/java/app/cash/paparazzi/internal/Renderer.kt

@@ -69,7 +69,9 @@ internal class Renderer(
       .plusFlag(RenderParamsFlags.FLAG_DO_NOT_RENDER_ON_CREATE, true)
       .withTheme("AppTheme", true)
 
-    val platformDataDir = File("${environment.platformDataDir}/data")
+    val platformDataRoot = System.getProperty("paparazzi.platform.data.root")
+      ?: throw RuntimeException("Missing system property for 'paparazzi.platform.data.root'")
+    val platformDataDir = File(platformDataRoot, "data")
     val fontLocation = File(platformDataDir, "fonts")
     val nativeLibLocation = File(platformDataDir, getNativeLibDir())
     val icuLocation = File(platformDataDir, "icu" + File.separator + "icudt68l.dat")