Skip to content

Commit 8670790

Browse files
autonomousappsautonomousapps
committed
revert: feat: check binary compatibility when there are duplicate classes on the classpath.
1 parent 9178693 commit 8670790

File tree

12 files changed

+320
-342
lines changed

12 files changed

+320
-342
lines changed

‎src/functionalTest/groovy/com/autonomousapps/AbstractFunctionalSpec.groovy‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,15 @@ abstract class AbstractFunctionalSpec extends Specification {
3939
*/
4040
private static boolean isCi = System.getenv("CI") == "true"
4141

42-
def cleanup() {
43-
// Delete fixtures on CI to prevent disk space growing out of bounds
44-
if (gradleProject != null && isCi) {
45-
try {
46-
gradleProject.rootDir.deleteDir()
47-
} catch (Throwable t) {
48-
}
49-
}
50-
}
42+
// def cleanup() {
43+
// // Delete fixtures on CI to prevent disk space growing out of bounds
44+
// if (gradleProject != null && isCi) {
45+
// try {
46+
// gradleProject.rootDir.deleteDir()
47+
// } catch (Throwable t) {
48+
// }
49+
// }
50+
// }
5151

5252
protected static Boolean quick() {
5353
return System.getProperty('com.autonomousapps.quick').toBoolean()

‎src/functionalTest/groovy/com/autonomousapps/jvm/DuplicateClasspathSpec.groovy‎

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ package com.autonomousapps.jvm
33
import com.autonomousapps.jvm.projects.BinaryIncompatibilityProject
44
import com.autonomousapps.jvm.projects.DuplicateClasspathProject
55
import com.autonomousapps.utils.Colors
6+
import org.gradle.testkit.runner.TaskOutcome
7+
import spock.lang.PendingFeature
68

79
import static com.autonomousapps.advice.truth.BuildHealthSubject.buildHealth
810
import static com.autonomousapps.utils.Runner.build
@@ -20,17 +22,17 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
2022
when:
2123
// This first invocation fixes the dependency declarations
2224
build(gradleVersion, gradleProject.rootDir, ':consumer:fixDependencies')
23-
// This used to fail because of classpath duplication and the wrong dep getting loaded first. Recent improvements
24-
// have improved this case, however.
25-
build(gradleVersion, gradleProject.rootDir, 'buildHealth')
25+
// This fails because of classpath duplication and the wrong dep getting loaded first
26+
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
2627
2728
then:
28-
assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
29-
'''\
30-
dependencies {
31-
implementation project(':producer-2')
32-
}'''.stripIndent()
33-
)
29+
assertThat(result.task(':consumer:compileJava').outcome).isEqualTo(TaskOutcome.FAILED)
30+
// assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
31+
// '''\
32+
// dependencies {
33+
// implementation project(':producer-2')
34+
// }'''.stripIndent()
35+
// )
3436
3537
where:
3638
gradleVersion << [GRADLE_LATEST]
@@ -69,6 +71,7 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
6971
gradleVersion << [GRADLE_LATEST]
7072
}
7173
74+
@PendingFeature(reason = "This feature was reverted")
7275
def "can report on which of the duplicates is needed for binary compatibility (#gradleVersion)"() {
7376
given:
7477
def project = new BinaryIncompatibilityProject()
@@ -123,6 +126,7 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
123126
gradleVersion << [GRADLE_LATEST]
124127
}
125128
129+
@PendingFeature(reason = "This feature was reverted")
126130
def "suggests removing a binary-incompatible duplicate (#gradleVersion)"() {
127131
given:
128132
def project = new BinaryIncompatibilityProject(true)

‎src/functionalTest/groovy/com/autonomousapps/jvm/projects/DuplicateClasspathProject.groovy‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,6 +176,9 @@ final class DuplicateClasspathProject extends AbstractProject {
176176

177177
private final Set<Advice> consumerAdvice = [
178178
Advice.ofRemove(projectCoordinates(':unused'), 'implementation'),
179+
// This is actually bad advice, but we can't detect it without re-reverting the change to detect binary
180+
// incompatibilities.
181+
Advice.ofAdd(projectCoordinates(':producer-1'), 'implementation')
179182
]
180183

181184
final Set<ProjectAdvice> expectedProjectAdvice = [

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

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import com.autonomousapps.internal.kotlin.AccessFlags
99
import com.autonomousapps.internal.utils.METHOD_DESCRIPTOR_REGEX
1010
import com.autonomousapps.internal.utils.efficient
1111
import com.autonomousapps.internal.utils.genericTypes
12-
import com.autonomousapps.model.internal.intermediates.producer.Member
1312
import com.autonomousapps.model.internal.intermediates.consumer.MemberAccess
1413
import kotlinx.metadata.jvm.Metadata
1514
import org.gradle.api.logging.Logger
@@ -26,13 +25,14 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
2625
private lateinit var access: Access
2726
private var outerClassName: String? = null
2827
private var superClassName: String? = null
29-
private var interfaces: Set<String>? = null
28+
29+
// private var interfaces: Set<String>? = null
3030
private val retentionPolicyHolder = AtomicReference("")
3131
private var isAnnotation = false
3232
private val methods = mutableSetOf<Method>()
3333
private val innerClasses = mutableSetOf<String>()
34-
private val effectivelyPublicFields = mutableSetOf<Member.Field>()
35-
private val effectivelyPublicMethods = mutableSetOf<Member.Method>()
34+
// private val effectivelyPublicFields = mutableSetOf<Member.Field>()
35+
// private val effectivelyPublicMethods = mutableSetOf<Member.Method>()
3636

3737
private var methodCount = 0
3838
private var fieldCount = 0
@@ -49,16 +49,16 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
4949
className = className,
5050
outerClassName = outerClassName,
5151
superClassName = superClassName!!,
52-
interfaces = interfaces.orEmpty(),
52+
// interfaces = interfaces.orEmpty(),
5353
retentionPolicy = retentionPolicyHolder.get(),
5454
isAnnotation = isAnnotation,
5555
hasNoMembers = hasNoMembers,
5656
access = access,
5757
methods = methods.efficient(),
5858
innerClasses = innerClasses.efficient(),
5959
constantClasses = constantClasses.efficient(),
60-
effectivelyPublicFields = effectivelyPublicFields,
61-
effectivelyPublicMethods = effectivelyPublicMethods,
60+
// effectivelyPublicFields = effectivelyPublicFields,
61+
// effectivelyPublicMethods = effectivelyPublicMethods,
6262
)
6363
}
6464

@@ -72,7 +72,7 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
7272
) {
7373
// This _must_ not be canonicalized, unless we also change accesses to be dotty instead of slashy
7474
this.superClassName = superName
75-
this.interfaces = interfaces?.toSortedSet().orEmpty()
75+
// this.interfaces = interfaces?.toSortedSet().orEmpty()
7676

7777
className = canonicalize(name)
7878
if (interfaces?.contains("java/lang/annotation/Annotation") == true) {
@@ -107,15 +107,15 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
107107
methods.add(Method(descriptor))
108108
}
109109

110-
if (isEffectivelyPublic(access)) {
111-
effectivelyPublicMethods.add(
112-
Member.Method(
113-
access = access,
114-
name = name,
115-
descriptor = descriptor,
116-
)
117-
)
118-
}
110+
// if (isEffectivelyPublic(access)) {
111+
// effectivelyPublicMethods.add(
112+
// Member.Method(
113+
// access = access,
114+
// name = name,
115+
// descriptor = descriptor,
116+
// )
117+
// )
118+
// }
119119

120120
return null
121121
}
@@ -131,15 +131,15 @@ internal class ClassNameAndAnnotationsVisitor(private val logger: Logger) : Clas
131131
constantClasses.add(name)
132132
}
133133

