Remote: Fixed a bug that remote cache is missed due to executable bit…

… is different. bazelbuild#13262

bazelbuild#12820 changed to set executable bit of input files based on its real value. However, this causes cache misses in --remote_download_toplevel mode since executable bit is changed after action execution by ActionMetadataHandler#getMetadata. This change effectively rolls back bazelbuild#12820.

src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java

@@ -74,6 +74,18 @@ static class FileNode extends Node {
     private final Digest digest;
     private final boolean isExecutable;
 
+    /**
+     * Create a FileNode with its executable bit set.
+     *
+     * <p>We always treat files as executable since Bazel will `chmod 555` on the output files of an
+     * action within ActionMetadataHandler#getMetadata after action execution if no metadata was
+     * injected. We can't use real executable bit of the file until this behaviour is changed. See
+     * https://github.com/bazelbuild/bazel/issues/13262 for more details.
+     */
+    static FileNode createExecutable(String pathSegment, Path path, Digest digest) {
+      return new FileNode(pathSegment, path, digest, /* isExecutable= */ true);
+    }
+
     FileNode(String pathSegment, Path path, Digest digest, boolean isExecutable) {
       super(pathSegment);
       this.path = Preconditions.checkNotNull(path, "path");

src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java

@@ -102,7 +102,7 @@ private static int buildFromPaths(
             throw new IOException(String.format("Input '%s' is not a file.", input));
           }
           Digest d = digestUtil.compute(input);
-          currDir.addChild(new FileNode(path.getBaseName(), input, d, input.isExecutable()));
+          currDir.addChild(FileNode.createExecutable(path.getBaseName(), input, d));
           return 1;
         });
   }
@@ -141,15 +141,7 @@ private static int buildFromActionInputs(
             case REGULAR_FILE:
               Digest d = DigestUtil.buildDigest(metadata.getDigest(), metadata.getSize());
               Path inputPath = ActionInputHelper.toInputPath(input, execRoot);
-
-              boolean isExecutable;
-              if (metadata instanceof RemoteActionFileArtifactValue) {
-                isExecutable = ((RemoteActionFileArtifactValue) metadata).isExecutable();
-              } else {
-                isExecutable = inputPath.isExecutable();
-              }
-
-              currDir.addChild(new FileNode(path.getBaseName(), inputPath, d, isExecutable));
+              currDir.addChild(FileNode.createExecutable(path.getBaseName(), inputPath, d));
               return 1;
 
             case DIRECTORY:

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -2194,26 +2194,70 @@ EOF
     @local_foo//:all
 }
 
-function test_remote_input_files_executable_bit() {
-  # Test that input files uploaded to remote executor have the same executable bit with local files. #12818
+function test_remote_cache_works_for_non_executable_intermediate_outputs() {
+  # test that remote cache is hit when intermediate output is not executable
   touch WORKSPACE
   cat > BUILD <<'EOF'
+genrule(
+  name = "dep",
+  srcs = [],
+  outs = ["dep"],
+  cmd = "echo 'dep' > $@",
+)
+
 genrule(
   name = "test",
-  srcs = ["foo.txt", "bar.sh"],
-  outs = ["out.txt"],
-  cmd = "ls -l $(SRCS); touch $@",
+  srcs = [":dep"],
+  outs = ["out"],
+  cmd = "cat $(SRCS) > $@",
 )
 EOF
-  touch foo.txt bar.sh
-  chmod a+x bar.sh
 
   bazel build \
-    --remote_executor=grpc://localhost:${worker_port} \
+    --remote_cache=grpc://localhost:${worker_port} \
+    //:test >& $TEST_log || fail "Failed to build //:test"
+
+  bazel clean
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    //:test >& $TEST_log || fail "Failed to build //:test"
+
+  expect_log "2 remote cache hit"
+}
+
+function test_remote_cache_works_for_non_executable_intermediate_outputs_with_toplevel_mode() {
+  # test that remote cache is hit when intermediate output is not executable in remote download toplevel mode
+  touch WORKSPACE
+  cat > BUILD <<'EOF'
+genrule(
+  name = "dep",
+  srcs = [],
+  outs = ["dep"],
+  cmd = "echo 'dep' > $@",
+)
+
+genrule(
+  name = "test",
+  srcs = [":dep"],
+  outs = ["out"],
+  cmd = "cat $(SRCS) > $@",
+)
+EOF
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    --remote_download_toplevel \
+    //:test >& $TEST_log || fail "Failed to build //:test"
+
+  bazel clean
+
+  bazel build \
+    --remote_cache=grpc://localhost:${worker_port} \
+    --remote_download_toplevel \
     //:test >& $TEST_log || fail "Failed to build //:test"
 
-  expect_log "-rwxr--r-- .* bar.sh"
-  expect_log "-rw-r--r-- .* foo.txt"
+  expect_log "2 remote cache hit"
 }
 
 function test_exclusive_tag() {