Make GlobsValue.Key#toString() returns deterministic results when c…

…ontaining multiple `GlobRequest`s PiperOrigin-RevId: 642744019 Change-Id: I5c9bfc50896963c8b4db4fbbb2488952262ecee4
bazelbuild · Jun 12, 2024 · fe3b2fb · fe3b2fb
1 parent 3695909
commit fe3b2fb
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 10 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobsFunction.java
@@ -131,7 +131,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
                 // IncludeParser.
                 PathFragment.EMPTY_FRAGMENT,
                 globRequest.getPattern(),
-                globRequest.getGlobOeration());
+                globRequest.getGlobOperation());
         state.globDrivers.add(
             new Driver(
                 new GlobComputationProducer(

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobsValue.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
+import static java.util.stream.Collectors.joining;
+
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.packages.Globber;
@@ -72,7 +74,7 @@ public String getPattern() {
       return pattern;
     }
 
-    public Operation getGlobOeration() {
+    public Operation getGlobOperation() {
       return globOperation;
     }
 
@@ -108,7 +110,7 @@ public int hashCode() {
      *
      * <p>@throws InvalidGlobPatternException if the pattern is not valid.
      */
-    public static GlobRequest create(String pattern, Globber.Operation globOeration)
+    public static GlobRequest create(String pattern, Globber.Operation globOperation)
         throws InvalidGlobPatternException {
       if (pattern.indexOf('?') != -1) {
         throw new InvalidGlobPatternException(pattern, "wildcard ? forbidden");
@@ -118,7 +120,7 @@ public static GlobRequest create(String pattern, Globber.Operation globOeration)
       if (error != null) {
         throw new InvalidGlobPatternException(pattern, error);
       }
-      return new GlobRequest(pattern, globOeration);
+      return new GlobRequest(pattern, globOperation);
     }
   }
 
@@ -226,12 +228,11 @@ public int hashCode() {
 
     @Override
     public String toString() {
-      StringBuilder stringBuilder = new StringBuilder();
-      stringBuilder.append(
-          String.format(
-              "<GlobsKey packageRoot = %s, packageIdentifier = %s, globRequests = %s>",
-              packageRoot, packageIdentifier, globRequests));
-      return stringBuilder.toString();
+      return String.format(
+          "<GlobsKey packageRoot = %s, packageIdentifier = %s, globRequests = [%s]>",
+          packageRoot,
+          packageIdentifier,
+          globRequests.stream().map(GlobRequest::toString).sorted().collect(joining(",")));
     }
 
     @Override

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -2076,6 +2076,7 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//third_party:guava",
+        "//third_party:guava-testlib",
         "//third_party:junit4",
         "//third_party:truth",
     ],

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobsValueTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobsValueTest.java
@@ -16,6 +16,7 @@
 import static com.google.common.truth.Truth.assertThat;
 
 import com.google.common.collect.ImmutableSet;
+import com.google.common.testing.EqualsTester;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.packages.Globber.Operation;
 import com.google.devtools.build.lib.skyframe.GlobsValue.GlobRequest;
@@ -51,4 +52,45 @@ public void testSerialization() throws Exception {
   private static void verifyEquivalent(GlobsValue.Key orig, GlobsValue.Key deserialized) {
     assertThat(deserialized).isSameInstanceAs(orig);
   }
+
+  @Test
+  public void testPrintingDeterministic() throws Exception {
+    PackageIdentifier packageId = PackageIdentifier.create("foo", PathFragment.create("//bar"));
+    Root packageRoot = Root.fromPath(FsUtils.TEST_FILESYSTEM.getPath("/packageRoot"));
+
+    GlobRequest globRequest1 = GlobRequest.create("*", Operation.FILES_AND_DIRS);
+    GlobRequest globRequest2 = GlobRequest.create("foo/**", Operation.SUBPACKAGES);
+    GlobRequest globRequest3 = GlobRequest.create("**/*", Operation.FILES);
+
+    GlobsValue.Key key1 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest1, globRequest2, globRequest3));
+    GlobsValue.Key key2 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest1, globRequest3, globRequest2));
+    GlobsValue.Key key3 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest2, globRequest1, globRequest3));
+    GlobsValue.Key key4 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest2, globRequest3, globRequest1));
+    GlobsValue.Key key5 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest3, globRequest1, globRequest2));
+    GlobsValue.Key key6 =
+        GlobsValue.key(
+            packageId, packageRoot, ImmutableSet.of(globRequest3, globRequest2, globRequest1));
+    new EqualsTester()
+        .addEqualityGroup(
+            key1.toString(),
+            key2.toString(),
+            key3.toString(),
+            key4.toString(),
+            key5.toString(),
+            key6.toString(),
+            "<GlobsKey packageRoot = /packageRoot, packageIdentifier = @@foo///bar,"
+                + " globRequests = [GlobRequest: * FILES_AND_DIRS,GlobRequest: **/* FILES,"
+                + "GlobRequest: foo/** SUBPACKAGES]>")
+        .testEquals();
+  }
 }