Correctly handle symlinks when prefetching.

In https://github.com/bazelbuild/bazel/commit/5c4cf47a131c84506aad9ce0e014c6643c31a4ac, we replaced `path = path.getRelative(path.readSymbolicLink())` with `path = path.resolveSymbolicLink`. While the former one is wrong for relative symlinks, it actually works for absolute symlinks. The latter one, however, doesn't work if the target doesn't exist on local file system -- and this is the case for most of builds using our internal version of BwoB.

This CL fixes that issue by correctly handle both absolute and relative symlinks. Additionally, it correctly handles the case where a symlink points to a tree output.

Moved symlink integration tests to the base test class.

PiperOrigin-RevId: 549921885
Change-Id: If4009525499f4d0625e7725faf4bb0675984c299
diff --git a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
index f1700d6..057ac04 100644
--- a/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
+++ b/src/main/java/com/google/devtools/build/lib/remote/AbstractActionInputPrefetcher.java
@@ -46,6 +46,7 @@
 import com.google.devtools.build.lib.remote.util.RxUtils;
 import com.google.devtools.build.lib.remote.util.RxUtils.TransferResult;
 import com.google.devtools.build.lib.remote.util.TempPathGenerator;
+import com.google.devtools.build.lib.vfs.FileSymlinkLoopException;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.OutputPermissions;
 import com.google.devtools.build.lib.vfs.Path;
@@ -426,6 +427,36 @@
     return null;
   }
 
