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 index 2dd79a67edcf9b..3ca5f30e479da1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java +++ b/src/main/java/com/google/devtools/build/lib/remote/UploadManifest.java @@ -202,6 +202,10 @@ private void setStdoutStderr(FileOutErr outErr) throws IOException { * non-symlink where a symlink was expected). Outputs are always uploaded according to the * filesystem state, possibly after applying the transformation implied by {@link followSymlinks}. * A type mismatch may later cause execution to fail, but that's an action-level concern. + * + *

All files are uploaded with the executable bit set, in accordance with input Merkle trees. + * This does not affect correctness since we always set the output permissions to 0555 or 0755 + * after execution, both for cache hits and misses. */ @VisibleForTesting void addFiles(Collection files) throws ExecException, IOException { @@ -332,7 +336,6 @@ private void addFile(Digest digest, Path file) throws IOException { .addOutputFilesBuilder() .setPath(remotePathResolver.localPathToOutputPath(file)) .setDigest(digest) - // The permission of output file is changed to 0555 after action execution .setIsExecutable(true); digestToFile.put(digest, file); @@ -395,7 +398,7 @@ private ByteString computeDirectory(Path path, LinkedHashSet directo Path child = path.getRelative(name); if (dirent.getType() == Dirent.Type.FILE) { Digest digest = digestUtil.compute(child); - b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(child.isExecutable()); + b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(true); digestToFile.put(digest, child); continue; } @@ -421,7 +424,7 @@ private ByteString computeDirectory(Path path, LinkedHashSet directo if (statFollow.isFile() && !statFollow.isSpecialFile()) { // Symlink to file uploaded as a file. Digest digest = digestUtil.compute(child); - b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(child.isExecutable()); + b.addFilesBuilder().setName(name).setDigest(digest).setIsExecutable(true); digestToFile.put(digest, child); continue; } 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 index f46a0be51741df..4df6a0cd31eddb 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcCacheClientTest.java @@ -640,7 +640,12 @@ public void testUploadDirectoryNested() throws Exception { fakeFileCache.createScratchInput(ActionInputHelper.fromPath("bar/qux"), "abc"); final Directory testDirMessage = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("wobble").setDigest(wobbleDigest).build()) + .addFiles( + FileNode.newBuilder() + .setName("wobble") + .setDigest(wobbleDigest) + .setIsExecutable(true) + .build()) .build(); final Digest testDigest = DIGEST_UTIL.compute(testDirMessage); final Tree barTree = @@ -649,9 +654,9 @@ public void testUploadDirectoryNested() throws Exception { Directory.newBuilder() .addFiles( FileNode.newBuilder() - .setIsExecutable(true) .setName("qux") - .setDigest(quxDigest)) + .setDigest(quxDigest) + .setIsExecutable(true)) .addDirectories( DirectoryNode.newBuilder().setName("test").setDigest(testDigest))) .addChildren(testDirMessage) 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 index 81a81e71d3bdc1..06693d92c34845 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/RemoteExecutionServiceTest.java @@ -306,7 +306,11 @@ public void downloadOutputs_executableBitIgnored() throws Exception { Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("bar").setDigest(barDigest))) + .addFiles( + FileNode.newBuilder() + .setName("bar") + .setDigest(barDigest) + .setIsExecutable(true))) .build(); Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); ActionResult.Builder builder = ActionResult.newBuilder(); @@ -401,8 +405,16 @@ public void downloadOutputs_outputDirectories() throws Exception { Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(fooDigest)) - .addFiles(FileNode.newBuilder().setName("subdir/bar").setDigest(barDigest))) + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(fooDigest) + .setIsExecutable(true)) + .addFiles( + FileNode.newBuilder() + .setName("subdir/bar") + .setDigest(barDigest) + .setIsExecutable(true))) .build(); Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); ActionResult.Builder builder = ActionResult.newBuilder(); @@ -469,7 +481,8 @@ public void downloadOutputs_nestedOutputDirectories_works() throws Exception { Digest quxDigest = cache.addContents(remoteActionExecutionContext, "qux-contents"); Directory wobbleDirMessage = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("qux").setDigest(quxDigest)) + .addFiles( + FileNode.newBuilder().setName("qux").setDigest(quxDigest).setIsExecutable(true)) .build(); Digest wobbleDirDigest = cache.addContents(remoteActionExecutionContext, wobbleDirMessage.toByteArray()); @@ -477,7 +490,11 @@ public void downloadOutputs_nestedOutputDirectories_works() throws Exception { Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("qux").setDigest(quxDigest)) + .addFiles( + FileNode.newBuilder() + .setName("qux") + .setDigest(quxDigest) + .setIsExecutable(true)) .addDirectories( DirectoryNode.newBuilder().setName("wobble").setDigest(wobbleDirDigest))) .addChildren(wobbleDirMessage) @@ -518,8 +535,16 @@ public void downloadOutputs_outputDirectoriesWithNestedFile_works() throws Excep Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(fooDigest)) - .addFiles(FileNode.newBuilder().setName("bar").setDigest(barDigest))) + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(fooDigest) + .setIsExecutable(true)) + .addFiles( + FileNode.newBuilder() + .setName("bar") + .setDigest(barDigest) + .setIsExecutable(true))) .build(); Digest subdirTreeDigest = cache.addContents(remoteActionExecutionContext, subdirTreeMessage.toByteArray()); @@ -559,7 +584,8 @@ public void downloadOutputs_outputDirectoriesWithSameHash_works() throws Excepti // arrange Digest fileDigest = cache.addContents(remoteActionExecutionContext, "file"); - FileNode file = FileNode.newBuilder().setName("file").setDigest(fileDigest).build(); + FileNode file = + FileNode.newBuilder().setName("file").setDigest(fileDigest).setIsExecutable(true).build(); Directory fooDir = Directory.newBuilder().addFiles(file).build(); Digest fooDigest = cache.addContents(remoteActionExecutionContext, fooDir.toByteArray()); DirectoryNode fooDirNode = @@ -815,7 +841,11 @@ public void downloadOutputs_onActionFailure_downloadEverything() throws Exceptio Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("bar").setDigest(barDigest))) + .addFiles( + FileNode.newBuilder() + .setName("bar") + .setDigest(barDigest) + .setIsExecutable(true))) .build(); Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); ActionResult.Builder builder = ActionResult.newBuilder(); @@ -850,7 +880,10 @@ public void downloadOutputs_onDownloadFailure_maintainDirectories() throws Excep .setRoot( Directory.newBuilder() .addFiles( - FileNode.newBuilder().setName("outputfile").setDigest(treeFileDigest))) + FileNode.newBuilder() + .setName("outputfile") + .setDigest(treeFileDigest) + .setIsExecutable(true))) .build(); Digest treeDigest = cache.addContents(remoteActionExecutionContext, tree.toByteArray()); Digest otherFileDigest = @@ -1201,8 +1234,10 @@ public void downloadOutputs_outputDirectories_partialDownload() throws Exception // dir/a/file2 Digest d1 = cache.addContents(remoteActionExecutionContext, "content1"); Digest d2 = cache.addContents(remoteActionExecutionContext, "content2"); - FileNode file1 = FileNode.newBuilder().setName("file1").setDigest(d1).build(); - FileNode file2 = FileNode.newBuilder().setName("file2").setDigest(d2).build(); + FileNode file1 = + FileNode.newBuilder().setName("file1").setDigest(d1).setIsExecutable(true).build(); + FileNode file2 = + FileNode.newBuilder().setName("file2").setDigest(d2).setIsExecutable(true).build(); Directory a = Directory.newBuilder().addFiles(file2).build(); Digest da = cache.addContents(remoteActionExecutionContext, a); Directory root = @@ -1251,8 +1286,10 @@ public void downloadOutputs_outputDirectories_noDownload() throws Exception { // dir/a/file2 Digest d1 = cache.addContents(remoteActionExecutionContext, "content1"); Digest d2 = cache.addContents(remoteActionExecutionContext, "content2"); - FileNode file1 = FileNode.newBuilder().setName("file1").setDigest(d1).build(); - FileNode file2 = FileNode.newBuilder().setName("file2").setDigest(d2).build(); + FileNode file1 = + FileNode.newBuilder().setName("file1").setDigest(d1).setIsExecutable(true).build(); + FileNode file2 = + FileNode.newBuilder().setName("file2").setDigest(d2).setIsExecutable(true).build(); Directory a = Directory.newBuilder().addFiles(file2).build(); Digest da = cache.addContents(remoteActionExecutionContext, a); Directory root = @@ -1303,8 +1340,10 @@ public void downloadOutputs_outputDirectories_doNotDownload_failProperly() throw // dir/a/file2 Digest d1 = cache.addContents(remoteActionExecutionContext, "content1"); Digest d2 = cache.addContents(remoteActionExecutionContext, "content2"); - FileNode file1 = FileNode.newBuilder().setName("file1").setDigest(d1).build(); - FileNode file2 = FileNode.newBuilder().setName("file2").setDigest(d2).build(); + FileNode file1 = + FileNode.newBuilder().setName("file1").setDigest(d1).setIsExecutable(true).build(); + FileNode file2 = + FileNode.newBuilder().setName("file2").setDigest(d2).setIsExecutable(true).build(); Directory a = Directory.newBuilder().addFiles(file2).build(); Digest da = cache.addContents(remoteActionExecutionContext, a); Directory root = @@ -1577,9 +1616,9 @@ public void uploadOutputs_uploadDirectory_works() throws Exception { Directory.newBuilder() .addFiles( FileNode.newBuilder() - .setIsExecutable(true) .setName("qux") .setDigest(quxDigest) + .setIsExecutable(true) .build()) .build()) .build()); @@ -1680,7 +1719,12 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception { fakeFileCache.createScratchInput(ActionInputHelper.fromPath("outputs/bar/qux"), "abc"); final Directory testDirMessage = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("wobble").setDigest(wobbleDigest).build()) + .addFiles( + FileNode.newBuilder() + .setName("wobble") + .setDigest(wobbleDigest) + .setIsExecutable(true) + .build()) .build(); final Digest testDigest = digestUtil.compute(testDirMessage); final Tree barTree = @@ -1689,9 +1733,9 @@ public void uploadOutputs_uploadNestedDirectory_works() throws Exception { Directory.newBuilder() .addFiles( FileNode.newBuilder() - .setIsExecutable(true) .setName("qux") - .setDigest(quxDigest)) + .setDigest(quxDigest) + .setIsExecutable(true)) .addDirectories( DirectoryNode.newBuilder().setName("test").setDigest(testDigest))) .addChildren(testDirMessage) 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 index 76aa2da64d152a..8d85f8930e7bd9 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/UploadManifestTest.java @@ -247,7 +247,11 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkAsDirectory() th Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -311,7 +315,11 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkAsDirectory() Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -375,7 +383,11 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkAsDirectory() th Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -631,7 +643,11 @@ public void actionResult_followSymlinks_absoluteFileSymlinkInDirectoryAsFile() t Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("link").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("link") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -671,7 +687,7 @@ public void actionResult_followSymlinks_absoluteDirectorySymlinkInDirectoryAsDir Directory barDir = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest)) + .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest).setIsExecutable(true)) .build(); Digest barDigest = digestUtil.compute(barDir); Tree tree = @@ -720,7 +736,11 @@ public void actionResult_noFollowSymlinks_absoluteFileSymlinkInDirectoryAsFile() Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("link").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("link") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -760,7 +780,7 @@ public void actionResult_noFollowSymlinks_absoluteDirectorySymlinkInDirectoryAsD Directory barDir = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest)) + .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest).setIsExecutable(true)) .build(); Digest barDigest = digestUtil.compute(barDir); Tree tree = @@ -808,7 +828,11 @@ public void actionResult_followSymlinks_relativeFileSymlinkInDirectoryAsFile() t Tree.newBuilder() .setRoot( Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("link").setDigest(digest))) + .addFiles( + FileNode.newBuilder() + .setName("link") + .setDigest(digest) + .setIsExecutable(true))) .build(); Digest treeDigest = digestUtil.compute(tree); @@ -848,7 +872,7 @@ public void actionResult_followSymlinks_relativeDirectorySymlinkInDirectoryAsDir Directory barDir = Directory.newBuilder() - .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest)) + .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest).setIsExecutable(true)) .build(); Digest barDigest = digestUtil.compute(barDir); Tree tree = @@ -1278,7 +1302,10 @@ public void actionResult_topologicallySortedAndDeduplicatedTree() throws Excepti Directory root = Directory.newBuilder() .addFiles( - FileNode.newBuilder().setName("file").setDigest(digestUtil.compute(fileContents))) + FileNode.newBuilder() + .setName("file") + .setDigest(digestUtil.compute(fileContents)) + .setIsExecutable(true)) .build(); for (int depth = 0; depth < 3; depth++) { Directory.Builder b = Directory.newBuilder();