Always add the task provider in variant api operations output paths

Not having the task provider in the output path of the `toAppend`
operation meant that all of the directory will be an input to the next
operation. Meanwhile, `toTransform` operation will also output to the
same directory that is used as an input, which creates a case where the
output of a task is an input to the same task on the next run, and also
incorrect clean runs since some files will be processed as an input
after they were just been created.

This caused an issue with plugins that use the transform APIs (like
Hilt) and plugins that use the toAppend variant API.

Fixes: 285170632
Test: AsmTransformApiInstrumentationTest.testInteractionWithVariantApi
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:4b894b3d7a78b06f08b860aeeb223e058adeac06)
Merged-In: Iee3a81b66d2057f2d3c821e3e6a02e252a5b9600
Change-Id: Iee3a81b66d2057f2d3c821e3e6a02e252a5b9600

3 files changed, 105 insertions, 91 deletions

diff --git a/build-system/gradle-core/src/main/java/com/android/build/api/artifact/impl/ScopedArtifactsImpl.kt b/build-system/gradle-core/src/main/java/com/android/build/api/artifact/impl/ScopedArtifactsImpl.kt
index 95af44948d..7594211a08 100644
--- a/build-system/gradle-core/src/main/java/com/android/build/api/artifact/impl/ScopedArtifactsImpl.kt
+++ b/build-system/gradle-core/src/main/java/com/android/build/api/artifact/impl/ScopedArtifactsImpl.kt
@@ -34,15 +34,14 @@ import org.gradle.api.file.RegularFile
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.ListProperty
 import org.gradle.api.provider.Property
