From 733239b38869c001b1864306f09e726fa830464f Mon Sep 17 00:00:00 2001 From: Oleg Golberg Date: Tue, 5 Nov 2024 20:33:50 -0500 Subject: [PATCH] Handle dependent artifacts that have multiple files (#130) Turns out, `ArtifactCollection.artifactFiles` coalesces multiple files associated with the same artifact. This manifests itself when e.g. a subproject dependency has a mix of kotlin and java sources and thus has two class output directories (build/classes/kotlin/main and build/classes/java/main). One of the output directories will be "lost" and won't be "seen" by the Gradle task. --- .../expediter/gradle/ExpediterTask.kt | 2 +- .../com/toasttab/expediter/gradle/Sources.kt | 55 +++++++++++++++---- .../gradle/ExpediterPluginIntegrationTest.kt | 5 ++ .../multiple outputs/app/build.gradle.kts | 16 ++++++ .../app/src/main/java/test/Caller.java | 21 +++++++ .../multiple outputs/build.gradle.kts | 9 +++ .../multiple outputs/lib/build.gradle.kts | 5 ++ .../lib/src/main/java/test/Callee1.java | 20 +++++++ .../lib/src/main/kotlin/test/Callee.kt | 18 ++++++ .../multiple outputs/settings.gradle.kts | 3 + 10 files changed, 142 insertions(+), 12 deletions(-) create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/build.gradle.kts create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/src/main/java/test/Caller.java create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/build.gradle.kts create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/build.gradle.kts create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/java/test/Callee1.java create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/kotlin/test/Callee.kt create mode 100644 plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/settings.gradle.kts diff --git a/plugin/src/main/kotlin/com/toasttab/expediter/gradle/ExpediterTask.kt b/plugin/src/main/kotlin/com/toasttab/expediter/gradle/ExpediterTask.kt index f439f7b..7086825 100644 --- a/plugin/src/main/kotlin/com/toasttab/expediter/gradle/ExpediterTask.kt +++ b/plugin/src/main/kotlin/com/toasttab/expediter/gradle/ExpediterTask.kt @@ -134,7 +134,7 @@ abstract class ExpediterTask : DefaultTask(), TaskWithProjectOutputs { if (platformConfigurationArtifacts.isNotEmpty()) { providers.add( InMemoryPlatformTypeProvider( - ClasspathScanner(platformConfigurationArtifacts.flatMap { it.artifacts.map { it.source() } }).scan { i, _ -> TypeParsers.typeDescriptor(i) } + ClasspathScanner(platformConfigurationArtifacts.sources()).scan { i, _ -> TypeParsers.typeDescriptor(i) } ) ) } diff --git a/plugin/src/main/kotlin/com/toasttab/expediter/gradle/Sources.kt b/plugin/src/main/kotlin/com/toasttab/expediter/gradle/Sources.kt index 177d10b..f17d1d5 100644 --- a/plugin/src/main/kotlin/com/toasttab/expediter/gradle/Sources.kt +++ b/plugin/src/main/kotlin/com/toasttab/expediter/gradle/Sources.kt @@ -5,25 +5,58 @@ import com.toasttab.expediter.types.ClassfileSourceType import org.gradle.api.artifacts.ArtifactCollection import org.gradle.api.artifacts.component.ModuleComponentIdentifier import org.gradle.api.artifacts.component.ProjectComponentIdentifier -import org.gradle.api.artifacts.result.ResolvedArtifactResult +import org.gradle.api.attributes.AttributeContainer import org.gradle.api.file.ConfigurableFileCollection import org.gradle.api.file.FileSystemLocation +import org.gradle.api.internal.artifacts.configurations.ArtifactCollectionInternal +import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ArtifactVisitor +import org.gradle.api.internal.artifacts.ivyservice.resolveengine.artifact.ResolvableArtifact import org.gradle.api.provider.ListProperty +import org.gradle.internal.DisplayName +import org.gradle.internal.component.external.model.ImmutableCapabilities import java.io.File -fun ResolvedArtifactResult.source() = - when (id.componentIdentifier) { - is ModuleComponentIdentifier -> ClassfileSource(file, ClassfileSourceType.EXTERNAL_DEPENDENCY) - is ProjectComponentIdentifier -> ClassfileSource(file, ClassfileSourceType.SUBPROJECT_DEPENDENCY) - else -> ClassfileSource(file, ClassfileSourceType.UNKNOWN) - } - fun File.source() = ClassfileSource(this, ClassfileSourceType.UNKNOWN) -fun ArtifactCollection.sources() = artifacts.map { it.source() } - fun ConfigurableFileCollection.sources() = map { it.source() } -fun Collection.sources() = flatMapTo(LinkedHashSet(), ArtifactCollection::sources) +fun ResolvableArtifact.source() = when (id.componentIdentifier) { + is ModuleComponentIdentifier -> ClassfileSource(file, ClassfileSourceType.EXTERNAL_DEPENDENCY) + is ProjectComponentIdentifier -> ClassfileSource(file, ClassfileSourceType.SUBPROJECT_DEPENDENCY) + else -> ClassfileSource(file, ClassfileSourceType.UNKNOWN) +} + +private fun ArtifactCollectionInternal.sources(): Collection { + val sources = mutableListOf() + + // Note that ArtifactCollection.artifactFiles coalesces multiple files associated + // with the same artifact; for example, when a project dependency has multiple outputs + // (e.g. build/classes/kotlin/main and build/classes/java/main), only one of those + // outputs will be present in ArtifactCollection.artifactFiles. + // + // To deal with that, we visit all artifacts, similarly to the implementation + // of ArtifactCollection.artifactFiles, but we don't coalesce the files. + visitArtifacts(object : ArtifactVisitor { + override fun visitArtifact( + variantName: DisplayName, + variantAttributes: AttributeContainer, + capabilities: ImmutableCapabilities, + artifact: ResolvableArtifact + ) { + sources.add(artifact.source()) + } + + override fun requireArtifactFiles() = true + + override fun visitFailure(failure: Throwable) { + } + }) + + return sources +} + +fun Collection.sources() = flatMapTo(LinkedHashSet()) { + (it as ArtifactCollectionInternal).sources() +} fun ListProperty.sources() = get().map { ClassfileSource(it.asFile, ClassfileSourceType.PROJECT) } diff --git a/plugin/src/test/kotlin/com/toasttab/expediter/gradle/ExpediterPluginIntegrationTest.kt b/plugin/src/test/kotlin/com/toasttab/expediter/gradle/ExpediterPluginIntegrationTest.kt index e61881e..e891f92 100644 --- a/plugin/src/test/kotlin/com/toasttab/expediter/gradle/ExpediterPluginIntegrationTest.kt +++ b/plugin/src/test/kotlin/com/toasttab/expediter/gradle/ExpediterPluginIntegrationTest.kt @@ -298,6 +298,11 @@ class ExpediterPluginIntegrationTest { expectThat(report.issues).filterIsInstance().isEmpty() } + @ParameterizedWithGradleVersions + fun `multiple outputs`(project: TestProject) { + project.build("check") + } + @ParameterizedWithGradleVersions fun `ignore`(project: TestProject) { project.build("check") diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/build.gradle.kts b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/build.gradle.kts new file mode 100644 index 0000000..389b7da --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/build.gradle.kts @@ -0,0 +1,16 @@ +plugins { + java + id("com.toasttab.expediter") version "@VERSION@" +} + +expediter { + failOnIssues = true + + platform { + jvmVersion = 8 + } +} + +dependencies { + implementation(project(":lib")) +} diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/src/main/java/test/Caller.java b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/src/main/java/test/Caller.java new file mode 100644 index 0000000..83c07c1 --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/app/src/main/java/test/Caller.java @@ -0,0 +1,21 @@ +/* + * Copyright (c) 2024 Toast Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package test; + +public class Caller { + Callee callee; + Callee1 callee1; +} diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/build.gradle.kts b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/build.gradle.kts new file mode 100644 index 0000000..92fb3f0 --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/build.gradle.kts @@ -0,0 +1,9 @@ +plugins { + id("com.toasttab.testkit.coverage") version "@TESTKIT_PLUGIN_VERSION@" +} + +subprojects { + repositories { + mavenCentral() + } +} \ No newline at end of file diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/build.gradle.kts b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/build.gradle.kts new file mode 100644 index 0000000..2183920 --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/build.gradle.kts @@ -0,0 +1,5 @@ +plugins { + java + kotlin("jvm") version "@KOTLIN_VERSION@" + id("com.toasttab.expediter") version "@VERSION@" +} diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/java/test/Callee1.java b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/java/test/Callee1.java new file mode 100644 index 0000000..b1f844f --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/java/test/Callee1.java @@ -0,0 +1,20 @@ +/* + * Copyright (c) 2024 Toast Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package test; + +public class Callee1 { + +} diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/kotlin/test/Callee.kt b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/kotlin/test/Callee.kt new file mode 100644 index 0000000..e9e4b35 --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/lib/src/main/kotlin/test/Callee.kt @@ -0,0 +1,18 @@ +/* + * Copyright (c) 2024 Toast Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package test + +class Callee diff --git a/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/settings.gradle.kts b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/settings.gradle.kts new file mode 100644 index 0000000..4792814 --- /dev/null +++ b/plugin/src/test/projects/ExpediterPluginIntegrationTest/multiple outputs/settings.gradle.kts @@ -0,0 +1,3 @@ +rootProject.name = "test" + +include(":lib", ":app") \ No newline at end of file