Skip to content

Commit

Permalink
fix: handle disjoint classpaths.
Browse files Browse the repository at this point in the history
If the main source set has a used dependency, and a custom source set has an unused transitive dependency,
and these are at different versions, StandardTransform was advising the removal of the used dependency.
Now we check that the usage is not undeclared before adding to the set of remove-advice.
  • Loading branch information
autonomousapps committed Feb 1, 2024
1 parent 500169e commit 46c0d5e
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 51 deletions.
2 changes: 2 additions & 0 deletions src/main/kotlin/com/autonomousapps/internal/Bundles.kt
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,15 @@ internal class Bundles private constructor(private val dependencyUsages: Map<Coo

fun hasUsedChild(coordinates: Coordinates): Boolean {
val children = parentKeyedBundle[coordinates] ?: return false

return children.any { child ->
dependencyUsages[child].orEmpty().any { it.bucket != Bucket.NONE }
}
}

fun findUsedChild(coordinates: Coordinates): Coordinates? {
val children = parentKeyedBundle[coordinates] ?: return null

return children.find { child ->
dependencyUsages[child].orEmpty().any { it.bucket != Bucket.NONE }
}
Expand Down
30 changes: 20 additions & 10 deletions src/main/kotlin/com/autonomousapps/internal/utils/gradleStrings.kt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import org.gradle.api.internal.artifacts.DefaultProjectComponentIdentifier
import org.gradle.api.provider.Provider
import org.gradle.internal.component.local.model.OpaqueComponentArtifactIdentifier
import org.gradle.internal.component.local.model.OpaqueComponentIdentifier
import java.io.Serializable

/** Converts this [ResolvedDependencyResult] to group-artifact-version (GAV) coordinates in a tuple of (GA, V?). */
internal fun ResolvedDependencyResult.toCoordinates(): Coordinates =
Expand Down Expand Up @@ -154,42 +155,51 @@ private fun ComponentIdentifier.resolvedVersion(): String? = when (this) {
else -> throw GradleException("Cannot identify ComponentIdentifier subtype. Was ${javaClass.simpleName}, named $this")
}?.intern()

/**
* This has to be public because it's used as part of a task input, but should otherwise be considered an internal
* implementation detail.
*/
class ModuleInfo(
val identifier: String,
val version: String? = null,
) : Serializable

/**
* Given [Configuration.getDependencies], return this dependency set as a set of identifiers, per
* [ComponentIdentifier.toIdentifier].
*/
internal fun DependencySet.toIdentifiers(): Set<Pair<String, GradleVariantIdentification>> = mapNotNullToSet {
internal fun DependencySet.toIdentifiers(): Set<Pair<ModuleInfo, GradleVariantIdentification>> = mapNotNullToSet {
it.toIdentifier()
}

internal fun Dependency.toCoordinates(): Coordinates? {
val identifier = toIdentifier() ?: return null
return when (this) {
is ProjectDependency -> ProjectCoordinates(identifier.first, identifier.second)
is ProjectDependency -> ProjectCoordinates(identifier.first.identifier, identifier.second)
is ModuleDependency -> {
resolvedVersion()?.let { resolvedVersion ->
ModuleCoordinates(identifier.first, resolvedVersion, identifier.second)
} ?: FlatCoordinates(identifier.first)
ModuleCoordinates(identifier.first.identifier, resolvedVersion, identifier.second)
} ?: FlatCoordinates(identifier.first.identifier)
}

else -> FlatCoordinates(identifier.first)
else -> FlatCoordinates(identifier.first.identifier)
}
}

/**
* Given a [Dependency] retrieved from a [Configuration], return it as a
* pair of 'identifier' and 'GradleVariantIdentification'
*/
internal fun Dependency.toIdentifier(): Pair<String, GradleVariantIdentification>? = when (this) {
internal fun Dependency.toIdentifier(): Pair<ModuleInfo, GradleVariantIdentification>? = when (this) {
is ProjectDependency -> {
val identifier = dependencyProject.path
Pair(identifier.intern(), targetGradleVariantIdentification())
Pair(ModuleInfo(identifier.intern()), targetGradleVariantIdentification())
}

is ModuleDependency -> {
// flat JAR/AAR files have no group.
val identifier = if (group != null) "${group}:${name}" else name
Pair(identifier.intern(), targetGradleVariantIdentification())
Pair(ModuleInfo(identifier.intern(), version), targetGradleVariantIdentification())
}

is FileCollectionDependency -> {
Expand All @@ -213,12 +223,12 @@ internal fun Dependency.toIdentifier(): Pair<String, GradleVariantIdentification
else -> firstFile?.toString()?.substringAfterLast('/')
}
}?.let {
Pair(it.intern(), GradleVariantIdentification.EMPTY)
Pair(ModuleInfo(it.intern()), GradleVariantIdentification.EMPTY)
}
}

is ConfigurableFileTree -> files.firstOrNull()?.name?.let {
Pair(it.intern(), GradleVariantIdentification.EMPTY)
Pair(ModuleInfo((it.intern())), GradleVariantIdentification.EMPTY)
}

else -> null
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import com.squareup.moshi.JsonClass
@JsonClass(generateAdapter = false)
internal data class Declaration(
val identifier: String,
val version: String? = null,
val configurationName: String,
val gradleVariantIdentification: GradleVariantIdentification
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal data class Usage(
}

internal class UsageBuilder(
reports: Set<DependencyTraceReport>,
traces: Set<DependencyTraceReport>,
private val variants: Collection<Variant>
) {

Expand All @@ -42,7 +42,7 @@ internal class UsageBuilder(
val theDependencyUsages = mutableMapOf<Coordinates, MutableSet<Usage>>()
val theAnnotationProcessingUsages = mutableMapOf<Coordinates, MutableSet<Usage>>()

reports.forEach { report ->
traces.forEach { report ->
report.dependencies.forEach { trace ->
theDependencyUsages.add(report, trace)
}
Expand Down
4 changes: 2 additions & 2 deletions src/main/kotlin/com/autonomousapps/tasks/ComputeAdviceTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -156,9 +156,9 @@ abstract class ComputeAdviceTask @Inject constructor(
.run { AndroidScore.ofVariants(this) }
.toSetOrEmpty()
val bundleRules = parameters.bundles.get()
val reports = parameters.dependencyUsageReports.get().mapToSet { it.fromJson<DependencyTraceReport>() }
val traces = parameters.dependencyUsageReports.get().mapToSet { it.fromJson<DependencyTraceReport>() }
val usageBuilder = UsageBuilder(
reports = reports,
traces = traces,
// TODO: it would be clearer to get this from a SyntheticProject
variants = dependencyGraph.values.map { it.variant }
)
Expand Down
15 changes: 10 additions & 5 deletions src/main/kotlin/com/autonomousapps/tasks/FindDeclarationsTask.kt
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package com.autonomousapps.tasks
import com.autonomousapps.Flags.shouldAnalyzeTests
import com.autonomousapps.TASK_GROUP_DEP_INTERNAL
import com.autonomousapps.internal.NoVariantOutputPaths
import com.autonomousapps.internal.utils.ModuleInfo
import com.autonomousapps.internal.utils.bufferWriteJsonSet
import com.autonomousapps.internal.utils.getAndDelete
import com.autonomousapps.internal.utils.toIdentifiers
Expand Down Expand Up @@ -57,11 +58,11 @@ abstract class FindDeclarationsTask : DefaultTask() {

task.projectPath.set(project.path)
task.shouldAnalyzeTest.set(shouldAnalyzeTests)
task.declarationContainer.set(computeLocations(project, shouldAnalyzeTests))
task.declarationContainer.set(computeDeclarations(project, shouldAnalyzeTests))
task.output.set(outputPaths.locationsPath)
}

private fun computeLocations(project: Project, shouldAnalyzeTests: Boolean): Provider<DeclarationContainer> {
private fun computeDeclarations(project: Project, shouldAnalyzeTests: Boolean): Provider<DeclarationContainer> {
val configurations = project.configurations
return project.provider {
DeclarationContainer.of(
Expand All @@ -87,11 +88,14 @@ abstract class FindDeclarationsTask : DefaultTask() {
}
}

class DeclarationContainer(@get:Input val mapping: Map<String, Set<Pair<String, GradleVariantIdentification>>>) {
class DeclarationContainer(
@get:Input
val mapping: Map<String, Set<Pair<ModuleInfo, GradleVariantIdentification>>>
) {

companion object {
internal fun of(
mapping: Map<String, Set<Pair<String, GradleVariantIdentification>>>
mapping: Map<String, Set<Pair<ModuleInfo, GradleVariantIdentification>>>
): DeclarationContainer = DeclarationContainer(mapping)
}
}
Expand All @@ -102,7 +106,8 @@ abstract class FindDeclarationsTask : DefaultTask() {
.flatMap { (conf, identifiers) ->
identifiers.map { id ->
Declaration(
identifier = id.first,
identifier = id.first.identifier,
version = id.first.version,
configurationName = conf,
gradleVariantIdentification = id.second
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import com.autonomousapps.model.declaration.Bucket
import com.autonomousapps.model.declaration.Declaration
import com.autonomousapps.model.declaration.SourceSetKind
import com.autonomousapps.model.declaration.Variant
import com.autonomousapps.model.intermediates.Reason
import com.autonomousapps.model.intermediates.Usage
import com.google.common.collect.SetMultimap
import org.gradle.api.attributes.Category
Expand Down Expand Up @@ -147,6 +148,8 @@ internal class StandardTransform(
declarationsForVariant.forEach { decl ->
if (
usage.bucket == Bucket.NONE
// Don't remove an undeclared usage (this would make no sense)
&& Reason.Undeclared !in usage.reasons
// Don't remove a declaration on compileOnly, compileOnlyApi, providedCompile
&& decl.bucket != Bucket.COMPILE_ONLY
// Don't remove a declaration on runtimeOnly
Expand All @@ -157,8 +160,9 @@ internal class StandardTransform(
declaration = decl
)
} else if (
// Don't change a match, it's correct!
!usage.bucket.matches(decl)
usage.bucket != Bucket.NONE
// Don't change a match, it's correct!
&& !usage.bucket.matches(decl)
// Don't change a declaration on compileOnly, compileOnlyApi, providedCompile
&& decl.bucket != Bucket.COMPILE_ONLY
// Don't change a declaration on runtimeOnly
Expand Down
Loading

0 comments on commit 46c0d5e

Please sign in to comment.