-import org.gradle.api.provider.Provider
 import org.gradle.api.tasks.TaskProvider
 import java.util.concurrent.atomic.AtomicBoolean
 
 class ScopedArtifactsImpl(
-    val scopeName: String,
-    val variantIdentifier: String,
-    val projectLayout: ProjectLayout,
-    val fileCollectionCreator: () -> ConfigurableFileCollection,
+    private val scopeName: String,
+    private val variantIdentifier: String,
+    private val projectLayout: ProjectLayout,
+    private val fileCollectionCreator: () -> ConfigurableFileCollection,
 ): ScopedArtifacts {
 
     /**
@@ -106,7 +105,7 @@ class ScopedArtifactsImpl(
 
         val artifactsAltered = AtomicBoolean(false)
 
-        val listOfProviders = mutableListOf(initialScopedContent)
+        private val listOfProviders = mutableListOf(initialScopedContent)
         /**
          * Reset the current provider of the artifact the new file collection and make sure the
          * final version points to the new content.
@@ -169,10 +168,10 @@ class ScopedArtifactsImpl(
         getScopedArtifactsContainer(type).finalScopedContent
 
     class ScopedArtifactsOperationImpl<T: Task>(
-        val scopedArtifacts: ScopedArtifactsImpl,
-        val taskProvider: TaskProvider<T>,
-        val projectLayout: ProjectLayout,
-        val fileCollectionCreator: () -> ConfigurableFileCollection,
+        private val scopedArtifacts: ScopedArtifactsImpl,
+        private val taskProvider: TaskProvider<T>,
+        private val projectLayout: ProjectLayout,
+        private val fileCollectionCreator: () -> ConfigurableFileCollection,
     ): ScopedArtifactsOperation<T> {
 
         override fun toAppend(to: ScopedArtifact, with: (T) -> Property<out FileSystemLocation>) {
@@ -182,8 +181,8 @@ class ScopedArtifactsImpl(
             // and sets the output path.
             taskProvider.configure {
                 when (val provider = with(it)) {
-                    is RegularFileProperty -> setContentPath(to, provider)
-                    is DirectoryProperty -> setContentPath(to, provider)
+                    is RegularFileProperty -> setContentPath(to, provider, taskProvider.name)
+                    is DirectoryProperty -> setContentPath(to, provider, taskProvider.name)
                     else -> throw RuntimeException("Only RegularFileProperty or DirectoryProperty" +
                             " instances are supported, got ${provider.javaClass}")
                 }
@@ -219,7 +218,7 @@ class ScopedArtifactsImpl(
                 inputDirectories(task).set(
                     currentScopedContent.getDirectories(projectLayout.projectDirectory)
                 )
-                setContentPath(type, into(task))
+                setContentPath(type, into(task), taskProvider.name)
             }
             resetContentProvider(type, into)
         }
@@ -296,21 +295,21 @@ class ScopedArtifactsImpl(
 
         override fun toReplace(type: ScopedArtifact, into: (T) -> RegularFileProperty) {
             taskProvider.configure { task ->
-                setContentPath(type, into(task))
+                setContentPath(type, into(task), taskProvider.name)
             }
             resetContentProvider(type, into)
-
         }
 
         private fun setContentPath(
             type: ScopedArtifact,
             into: RegularFileProperty,
+            vararg paths: String,
         ) {
             into.set(
                 type.getIntermediateOutputPath(
                     buildDirectory = projectLayout.buildDirectory,
                     variantIdentifier = scopedArtifacts.variantIdentifier,
-                    paths = arrayOf(scopedArtifacts.scopeName) ,
+                    paths = arrayOf(scopedArtifacts.scopeName, *paths) ,
                     forceFilename = type.name().lowercase().plus(".jar")
                 )
             )
diff --git a/build-system/gradle-core/src/main/java/com/android/build/gradle/tasks/TransformClassesWithAsmTask.kt b/build-system/gradle-core/src/main/java/com/android/build/gradle/tasks/TransformClassesWithAsmTask.kt
index 4f42a8c8b2..00e78db697 100644
--- a/build-system/gradle-core/src/main/java/com/android/build/gradle/tasks/TransformClassesWithAsmTask.kt
+++ b/build-system/gradle-core/src/main/java/com/android/build/gradle/tasks/TransformClassesWithAsmTask.kt
@@ -21,8 +21,6 @@ import com.android.SdkConstants.DOT_JAR
 import com.android.SdkConstants.DOT_JSON
 import com.android.build.api.instrumentation.AsmClassVisitorFactory
 import com.android.build.api.instrumentation.FramesComputationMode
-import com.android.build.api.artifact.ScopedArtifact
-import com.android.build.api.variant.ScopedArtifacts
 import com.android.build.gradle.internal.component.ApkCreationConfig
 import com.android.build.gradle.internal.component.ComponentCreationConfig
 import com.android.build.gradle.internal.instrumentation.AsmInstrumentationManager
@@ -31,9 +29,6 @@ import com.android.build.gradle.internal.instrumentation.loadClassData
 import com.android.build.gradle.internal.instrumentation.saveClassData
 import com.android.build.gradle.internal.profile.ProfileAwareWorkAction
 import com.android.build.gradle.internal.publishing.AndroidArtifacts
-import com.android.build.gradle.internal.scope.InternalArtifactType
-import com.android.build.gradle.internal.scope.getDirectories
-import com.android.build.gradle.internal.scope.getRegularFiles
 import com.android.build.gradle.internal.services.ClassesHierarchyBuildService
 import com.android.build.gradle.internal.services.getBuildService
 import com.android.build.gradle.internal.tasks.BuildAnalyzer
@@ -47,12 +42,10 @@ import com.android.build.gradle.internal.utils.setDisallowChanges
 import com.android.buildanalyzer.common.TaskCategory
 import com.android.builder.files.SerializableFileChanges
 import com.android.builder.utils.isValidZipEntryName
-import com.android.ide.common.resources.FileStatus
 import com.android.utils.FileUtils
 import com.google.common.io.ByteStreams
 import com.google.common.io.Files
 import org.gradle.api.file.ConfigurableFileCollection
-import org.gradle.api.file.Directory
 import org.gradle.api.file.DirectoryProperty
 import org.gradle.api.file.RegularFileProperty
 import org.gradle.api.provider.ListProperty
@@ -66,7 +59,6 @@ import org.gradle.api.tasks.Internal
 import org.gradle.api.tasks.Nested
 import org.gradle.api.tasks.Optional
 import org.gradle.api.tasks.OutputDirectory
-import org.gradle.api.tasks.TaskProvider
 import org.gradle.work.ChangeType
 import org.gradle.work.Incremental
 import org.gradle.work.InputChanges
@@ -100,12 +92,6 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
     @get:Classpath
     abstract val inputClassesDir: ConfigurableFileCollection
 
-    // This is used when jacoco instrumented jars are used as inputs
-    @get:Incremental
-    @get:Classpath
-    @get:Optional
-    abstract val inputJarsDir: DirectoryProperty
-
     @get:Nested
     abstract val inputJarsWithIdentity: JarsClasspathInputsWithIdentity
 
@@ -178,7 +164,6 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
         workerExecutor.noIsolation().submit(TransformClassesFullAction::class.java) {
             configureParams(it, inputChanges)
             it.inputClassesDir.from(inputClassesDir)
-            it.inputJarsDir.set(inputJarsDir)
         }
     }
 
@@ -226,9 +211,6 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
             it.inputClassesDirChanges.set(
                 inputChanges.getFileChanges(inputClassesDir).toSerializable()
             )
-            if (inputJarsDir.isPresent) {
-                it.inputJarsChanges.set(inputChanges.getFileChanges(inputJarsDir).toSerializable())
-            }
         }
     }
 
@@ -275,7 +257,7 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
          * Extract profiler dependency jars and add them to the project jars as they need to be
          * packaged with the rest of the classes.
          */
-        protected fun extractProfilerDependencyJars() {
+        private fun extractProfilerDependencyJars() {
             if (parameters.shouldPackageProfilerDependencies.getOrElse(false)) {
                 parameters.profilingTransforms.get().forEach { path ->
                     val profilingTransformFile = File(path)
@@ -311,24 +293,18 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
             }
         }
 
-        abstract fun maybeProcessJacocoInstrumentedJars(
-            instrumentationManager: AsmInstrumentationManager
-        ): Boolean
-
         fun processJars(instrumentationManager: AsmInstrumentationManager) {
-             if (!maybeProcessJacocoInstrumentedJars(instrumentationManager)) {
-                val mappingState = parameters.mappingState.get()
-                if (mappingState.reprocessAll) {
-                    FileUtils.deleteDirectoryContents(parameters.jarsOutputDir.get().asFile)
-                    extractProfilerDependencyJars()
-                }
-                mappingState.jarsInfo.forEach { (file, info) ->
-                    if (info.hasChanged) {
-                        val instrumentedJar =
-                            File(parameters.jarsOutputDir.get().asFile, info.identity + DOT_JAR)
-                        FileUtils.deleteIfExists(instrumentedJar)
-                        instrumentationManager.instrumentClassesFromJarToJar(file, instrumentedJar)
-                    }
+            val mappingState = parameters.mappingState.get()
+            if (mappingState.reprocessAll) {
+                FileUtils.deleteDirectoryContents(parameters.jarsOutputDir.get().asFile)
+                extractProfilerDependencyJars()
+            }
+            mappingState.jarsInfo.forEach { (file, info) ->
+                if (info.hasChanged) {
+                    val instrumentedJar =
+                        File(parameters.jarsOutputDir.get().asFile, info.identity + DOT_JAR)
+                    FileUtils.deleteIfExists(instrumentedJar)
+                    instrumentationManager.instrumentClassesFromJarToJar(file, instrumentedJar)
                 }
             }
         }
@@ -358,7 +334,6 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
 
     abstract class IncrementalWorkerParams: BaseWorkerParams() {
         abstract val inputClassesDirChanges: Property<SerializableFileChanges>
-        abstract val inputJarsChanges: Property<SerializableFileChanges>
     }
 
     abstract class TransformClassesIncrementalAction:
@@ -397,33 +372,10 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
                 classesHierarchyResolver = classesHierarchyResolver
             )
         }
-
-        override fun maybeProcessJacocoInstrumentedJars(
-            instrumentationManager: AsmInstrumentationManager
-        ): Boolean {
-            if (!parameters.inputJarsChanges.isPresent) {
-                return false
-            }
-            parameters.inputJarsChanges.get().fileChanges.forEach { inputJar ->
-                val instrumentedJar =
-                    File(parameters.jarsOutputDir.get().asFile, inputJar.file.name)
-                FileUtils.deleteIfExists(instrumentedJar)
-                if (inputJar.fileStatus == FileStatus.NEW ||
-                    inputJar.fileStatus == FileStatus.CHANGED
-                ) {
-                    instrumentationManager.instrumentClassesFromJarToJar(
-                        inputJar.file,
-                        instrumentedJar
-                    )
-                }
-            }
-            return true
-        }
     }
 
     abstract class FullActionWorkerParams: BaseWorkerParams() {
         abstract val inputClassesDir: ConfigurableFileCollection
-        abstract val inputJarsDir: DirectoryProperty
     }
 
     abstract class TransformClassesFullAction:
@@ -444,21 +396,6 @@ abstract class TransformClassesWithAsmTask : NewIncrementalTask() {
 
             updateIncrementalState(emptySet(), classesHierarchyResolver)
         }
-
-        override fun maybeProcessJacocoInstrumentedJars(
-            instrumentationManager: AsmInstrumentationManager
-        ): Boolean {
-            if (!parameters.inputJarsDir.isPresent) {
-                return false
-            }
-            FileUtils.deleteDirectoryContents(parameters.jarsOutputDir.get().asFile)
-            extractProfilerDependencyJars()
-            parameters.inputJarsDir.get().asFile.listFiles()?.forEach { inputJar ->
-                val instrumentedJar = File(parameters.jarsOutputDir.get().asFile, inputJar.name)
-                instrumentationManager.instrumentClassesFromJarToJar(inputJar, instrumentedJar)
-            }
-            return true
-        }
     }
 
     class CreationAction(
diff --git a/build-system/integration-test/application/src/test/java/com/android/build/gradle/integration/instrumentation/AsmTransformApiInstrumentationTest.kt b/build-system/integration-test/application/src/test/java/com/android/build/gradle/integration/instrumentation/AsmTransformApiInstrumentationTest.kt
index 9d4eb8a470..e10d0713b1 100644
--- a/build-system/integration-test/application/src/test/java/com/android/build/gradle/integration/instrumentation/AsmTransformApiInstrumentationTest.kt
+++ b/build-system/integration-test/application/src/test/java/com/android/build/gradle/integration/instrumentation/AsmTransformApiInstrumentationTest.kt
@@ -385,4 +385,82 @@ class AsmTransformApiInstrumentationTest {
                 expectedInstrumentedClasses = listOf("InterfaceExtendsI")
         )
     }
+
+    /**
+     * regression test for b/285170632.
+     */
+    @Test
+    fun testInteractionWithVariantApi() {
+        configureExtensionForAnnotationAddingVisitor(project)
+        configureExtensionForInterfaceAddingVisitor(project)
+
+        TestFileUtils.searchAndReplace(
+            project.getSubproject(":app").buildFile,
+            "plugins {",
+            """
+                buildscript {
+                    apply from: '../../commonBuildScript.gradle'
+                    apply from: '../../commonHeader.gradle'
+                    dependencies {
+                        classpath("org.javassist:javassist:3.26.0-GA")
+                    }
+                }
+
+                plugins {
+            """.trimIndent()
+        )
+
+        TestFileUtils.appendToFile(
+            project.getSubproject(":app").buildFile,
+            // language=groovy
+            """
+                import com.android.build.api.variant.ScopedArtifacts
+                import com.android.build.api.artifact.ScopedArtifact
+
+                import org.gradle.api.DefaultTask
+                import org.gradle.api.tasks.OutputDirectory
+                import org.gradle.api.tasks.TaskAction
+                import javassist.ClassPool
+                import javassist.CtClass
+
+                abstract class AddClassesTask extends DefaultTask {
+
+                    @OutputDirectory
+                    abstract DirectoryProperty getOutput();
+
+                    @TaskAction
+                    void taskAction() {
+
+                        ClassPool pool = new ClassPool(ClassPool.getDefault())
+                        CtClass interfaceClass = pool.makeInterface(
+                            "com.android.api.tests.SomeInterface"
+                        )
+                        System.out.println("Adding ${'$'}interfaceClass")
+                        interfaceClass.writeFile(output.get().asFile.absolutePath)
+                    }
+                }
+
+                androidComponents {
+                    onVariants(selector().all(), { variant ->
+                        TaskProvider<AddClassesTask> taskProvider = project.tasks.register(
+                            variant.getName() + "AddAllClasses", AddClassesTask.class
+                        )
+                        variant.artifacts
+                            .forScope(ScopedArtifacts.Scope.PROJECT)
+                            .use(taskProvider)
+                            .toAppend(
+                                ScopedArtifact.CLASSES.INSTANCE,
+                                { it.getOutput() }
+                            )
+                    })
+                }
+            """.trimIndent()
+        )
+
+        // run twice to catch recursive input
+        project.executor().run(":app:assembleDebug")
+        project.executor().run(":app:assembleDebug")
+
+        assertClassesAreInstrumentedInDebugVariant()
+    }
 }