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 655877b5fcffe4..8e5a0f868dabc5 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 @@ -102,11 +102,7 @@ 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, - options.incompatibleRemoteSymlinks); + new TreeNodeRepository(execRoot, context.getMetadataProvider(), digestUtil); 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 92eb77b251fdd2..6f85f390346022 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 @@ -149,9 +149,7 @@ 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, remoteOptions.incompatibleRemoteSymlinks); + TreeNodeRepository repository = new TreeNodeRepository(execRoot, inputFileCache, digestUtil); 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 78541425de22f9..5732aedc158a52 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,7 +21,6 @@ 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; @@ -34,7 +33,6 @@ 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; @@ -80,10 +78,6 @@ 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 547c7adb918d46..5d21fe6c83a66e 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,7 +38,6 @@ 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; @@ -64,6 +63,9 @@ 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)); @@ -231,17 +233,11 @@ 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, - boolean uploadSymlinks) { + public TreeNodeRepository(Path execRoot, MetadataProvider inputFileCache, DigestUtil digestUtil) { this.execRoot = execRoot; this.inputFileCache = inputFileCache; this.digestUtil = digestUtil; - this.uploadSymlinks = uploadSymlinks; } public MetadataProvider getInputFileCache() { @@ -291,7 +287,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(Symlinks.NOFOLLOW)); + List sortedDirent = new ArrayList<>(path.readdir(SYMLINK_POLICY)); sortedDirent.sort(Comparator.comparing(Dirent::getName)); List entries = new ArrayList<>(sortedDirent.size()); @@ -301,17 +297,6 @@ 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()))); } @@ -340,25 +325,14 @@ private TreeNode buildParentNode( Preconditions.checkArgument( inputsStart == inputsEnd - 1, "Encountered two inputs with the same path."); ActionInput input = inputs.get(inputsStart); - Path leafPath = ActionInputHelper.toInputPath(input, execRoot); - 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)); @@ -392,30 +366,17 @@ 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 digest = digestUtil.compute(virtualInput); + 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); - 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 = ActionInputHelper.toInputPath(input, execRoot); - 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; - } - } + } else { + digest = DigestUtil.getFromInputCache(input, inputFileCache); } - 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 bd677f62d6b484..92894f8928cebf 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 @@ -249,7 +249,7 @@ public PathFragment getExecPath() { public void testVirtualActionInputSupport() throws Exception { GrpcRemoteCache client = newClient(); TreeNodeRepository treeNodeRepository = - new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL, true); + new TreeNodeRepository(execRoot, fakeFileCache, DIGEST_UTIL); 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 1f07a53266ab46..eba2ebb5ae6dbc 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,13 +14,9 @@ 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; @@ -63,14 +59,10 @@ public final void setRootDir() throws Exception { rootDir = ArtifactRoot.asSourceRoot(Root.fromPath(scratch.dir("/exec/root"))); } - private TreeNodeRepository createTestTreeNodeRepository(boolean uploadSymlinks) { + private TreeNodeRepository createTestTreeNodeRepository() { MetadataProvider inputFileCache = new SingleBuildFileCache(execRoot.getPathString(), scratch.getFileSystem()); - return new TreeNodeRepository(execRoot, inputFileCache, digestUtil, uploadSymlinks); - } - - private TreeNodeRepository createTestTreeNodeRepository() { - return createTestTreeNodeRepository(true); + return new TreeNodeRepository(execRoot, inputFileCache, digestUtil); } private TreeNode buildFromActionInputs(TreeNodeRepository repo, ActionInput... inputs) @@ -224,307 +216,4 @@ 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 afcd7eb456a2c7..9b71efd15bac29 100755 --- a/src/test/shell/bazel/remote/remote_execution_http_test.sh +++ b/src/test/shell/bazel/remote/remote_execution_http_test.sh @@ -34,7 +34,6 @@ 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 @@ -169,12 +168,8 @@ def _gen_output_dir_impl(ctx): inputs = [], command = """ mkdir -p $1/sub1; \ - 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 + echo "Hello, world!" > $1/foo.txt; \ + echo "Shuffle, duffle, muzzle, muff" > $1/sub1/bar.txt """, arguments = [output_dir.path], ) @@ -237,9 +232,7 @@ EOF function test_directory_artifact_skylark_local() { set_directory_artifact_skylark_testfixtures - bazel build \ - --spawn_strategy=local \ - //a:test >& $TEST_log \ + bazel build //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" @@ -250,7 +243,6 @@ 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" @@ -259,7 +251,6 @@ 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" @@ -273,8 +264,6 @@ 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 \ @@ -282,8 +271,6 @@ 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" @@ -296,16 +283,12 @@ 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" @@ -320,16 +303,12 @@ 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 ed24df60e9292f..b9450244ccd976 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,14 +350,6 @@ 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; }