diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java index 19d4b33c4e1ccb..5d3cb7eefa3014 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTree.java +++ b/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. + * + *

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"); diff --git a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java b/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java index 4503f741dbbfae..87b6fe9905f42e 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/merkletree/DirectoryTreeBuilder.java +++ b/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: diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index dce00f2b848d26..b5ebe0cfe517e2 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/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() {