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 https://github.com/bazelbuild/bazel/issues/2238
--
Change-Id: I5827e2e70e8e147f5f102fabf95fa9a148b3bcdc
Reviewed-on: https://cr.bazel.build/8896
PiperOrigin-RevId: 147598107
MOS_MIGRATED_REVID=147598107
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/WindowsFileSystem.java
index 7a144f5..4167d81 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/WindowsFileSystem.java
@@ -327,20 +327,17 @@
@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 @@
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 {
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
index 4750ff5..0d68c36 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileOperations.java
@@ -53,6 +53,8 @@
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 @@
? ("\\\\?\\" + 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]));
+ }
+ }
}
diff --git a/src/main/native/windows_processes.cc b/src/main/native/windows_processes.cc
index f016550..a014a8c 100644
--- a/src/main/native/windows_processes.cc
+++ b/src/main/native/windows_processes.cc
@@ -625,3 +625,16 @@
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;
+}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/WindowsFileSystemTest.java b/src/test/java/com/google/devtools/build/lib/vfs/WindowsFileSystemTest.java
index 6f035c7..ea08b22 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/WindowsFileSystemTest.java
+++ b/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 @@
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();
+ }
+ }
}
diff --git a/src/test/java/com/google/devtools/build/lib/windows/util/WindowsTestUtil.java b/src/test/java/com/google/devtools/build/lib/windows/util/WindowsTestUtil.java
index 7726875..7a73b57 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/util/WindowsTestUtil.java
+++ b/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 @@
*
* <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(
