[8.5.0] Add a createSymbolicLink argument indicating the file type of the target.

src/main/java/com/google/devtools/build/lib/remote/RemoteActionFileSystem.java

@@ -57,6 +57,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.vfs.SymlinkTargetType;
 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;
@@ -579,15 +580,16 @@ protected PathFragment readSymbolicLink(PathFragment path) throws IOException {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
     linkPath = resolveSymbolicLinksForParent(linkPath);
 
     if (isOutput(linkPath)) {
-      remoteOutputTree.getPath(linkPath).createSymbolicLink(targetFragment);
+      remoteOutputTree.getPath(linkPath).createSymbolicLink(targetFragment, type);
     }
 
-    localFs.getPath(linkPath).createSymbolicLink(targetFragment);
+    localFs.getPath(linkPath).createSymbolicLink(targetFragment, type);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/unix/UnixFileSystem.java

@@ -29,6 +29,7 @@
 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.SymlinkTargetType;
 import java.io.File;
 import java.io.FileInputStream;
 import java.io.FileNotFoundException;
@@ -390,7 +391,8 @@ public void createDirectoryAndParents(PathFragment path) throws IOException {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
     var comp = Blocker.begin();
     try {

src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java

@@ -569,14 +569,27 @@ protected FileStatus statIfFound(PathFragment path, boolean followSymlinks) thro
   protected abstract boolean isSpecialFile(PathFragment path, boolean followSymlinks);
 
   /**
-   * Creates a symbolic link. See {@link Path#createSymbolicLink(Path)} for specification.
+   * Creates a symbolic link. See {@link Path#createSymbolicLink(Path, SymlinkTargetType)} for
+   * specification.
    *
    * <p>Note: for {@link FileSystem}s where {@link #supportsSymbolicLinksNatively(PathFragment)}
    * returns false, this method will throw an {@link UnsupportedOperationException}
    */
-  protected abstract void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected abstract void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType hint)
       throws IOException;
 
+  /**
+   * Creates a symbolic link. See {@link Path#createSymbolicLink(Path)} for specification.
+   *
+   * <p>Note: for {@link FileSystem}s where {@link #supportsSymbolicLinksNatively(PathFragment)}
+   * returns false, this method will throw an {@link UnsupportedOperationException}
+   */
+  protected final void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+      throws IOException {
+    createSymbolicLink(linkPath, targetFragment, SymlinkTargetType.UNSPECIFIED);
+  }
+
   /**
    * Returns the target of a symbolic link. See {@link Path#readSymbolicLink} for specification.
    *

src/main/java/com/google/devtools/build/lib/vfs/JavaIoFileSystem.java

@@ -279,10 +279,12 @@ private boolean linkExists(File file) {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
     java.nio.file.Path nioPath = getNioPath(linkPath);
     try {
+      // Files.createSymbolicLink does not let us specify the target type.
       Files.createSymbolicLink(
           nioPath,
           Paths.get(StringEncoding.internalToPlatform(targetFragment.getSafePathString())));

src/main/java/com/google/devtools/build/lib/vfs/Path.java

@@ -486,11 +486,43 @@ public Path createTempDirectory(String prefix) throws IOException {
    * referent of the created symlink is is the absolute path "target"; it is not possible to create
    * relative symbolic links via this method.
    *
+   * <p>The {@code type} argument denotes the file type of the target, if known. Some filesystems
+   * require this information to correctly create a symlink. This argument may be ignored if the
+   * target can be observed to exist and is of a different type.
+   *
+   * @param type the target file type
    * @throws IOException if the creation of the symbolic link was unsuccessful for any reason
    */
-  public void createSymbolicLink(Path target) throws IOException {
+  public void createSymbolicLink(Path target, SymlinkTargetType type) throws IOException {
     checkSameFileSystem(target);
-    fileSystem.createSymbolicLink(asFragment(), target.asFragment());
+    fileSystem.createSymbolicLink(asFragment(), target.asFragment(), type);
+  }
+
+  /**
+   * Creates a symbolic link with the name of the current path, following symbolic links. The
+   * referent of the created symlink is is the absolute path "target"; it is not possible to create
+   * relative symbolic links via this method.
+   *
+   * @throws IOException if the creation of the symbolic link was unsuccessful for any reason
+   */
+  public void createSymbolicLink(Path target) throws IOException {
+    createSymbolicLink(target, SymlinkTargetType.UNSPECIFIED);
+  }
+
+  /**
+   * Creates a symbolic link with the name of the current path, following symbolic links. The
+   * referent of the created symlink is is the path fragment "target", which may be absolute or
+   * relative.
+   *
+   * <p>The {@code type} argument denotes the file type of the target, if known. Some filesystems
+   * require this information to correctly create a symlink. This argument may be ignored if the
+   * target can be observed to exist and is of a different type.
+   *
+   * @param type the target file type
+   * @throws IOException if the creation of the symbolic link was unsuccessful for any reason
+   */
+  public void createSymbolicLink(PathFragment target, SymlinkTargetType type) throws IOException {
+    fileSystem.createSymbolicLink(asFragment(), target, type);
   }
 
   /**
@@ -501,7 +533,7 @@ public void createSymbolicLink(Path target) throws IOException {
    * @throws IOException if the creation of the symbolic link was unsuccessful for any reason
    */
   public void createSymbolicLink(PathFragment target) throws IOException {
-    fileSystem.createSymbolicLink(asFragment(), target);
+    createSymbolicLink(target, SymlinkTargetType.UNSPECIFIED);
   }
 
   /**

src/main/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystem.java

@@ -119,9 +119,10 @@ protected boolean isSpecialFile(PathFragment path, boolean followSymlinks) {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
-    delegateFs.createSymbolicLink(toDelegatePath(linkPath), targetFragment);
+    delegateFs.createSymbolicLink(toDelegatePath(linkPath), targetFragment, type);
   }
 
   @Override

src/main/java/com/google/devtools/build/lib/vfs/ReadonlyFileSystemWithCustomStat.java

@@ -81,7 +81,8 @@ public void createDirectoryAndParents(PathFragment path) throws IOException {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
     throw modificationException();
   }

src/main/java/com/google/devtools/build/lib/vfs/SymlinkTargetType.java

@@ -0,0 +1,31 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+package com.google.devtools.build.lib.vfs;
+
+/**
+ * Indicates the file type at the other end of a symlink.
+ *
+ * <p>Required by some filesystems (notably on Windows) to correctly create a symlink when its
+ * target does not yet exist, as a different kind of filesystem object might be required depending
+ * on the target type.
+ */
+public enum SymlinkTargetType {
+  /** The target is of unspecified type. */
+  UNSPECIFIED,
+  /** The target is a regular file. */
+  FILE,
+  /** The target is a directory. */
+  DIRECTORY,
+}

src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/InMemoryFileSystem.java

@@ -29,6 +29,7 @@
 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.SymlinkTargetType;
 import com.google.errorprone.annotations.CheckReturnValue;
 import java.io.FileNotFoundException;
 import java.io.IOException;
@@ -521,8 +522,8 @@ public void createDirectoryAndParents(PathFragment path) throws IOException {
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment path, PathFragment targetFragment)
-      throws IOException {
+  protected void createSymbolicLink(
+      PathFragment path, PathFragment targetFragment, SymlinkTargetType type) throws IOException {
     if (isRootDirectory(path)) {
       throw Errno.EACCES.exception(path);
     }

src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java

@@ -22,11 +22,15 @@
 import com.google.devtools.build.lib.vfs.FileStatus;
 import com.google.devtools.build.lib.vfs.JavaIoFileSystem;
 import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.SymlinkTargetType;
 import java.io.File;
 import java.io.FileNotFoundException;
 import java.io.IOException;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.FileAlreadyExistsException;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.attribute.DosFileAttributes;
 
 /** File system implementation for Windows. */
@@ -72,30 +76,39 @@ protected boolean createWritableDirectory(PathFragment path) throws IOException
   }
 
   @Override
-  protected void createSymbolicLink(PathFragment linkPath, PathFragment targetFragment)
+  protected void createSymbolicLink(
+      PathFragment linkPath, PathFragment targetFragment, SymlinkTargetType type)
       throws IOException {
     PathFragment targetPath =
         targetFragment.isAbsolute()
             ? targetFragment
             : linkPath.getParentDirectory().getRelative(targetFragment);
+
+    FileStatus stat = statIfFound(targetPath, /* followSymlinks= */ true);
+    boolean existingFile = stat != null && stat.isFile();
+    boolean existingDirectory = stat != null && stat.isDirectory();
+
     try {
       File link = getIoFile(linkPath);
       File target = getIoFile(targetPath);
-      if (target.isDirectory()) {
-        WindowsFileOperations.createJunction(link.toString(), target.toString());
-      } else if (createSymbolicLinks) {
+
+      if (!createSymbolicLinks && existingFile) {
+        // If symlinks aren't enabled and the target is an existing file, fall back to a copy.
+        Files.copy(target.toPath(), link.toPath());
+      } else if (createSymbolicLinks
+          && (existingFile || (!existingDirectory && type != SymlinkTargetType.DIRECTORY))) {
+        // If symlinks are enabled and the target is not an existing or future directory, create a
+        // symlink.
         WindowsFileOperations.createSymlink(link.toString(), target.toString());
-      } else if (!target.exists()) {
-        // Still Create a dangling junction if the target doesn't exist.
-        WindowsFileOperations.createJunction(link.toString(), target.toString());
       } else {
-        Files.copy(target.toPath(), link.toPath());
+        // Otherwise, create a junction.
+        WindowsFileOperations.createJunction(link.toString(), target.toString());
       }
-    } catch (java.nio.file.FileAlreadyExistsException e) {
+    } catch (FileAlreadyExistsException e) {
       throw new IOException(linkPath + ERR_FILE_EXISTS, e);
-    } catch (java.nio.file.AccessDeniedException e) {
+    } catch (AccessDeniedException e) {
       throw new IOException(linkPath + ERR_PERMISSION_DENIED, e);
-    } catch (java.nio.file.NoSuchFileException e) {
+    } catch (NoSuchFileException e) {
       throw new FileNotFoundException(linkPath + ERR_NO_SUCH_FILE_OR_DIR);
     }
   }

src/test/java/com/google/devtools/build/lib/vfs/PathTransformingDelegateFileSystemTest.java

@@ -101,10 +101,12 @@ public void resolveSymbolicLinks_callsDelegateWithRewrittenPathAndTransformsItBa
   public void createSymbolicLink_callsDelegateWithRewrittenPathNotTarget() throws Exception {
     PathFragment target = PathFragment.create("/original/target");
 
-    fileSystem.createSymbolicLink(PathFragment.create("/original/dir/file"), target);
+    fileSystem.createSymbolicLink(
+        PathFragment.create("/original/dir/file"), target, SymlinkTargetType.UNSPECIFIED);
 
     verify(delegateFileSystem)
-        .createSymbolicLink(PathFragment.create("/transformed/dir/file"), target);
+        .createSymbolicLink(
+            PathFragment.create("/transformed/dir/file"), target, SymlinkTargetType.UNSPECIFIED);
     verifyNoMoreInteractions(delegateFileSystem);
   }
 
@@ -158,7 +160,11 @@ private static class FileSystemMethodProvider extends TestParametersValuesProvid
             getFileSystemMethod("getPath", PathFragment.class),
             getFileSystemMethod("readSymbolicLink", PathFragment.class),
             getFileSystemMethod("resolveSymbolicLinks", PathFragment.class),
-            getFileSystemMethod("createSymbolicLink", PathFragment.class, PathFragment.class));
+            getFileSystemMethod(
+                "createSymbolicLink",
+                PathFragment.class,
+                PathFragment.class,
+                SymlinkTargetType.class));
 
     private static Method getFileSystemMethod(String name, Class<?>... parameterTypes) {
       try {

src/test/java/com/google/devtools/build/lib/windows/BUILD

@@ -43,11 +43,13 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/windows:processes",
         "//src/main/java/com/google/devtools/build/lib/windows:windows_path_operations",
         "//src/test/java/com/google/devtools/build/lib/testutil",
+        "//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
         "//src/test/java/com/google/devtools/build/lib/windows/util",
         "//third_party:guava",
         "//third_party:junit4",
         "//third_party:truth",
         "@bazel_tools//tools/java/runfiles",
+        "@maven//:com_google_testparameterinjector_test_parameter_injector",
     ],
 )