Make `GlobsValue.Key#toString()` returns deterministic results when containing multiple `GlobRequest`s PiperOrigin-RevId: 642744019 Change-Id: I5c9bfc50896963c8b4db4fbbb2488952262ecee4
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 index d9c56f4..34724f5 100644 --- 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 @@ // 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 index 10b9ebb..9cb7436 100644 --- 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 @@ return pattern; } - public Operation getGlobOeration() { + public Operation getGlobOperation() { return globOperation; } @@ -108,7 +110,7 @@ * * <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 @@ if (error != null) { throw new InvalidGlobPatternException(pattern, error); } - return new GlobRequest(pattern, globOeration); + return new GlobRequest(pattern, globOperation); } } @@ -226,12 +228,11 @@ @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 index 82961cd..73d5aec 100644 --- 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 @@ "//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 index 9a24dbb..a468742 100644 --- 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 @@ 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(); + } }