[7.1.0] Set the executable bit on files in output directories uploade…

…d to a disk or remote cache.

Bazel doesn't preserve executable bits for outputs; all files in the output tree are chmodded to either 0555 or 0755 after action execution, depending on --experimental_writable_outputs.

With respect to the disk/remote cache protocol, Bazel always marks uploaded inputs as executable and always ignores the executable bit on downloaded outputs. For uploaded outputs, the behavior currently differs between directories and non-directories; this CL makes the behavior consistent.

This makes it more likely that the input Merkle tree to a remote action can hit a cache populated by a previous local action. (See unknown commit where the behavior was changed for non-directory outputs, with the same rationale.)

As a minor effect, it also avoids additional I/O to obtain the permission bits from the filesystem, which adds up for very large tree artifacts.

PiperOrigin-RevId: 607367059
Change-Id: Ib507a98f32c0a5c89b1ed0f1ec3777f1c6430e28

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.
+ *
+ * <p>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<Path> 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<ByteString> 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<ByteString> 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;
  }

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)

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,15 +481,20 @@ 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());
  Tree barTreeMessage =
  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)

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();