Skip to content

Commit a306f4f

Browse files
mohamadkcopybara-github
mohamadk
authored and
copybara-github
committed
make desugar dependencies deterministic
Desugar dependencies are added to the result jar file as metadata, this desugar dependency object contains a few lists that we found out the order of this list can be different in different builds with the same inputs, therefore the final result will have a different hash and it cause cache miss in the builds solution is to make the lists in this object sorted. So for the same input, we always get the same output Closes bazelbuild#16859. PiperOrigin-RevId: 500842557 Change-Id: I051e74e7147d590fcfe5cda9731b8e012396bb65
1 parent 88c426e commit a306f4f

File tree

2 files changed

+100
-46
lines changed

2 files changed

+100
-46
lines changed

‎src/test/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollectorTest.java

+48-35
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,13 @@
1313
// limitations under the License.
1414
package com.google.devtools.build.android.desugar.dependencies;
1515

16+
import static com.google.common.collect.ImmutableList.toImmutableList;
1617
import static com.google.common.truth.Truth.assertThat;
1718

18-
import com.google.common.collect.ImmutableList;
1919
import com.google.devtools.build.android.desugar.proto.DesugarDeps;
20-
import com.google.devtools.build.android.desugar.proto.DesugarDeps.Dependency;
2120
import com.google.devtools.build.android.desugar.proto.DesugarDeps.DesugarDepsInfo;
2221
import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceDetails;
23-
import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceWithCompanion;
22+
import java.util.Arrays;
2423
import org.junit.Test;
2524
import org.junit.runner.RunWith;
2625
import org.junit.runners.JUnit4;
@@ -42,65 +41,79 @@ public void testAssumeCompanionClass() throws Exception {
4241
collector.assumeCompanionClass("a", "a$$CC");
4342

4443
DesugarDepsInfo info = extractProto(collector);
44+
4545
assertThat(info.getAssumePresentList())
4646
.containsExactly(
47-
Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b$$CC")).build(),
48-
Dependency.newBuilder().setOrigin(wrapType("b")).setTarget(wrapType("b$$CC")).build(),
49-
Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("a$$CC")).build());
47+
dependency("a", "a$$CC"), dependency("a", "b$$CC"), dependency("b", "b$$CC"));
5048
}
5149

5250
@Test
5351
public void testMissingImplementedInterface() throws Exception {
5452
MetadataCollector collector = new MetadataCollector(true);
5553
collector.missingImplementedInterface("a", "b");
56-
collector.missingImplementedInterface("a", "c");
5754
collector.missingImplementedInterface("c", "b");
55+
collector.missingImplementedInterface("a", "c");
5856

5957
DesugarDepsInfo info = extractProto(collector);
60-
assertThat(info.getMissingInterfaceList())
61-
.containsExactly(
62-
Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("b")).build(),
63-
Dependency.newBuilder().setOrigin(wrapType("a")).setTarget(wrapType("c")).build(),
64-
Dependency.newBuilder().setOrigin(wrapType("c")).setTarget(wrapType("b")).build());
58+
assertThat(info.getMissingInterfaceList().get(0)).isEqualTo(dependency("a", "b"));
59+
assertThat(info.getMissingInterfaceList().get(1)).isEqualTo(dependency("a", "c"));
60+
assertThat(info.getMissingInterfaceList().get(2)).isEqualTo(dependency("c", "b"));
6561
}
6662

6763
@Test
6864
public void testRecordExtendedInterfaces() throws Exception {
6965
MetadataCollector collector = new MetadataCollector(false);
70-
collector.recordExtendedInterfaces("a", "b", "c");
71-
collector.recordExtendedInterfaces("b");
7266
collector.recordExtendedInterfaces("c", "d");
67+
collector.recordExtendedInterfaces("a", "c", "b");
68+
collector.recordExtendedInterfaces("b");
7369

7470
DesugarDepsInfo info = extractProto(collector);
75-
assertThat(info.getInterfaceWithSupertypesList())
76-
.containsExactly(
77-
InterfaceDetails.newBuilder()
78-
.setOrigin(wrapType("a"))
79-
.addAllExtendedInterface(ImmutableList.of(wrapType("b"), wrapType("c")))
80-
.build(),
81-
InterfaceDetails.newBuilder()
82-
.setOrigin(wrapType("c"))
83-
.addAllExtendedInterface(ImmutableList.of(wrapType("d")))
84-
.build());
71+
72+
assertThat(info.getInterfaceWithSupertypesList().get(0))
73+
.isEqualTo(interfaceDetails("a", "b", "c"));
74+
assertThat(info.getInterfaceWithSupertypesList().get(0).getExtendedInterfaceList().get(0))
75+
.isEqualTo(wrapType("b"));
76+
assertThat(info.getInterfaceWithSupertypesList().get(0).getExtendedInterfaceList().get(1))
77+
.isEqualTo(wrapType("c"));
78+
79+
assertThat(info.getInterfaceWithSupertypesList().get(1)).isEqualTo(interfaceDetails("c", "d"));
8580
}
8681

