Skip to content

Commit e0ee278

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

40 files changed

+1346
-290
lines changed

‎build.gradle.kts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// SPDX-License-Identifier: Apache-2.0
33
@file:Suppress("UnstableApiUsage", "HasPlatformType", "PropertyName")
44

5-
import org.jetbrains.kotlin.cli.common.toBooleanLenient
6-
75
plugins {
86
id("java-gradle-plugin")
97
id("com.gradle.plugin-publish")
@@ -182,7 +180,7 @@ fun maxParallelForks() =
182180

183181
val isCi = providers.environmentVariable("CI")
184182
.getOrElse("false")
185-
.toBooleanLenient()!!
183+
.toBoolean()
186184

187185
// This will slow down tests on CI, but maybe it won't run out of metaspace.
188186
fun forkEvery(): Long = if (isCi) 40 else 0

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ import spock.lang.Specification
1111

1212
abstract class AbstractFunctionalSpec extends Specification {
1313

14+
@SuppressWarnings('unused')
15+
protected static final String FLAG_LOG_BYTECODE = "-D${Flags.FLAG_BYTECODE_LOGGING}=true"
16+
1417
protected static final GRADLE_7_5 = GradleVersion.version('7.5.1')
1518
protected static final GRADLE_7_6 = GradleVersion.version('7.6.2')
1619
protected static final GRADLE_8_0 = GradleVersion.version('8.0.2')

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

Lines changed: 118 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.autonomousapps.jvm
22

3+
import com.autonomousapps.Flags
4+
import com.autonomousapps.jvm.projects.BinaryIncompatibilityProject
35
import com.autonomousapps.jvm.projects.DuplicateClasspathProject
4-
import org.gradle.testkit.runner.TaskOutcome
6+
import com.autonomousapps.utils.Colors
57

68
import static com.autonomousapps.advice.truth.BuildHealthSubject.buildHealth
79
import static com.autonomousapps.utils.Runner.build
@@ -17,13 +19,19 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
1719
gradleProject = project.gradleProject
1820
1921
when:
20-
// This first invocation "fixes" the dependency declarations
22+
// This first invocation fixes the dependency declarations
2123
build(gradleVersion, gradleProject.rootDir, ':consumer:fixDependencies')
22-
// This second invocation fails because the fix (+ sort) causes the JVM to load the wrong class during compilation.
23-
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
24+
// This used to fail because of classpath duplication and the wrong dep getting loaded first. Recent improvements
25+
// have improved this case, however.
26+
build(gradleVersion, gradleProject.rootDir, 'buildHealth')
2427
2528
then:
26-
assertThat(result.task(':consumer:compileJava').outcome).isEqualTo(TaskOutcome.FAILED)
29+
assertThat(gradleProject.rootDir.toPath().resolve('consumer/build.gradle').text).contains(
30+
'''\
31+
dependencies {
32+
implementation project(':producer-2')
33+
}'''.stripIndent()
34+
)
2735
2836
where:
2937
gradleVersion << [GRADLE_LATEST]
@@ -35,7 +43,7 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
3543
gradleProject = project.gradleProject
3644
3745
when:
38-
def result = build(gradleVersion, gradleProject.rootDir, 'buildHealth')
46+
def result = buildAndFail(gradleVersion, gradleProject.rootDir, 'buildHealth')
3947
4048
then:
4149
assertThat(result.output).contains(
@@ -61,4 +69,108 @@ final class DuplicateClasspathSpec extends AbstractJvmSpec {
6169
where:
6270
gradleVersion << [GRADLE_LATEST]
6371
}
72+
73+
def "can report on which of the duplicates is needed for binary compatibility (#gradleVersion)"() {
74+
given:
75+
def project = new BinaryIncompatibilityProject()
76+
gradleProject = project.gradleProject
77+
78+
when:
79+
def result = build(
80+
gradleVersion, gradleProject.rootDir,
81+
':consumer:reason', '--id', ':producer-1',
82+
//FLAG_LOG_BYTECODE,
83+
)
84+
85+
then:
86+
assertThat(Colors.decolorize(result.output)).contains(
87+
'''\
88+
------------------------------------------------------------
89+
You asked about the dependency ':producer-1'.
90+
There is no advice regarding this dependency.
91+
------------------------------------------------------------
92+
93+
Shortest path from :consumer to :producer-1 for compileClasspath:
94+
:consumer
95+
\\--- :unused
96+
\\--- :producer-1
97+
98+
Shortest path from :consumer to :producer-1 for runtimeClasspath:
99+
:consumer
100+
\\--- :unused
101+
\\--- :producer-1
102+
103+
Shortest path from :consumer to :producer-1 for testCompileClasspath:
104+
:consumer
105+
\\--- :unused
106+
\\--- :producer-1
107+
108+
Shortest path from :consumer to :producer-1 for testRuntimeClasspath:
109+
:consumer
110+
\\--- :unused
111+
\\--- :producer-1
112+
113+
Source: main
114+
------------
115+
* Is binary-incompatible, and should be removed from the classpath:
116+
Expected METHOD com/example/producer/Person.<init>(Ljava/lang/String;Ljava/lang/String;)V, but was com/example/producer/Person.<init>(Ljava/lang/String;)V
117+
118+
Source: test
119+
------------
120+
(no usages)'''.stripIndent()
121+
)
122+
123+
where:
124+
gradleVersion << [GRADLE_LATEST]
125+
}
126+
127+
def "suggests removing a binary-incompatible duplicate (#gradleVersion)"() {
128+
given:
129+
def project = new BinaryIncompatibilityProject(true)
130+
gradleProject = project.gradleProject
131+
132+
when:
133+
def result = build(
134+
gradleVersion, gradleProject.rootDir,
135+
':consumer:reason', '--id', ':producer-1',
136+
//FLAG_LOG_BYTECODE,
137+
)
138+
139+
then:
140+
assertThat(Colors.decolorize(result.output)).contains(
141+
'''\
142+
------------------------------------------------------------
143+
You asked about the dependency ':producer-1'.
144+
You have been advised to remove this dependency from 'implementation'.
145+
------------------------------------------------------------
146+
147+
Shortest path from :consumer to :producer-1 for compileClasspath:
148+
:consumer
149+
\\--- :producer-1
150+
151+
Shortest path from :consumer to :producer-1 for runtimeClasspath:
152+
:consumer
153+
\\--- :producer-1
154+
155+
Shortest path from :consumer to :producer-1 for testCompileClasspath:
156+
:consumer
157+
\\--- :producer-1
158+
159+
Shortest path from :consumer to :producer-1 for testRuntimeClasspath:
160+
:consumer
161+
\\--- :producer-1
162+
163+
Source: main
164+
------------
165+
* Is binary-incompatible, and should be removed from the classpath:
166+
Expected METHOD com/example/producer/Person.<init>(Ljava/lang/String;Ljava/lang/String;)V, but was com/example/producer/Person.<init>(Ljava/lang/String;)V
167+
168+
Source: test
169+
------------
170+
(no usages)'''.stripIndent()
171+
)
172+
173+
where:
174+
gradleVersion << [GRADLE_LATEST]
175+
}
64176
}

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

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
package com.autonomousapps.jvm.projects
2+
3+
import com.autonomousapps.AbstractProject
4+
import com.autonomousapps.kit.GradleProject
5+
import com.autonomousapps.kit.Source
6+
7+
import static com.autonomousapps.kit.gradle.Dependency.project
8+
9+
final class BinaryIncompatibilityProject extends AbstractProject {
10+
11+
final GradleProject gradleProject
12+
13+
BinaryIncompatibilityProject(boolean extra = false) {
14+
this.gradleProject = build(extra)
15+
}
16+
17+
private GradleProject build(boolean extra) {
18+
return newGradleProjectBuilder()
19+
// :consumer uses the Producer class.
20+
// This class is provided by both
21+
// 1. :producer-2, which is a direct dependency, and
22+
// 2. :producer-1, which is a transitive dependency (of :unused)
23+
// These classes have incompatible definitions. :consumer _requires_ the version provided by :producer-2.
24+
.withSubproject('consumer') { s ->
25+
s.sources = sourcesConsumer
26+
s.withBuildScript { bs ->
27+
bs.plugins = javaLibrary + plugins.gradleDependenciesSorter
28+
bs.dependencies(
29+
project('implementation', ':unused'),
30+
project('implementation', ':producer-2'),
31+
)
32+
33+
// If this is a direct dep, then we should suggest removing it due to binary-incompatibility
34+
if (extra) {
35+
bs.dependencies += project('implementation', ':producer-1')
36+
}
37+
}
38+
}
39+
// Used
40+
.withSubproject('producer-1') { s ->
41+
s.sources = sourcesProducer1
42+
s.withBuildScript { bs ->
43+
bs.plugins = javaLibrary
44+
}
45+
}
46+
// Unused, except its transitive is
47+
.withSubproject('unused') { s ->
48+
s.sources = sourcesUnused
49+
s.withBuildScript { bs ->
50+
bs.plugins = javaLibrary
51+
bs.dependencies(
52+
project('api', ':producer-1'),
53+
)
54+
}
55+
}
56+
// Used?
57+
.withSubproject('producer-2') { s ->
58+
s.sources = sourcesProducer2
59+
s.withBuildScript { bs ->
60+
bs.plugins = javaLibrary
61+
}
62+
}
63+
.write()
64+
}
65+
66+
private static final List<Source> sourcesConsumer = [
67+
Source.java(
68+
'''\
69+
package com.example.consumer;
70+
71+
import com.example.producer.Person;
72+
73+
public class Consumer {
74+
private Person person = new Person("Emma", "Goldman");
75+
private String usesField = person.firstName;
76+
private String usesMethod = person.getLastName();
77+
78+
private void usePerson() {
79+
String m = Person.MAGIC;
80+
System.out.println(person.firstName);
81+
System.out.println(person.lastName);
82+
System.out.println(person.getFirstName());
83+
System.out.println(person.getLastName());
84+
85+
// Person::ugh takes and returns Object, here we pass Void and print a String.
86+
System.out.println(person.ugh(null));
87+
}
88+
}
89+
'''
90+
)
91+
.withPath('com.example.consumer', 'Consumer')
92+
.build(),
93+
]
94+
95+
private static final List<Source> sourcesProducer1 = [
96+
Source.java(
97+
'''\
98+
package com.example.producer;
99+
100+
public class Person {
101+
102+
private final String name;
103+
104+
public Person(String name) {
105+
this.name = name;
106+
}
107+
}
108+
'''
109+
)
110+
.withPath('com.example.producer', 'Person')
111+
.build(),
112+
]
113+
114+
// Same class file as sourcesProducer1, incompatible definition
115+
private static final List<Source> sourcesProducer2 = [
116+
Source.java(
117+
'''\
118+
package com.example.producer;
119+
120+
import java.util.ArrayList;
121+
import java.util.List;
122+
123+
/**
124+
* This class contains fields with different visibilities so we can exercise our bytecode analysis thoroughly.
125+
*/
126+
public class Person {
127+
128+
public static final String MAGIC = "42";
129+
130+
public final String firstName;
131+
public final String lastName;
132+
133+
protected int nameLength;
134+
135+
String[] names = new String[2];
136+
137+
private final boolean unused = false;
138+
139+
public Person(String firstName, String lastName) {
140+
this.firstName = firstName;
141+
this.lastName = lastName;
142+
this.nameLength = firstName.length() + lastName.length();
143+
144+
this.names[0] = firstName;
145+
this.names[1] = lastName;
146+
}
147+
148+
public String getFirstName() {
149+
return firstName;
150+
}
151+
152+
public String getLastName() {
153+
return lastName;
154+
}
155+
156+
protected int getNameLength() {
157+
return nameLength;
158+
}
159+
160+
List<String> getNames() {
161+
List<String> list = new ArrayList<>();
162+
list.add(firstName);
163+
list.add(lastName);
164+
return list;
165+
}
166+
167+
public Object ugh(Object anything) {
168+
return firstName;
169+
}
170+
171+
private void notImplemented() {
172+
throw new RuntimeException("ohnoes");
173+
}
174+
}
175+
'''
176+
)
177+
.withPath('com.example.producer', 'Person')
178+
.build(),
179+
]
180+
181+
private static final List<Source> sourcesUnused = [
182+
Source.java(
183+
'''\
184+
package com.example.unused;
185+
186+
import com.example.producer.Person;
187+
188+
public class Unused {
189+
public Person producer;
190+
}
191+
'''
192+
)
193+
.withPath('com.example.unused', 'Unused')
194+
.build(),
195+
]
196+
}

0 commit comments

Comments
 (0)