Windows: use JNI CreateJunction in Java code

Use the new CreateJunction in the Windows JNI code
every time we need to create junctions. This means
updating WindowsFileOperations and related tests.

Add test for WindowsFileSystem.createSymbolicLink.

See bazelbuild#2238

--
Change-Id: I5827e2e70e8e147f5f102fabf95fa9a148b3bcdc
Reviewed-on: https://cr.bazel.build/8896
PiperOrigin-RevId: 147598107
MOS_MIGRATED_REVID=147598107

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

@@ -327,20 +327,17 @@ public String getFileSystemType(Path path) {
 
   @Override
   protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) throws IOException {
-    // TODO(lberki): Add some JNI to create hard links/junctions instead of calling out to
-    // cmd.exe
-    File file = getIoFile(linkPath);
+    Path targetPath =
+        targetFragment.isAbsolute()
+            ? getPath(targetFragment)
+            : linkPath.getParentDirectory().getRelative(targetFragment);
     try {
-      File targetFile = new File(targetFragment.getPathString());
-      if (targetFile.isDirectory()) {
-        createDirectoryJunction(targetFile, file);
+      java.nio.file.Path link = getIoFile(linkPath).toPath();
+      java.nio.file.Path target = getIoFile(targetPath).toPath();
+      if (target.toFile().isDirectory()) {
+        WindowsFileOperations.createJunction(link.toString(), target.toString());
       } else {
-        if (targetFile.isAbsolute()) {
-          Files.copy(targetFile.toPath(), file.toPath());
-        } else {
-          // When targetFile is a relative path to linkPath, resolve it to an absolute path first.
-          Files.copy(file.toPath().getParent().resolve(targetFile.toPath()), file.toPath());
-        }
+        Files.copy(target, link);
       }
     } catch (java.nio.file.FileAlreadyExistsException e) {
       throw new IOException(linkPath + ERR_FILE_EXISTS);
@@ -361,28 +358,6 @@ public boolean isFilePathCaseSensitive() {
     return false;
   }
 
-  private void createDirectoryJunction(File sourceDirectory, File targetPath) throws IOException {
-    StringBuilder cl = new StringBuilder("cmd.exe /c ");
-    cl.append("mklink /J ");
-    cl.append('"');
-    cl.append(targetPath.getAbsolutePath());
-    cl.append('"');
-    cl.append(' ');
-    cl.append('"');
-    cl.append(sourceDirectory.getAbsolutePath());
-    cl.append('"');
-    Process process = Runtime.getRuntime().exec(cl.toString());
-    try {
-      process.waitFor();
-      if (process.exitValue() != 0) {
-        throw new IOException("Command failed " + cl);
-      }
-    } catch (InterruptedException e) {
-      Thread.currentThread().interrupt();
-      throw new IOException("Command failed ", e);
-    }
-  }
-
   @Override
   protected boolean fileIsSymbolicLink(File file) {
     try {

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

@@ -53,6 +53,8 @@ private WindowsFileOperations() {
 
   static native boolean nativeGetLongPath(String path, String[] result, String[] error);
 
+  static native boolean nativeCreateJunction(String name, String target, String[] error);
+
   /** Determines whether `path` is a junction point or directory symlink. */
   public static boolean isJunction(String path) throws IOException {
     WindowsJniLoader.loadJni();
@@ -101,4 +103,22 @@ static String asLongPath(String path) {
         ? ("\\\\?\\" + path.replace('/', '\\'))
         : path;
   }
+
+  /**
+   * Creates a junction at `name`, pointing to `target`.
+   *
+   * <p>Both `name` and `target` may be Unix-style Windows paths (i.e. use forward slashes), and
+   * they don't need to have a UNC prefix, not even if they are longer than `MAX_PATH`. The
+   * underlying logic will take care of adding the prefixes if necessary.
+   *
+   * @throws IOException if some error occurs
+   */
+  public static void createJunction(String name, String target) throws IOException {
+    WindowsJniLoader.loadJni();
+    String[] error = new String[] {null};
+    if (!nativeCreateJunction(name.replace('/', '\\'), target.replace('/', '\\'), error)) {
+      throw new IOException(
+          String.format("Cannot create junction (name=%s, target=%s): %s", name, target, error[0]));
+    }
+  }
 }

src/main/native/windows_processes.cc

@@ -625,3 +625,16 @@ Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeGetLongPa
                      wcslen(result.get())));
   return JNI_TRUE;
 }
+
+extern "C" JNIEXPORT jboolean JNICALL
+Java_com_google_devtools_build_lib_windows_WindowsFileOperations_nativeCreateJunction(
+    JNIEnv* env, jclass clazz, jstring name, jstring target,
+    jobjectArray error_msg_holder) {
+  std::string error = windows_util::CreateJunction(GetJavaWstring(env, name),
+                                                   GetJavaWstring(env, target));
+  if (!error.empty()) {
+    MaybeReportLastError(error, env, error_msg_holder);
+    return JNI_FALSE;
+  }
+  return JNI_TRUE;
+}

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

@@ -17,10 +17,14 @@
 import static com.google.common.truth.Truth.assertThat;
 import static org.junit.Assert.assertSame;
 
+import com.google.common.base.Function;
 import com.google.common.base.Predicate;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.testutil.TestSpec;
 import com.google.devtools.build.lib.util.OS;
+import com.google.devtools.build.lib.windows.WindowsFileOperations;
 import com.google.devtools.build.lib.windows.util.WindowsTestUtil;
 import java.io.File;
 import java.io.IOException;
@@ -320,4 +324,46 @@ public void testShortPathResolvesToDifferentPathsOverTime() throws Exception {
     assertThat(r1).isNotEqualTo(q1);
     assertThat(r1).isNotSameAs(q1);
   }
+
+  @Test
+  public void testCreateSymbolicLink() throws Exception {
+    // Create the `scratchRoot` directory.
+    assertThat(fs.getPath(scratchRoot).createDirectory()).isTrue();
+    // Create symlink with directory target, relative path.
+    Path link1 = fs.getPath(scratchRoot).getRelative("link1");
+    fs.createSymbolicLink(link1, new PathFragment(".."));
+    // Create symlink with directory target, absolute path.
+    Path link2 = fs.getPath(scratchRoot).getRelative("link2");
+    fs.createSymbolicLink(link2, fs.getPath(scratchRoot).getRelative("link1").asFragment());
+    // Create scratch files that'll be symlink targets.
+    testUtil.scratchFile("foo.txt", "hello");
+    testUtil.scratchFile("bar.txt", "hello");
+    // Create symlink with file target, relative path.
+    Path link3 = fs.getPath(scratchRoot).getRelative("link3");
+    fs.createSymbolicLink(link3, new PathFragment("foo.txt"));
+    // Create symlink with file target, absolute path.
+    Path link4 = fs.getPath(scratchRoot).getRelative("link4");
+    fs.createSymbolicLink(link4, fs.getPath(scratchRoot).getRelative("bar.txt").asFragment());
+    // Assert that link1 and link2 are true junctions and have the right contents.
+    for (Path p : ImmutableList.of(link1, link2)) {
+      assertThat(WindowsFileOperations.isJunction(p.getPathString())).isTrue();
+      assertThat(p.isSymbolicLink()).isTrue();
+      assertThat(
+              Iterables.transform(
+                  Arrays.asList(new File(p.getPathString()).listFiles()),
+                  new Function<File, String>() {
+                    @Override
+                    public String apply(File input) {
+                      return input.getName();
+                    }
+                  }))
+          .containsExactly("x");
+    }
+    // Assert that link3 and link4 are copies of files.
+    for (Path p : ImmutableList.of(link3, link4)) {
+      assertThat(WindowsFileOperations.isJunction(p.getPathString())).isFalse();
+      assertThat(p.isSymbolicLink()).isFalse();
+      assertThat(p.isFile()).isTrue();
+    }
+  }
 }

src/test/java/com/google/devtools/build/lib/windows/util/WindowsTestUtil.java

@@ -21,13 +21,13 @@
 import com.google.common.base.Strings;
 import com.google.devtools.build.lib.vfs.FileSystem;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.windows.WindowsFileOperations;
 import com.google.devtools.build.lib.windows.WindowsJniLoader;
 import com.google.devtools.build.lib.windows.WindowsRunfiles;
 import java.io.File;
 import java.io.FileWriter;
 import java.io.IOException;
 import java.nio.file.Files;
-import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
@@ -55,31 +55,12 @@ public static void loadJni() {
    *
    * <p>Each value in the map is a directory or junction path, also relative to {@link
    * #scratchRoot}. These are the link targets.
-   *
-   * <p>This method creates all junctions in one invocation to "cmd.exe".
    */
-  // Do not use WindowsFileSystem.createDirectoryJunction but reimplement junction creation here.
-  // If that method were buggy, using it here would compromise the test.
   public void createJunctions(Map<String, String> links) throws Exception {
-    List<String> args = new ArrayList<>();
-    boolean first = true;
-
-    // Shell out to cmd.exe to create all junctions in one go.
-    // Running "cmd.exe /c command1 arg1 arg2 && command2 arg1 ... argN && ..." will run all
-    // commands within one cmd.exe invocation.
     for (Map.Entry<String, String> e : links.entrySet()) {
-      if (first) {
-        args.add("cmd.exe /c");
-        first = false;
-      } else {
-        args.add("&&");
-      }
-
-      args.add(
-          String.format(
-              "mklink /j \"%s/%s\" \"%s/%s\"", scratchRoot, e.getKey(), scratchRoot, e.getValue()));
+      WindowsFileOperations.createJunction(
+          scratchRoot + "/" + e.getKey(), scratchRoot + "/" + e.getValue());
     }
-    runCommand(args);
 
     for (Map.Entry<String, String> e : links.entrySet()) {
       assertWithMessage(