8782
@Test
8883
public void testRecordDefaultMethods() throws Exception {
8984
MetadataCollector collector = new MetadataCollector(false);
90-
collector.recordDefaultMethods("a", 0);
9185
collector.recordDefaultMethods("b", 1);
86+
collector.recordDefaultMethods("a", 0);
9287

9388
DesugarDepsInfo info = extractProto(collector);
94-
assertThat(info.getInterfaceWithCompanionList())
95-
.containsExactly(
96-
InterfaceWithCompanion.newBuilder()
97-
.setOrigin(wrapType("a"))
98-
.setNumDefaultMethods(0)
99-
.build(),
100-
InterfaceWithCompanion.newBuilder()
101-
.setOrigin(wrapType("b"))
102-
.setNumDefaultMethods(1)
103-
.build());
89+
assertThat(info.getInterfaceWithCompanionList().get(0))
90+
.isEqualTo(interfaceWithCompanion("a", 0));
91+
assertThat(info.getInterfaceWithCompanionList().get(1))
92+
.isEqualTo(interfaceWithCompanion("b", 1));
93+
}
94+
95+
private DesugarDeps.InterfaceWithCompanion interfaceWithCompanion(String origin, int count) {
96+
return DesugarDeps.InterfaceWithCompanion.newBuilder()
97+
.setOrigin(wrapType(origin))
98+
.setNumDefaultMethods(count)
99+
.build();
100+
}
101+
102+
private DesugarDeps.InterfaceDetails interfaceDetails(String originName, String... interfaces) {
103+
return InterfaceDetails.newBuilder()
104+
.setOrigin(wrapType(originName))
105+
.addAllExtendedInterface(
106+
Arrays.stream(interfaces)
107+
.map(MetadataCollectorTest::wrapType)
108+
.collect(toImmutableList()))
109+
.build();
110+
}
111+
112+
private DesugarDeps.Dependency dependency(String origin, String target) {
113+
return DesugarDeps.Dependency.newBuilder()
114+
.setOrigin(wrapType(origin))
115+
.setTarget(wrapType(target))
116+
.build();
104117
}
105118

106119
private static DesugarDeps.Type wrapType(String name) {

‎src/tools/android/java/com/google/devtools/build/android/desugar/dependencies/MetadataCollector.java

+52-11
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,21 @@
2222
import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceDetails;
2323
import com.google.devtools.build.android.desugar.proto.DesugarDeps.InterfaceWithCompanion;
2424
import com.google.devtools.build.android.r8.DependencyCollector;
25+
import java.util.ArrayList;
26+
import java.util.Comparator;
2527
import javax.annotation.Nullable;
2628

2729
/** Dependency collector that emits collected metadata as a {@link DesugarDepsInfo} proto. */
2830
public final class MetadataCollector implements DependencyCollector {
2931

3032
private final boolean tolerateMissingDeps;
31-
private final DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder();
33+
34+
private final ArrayList<DesugarDeps.Dependency> assumePresents = new ArrayList<>();
35+
private final ArrayList<DesugarDeps.Dependency> missingInterfaces = new ArrayList<>();
36+
private final ArrayList<DesugarDeps.InterfaceDetails> interfacesWithSupertypes =
37+
new ArrayList<>();
38+
private final ArrayList<DesugarDeps.InterfaceWithCompanion> interfacesWithCompanion =
39+
new ArrayList<>();
3240

3341
public MetadataCollector(boolean tolerateMissingDeps) {
3442
this.tolerateMissingDeps = tolerateMissingDeps;
@@ -43,8 +51,8 @@ private static boolean isInterfaceCompanionClass(String name) {
4351
public void assumeCompanionClass(String origin, String target) {
4452
checkArgument(
4553
isInterfaceCompanionClass(target), "target not a companion: %s -> %s", origin, target);
46-
info.addAssumePresent(
47-
Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)));
54+
assumePresents.add(
55+
Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)).build());
4856
}
4957