+  private Path resolveOneSymlink(Path path) throws IOException {
+    var targetPathFragment = path.readSymbolicLink();
+    if (targetPathFragment.isAbsolute()) {
+      return path.getFileSystem().getPath(targetPathFragment);
+    } else {
+      return checkNotNull(path.getParentDirectory()).getRelative(targetPathFragment);
+    }
+  }
+
+  private Path maybeResolveSymlink(Path path) throws IOException {
+    // Potentially resolves a symlink to its target path. This differs from
+    // Path#resolveSymbolicLinks() that:
+    //   1. Path#resolveSymbolicLinks() checks each segment of the path, but we assume there is no
+    //      intermediate symlink because they should've been already normalized for outputs.
+    //   2. In case of dangling symlink, we return the target path instead of throwing
+    //      FileNotFoundException because we want to download output to that target path.
+    var maxAttempt = 32;
+    while (path.isSymbolicLink() && maxAttempt-- > 0) {
+      var resolvedPath = resolveOneSymlink(path);
+      if (resolvedPath.asFragment().equals(path.asFragment())) {
+        throw new FileSymlinkLoopException(path.asFragment());
+      }
+      path = resolvedPath;
+    }
+    if (maxAttempt <= 0) {
+      throw new FileSymlinkLoopException(path.asFragment());
+    }
+    return path;
+  }
+
   private Completable downloadFileNoCheckRx(
       ActionExecutionMetadata action,
       DirectoryContext dirCtx,
@@ -434,14 +465,23 @@
       ActionInput actionInput,
       FileArtifactValue metadata,
       Priority priority) {
-    if (path.isSymbolicLink()) {
-      try {
-        path = path.resolveSymbolicLinks();
-      } catch (IOException e) {
-        return Completable.error(e);
+    // If the path to be prefetched is a non-dangling symlink, prefetch its target path instead.
+    // Note that this only applies to symlinks created by spawns (or, currently, with the internal
+    // version of BwoB); symlinks created in-process through an ActionFileSystem should have already
+    // been resolved into their materializationExecPath in maybeGetSymlink.
+    try {
+      if (treeRoot != null) {
+        var treeRootRelativePath = path.relativeTo(treeRoot);
+        treeRoot = maybeResolveSymlink(treeRoot);
+        path = treeRoot.getRelative(treeRootRelativePath);
+      } else {
+        path = maybeResolveSymlink(path);
       }
+    } catch (IOException e) {
+      return Completable.error(e);
     }
 
+    Path finalTreeRoot = treeRoot;
     Path finalPath = path;
 
     AtomicBoolean completed = new AtomicBoolean(false);
@@ -461,7 +501,7 @@
                             directExecutor())
                         .doOnComplete(
                             () -> {
-                              finalizeDownload(dirCtx, treeRoot, tempPath, finalPath);
+                              finalizeDownload(dirCtx, finalTreeRoot, tempPath, finalPath);
                               completed.set(true);
                             }),
                 tempPath -> {
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSymlinkLoopException.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSymlinkLoopException.java
index 4026e39..6fc2f21 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSymlinkLoopException.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSymlinkLoopException.java
@@ -16,12 +16,13 @@
 
 import com.google.devtools.build.lib.io.FileSymlinkException;
 
-final class FileSymlinkLoopException extends FileSymlinkException {
+/** A {@link FileSymlinkException} that indicates a symlink loop. */
+public final class FileSymlinkLoopException extends FileSymlinkException {
   FileSymlinkLoopException(String message) {
     super(message);
   }
 
-  FileSymlinkLoopException(PathFragment pathFragment) {
+  public FileSymlinkLoopException(PathFragment pathFragment) {
     this(pathFragment.getPathString() + " (Too many levels of symbolic links)");
   }
 
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
index b568707..5eda1fb 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTest.java
@@ -112,6 +112,9 @@
     return true;
   }
 
+  @Override
+  protected void injectFile(byte[] content) {}
+
   @After
   public void tearDown() throws IOException {
     if (worker != null) {
@@ -233,217 +236,6 @@
   }
 
   @Test
-  public void symlinkToGeneratedFile() throws Exception {
-    write(
-        "a/defs.bzl",
-        "def _impl(ctx):",
-        "  if ctx.attr.chain_length < 1:",
-        "    fail('chain_length must be > 0')",
-        "",
-        "  file = ctx.actions.declare_file(ctx.label.name + '.file')",
-        // Use ctx.actions.run_shell instead of ctx.actions.write, so that it runs remotely.
-        "  ctx.actions.run_shell(",
-        "    outputs = [file],",
-        "    command = 'echo hello > $1',",
-        "    arguments = [file.path],",
-        "  )",
-        "",
-        "  for i in range(ctx.attr.chain_length):",
-        "    sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))",
-        "    ctx.actions.symlink(output = sym, target_file = file)",
-        "    file = sym",
-        "",
-        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
-        "  ctx.actions.run_shell(",
-        "    inputs = [sym],",
-        "    outputs = [out],",
-        "    command = '[[ hello == $(cat $1) ]] && touch $2',",
-        "    arguments = [sym.path, out.path],",
-        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
-        "  )",
-        "",
-        "  return DefaultInfo(files = depset([out]))",
-        "",
-        "my_rule = rule(",
-        "  implementation = _impl,",
-        "  attrs = {",
-        "    'chain_length': attr.int(),",
-        "    'local': attr.bool(),",
-        "  },",
-        ")");
-
-    write(
-        "a/BUILD",
-        "load(':defs.bzl', 'my_rule')",
-        "",
-        "my_rule(name = 'one_local', local = True, chain_length = 1)",
-        "my_rule(name = 'two_local', local = True, chain_length = 2)",
-        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
-        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
-
-    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
-  }
-
-  @Test
-  public void symlinkToDirectory() throws Exception {
-    write(
-        "a/defs.bzl",
-        "def _impl(ctx):",
-        "  if ctx.attr.chain_length < 1:",
-        "    fail('chain_length must be > 0')",
-        "",
-        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
-        "  ctx.actions.run_shell(",
-        "    outputs = [dir],",
-        "    command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',",
-        "    arguments = [dir.path],",
-        "  )",
-        "",
-        "  for i in range(ctx.attr.chain_length):",
-        "    sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))",
-        "    ctx.actions.symlink(output = sym, target_file = dir)",
-        "    dir = sym",
-        "",
-        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
-        "  ctx.actions.run_shell(",
-        "    inputs = [sym],",
-        "    outputs = [out],",
-        "    command = '[[ hello == $(cat $1/some/path/inside.txt) ]] && touch $2',",
-        "    arguments = [sym.path, out.path],",
-        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
-        "  )",
-        "",
-        "  return DefaultInfo(files = depset([out]))",
-        "",
-        "my_rule = rule(",
-        "  implementation = _impl,",
-        "  attrs = {",
-        "    'chain_length': attr.int(),",
-        "    'local': attr.bool()",
-        "  },",
-        ")");
-
-    write(
-        "a/BUILD",
-        "load(':defs.bzl', 'my_rule')",
-        "",
-        "my_rule(name = 'one_local', local = True, chain_length = 1)",
-        "my_rule(name = 'two_local', local = True, chain_length = 2)",
-        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
-        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
-
-    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
-  }
-
-  @Test
-  public void symlinkToNestedFile() throws Exception {
-    addOptions("--noincompatible_strict_conflict_checks");
-
-    write(
-        "a/defs.bzl",
-        "def _impl(ctx):",
-        "  if ctx.attr.chain_length < 1:",
-        "    fail('chain_length must be > 0')",
-        "",
-        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
-        "  file = ctx.actions.declare_file(ctx.label.name + '.dir/some/path/inside.txt')",
-        "  ctx.actions.run_shell(",
-        "    outputs = [dir, file],",
-        "    command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',",
-        "    arguments = [dir.path],",
-        "  )",
-        "",
-        "  for i in range(ctx.attr.chain_length):",
-        "    sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))",
-        "    ctx.actions.symlink(output = sym, target_file = file)",
-        "    file = sym",
-        "",
-        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
-        "  ctx.actions.run_shell(",
-        "    inputs = [sym],",
-        "    outputs = [out],",
-        "    command = '[[ hello == $(cat $1) ]] && touch $2',",
-        "    arguments = [sym.path, out.path],",
-        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
-        "  )",
-        "",
-        "  return DefaultInfo(files = depset([out]))",
-        "",
-        "my_rule = rule(",
-        "  implementation = _impl,",
-        "  attrs = {",
-        "    'chain_length': attr.int(),",
-        "    'local': attr.bool(),",
-        "  },",
-        ")");
-
-    write(
-        "a/BUILD",
-        "load(':defs.bzl', 'my_rule')",
-        "",
-        "my_rule(name = 'one_local', local = True, chain_length = 1)",
-        "my_rule(name = 'two_local', local = True, chain_length = 2)",
-        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
-        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
-
-    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
-  }
-
-  @Test
-  public void symlinkToNestedDirectory() throws Exception {
-    addOptions("--noincompatible_strict_conflict_checks");
-
-    write(
-        "a/defs.bzl",
-        "def _impl(ctx):",
-        "  if ctx.attr.chain_length < 1:",
-        "    fail('chain_length must be > 0')",
-        "",
-        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
-        "  subdir = ctx.actions.declare_directory(ctx.label.name + '.dir/some/path')",
-        "  ctx.actions.run_shell(",
-        "    outputs = [dir, subdir],",
-        "    command = 'mkdir -p $1/some/path && echo hello > $1/some/path/inside.txt',",
-        "    arguments = [dir.path],",
-        "  )",
-        "",
-        "  for i in range(ctx.attr.chain_length):",
-        "    sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))",
-        "    ctx.actions.symlink(output = sym, target_file = subdir)",
-        "    subdir = sym",
-        "",
-        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
-        "  ctx.actions.run_shell(",
-        "    inputs = [sym],",
-        "    outputs = [out],",
-        "    command = '[[ hello == $(cat $1/inside.txt) ]] && touch $2',",
-        "    arguments = [sym.path, out.path],",
-        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
-        "  )",
-        "",
-        "  return DefaultInfo(files = depset([out]))",
-        "",
-        "my_rule = rule(",
-        "  implementation = _impl,",
-        "  attrs = {",
-        "    'chain_length': attr.int(),",
-        "    'local': attr.bool(),",
-        "  },",
-        ")");
-
-    write(
-        "a/BUILD",
-        "load(':defs.bzl', 'my_rule')",
-        "",
-        "my_rule(name = 'one_local', local = True, chain_length = 1)",
-        "my_rule(name = 'two_local', local = True, chain_length = 2)",
-        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
-        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
-
-    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
-  }
-
-  @Test
   public void outputSymlinkHandledGracefully() throws Exception {
     // Symlinks may not be supported on Windows
     assumeFalse(OS.getCurrent() == OS.WINDOWS);
@@ -693,118 +485,6 @@
   }
 
   @Test
-  public void downloadToplevel_symlinkFile() throws Exception {
-    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
-    assumeFalse(OS.getCurrent() == OS.WINDOWS);
-
-    setDownloadToplevel();
-    writeSymlinkRule();
-    write(
-        "BUILD",
-        "load(':symlink.bzl', 'symlink')",
-        "genrule(",
-        "  name = 'foo',",
-        "  srcs = [],",
-        "  outs = ['out/foo.txt'],",
-        "  cmd = 'echo foo > $@',",
-        ")",
-        "symlink(",
-        "  name = 'foo-link',",
-        "  target = ':foo'",
-        ")");
-
-    buildTarget("//:foo-link");
-
-    assertValidOutputFile("foo-link", "foo\n");
-
-    // Delete link, re-plant symlink
-    getOutputPath("foo-link").delete();
-
-    buildTarget("//:foo-link");
-
-    assertValidOutputFile("foo-link", "foo\n");
-
-    // Delete target, re-download it
-    getOutputPath("foo").delete();
-
-    assertValidOutputFile("foo-link", "foo\n");
-  }
-
-  @Test
-  public void downloadToplevel_symlinkSourceFile() throws Exception {
-    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
-    assumeFalse(OS.getCurrent() == OS.WINDOWS);
-
-    setDownloadToplevel();
-    writeSymlinkRule();
-    write(
-        "BUILD",
-        "load(':symlink.bzl', 'symlink')",
-        "symlink(",
-        "  name = 'foo-link',",
-        "  target = ':foo.txt'",
-        ")");
-    write("foo.txt", "foo");
-
-    buildTarget("//:foo-link");
-
-    assertOnlyOutputContent("//:foo-link", "foo-link", "foo" + lineSeparator());
-
-    // Delete link, re-plant symlink
-    getOutputPath("foo-link").delete();
-
-    buildTarget("//:foo-link");
-
-    assertOnlyOutputContent("//:foo-link", "foo-link", "foo" + lineSeparator());
-  }
-
-  @Test
-  public void downloadToplevel_symlinkTree() throws Exception {
-    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
-    assumeFalse(OS.getCurrent() == OS.WINDOWS);
-
-    setDownloadToplevel();
-    writeSymlinkRule();
-    writeOutputDirRule();
-    write(
-        "BUILD",
-        "load(':output_dir.bzl', 'output_dir')",
-        "load(':symlink.bzl', 'symlink')",
-        "output_dir(",
-        "  name = 'foo',",
-        "  content_map = {'file-1': '1', 'file-2': '2', 'file-3': '3'},",
-        ")",
-        "symlink(",
-        "  name = 'foo-link',",
-        "  target = ':foo'",
-        ")");
-
-    buildTarget("//:foo-link");
-
-    assertValidOutputFile("foo-link/file-1", "1");
-    assertValidOutputFile("foo-link/file-2", "2");
-    assertValidOutputFile("foo-link/file-3", "3");
-
-    getOutputPath("foo-link").deleteTree();
-
-    // Delete link, re-plant symlink
-    buildTarget("//:foo-link");
-
-    assertValidOutputFile("foo-link/file-1", "1");
-    assertValidOutputFile("foo-link/file-2", "2");
-    assertValidOutputFile("foo-link/file-3", "3");
-
-    // Delete target, re-download them
-    getOutputPath("foo").deleteTree();
-
-    buildTarget("//:foo-link");
-
-    assertValidOutputFile("foo-link/file-1", "1");
-    assertValidOutputFile("foo-link/file-2", "2");
-    assertValidOutputFile("foo-link/file-3", "3");
-  }
-
-  @Test
   public void leaseExtension() throws Exception {
     // Test that Bazel will extend the leases for remote output by sending FindMissingBlobs calls
     // periodically to remote server. The test assumes remote server will set mtime of referenced
diff --git a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
index 8f6c0b3..36ce7f4 100644
--- a/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/remote/BuildWithoutTheBytesIntegrationTestBase.java
@@ -19,6 +19,7 @@
 import static com.google.devtools.build.lib.vfs.FileSystemUtils.writeContent;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.junit.Assert.assertThrows;
+import static org.junit.Assume.assumeFalse;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
@@ -55,6 +56,8 @@
 
   protected abstract boolean hasAccessToRemoteOutputs();
 
+  protected abstract void injectFile(byte[] content);
+
   protected void waitDownloads() throws Exception {
     // Trigger afterCommand of modules so that downloads are waited.
     runtimeWrapper.newCommand();
@@ -394,6 +397,221 @@
   }
 
   @Test
+  public void symlinkToGeneratedFile() throws Exception {
+    injectFile("hello".getBytes(UTF_8));
+    write(
+        "a/defs.bzl",
+        "def _impl(ctx):",
+        "  if ctx.attr.chain_length < 1:",
+        "    fail('chain_length must be > 0')",
+        "",
+        "  file = ctx.actions.declare_file(ctx.label.name + '.file')",
+        // Use ctx.actions.run_shell instead of ctx.actions.write, so that it runs remotely.
+        "  ctx.actions.run_shell(",
+        "    outputs = [file],",
+        "    command = 'echo -n hello > $1',",
+        "    arguments = [file.path],",
+        "  )",
+        "",
+        "  for i in range(ctx.attr.chain_length):",
+        "    sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))",
+        "    ctx.actions.symlink(output = sym, target_file = file)",
+        "    file = sym",
+        "",
+        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
+        "  ctx.actions.run_shell(",
+        "    inputs = [sym],",
+        "    outputs = [out],",
+        "    command = '[[ hello == $(cat $1) ]] && touch $2',",
+        "    arguments = [sym.path, out.path],",
+        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
+        "  )",
+        "",
+        "  return DefaultInfo(files = depset([out]))",
+        "",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'chain_length': attr.int(),",
+        "    'local': attr.bool(),",
+        "  },",
+        ")");
+
+    write(
+        "a/BUILD",
+        "load(':defs.bzl', 'my_rule')",
+        "",
+        "my_rule(name = 'one_local', local = True, chain_length = 1)",
+        "my_rule(name = 'two_local', local = True, chain_length = 2)",
+        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
+        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
+
+    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
+  }
+
+  @Test
+  public void symlinkToDirectory() throws Exception {
+    injectFile("hello".getBytes(UTF_8));
+    write(
+        "a/defs.bzl",
+        "def _impl(ctx):",
+        "  if ctx.attr.chain_length < 1:",
+        "    fail('chain_length must be > 0')",
+        "",
+        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
+        "  ctx.actions.run_shell(",
+        "    outputs = [dir],",
+        "    command = 'mkdir -p $1/some/path && echo -n hello > $1/some/path/inside.txt',",
+        "    arguments = [dir.path],",
+        "  )",
+        "",
+        "  for i in range(ctx.attr.chain_length):",
+        "    sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))",
+        "    ctx.actions.symlink(output = sym, target_file = dir)",
+        "    dir = sym",
+        "",
+        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
+        "  ctx.actions.run_shell(",
+        "    inputs = [sym],",
+        "    outputs = [out],",
+        "    command = '[[ hello == $(cat $1/some/path/inside.txt) ]] && touch $2',",
+        "    arguments = [sym.path, out.path],",
+        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
+        "  )",
+        "",
+        "  return DefaultInfo(files = depset([out]))",
+        "",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'chain_length': attr.int(),",
+        "    'local': attr.bool()",
+        "  },",
+        ")");
+
+    write(
+        "a/BUILD",
+        "load(':defs.bzl', 'my_rule')",
+        "",
+        "my_rule(name = 'one_local', local = True, chain_length = 1)",
+        "my_rule(name = 'two_local', local = True, chain_length = 2)",
+        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
+        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
+
+    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
+  }
+
+  @Test
+  public void symlinkToNestedFile() throws Exception {
+    injectFile("hello".getBytes(UTF_8));
+    addOptions("--noincompatible_strict_conflict_checks");
+
+    write(
+        "a/defs.bzl",
+        "def _impl(ctx):",
+        "  if ctx.attr.chain_length < 1:",
+        "    fail('chain_length must be > 0')",
+        "",
+        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
+        "  file = ctx.actions.declare_file(ctx.label.name + '.dir/some/path/inside.txt')",
+        "  ctx.actions.run_shell(",
+        "    outputs = [dir, file],",
+        "    command = 'mkdir -p $1/some/path && echo -n hello > $1/some/path/inside.txt',",
+        "    arguments = [dir.path],",
+        "  )",
+        "",
+        "  for i in range(ctx.attr.chain_length):",
+        "    sym = ctx.actions.declare_file(ctx.label.name + '.sym' + str(i))",
+        "    ctx.actions.symlink(output = sym, target_file = file)",
+        "    file = sym",
+        "",
+        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
+        "  ctx.actions.run_shell(",
+        "    inputs = [sym],",
+        "    outputs = [out],",
+        "    command = '[[ hello == $(cat $1) ]] && touch $2',",
+        "    arguments = [sym.path, out.path],",
+        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
+        "  )",
+        "",
+        "  return DefaultInfo(files = depset([out]))",
+        "",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'chain_length': attr.int(),",
+        "    'local': attr.bool(),",
+        "  },",
+        ")");
+
+    write(
+        "a/BUILD",
+        "load(':defs.bzl', 'my_rule')",
+        "",
+        "my_rule(name = 'one_local', local = True, chain_length = 1)",
+        "my_rule(name = 'two_local', local = True, chain_length = 2)",
+        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
+        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
+
+    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
+  }
+
+  @Test
+  public void symlinkToNestedDirectory() throws Exception {
+    injectFile("hello".getBytes(UTF_8));
+    addOptions("--noincompatible_strict_conflict_checks");
+
+    write(
+        "a/defs.bzl",
+        "def _impl(ctx):",
+        "  if ctx.attr.chain_length < 1:",
+        "    fail('chain_length must be > 0')",
+        "",
+        "  dir = ctx.actions.declare_directory(ctx.label.name + '.dir')",
+        "  subdir = ctx.actions.declare_directory(ctx.label.name + '.dir/some/path')",
+        "  ctx.actions.run_shell(",
+        "    outputs = [dir, subdir],",
+        "    command = 'mkdir -p $1/some/path && echo -n hello > $1/some/path/inside.txt',",
+        "    arguments = [dir.path],",
+        "  )",
+        "",
+        "  for i in range(ctx.attr.chain_length):",
+        "    sym = ctx.actions.declare_directory(ctx.label.name + '.sym' + str(i))",
+        "    ctx.actions.symlink(output = sym, target_file = subdir)",
+        "    subdir = sym",
+        "",
+        "  out = ctx.actions.declare_file(ctx.label.name + '.out')",
+        "  ctx.actions.run_shell(",
+        "    inputs = [sym],",
+        "    outputs = [out],",
+        "    command = '[[ hello == $(cat $1/inside.txt) ]] && touch $2',",
+        "    arguments = [sym.path, out.path],",
+        "    execution_requirements = {'no-remote': ''} if ctx.attr.local else {},",
+        "  )",
+        "",
+        "  return DefaultInfo(files = depset([out]))",
+        "",
+        "my_rule = rule(",
+        "  implementation = _impl,",
+        "  attrs = {",
+        "    'chain_length': attr.int(),",
+        "    'local': attr.bool(),",
+        "  },",
+        ")");
+
+    write(
+        "a/BUILD",
+        "load(':defs.bzl', 'my_rule')",
+        "",
+        "my_rule(name = 'one_local', local = True, chain_length = 1)",
+        "my_rule(name = 'two_local', local = True, chain_length = 2)",
+        "my_rule(name = 'one_remote', local = False, chain_length = 1)",
+        "my_rule(name = 'two_remote', local = False, chain_length = 2)");
+
+    buildTarget("//a:one_local", "//a:two_local", "//a:one_remote", "//a:two_remote");
+  }
+
+  @Test
   public void localAction_stdoutIsReported() throws Exception {
     // Disable on Windows since it fails for unknown reasons.
     // TODO(chiwang): Enable it on windows.
@@ -830,6 +1048,118 @@
   }
 
   @Test
+  public void downloadToplevel_symlinkFile() throws Exception {
+    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
+    assumeFalse(OS.getCurrent() == OS.WINDOWS);
+
+    setDownloadToplevel();
+    writeSymlinkRule();
+    write(
+        "BUILD",
+        "load(':symlink.bzl', 'symlink')",
+        "genrule(",
+        "  name = 'foo',",
+        "  srcs = [],",
+        "  outs = ['out/foo.txt'],",
+        "  cmd = 'echo foo > $@',",
+        ")",
+        "symlink(",
+        "  name = 'foo-link',",
+        "  target = ':foo'",
+        ")");
+
+    buildTarget("//:foo-link");
+
+    assertValidOutputFile("foo-link", "foo\n");
+
+    // Delete link, re-plant symlink
+    getOutputPath("foo-link").delete();
+
+    buildTarget("//:foo-link");
+
+    assertValidOutputFile("foo-link", "foo\n");
+
+    // Delete target, re-download it
+    getOutputPath("foo").delete();
+
+    assertValidOutputFile("foo-link", "foo\n");
+  }
+
+  @Test
+  public void downloadToplevel_symlinkSourceFile() throws Exception {
+    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
+    assumeFalse(OS.getCurrent() == OS.WINDOWS);
+
+    setDownloadToplevel();
+    writeSymlinkRule();
+    write(
+        "BUILD",
+        "load(':symlink.bzl', 'symlink')",
+        "symlink(",
+        "  name = 'foo-link',",
+        "  target = ':foo.txt'",
+        ")");
+    write("foo.txt", "foo");
+
+    buildTarget("//:foo-link");
+
+    assertOnlyOutputContent("//:foo-link", "foo-link", "foo" + lineSeparator());
+
+    // Delete link, re-plant symlink
+    getOutputPath("foo-link").delete();
+
+    buildTarget("//:foo-link");
+
+    assertOnlyOutputContent("//:foo-link", "foo-link", "foo" + lineSeparator());
+  }
+
+  @Test
+  public void downloadToplevel_symlinkTree() throws Exception {
+    // TODO(chiwang): Make metadata for downloaded symlink non-remote.
+    assumeFalse(OS.getCurrent() == OS.WINDOWS);
+
+    setDownloadToplevel();
+    writeSymlinkRule();
+    writeOutputDirRule();
+    write(
+        "BUILD",
+        "load(':output_dir.bzl', 'output_dir')",
+        "load(':symlink.bzl', 'symlink')",
+        "output_dir(",
+        "  name = 'foo',",
+        "  content_map = {'file-1': '1', 'file-2': '2', 'file-3': '3'},",
+        ")",
+        "symlink(",
+        "  name = 'foo-link',",
+        "  target = ':foo'",
+        ")");
+
+    buildTarget("//:foo-link");
+
+    assertValidOutputFile("foo-link/file-1", "1");
+    assertValidOutputFile("foo-link/file-2", "2");
+    assertValidOutputFile("foo-link/file-3", "3");
+
+    getOutputPath("foo-link").deleteTree();
+
+    // Delete link, re-plant symlink
+    buildTarget("//:foo-link");
+
+    assertValidOutputFile("foo-link/file-1", "1");
+    assertValidOutputFile("foo-link/file-2", "2");
+    assertValidOutputFile("foo-link/file-3", "3");
+
+    // Delete target, re-download them
+    getOutputPath("foo").deleteTree();
+
+    buildTarget("//:foo-link");
+
+    assertValidOutputFile("foo-link/file-1", "1");
+    assertValidOutputFile("foo-link/file-2", "2");
+    assertValidOutputFile("foo-link/file-3", "3");
+  }
+
+  @Test
   public void treeOutputsFromLocalFileSystem_works() throws Exception {
     // Disable on Windows since it fails for unknown reasons.
     // TODO(chiwang): Enable it on windows.