diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 925dfcc790291a..49b53d7d43bde8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -54,6 +54,7 @@ java_library( "LocalHostResourceManagerDarwin.java", "LocalHostResourceFallback.java", "MiddlemanType.java", + "RemoteFileStatus.java", "ResourceManager.java", "ResourceSet.java", "ResourceSetOrBuilder.java", @@ -263,6 +264,7 @@ java_library( "FileStateType.java", "FileStateValue.java", "FileValue.java", + "RemoteFileStatus.java", "cache/MetadataDigestUtils.java", ], deps = [ diff --git a/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java b/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java new file mode 100644 index 00000000000000..365be160aab4fc --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/actions/RemoteFileStatus.java @@ -0,0 +1,8 @@ +package com.google.devtools.build.lib.actions; + +import com.google.devtools.build.lib.actions.FileArtifactValue.RemoteFileArtifactValue; +import com.google.devtools.build.lib.vfs.FileStatusWithDigest; + +public interface RemoteFileStatus extends FileStatusWithDigest { + RemoteFileArtifactValue getRemoteMetadata(); +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java index da3215392b6d53..b584de22edadb3 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java @@ -40,6 +40,7 @@ import com.google.devtools.build.lib.vfs.FileSystem; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.inmemoryfs.FileInfo; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryContentInfo; @@ -128,41 +129,7 @@ void flush() throws IOException { PathFragment path = execRoot.getRelative(entry.getKey()); Artifact output = entry.getValue(); - if (maybeInjectMetadataForSymlink(path, output)) { - continue; - } - - if (output.isTreeArtifact()) { - if (remoteOutputTree.exists(path)) { - SpecialArtifact parent = (SpecialArtifact) output; - TreeArtifactValue.Builder tree = TreeArtifactValue.newBuilder(parent); - - // TODO: Check directory content on the local fs to support mixed tree. - TreeArtifactValue.visitTree( - remoteOutputTree.getPath(path), - (parentRelativePath, type) -> { - if (type == Dirent.Type.DIRECTORY) { - return; - } - RemoteFileInfo remoteFile = - remoteOutputTree.getRemoteFileInfo( - path.getRelative(parentRelativePath), /* followSymlinks= */ true); - if (remoteFile != null) { - TreeFileArtifact child = - TreeFileArtifact.createTreeOutput(parent, parentRelativePath); - tree.putChild(child, createRemoteMetadata(remoteFile)); - } - }); - - metadataInjector.injectTree(parent, tree.build()); - } - } else { - RemoteFileInfo remoteFile = - remoteOutputTree.getRemoteFileInfo(path, /* followSymlinks= */ true); - if (remoteFile != null) { - metadataInjector.injectFile(output, createRemoteMetadata(remoteFile)); - } - } + maybeInjectMetadataForSymlink(path, output); } } @@ -498,7 +465,12 @@ protected FileStatus stat(PathFragment path, boolean followSymlinks) throws IOEx } private static FileStatus statFromRemoteMetadata(RemoteFileArtifactValue m) { - return new FileStatus() { + return new RemoteFileStatus() { + @Override + public byte[] getDigest() { + return m.getDigest(); + } + @Override public boolean isFile() { return m.getType().isFile(); @@ -538,6 +510,11 @@ public long getLastChangeTime() { public long getNodeId() { throw new UnsupportedOperationException("Cannot get node id for " + m); } + + @Override + public RemoteFileArtifactValue getRemoteMetadata() { + return m; + } }; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java index 77104d996b1ee3..171c664f439ead 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/ActionMetadataHandler.java @@ -47,6 +47,7 @@ import com.google.devtools.build.lib.vfs.FileStatusWithDigestAdapter; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.actions.RemoteFileStatus; import com.google.devtools.build.lib.vfs.RootedPath; import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.XattrProvider; @@ -656,13 +657,19 @@ private static FileArtifactValue fileArtifactValueFromStat( return FileArtifactValue.MISSING_FILE_MARKER; } + if (stat.isDirectory()) { + return FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime()); + } + + if (stat instanceof RemoteFileStatus) { + return ((RemoteFileStatus) stat).getRemoteMetadata(); + } + FileStateValue fileStateValue = FileStateValue.createWithStatNoFollow( rootedPath, stat, digestWillBeInjected, xattrProvider, tsgm); - return stat.isDirectory() - ? FileArtifactValue.createForDirectoryWithMtime(stat.getLastModifiedTime()) - : FileArtifactValue.createForNormalFile( + return FileArtifactValue.createForNormalFile( fileStateValue.getDigest(), fileStateValue.getContentsProxy(), stat.getSize()); } diff --git a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh index 477a2f0bd8335f..e0c4bb09fe9eef 100755 --- a/src/test/shell/bazel/remote/build_without_the_bytes_test.sh +++ b/src/test/shell/bazel/remote/build_without_the_bytes_test.sh @@ -1341,4 +1341,46 @@ EOF expect_log "bin-message" } +function test_tree_artifact_from_local_file_system() { + # Test that tree artifact generated locally can be consumed by other actions. + # See https://github.com/bazelbuild/bazel/issues/16789 + + mkdir -p a + cat > a/my_rule.bzl <<'EOF' +def _impl(ctx): + out = ctx.actions.declare_directory(ctx.label.name) + ctx.actions.run_shell( + outputs = [out], + command = "echo '1' > {out}/my_file".format(out = out.path), + ) + return DefaultInfo( + files = depset([out]), + runfiles = ctx.runfiles(files = [out]) + ) +my_rule = rule( + implementation = _impl, + attrs = {}, +) +EOF + cat > a/out_dir_test.sh <<'EOF' +cat $1/my_file +EOF + chmod a+x a/out_dir_test.sh + cat > a/BUILD <<'EOF' +load(":my_rule.bzl", "my_rule") +my_rule(name = "out_dir") +sh_test( + name = "out_dir_test", + srcs = ["out_dir_test.sh"], + args = ["$(rootpath :out_dir)"], + data = [":out_dir"], +) +EOF + + bazel test \ + --remote_cache=grpc://localhost:${worker_port} \ + --remote_download_minimal \ + //a:out_dir_test >& $TEST_log || fail "Failed to test //a:out_dir_test" +} + run_suite "Build without the Bytes tests"