diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java index 97c25704c511bd..5057f8f1ea6bc1 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnCache.java @@ -98,7 +98,11 @@ public CacheHandle lookup(Spawn spawn, SpawnExecutionContext context) SortedMap inputMap = context.getInputMapping(true); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! TreeNodeRepository repository = - new TreeNodeRepository(execRoot, context.getMetadataProvider(), digestUtil); + new TreeNodeRepository( + execRoot, + context.getMetadataProvider(), + digestUtil, + options.incompatibleRemoteSymlinks); TreeNode inputRoot; try (SilentCloseable c = Profiler.instance().profile("RemoteCache.computeMerkleDigests")) { inputRoot = repository.buildFromActionInputs(inputMap); diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java index 4417925575b5db..dff90981e433f3 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnRunner.java @@ -146,7 +146,9 @@ public SpawnResult exec(Spawn spawn, SpawnExecutionContext context) context.report(ProgressStatus.EXECUTING, getName()); // Temporary hack: the TreeNodeRepository should be created and maintained upstream! MetadataProvider inputFileCache = context.getMetadataProvider(); - TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); + TreeNodeRepository repository = + new TreeNodeRepository( + execRoot, inputFileCache, digestUtil, remoteOptions.incompatibleRemoteSymlinks); SortedMap inputMap = context.getInputMapping(true); TreeNode inputRoot; try (SilentCloseable c = Profiler.instance().profile("Remote.computeMerkleDigests")) { diff --git a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java index 3427a6c68cf7e0..13331981e06180 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java +++ b/src/main/java/com/google/devtools/build/lib/remote/SimpleBlobStoreActionCache.java @@ -21,6 +21,7 @@ import build.bazel.remote.execution.v2.Directory; import build.bazel.remote.execution.v2.DirectoryNode; import build.bazel.remote.execution.v2.FileNode; +import build.bazel.remote.execution.v2.SymlinkNode; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -37,6 +38,7 @@ import com.google.devtools.build.lib.remote.util.DigestUtil.ActionKey; import com.google.devtools.build.lib.util.io.FileOutErr; import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.PathFragment; import com.google.protobuf.ByteString; import com.google.protobuf.InvalidProtocolBufferException; import java.io.ByteArrayInputStream; @@ -97,6 +99,10 @@ public void downloadTree(Digest rootDigest, Path rootLocation) for (DirectoryNode child : directory.getDirectoriesList()) { downloadTree(child.getDigest(), rootLocation.getRelative(child.getName())); } + for (SymlinkNode symlink : directory.getSymlinksList()) { + PathFragment targetPath = PathFragment.create(symlink.getTarget()); + rootLocation.getRelative(symlink.getName()).createSymbolicLink(targetPath); + } } private Digest uploadFileContents(Path file) throws IOException, InterruptedException { diff --git a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java index 5d21fe6c83a66e..f684ac5fb96309 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java +++ b/src/main/java/com/google/devtools/build/lib/remote/TreeNodeRepository.java @@ -38,6 +38,7 @@ import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import com.google.devtools.build.lib.remote.util.DigestUtil; import com.google.devtools.build.lib.vfs.Dirent; +import com.google.devtools.build.lib.vfs.FileStatus; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.Symlinks; @@ -63,9 +64,6 @@ public final class TreeNodeRepository { private static final BaseEncoding LOWER_CASE_HEX = BaseEncoding.base16().lowerCase(); - // In this implementation, symlinks are NOT followed when expanding directory artifacts - public static final Symlinks SYMLINK_POLICY = Symlinks.NOFOLLOW; - private final Traverser traverser = Traverser.forTree((TreeNode node) -> children(node)); @@ -233,11 +231,17 @@ public String toDebugString() { private final Map virtualInputDigestCache = new HashMap<>(); private final Map digestVirtualInputCache = new HashMap<>(); private final DigestUtil digestUtil; + private final boolean uploadSymlinks; - public TreeNodeRepository(Path execRoot, MetadataProvider inputFileCache, DigestUtil digestUtil) { + public TreeNodeRepository( + Path execRoot, + MetadataProvider inputFileCache, + DigestUtil digestUtil, + boolean uploadSymlinks) { this.execRoot = execRoot; this.inputFileCache = inputFileCache; this.digestUtil = digestUtil; + this.uploadSymlinks = uploadSymlinks; } public MetadataProvider getInputFileCache() { @@ -287,7 +291,7 @@ public TreeNode buildFromActionInputs(SortedMap sorte // Expand the descendant of an artifact (input) directory private List buildInputDirectoryEntries(Path path) throws IOException { - List sortedDirent = new ArrayList<>(path.readdir(SYMLINK_POLICY)); + List sortedDirent = new ArrayList<>(path.readdir(Symlinks.NOFOLLOW)); sortedDirent.sort(Comparator.comparing(Dirent::getName)); List entries = new ArrayList<>(sortedDirent.size()); @@ -297,6 +301,17 @@ private List buildInputDirectoryEntries(Path path) throws I TreeNode childNode; if (dirent.getType() == Dirent.Type.DIRECTORY) { childNode = interner.intern(new TreeNode(buildInputDirectoryEntries(child), null)); + } else if (dirent.getType() == Dirent.Type.SYMLINK) { + PathFragment target = child.readSymbolicLink(); + // Need to resolve the symbolic link to know what to expand it to, file or directory. + FileStatus statFollow = child.statIfFound(Symlinks.FOLLOW); + Preconditions.checkNotNull(statFollow, "Dangling symbolic link %s to %s", child, target); + boolean uploadSymlinkAsDirectory = !uploadSymlinks || target.isAbsolute(); + if (statFollow.isDirectory() && uploadSymlinkAsDirectory) { + childNode = interner.intern(new TreeNode(buildInputDirectoryEntries(child), null)); + } else { + childNode = interner.intern(new TreeNode(ActionInputHelper.fromPath(child.asFragment()))); + } } else { childNode = interner.intern(new TreeNode(ActionInputHelper.fromPath(child.asFragment()))); } @@ -325,14 +340,25 @@ private TreeNode buildParentNode( Preconditions.checkArgument( inputsStart == inputsEnd - 1, "Encountered two inputs with the same path."); ActionInput input = inputs.get(inputsStart); + Path leafPath = execRoot.getRelative(input.getExecPathString()); + if (!(input instanceof VirtualActionInput) && uploadSymlinks) { + FileStatus stat = leafPath.stat(Symlinks.NOFOLLOW); + if (stat.isSymbolicLink()) { + PathFragment target = leafPath.readSymbolicLink(); + FileStatus statFollow = leafPath.statIfFound(Symlinks.FOLLOW); + Preconditions.checkNotNull( + statFollow, "Action input %s is a dangling symbolic link to %s ", leafPath, target); + if (!target.isAbsolute()) { + return interner.intern(new TreeNode(input)); + } + } + } try { if (!(input instanceof VirtualActionInput) && getInputMetadata(input).getType().isDirectory()) { - Path leafPath = execRoot.getRelative(input.getExecPathString()); return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input)); } } catch (DigestOfDirectoryException e) { - Path leafPath = execRoot.getRelative(input.getExecPathString()); return interner.intern(new TreeNode(buildInputDirectoryEntries(leafPath), input)); } return interner.intern(new TreeNode(input)); @@ -366,17 +392,30 @@ private synchronized Directory getOrComputeDirectory(TreeNode node) throws IOExc TreeNode child = entry.getChild(); if (child.isLeaf()) { ActionInput input = child.getActionInput(); - final Digest digest; if (input instanceof VirtualActionInput) { VirtualActionInput virtualInput = (VirtualActionInput) input; - digest = digestUtil.compute(virtualInput); + Digest digest = digestUtil.compute(virtualInput); virtualInputDigestCache.put(virtualInput, digest); // There may be multiple inputs with the same digest. In that case, we don't care which // one we get back from the digestVirtualInputCache later. digestVirtualInputCache.put(digest, virtualInput); - } else { - digest = DigestUtil.getFromInputCache(input, inputFileCache); + b.addFilesBuilder().setName(entry.getSegment()).setDigest(digest).setIsExecutable(true); + continue; + } + if (uploadSymlinks) { + // We need to stat the input to check whether it is a symlink. + // getInputMetadata only gives target metadata. + Path inputPath = execRoot.getRelative(input.getExecPath()); + FileStatus stat = inputPath.stat(Symlinks.NOFOLLOW); + if (stat.isSymbolicLink()) { + PathFragment target = inputPath.readSymbolicLink(); + if (!target.isAbsolute()) { + b.addSymlinksBuilder().setName(entry.getSegment()).setTarget(target.toString()); + continue; + } + } } + Digest digest = DigestUtil.getFromInputCache(input, inputFileCache); b.addFilesBuilder().setName(entry.getSegment()).setDigest(digest).setIsExecutable(true); } else { Digest childDigest = Preconditions.checkNotNull(treeNodeDigestCache.get(child)); diff --git a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java index a38371986265dd..4dbf20a9b3118c 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/GrpcRemoteCacheTest.java @@ -245,7 +245,7 @@ public PathFragment getExecPath() { public void testVirtualActionInputSupport() throws Exception { GrpcRemoteCache client = newClient(); TreeNodeRepository treeNodeRepository = - new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL); + new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL, true); PathFragment execPath = PathFragment.create("my/exec/path"); VirtualActionInput virtualActionInput = new StringVirtualActionInput("hello", execPath); Digest digest = DIGEST_UTIL.compute(virtualActionInput.getBytes().toByteArray()); diff --git a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java index eba2ebb5ae6dbc..1f07a53266ab46 100644 --- a/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java +++ b/src/test/java/com/google/devtools/build/lib/remote/TreeNodeRepositoryTest.java @@ -14,9 +14,13 @@ package com.google.devtools.build.lib.remote; import static com.google.common.truth.Truth.assertThat; +import static org.junit.Assert.fail; import build.bazel.remote.execution.v2.Digest; import build.bazel.remote.execution.v2.Directory; +import build.bazel.remote.execution.v2.DirectoryNode; +import build.bazel.remote.execution.v2.FileNode; +import build.bazel.remote.execution.v2.SymlinkNode; import com.google.common.collect.ImmutableCollection; import com.google.devtools.build.lib.actions.ActionInput; import com.google.devtools.build.lib.actions.ActionInputHelper; @@ -59,10 +63,14 @@ public final void setRootDir() throws Exception { rootDir = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.dir("/exec/root"))); } - private TreeNodeRepository createTestTreeNodeRepository() { + private TreeNodeRepository createTestTreeNodeRepository(boolean uploadSymlinks) { MetadataProvider inputFileCache = new SingleBuildFileCache(execRoot.getPathString(), scratch.getFileSystem()); - return new TreeNodeRepository(execRoot, inputFileCache, digestUtil); + return new TreeNodeRepository(execRoot, inputFileCache, digestUtil, uploadSymlinks); + } + + private TreeNodeRepository createTestTreeNodeRepository() { + return createTestTreeNodeRepository(true); } private TreeNode buildFromActionInputs(TreeNodeRepository repo, ActionInput... inputs) @@ -216,4 +224,307 @@ public void testDirectoryInput() throws Exception { assertThat(aClientDirectory.getFiles(0).getName()).isEqualTo("baz.txt"); assertThat(aClientDirectory.getFiles(0).getDigest()).isEqualTo(bazDigest); } + + @Test + public void testAbsoluteFileSymlinkAsFile() throws Exception { + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + Path target = scratch.file("/exec/root/target", "bla"); + link.createSymbolicLink(target); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(target); + Directory rootDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("link").setDigest(digest).setIsExecutable(true)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir); + } + + @Test + public void testAbsoluteFileSymlinkInDirectoryAsFile() throws Exception { + Path foo = scratch.file("/exec/root/foo", "bla"); + Path dir = scratch.dir("/exec/root/dir"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(foo); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(foo); + Directory childDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("link").setDigest(digest).setIsExecutable(true)) + .build(); + Digest dirDigest = digestUtil.compute(childDir); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, childDir); + } + + @Test + public void testRelativeFileSymlinkAsFile() throws Exception { + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + Path target = scratch.file("/exec/root/target", "bla"); + link.createSymbolicLink(target.relativeTo(execRoot)); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(false); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(target); + Directory rootDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("link").setDigest(digest).setIsExecutable(true)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir); + } + + @Test + public void testRelativeFileSymlinkInDirectoryAsFile() throws Exception { + Path foo = scratch.file("/exec/root/foo", "bla"); + Path dir = scratch.dir("/exec/root/dir"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(PathFragment.create("../foo")); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(false); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(foo); + Directory childDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("link").setDigest(digest).setIsExecutable(true)) + .build(); + Digest dirDigest = digestUtil.compute(childDir); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, childDir); + } + + @Test + public void testRelativeFileSymlink() throws Exception { + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + Path target = scratch.file("/exec/root/target", "bla"); + link.createSymbolicLink(target.relativeTo(execRoot)); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Directory rootDir = + Directory.newBuilder() + .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("target")) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir); + } + + @Test + public void testRelativeFileSymlinkInDirectory() throws Exception { + scratch.file("/exec/root/foo", "bla"); + Path dir = scratch.dir("/exec/root/dir"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(PathFragment.create("../foo")); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Directory childDir = + Directory.newBuilder() + .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("../foo")) + .build(); + Digest dirDigest = digestUtil.compute(childDir); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, childDir); + } + + @Test + public void testDanglingSymlinkFail() throws Exception { + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + Path target = scratch.getFileSystem().getPath("/exec/root/target"); + link.createSymbolicLink(target.relativeTo(execRoot)); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + try { + buildFromActionInputs(repo, linkInput); + fail("Expected exception"); + } catch (Exception e) { + assertThat(e).hasMessageThat().contains("dangling"); + assertThat(e).hasMessageThat().contains("/exec/root/link"); + assertThat(e).hasMessageThat().contains("target"); + } + } + + @Test + public void testDanglingSymlinkInDirectoryFail() throws Exception { + scratch.getFileSystem().getPath("/exec/root/foo"); + Path dir = scratch.dir("/exec/root/dir"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(PathFragment.create("../foo")); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + try { + buildFromActionInputs(repo, dirInput); + fail("Expected exception"); + } catch (Exception e) { + assertThat(e).hasMessageThat().contains("Dangling"); + assertThat(e).hasMessageThat().contains("/exec/root/dir/link"); + assertThat(e).hasMessageThat().contains("../foo"); + } + } + + @Test + public void testAbsoluteDirectorySymlinkAsDirectory() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + Path foo = scratch.file("/exec/root/dir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + link.createSymbolicLink(dir); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(foo); + Directory childDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest).setIsExecutable(true)) + .build(); + Digest dirDigest = digestUtil.compute(childDir); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("link").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, childDir); + } + + @Test + public void testAbsoluteDirectorySymlinkInDirectoryAsDirectory() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + Path bardir = scratch.dir("/exec/root/bardir"); + Path foo = scratch.file("/exec/root/bardir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(bardir); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Directory barDir = + Directory.newBuilder() + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(digestUtil.compute(foo)) + .setIsExecutable(true)) + .build(); + Digest barDigest = digestUtil.compute(barDir); + Directory dirNode = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("link").setDigest(barDigest)) + .build(); + Digest dirDigest = digestUtil.compute(dirNode); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, dirNode, barDir); + } + + @Test + public void testRelativeDirectorySymlinkAsDirectory() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + Path foo = scratch.file("/exec/root/dir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + link.createSymbolicLink(dir.relativeTo(execRoot)); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(false); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Digest digest = digestUtil.compute(foo); + Directory childDir = + Directory.newBuilder() + .addFiles(FileNode.newBuilder().setName("foo").setDigest(digest).setIsExecutable(true)) + .build(); + Digest dirDigest = digestUtil.compute(childDir); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("link").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, childDir); + } + + @Test + public void testRelativeDirectorySymlinkInDirectoryAsDirectory() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + scratch.dir("/exec/root/bardir"); + Path foo = scratch.file("/exec/root/bardir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(PathFragment.create("../bardir")); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(false); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Directory barDir = + Directory.newBuilder() + .addFiles( + FileNode.newBuilder() + .setName("foo") + .setDigest(digestUtil.compute(foo)) + .setIsExecutable(true)) + .build(); + Digest barDigest = digestUtil.compute(barDir); + Directory dirNode = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("link").setDigest(barDigest)) + .build(); + Digest dirDigest = digestUtil.compute(dirNode); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, dirNode, barDir); + } + + @Test + public void testRelativeDirectorySymlink() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + scratch.file("/exec/root/dir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/link"); + link.createSymbolicLink(dir.relativeTo(execRoot)); + Artifact linkInput = new Artifact(link, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, linkInput); + repo.computeMerkleDigests(root); + Directory rootDir = + Directory.newBuilder() + .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("dir")) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir); + } + + @Test + public void testRelativeDirectorySymlinkInDirectory() throws Exception { + Path dir = scratch.dir("/exec/root/dir"); + scratch.dir("/exec/root/bardir"); + scratch.file("/exec/root/bardir/foo", "bla"); + Path link = scratch.getFileSystem().getPath("/exec/root/dir/link"); + link.createSymbolicLink(PathFragment.create("../bardir")); + Artifact dirInput = new Artifact(dir, rootDir); + TreeNodeRepository repo = createTestTreeNodeRepository(true); + TreeNode root = buildFromActionInputs(repo, dirInput); + repo.computeMerkleDigests(root); + Directory dirNode = + Directory.newBuilder() + .addSymlinks(SymlinkNode.newBuilder().setName("link").setTarget("../bardir")) + .build(); + Digest dirDigest = digestUtil.compute(dirNode); + Directory rootDir = + Directory.newBuilder() + .addDirectories(DirectoryNode.newBuilder().setName("dir").setDigest(dirDigest)) + .build(); + assertThat(repo.treeToDirectories(root)).containsExactly(rootDir, dirNode); + } } diff --git a/src/test/shell/bazel/remote/remote_execution_http_test.sh b/src/test/shell/bazel/remote/remote_execution_http_test.sh index 9b71efd15bac29..afcd7eb456a2c7 100755 --- a/src/test/shell/bazel/remote/remote_execution_http_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_http_test.sh @@ -34,6 +34,7 @@ function set_up() { --work_path="${work_path}" \ --listen_port=${worker_port} \ --http_listen_port=${http_port} \ + --incompatible_remote_symlinks \ --pid_file="${pid_file}" & local wait_seconds=0 until [ -s "${pid_file}" ] || [ "$wait_seconds" -eq 15 ]; do @@ -168,8 +169,12 @@ def _gen_output_dir_impl(ctx): inputs = [], command = """ mkdir -p $1/sub1; \ - echo "Hello, world!" > $1/foo.txt; \ - echo "Shuffle, duffle, muzzle, muff" > $1/sub1/bar.txt + cd $1; \ + echo "Hello, world!" > hello.txt; \ + ln -s hello.txt foo.txt; \ + cd sub1; \ + echo "Shuffle, duffle, muzzle, muff" > shuffle.txt; \ + ln -s $PWD/shuffle.txt bar.txt """, arguments = [output_dir.path], ) @@ -232,7 +237,9 @@ EOF function test_directory_artifact_skylark_local() { set_directory_artifact_skylark_testfixtures - bazel build //a:test >& $TEST_log \ + bazel build \ + --spawn_strategy=local \ + //a:test >& $TEST_log \ || fail "Failed to build //a:test without remote execution" diff bazel-genfiles/a/qux/out.txt a/test_expected \ || fail "Local execution generated different result" @@ -243,6 +250,7 @@ function test_directory_artifact_skylark() { bazel build \ --spawn_strategy=remote \ + --incompatible_remote_symlinks \ --remote_executor=localhost:${worker_port} \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote execution" @@ -251,6 +259,7 @@ function test_directory_artifact_skylark() { bazel clean --expunge bazel build \ --spawn_strategy=remote \ + --incompatible_remote_symlinks \ --remote_executor=localhost:${worker_port} \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote execution" @@ -264,6 +273,8 @@ function test_directory_artifact_skylark_grpc_cache() { bazel build \ --remote_cache=localhost:${worker_port} \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote gRPC cache" diff bazel-genfiles/a/qux/out.txt a/test_expected \ @@ -271,6 +282,8 @@ function test_directory_artifact_skylark_grpc_cache() { bazel clean --expunge bazel build \ --remote_cache=localhost:${worker_port} \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote gRPC cache" expect_log "remote cache hit" @@ -283,12 +296,16 @@ function test_directory_artifact_skylark_rest_cache() { bazel build \ --remote_rest_cache=http://localhost:${http_port} \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote REST cache" diff bazel-genfiles/a/qux/out.txt a/test_expected \ || fail "Remote cache miss generated different result" bazel clean --expunge bazel build \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ --remote_rest_cache=http://localhost:${http_port} \ //a:test >& $TEST_log \ || fail "Failed to build //a:test with remote REST cache" @@ -303,12 +320,16 @@ function test_directory_artifact_in_runfiles_skylark_rest_cache() { bazel build \ --remote_rest_cache=http://localhost:${http_port} \ //a:test2 >& $TEST_log \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ || fail "Failed to build //a:test2 with remote REST cache" diff bazel-genfiles/a/test2-out.txt a/test_expected \ || fail "Remote cache miss generated different result" bazel clean --expunge bazel build \ --remote_rest_cache=http://localhost:${http_port} \ + --spawn_strategy=local \ + --incompatible_remote_symlinks \ //a:test2 >& $TEST_log \ || fail "Failed to build //a:test2 with remote REST cache" expect_log "remote cache hit" diff --git a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java index de76ed8d38ff57..0003e6f3109a20 100644 --- a/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java +++ b/src/tools/remote/src/main/java/com/google/devtools/build/remote/worker/ExecutionServer.java @@ -350,6 +350,14 @@ private ActionResult execute(Digest actionDigest, Path execRoot) } else if (setResult) { cache.setCachedActionResult(actionKey, finalResult); } + if (finalResult.getOutputFilesCount() + + finalResult.getOutputFileSymlinksCount() + + finalResult.getOutputDirectoriesCount() + + finalResult.getOutputDirectorySymlinksCount() + <= 0) { + logger.warning( + String.format("Unexpected result of remote execution: no output files: %s", finalResult)); + } return finalResult; }