Announce Tree objects as being topologically sorted

Now that UploadManifest has been patched up to emit topologically sorted Tree objects, let's announce them as such. This allows consumers to make stronger assumptions about their shape.
bazelbuild · Oct 23, 2022 · 530c6a7 · 530c6a7
1 parent 8cd2a09
commit 530c6a7
Show file tree

Hide file tree

Showing 4 changed files with 82 additions and 21 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java
@@ -335,7 +335,8 @@ private void addDirectory(Path dir) throws ExecException, IOException {
       result
           .addOutputDirectoriesBuilder()
           .setPath(remotePathResolver.localPathToOutputPath(dir))
-          .setTreeDigest(digest);
+          .setTreeDigest(digest)
+          .setIsTopologicallySorted(true);
     }
 
     digestToBlobs.put(digest, data);

diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java
@@ -369,7 +369,10 @@ public void updateActionResult(
         .setPath("a/foo")
         .setDigest(fooDigest)
         .setIsExecutable(true);
-    expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result).isEqualTo(expectedResult.build());
   }
 
@@ -409,7 +412,10 @@ public void updateActionResult(
 
     ActionResult result = uploadDirectory(remoteCache, ImmutableList.<Path>of(barDir));
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result).isEqualTo(expectedResult.build());
   }
 
@@ -472,7 +478,10 @@ public void updateActionResult(
 
     ActionResult result = uploadDirectory(remoteCache, ImmutableList.of(barDir));
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result).isEqualTo(expectedResult.build());
   }
 

diff --git a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java
@@ -1345,7 +1345,10 @@ public void uploadOutputs_uploadDirectory_works() throws Exception {
         .setPath("outputs/a/foo")
         .setDigest(fooDigest)
         .setIsExecutable(true);
-    expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("outputs/bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());
 
     ImmutableList<Digest> toQuery = ImmutableList.of(fooDigest, quxDigest, barDigest);
@@ -1383,7 +1386,10 @@ public void uploadOutputs_uploadEmptyDirectory_works() throws Exception {
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("outputs/bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());
     assertThat(
             getFromFuture(
@@ -1448,7 +1454,10 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception {
 
     // assert
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("outputs/bar").setTreeDigest(barDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("outputs/bar")
+        .setTreeDigest(barDigest)
+        .setIsTopologicallySorted(true);
     assertThat(manifest.getActionResult()).isEqualTo(expectedResult.build());
 
     ImmutableList<Digest> toQuery = ImmutableList.of(wobbleDigest, quxDigest, barDigest);

diff --git a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java
@@ -251,7 +251,10 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkAsDirectory() th
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("link").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("link")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -311,7 +314,10 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkAsDirectory()
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("link").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("link")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -371,7 +377,10 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkAsDirectory() th
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("link").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("link")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -572,7 +581,10 @@ public void actionResult_followSymlinks_absoluteFileSymlinkInDirectoryAsFile() t
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -617,7 +629,10 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkInDirectoryAsDir
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -653,7 +668,10 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkInDirectoryAsFile()
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -698,7 +716,10 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkInDirectoryAsD
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -733,7 +754,10 @@ public void actionResult_followSymlinks_relativeFileSymlinkInDirectoryAsFile() t
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -778,7 +802,10 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkInDirectoryAsDir
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -813,7 +840,10 @@ public void actionResult_noFollowSymlinks_relativeFileSymlinkInDirectoryAsSymlin
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -850,7 +880,10 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -935,7 +968,10 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -970,7 +1006,10 @@ public void actionResult_noFollowSymlinks_relativeDirectorySymlinkInDirectoryAsS
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }
 
@@ -1118,7 +1157,10 @@ public void actionResult_topologicallySortedAndDeduplicatedTree()
     Digest treeDigest = digestUtil.compute(tree);
 
     ActionResult.Builder expectedResult = ActionResult.newBuilder();
-    expectedResult.addOutputDirectoriesBuilder().setPath("dir").setTreeDigest(treeDigest);
+    expectedResult.addOutputDirectoriesBuilder()
+        .setPath("dir")
+        .setTreeDigest(treeDigest)
+        .setIsTopologicallySorted(true);
     assertThat(result.build()).isEqualTo(expectedResult.build());
   }