5058
@Override
@@ -59,37 +67,70 @@ public void missingImplementedInterface(String origin, String target) {
5967
"Couldn't find interface %s on the classpath for desugaring %s",
6068
target,
6169
origin);
62-
info.addMissingInterface(
63-
Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)));
70+
missingInterfaces.add(
71+
Dependency.newBuilder().setOrigin(wrapType(origin)).setTarget(wrapType(target)).build());
6472
}
6573

6674
@Override
6775
public void recordExtendedInterfaces(String origin, String... targets) {
6876
if (targets.length > 0) {
6977
InterfaceDetails.Builder details = InterfaceDetails.newBuilder().setOrigin(wrapType(origin));
78+
ArrayList<DesugarDeps.Type> types = new ArrayList<>();
7079
for (String target : targets) {
71-
details.addExtendedInterface(wrapType(target));
80+
types.add(wrapType(target));
7281
}
73-
info.addInterfaceWithSupertypes(details);
82+
types.sort(Comparator.comparing(DesugarDeps.Type::getBinaryName));
83+
details.addAllExtendedInterface(types);
84+
interfacesWithSupertypes.add(details.build());
7485
}
7586
}
7687

7788
@Override
7889
public void recordDefaultMethods(String origin, int count) {
7990
checkArgument(!isInterfaceCompanionClass(origin), "seems to be a companion: %s", origin);
80-
info.addInterfaceWithCompanion(
91+
interfacesWithCompanion.add(
8192
InterfaceWithCompanion.newBuilder()
8293
.setOrigin(wrapType(origin))
83-
.setNumDefaultMethods(count));
94+
.setNumDefaultMethods(count)
95+
.build());
8496
}
8597

8698
@Override
8799
@Nullable
88100
public byte[] toByteArray() {
89-
DesugarDepsInfo result = info.build();
90-
return DesugarDepsInfo.getDefaultInstance().equals(result) ? null : result.toByteArray();
101+
DesugarDeps.DesugarDepsInfo result = buildInfo();
102+
return DesugarDeps.DesugarDepsInfo.getDefaultInstance().equals(result)
103+
? null
104+
: result.toByteArray();
91105
}
92106

107+
private DesugarDeps.DesugarDepsInfo buildInfo() {
108+
109+
// Sort these for determinism.
110+
assumePresents.sort(dependencyComparator);
111+
missingInterfaces.sort(dependencyComparator);
112+
interfacesWithSupertypes.sort(interfaceDetailComparator);
113+
interfacesWithCompanion.sort(interFaceWithCompanionComparator);
114+
115+
DesugarDeps.DesugarDepsInfo.Builder info = DesugarDeps.DesugarDepsInfo.newBuilder();
116+
info.addAllAssumePresent(assumePresents);
117+
info.addAllMissingInterface(missingInterfaces);
118+
info.addAllInterfaceWithSupertypes(interfacesWithSupertypes);
119+
info.addAllInterfaceWithCompanion(interfacesWithCompanion);
120+
121+
return info.build();
122+
}
123+
124+
private static final Comparator<? super DesugarDeps.Dependency> dependencyComparator =
125+
Comparator.comparing((DesugarDeps.Dependency o) -> o.getOrigin().getBinaryName())
126+
.thenComparing((DesugarDeps.Dependency o) -> o.getTarget().getBinaryName());
127+
128+
private static final Comparator<? super DesugarDeps.InterfaceDetails> interfaceDetailComparator =
129+
Comparator.comparing((DesugarDeps.InterfaceDetails o) -> o.getOrigin().getBinaryName());
130+
131+
private static final Comparator<? super DesugarDeps.InterfaceWithCompanion>
132+
interFaceWithCompanionComparator = Comparator.comparing(o -> o.getOrigin().getBinaryName());
133+
93134
private static DesugarDeps.Type wrapType(String internalName) {
94135
return DesugarDeps.Type.newBuilder().setBinaryName(internalName).build();
95136
}

0 commit comments

Comments
 (0)