Make BuildOptions$OptionsDiffForReconstruction deterministic in its construction. Also restrict visibility of some test-only OptionsDiff methods, and remove a server-specific part of the ConfiguredTargetKey #toString representation.

PiperOrigin-RevId: 197830577

2 files changed, 25 insertions, 15 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
index a91649bf30..33cd98afc3 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/config/BuildOptions.java
@@ -426,10 +426,19 @@ public final class BuildOptions implements Cloneable, Serializable {
     }
     LinkedHashMap<Class<? extends FragmentOptions>, Map<String, Object>> differingOptions =
         new LinkedHashMap<>(diff.differingOptions.keySet().size());
-    for (Class<? extends FragmentOptions> clazz : diff.differingOptions.keySet()) {
+    for (Class<? extends FragmentOptions> clazz :
+        diff.differingOptions
+            .keySet()
+            .stream()
+            .sorted(lexicalFragmentOptionsComparator)
+            .collect(Collectors.toList())) {
       Collection<OptionDefinition> fields = diff.differingOptions.get(clazz);
       LinkedHashMap<String, Object> valueMap = new LinkedHashMap<>(fields.size());
-      for (OptionDefinition optionDefinition : fields) {
+      for (OptionDefinition optionDefinition :
+          fields
+              .stream()
+              .sorted(Comparator.comparing(o -> o.getField().getName()))
+              .collect(Collectors.toList())) {
         Object secondValue;
         try {
           secondValue = Iterables.getOnlyElement(diff.second.get(optionDefinition));
@@ -448,8 +457,14 @@ public final class BuildOptions implements Cloneable, Serializable {
     first.maybeInitializeFingerprintAndHashCode();
     return new OptionsDiffForReconstruction(
         differingOptions,
-        ImmutableSet.copyOf(diff.extraFirstFragments),
-        ImmutableList.copyOf(diff.extraSecondFragments),
+        diff.extraFirstFragments
+            .stream()
+            .sorted(lexicalFragmentOptionsComparator)
+            .collect(ImmutableSet.toImmutableSet()),
+        diff.extraSecondFragments
+            .stream()
+            .sorted(Comparator.comparing(o -> o.getClass().getName()))
+            .collect(ImmutableList.toImmutableList()),
         first.fingerprint,
         second.computeChecksum());
   }
@@ -480,14 +495,14 @@ public final class BuildOptions implements Cloneable, Serializable {
       extraSecondFragments.add(options);
     }
 
-    /** Return the extra fragments classes from the first configuration. */
-    public Set<Class<? extends FragmentOptions>> getExtraFirstFragmentClasses() {
+    @VisibleForTesting
+    Set<Class<? extends FragmentOptions>> getExtraFirstFragmentClassesForTesting() {
       return extraFirstFragments;
     }
 
-    /** Return the extra fragments classes from the second configuration. */
-    public Set<Class<?>> getExtraSecondFragmentClasses() {
-      return extraSecondFragments.stream().map(Object::getClass).collect(Collectors.toSet());
+    @VisibleForTesting
+    Set<FragmentOptions> getExtraSecondFragmentsForTesting() {
+      return extraSecondFragments;
     }
 
     public Map<OptionDefinition, Object> getFirst() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
index f9ac4b53d6..fa9bbd723d 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetKey.java
@@ -170,12 +170,7 @@ public class ConfiguredTargetKey extends ActionLookupKey {
 
   @Override
   public String toString() {
-    return String.format(
-        "%s %s %s (%s)",
-        label,
-        configurationKey,
-        isHostConfiguration(),
-        System.identityHashCode(this));
+    return String.format("%s %s %s", label, configurationKey, isHostConfiguration());
   }
 
   static class HostConfiguredTargetKey extends ConfiguredTargetKey {