134-
if (isEffectivelyPublic(access)) {
135-
effectivelyPublicFields.add(
136-
Member.Field(
137-
access = access,
138-
name = name,
139-
descriptor = descriptor,
140-
)
141-
)
142-
}
134+
// if (isEffectivelyPublic(access)) {
135+
// effectivelyPublicFields.add(
136+
// Member.Field(
137+
// access = access,
138+
// name = name,
139+
// descriptor = descriptor,
140+
// )
141+
// )
142+
// }
143143

144144
return null
145145
}

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

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import com.autonomousapps.internal.asm.Opcodes
66
import com.autonomousapps.internal.utils.efficient
77
import com.autonomousapps.internal.utils.filterNotToSet
88
import com.autonomousapps.internal.utils.mapToSet
9-
import com.autonomousapps.model.internal.intermediates.producer.BinaryClass
10-
import com.autonomousapps.model.internal.intermediates.producer.Member
119
import com.squareup.moshi.JsonClass
1210
import java.lang.annotation.RetentionPolicy
1311
import java.util.regex.Pattern
@@ -45,22 +43,22 @@ internal data class AnalyzedClass(
4543
val methods: Set<Method>,
4644
val innerClasses: Set<String>,
4745
val constantFields: Set<String>,
48-
val binaryClass: BinaryClass,
46+
// val binaryClass: BinaryClass,
4947
) : Comparable<AnalyzedClass> {
5048
constructor(
5149
className: String,
5250
outerClassName: String?,
5351
superClassName: String,
54-
interfaces: Set<String>,
52+
// interfaces: Set<String>,
5553
retentionPolicy: String?,
5654
isAnnotation: Boolean,
5755
hasNoMembers: Boolean,
5856
access: Access,
5957
methods: Set<Method>,
6058
innerClasses: Set<String>,
6159
constantClasses: Set<String>,
62-
effectivelyPublicFields: Set<Member.Field>,
63-
effectivelyPublicMethods: Set<Member.Method>,
60+
// effectivelyPublicFields: Set<Member.Field>,
61+
// effectivelyPublicMethods: Set<Member.Method>,
6462
) : this(
6563
className = className,
6664
outerClassName = outerClassName,
@@ -71,13 +69,13 @@ internal data class AnalyzedClass(
7169
methods = methods,
7270
innerClasses = innerClasses,
7371
constantFields = constantClasses,
74-
binaryClass = BinaryClass(
75-
className = className.replace('.', '/'),
76-
superClassName = superClassName.replace('.', '/'),
77-
interfaces = interfaces,
78-
effectivelyPublicFields = effectivelyPublicFields,
79-
effectivelyPublicMethods = effectivelyPublicMethods,
80-
),
72+
// binaryClass = BinaryClass(
73+
// className = className.replace('.', '/'),
74+
// superClassName = superClassName.replace('.', '/'),
75+
// interfaces = interfaces,
76+
// effectivelyPublicFields = effectivelyPublicFields,
77+
// effectivelyPublicMethods = effectivelyPublicMethods,
78+
// ),
8179
)
8280

8381
companion object {

0 commit comments

Comments
 (0)