Skip to content

Commit 46c0d5e

Browse files
autonomousappsautonomousapps
committed
fix: handle disjoint classpaths.
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.
1 parent 500169e commit 46c0d5e

8 files changed

Lines changed: 218 additions & 51 deletions

File tree

‎src/main/kotlin/com/autonomousapps/internal/Bundles.kt‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,15 @@ internal class Bundles private constructor(private val dependencyUsages: Map<Coo
4747

4848
fun hasUsedChild(coordinates: Coordinates): Boolean {
4949
val children = parentKeyedBundle[coordinates] ?: return false
50+
5051
return children.any { child ->
5152
dependencyUsages[child].orEmpty().any { it.bucket != Bucket.NONE }
5253
}
5354
}
5455

5556
fun findUsedChild(coordinates: Coordinates): Coordinates? {
5657
val children = parentKeyedBundle[coordinates] ?: return null
58+
5759
return children.find { child ->
5860
dependencyUsages[child].orEmpty().any { it.bucket != Bucket.NONE }
5961
}

‎src/main/kotlin/com/autonomousapps/internal/utils/gradleStrings.kt‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ import org.gradle.api.internal.artifacts.DefaultProjectComponentIdentifier
2424
import org.gradle.api.provider.Provider
2525
import org.gradle.internal.component.local.model.OpaqueComponentArtifactIdentifier
2626
import org.gradle.internal.component.local.model.OpaqueComponentIdentifier
27+
import java.io.Serializable
2728

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

158+
/**
159+
* This has to be public because it's used as part of a task input, but should otherwise be considered an internal
160+
* implementation detail.
161+
*/
162+
class ModuleInfo(
163+
val identifier: String,
164+
val version: String? = null,
165+
) : Serializable
166+
157167
/**
158168
* Given [Configuration.getDependencies], return this dependency set as a set of identifiers, per
159169
* [ComponentIdentifier.toIdentifier].
160170
*/
161-
internal fun DependencySet.toIdentifiers(): Set<Pair<String, GradleVariantIdentification>> = mapNotNullToSet {
171+
internal fun DependencySet.toIdentifiers(): Set<Pair<ModuleInfo, GradleVariantIdentification>> = mapNotNullToSet {
162172
it.toIdentifier()
163173
}
164174

165175
internal fun Dependency.toCoordinates(): Coordinates? {
166176
val identifier = toIdentifier() ?: return null
167177
return when (this) {
168-
is ProjectDependency -> ProjectCoordinates(identifier.first, identifier.second)
178+
is ProjectDependency -> ProjectCoordinates(identifier.first.identifier, identifier.second)
169179
is ModuleDependency -> {
170180
resolvedVersion()?.let { resolvedVersion ->
171-
ModuleCoordinates(identifier.first, resolvedVersion, identifier.second)
172-
} ?: FlatCoordinates(identifier.first)
181+
ModuleCoordinates(identifier.first.identifier, resolvedVersion, identifier.second)
182+
} ?: FlatCoordinates(identifier.first.identifier)
173183
}
174184

175-
else -> FlatCoordinates(identifier.first)
185+
else -> FlatCoordinates(identifier.first.identifier)
176186
}
177187
}
178188

179189
/**
180190
* Given a [Dependency] retrieved from a [Configuration], return it as a
181191
* pair of 'identifier' and 'GradleVariantIdentification'
182192
*/
183-
internal fun Dependency.toIdentifier(): Pair<String, GradleVariantIdentification>? = when (this) {
193+
internal fun Dependency.toIdentifier(): Pair<ModuleInfo, GradleVariantIdentification>? = when (this) {
184194
is ProjectDependency -> {
185195
val identifier = dependencyProject.path
186-
Pair(identifier.intern(), targetGradleVariantIdentification())
196+
Pair(ModuleInfo(identifier.intern()), targetGradleVariantIdentification())
187197
}
188198

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

195205
is FileCollectionDependency -> {
@@ -213,12 +223,12 @@ internal fun Dependency.toIdentifier(): Pair<String, GradleVariantIdentification
213223
else -> firstFile?.toString()?.substringAfterLast('/')
214224
}
215225
}?.let {
216-
Pair(it.intern(), GradleVariantIdentification.EMPTY)
226+
Pair(ModuleInfo(it.intern()), GradleVariantIdentification.EMPTY)
217227
}
218228
}
219229

220230
is ConfigurableFileTree -> files.firstOrNull()?.name?.let {
221-
Pair(it.intern(), GradleVariantIdentification.EMPTY)
231+
Pair(ModuleInfo((it.intern())), GradleVariantIdentification.EMPTY)
222232
}
223233

224234
else -> null

‎src/main/kotlin/com/autonomousapps/model/declaration/Declaration.kt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import com.squareup.moshi.JsonClass
2020
@JsonClass(generateAdapter = false)
2121
internal data class Declaration(
2222
val identifier: String,
23+
val version: String? = null,
2324
val configurationName: String,
2425
val gradleVariantIdentification: GradleVariantIdentification
2526
) {

‎src/main/kotlin/com/autonomousapps/model/intermediates/Usage.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ internal data class Usage(
3131
}
3232

3333
internal class UsageBuilder(
34-
reports: Set<DependencyTraceReport>,
34+
traces: Set<DependencyTraceReport>,
3535
private val variants: Collection<Variant>
3636
) {
3737

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

45-
reports.forEach { report ->
45+
traces.forEach { report ->
4646
report.dependencies.forEach { trace ->
4747
theDependencyUsages.add(report, trace)
4848
}

‎src/main/kotlin/com/autonomousapps/tasks/ComputeAdviceTask.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -156,9 +156,9 @@ abstract class ComputeAdviceTask @Inject constructor(
156156
.run { AndroidScore.ofVariants(this) }
157157
.toSetOrEmpty()
158158
val bundleRules = parameters.bundles.get()
159-
val reports = parameters.dependencyUsageReports.get().mapToSet { it.fromJson<DependencyTraceReport>() }
159+
val traces = parameters.dependencyUsageReports.get().mapToSet { it.fromJson<DependencyTraceReport>() }
160160
val usageBuilder = UsageBuilder(
161-
reports = reports,
161+
traces = traces,
162162
// TODO: it would be clearer to get this from a SyntheticProject
163163
variants = dependencyGraph.values.map { it.variant }
164164
)

‎src/main/kotlin/com/autonomousapps/tasks/FindDeclarationsTask.kt‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ package com.autonomousapps.tasks
55
import com.autonomousapps.Flags.shouldAnalyzeTests
66
import com.autonomousapps.TASK_GROUP_DEP_INTERNAL
77
import com.autonomousapps.internal.NoVariantOutputPaths
8+
import com.autonomousapps.internal.utils.ModuleInfo
89
import com.autonomousapps.internal.utils.bufferWriteJsonSet
910
import com.autonomousapps.internal.utils.getAndDelete
1011
import com.autonomousapps.internal.utils.toIdentifiers
@@ -57,11 +58,11 @@ abstract class FindDeclarationsTask : DefaultTask() {
5758

5859
task.projectPath.set(project.path)
5960
task.shouldAnalyzeTest.set(shouldAnalyzeTests)
60-
task.declarationContainer.set(computeLocations(project, shouldAnalyzeTests))
61+
task.declarationContainer.set(computeDeclarations(project, shouldAnalyzeTests))
6162
task.output.set(outputPaths.locationsPath)
6263
}
6364

64-
private fun computeLocations(project: Project, shouldAnalyzeTests: Boolean): Provider<DeclarationContainer> {
65+
private fun computeDeclarations(project: Project, shouldAnalyzeTests: Boolean): Provider<DeclarationContainer> {
6566
val configurations = project.configurations
6667
return project.provider {
6768
DeclarationContainer.of(
@@ -87,11 +88,14 @@ abstract class FindDeclarationsTask : DefaultTask() {
8788
}
8889
}
8990

90-
class DeclarationContainer(@get:Input val mapping: Map<String, Set<Pair<String, GradleVariantIdentification>>>) {
91+
class DeclarationContainer(
92+
@get:Input
93+
val mapping: Map<String, Set<Pair<ModuleInfo, GradleVariantIdentification>>>
94+
) {
9195

9296
companion object {
9397
internal fun of(
94-
mapping: Map<String, Set<Pair<String, GradleVariantIdentification>>>
98+
mapping: Map<String, Set<Pair<ModuleInfo, GradleVariantIdentification>>>
9599
): DeclarationContainer = DeclarationContainer(mapping)
96100
}
97101
}
@@ -102,7 +106,8 @@ abstract class FindDeclarationsTask : DefaultTask() {
102106
.flatMap { (conf, identifiers) ->
103107
identifiers.map { id ->
104108
Declaration(
105-
identifier = id.first,
109+
identifier = id.first.identifier,
110+
version = id.first.version,
106111
configurationName = conf,
107112
gradleVariantIdentification = id.second
108113
)

‎src/main/kotlin/com/autonomousapps/transform/StandardTransform.kt‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import com.autonomousapps.model.declaration.Bucket
1212
import com.autonomousapps.model.declaration.Declaration
1313
import com.autonomousapps.model.declaration.SourceSetKind
1414
import com.autonomousapps.model.declaration.Variant
15+
import com.autonomousapps.model.intermediates.Reason
1516
import com.autonomousapps.model.intermediates.Usage
1617
import com.google.common.collect.SetMultimap
1718
import org.gradle.api.attributes.Category
@@ -147,6 +148,8 @@ internal class StandardTransform(
147148
declarationsForVariant.forEach { decl ->
148149
if (
149150
usage.bucket == Bucket.NONE
151+
// Don't remove an undeclared usage (this would make no sense)
152+
&& Reason.Undeclared !in usage.reasons
150153
// Don't remove a declaration on compileOnly, compileOnlyApi, providedCompile
151154
&& decl.bucket != Bucket.COMPILE_ONLY
152155
// Don't remove a declaration on runtimeOnly
@@ -157,8 +160,9 @@ internal class StandardTransform(
157160
declaration = decl
158161
)
159162
} else if (
160-
// Don't change a match, it's correct!
161-
!usage.bucket.matches(decl)
163+
usage.bucket != Bucket.NONE
164+
// Don't change a match, it's correct!
165+
&& !usage.bucket.matches(decl)
162166
// Don't change a declaration on compileOnly, compileOnlyApi, providedCompile
163167
&& decl.bucket != Bucket.COMPILE_ONLY
164168
// Don't change a declaration on runtimeOnly

0 commit comments

Comments
 (0)