diff options
5 files changed, 343 insertions, 62 deletions
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 325176cb66..da04735831 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 @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.vfs; +import com.google.common.annotations.VisibleForTesting; import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe; import java.io.File; import java.io.FileNotFoundException; @@ -20,19 +21,17 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.attribute.BasicFileAttributes; +import java.nio.file.attribute.DosFileAttributes; -/** - * Jury-rigged file system for Windows. - */ +/** Jury-rigged file system for Windows. */ @ThreadSafe public class WindowsFileSystem extends JavaIoFileSystem { public static final LinkOption[] NO_OPTIONS = new LinkOption[0]; - public static final LinkOption[] NO_FOLLOW = new LinkOption[]{LinkOption.NOFOLLOW_LINKS}; + public static final LinkOption[] NO_FOLLOW = new LinkOption[] {LinkOption.NOFOLLOW_LINKS}; @Override - protected void createSymbolicLink(Path linkPath, PathFragment targetFragment) - throws IOException { + 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); @@ -96,7 +95,7 @@ public class WindowsFileSystem extends JavaIoFileSystem { return super.fileIsSymbolicLink(file); } - private LinkOption[] linkOpts(boolean followSymlinks) { + public static LinkOption[] symlinkOpts(boolean followSymlinks) { return followSymlinks ? NO_OPTIONS : NO_FOLLOW; } @@ -105,56 +104,58 @@ public class WindowsFileSystem extends JavaIoFileSystem { File file = getIoFile(path); final BasicFileAttributes attributes; try { - attributes = Files.readAttributes( - file.toPath(), BasicFileAttributes.class, linkOpts(followSymlinks)); + attributes = + Files.readAttributes( + file.toPath(), BasicFileAttributes.class, symlinkOpts(followSymlinks)); } catch (java.nio.file.FileSystemException e) { throw new FileNotFoundException(path + ERR_NO_SUCH_FILE_OR_DIR); } final boolean isSymbolicLink = !followSymlinks && fileIsSymbolicLink(file); - FileStatus status = new FileStatus() { - @Override - public boolean isFile() { - return attributes.isRegularFile() || (isSpecialFile() && !isDirectory()); - } - - @Override - public boolean isSpecialFile() { - return attributes.isOther(); - } - - @Override - public boolean isDirectory() { - return attributes.isDirectory(); - } - - @Override - public boolean isSymbolicLink() { - return isSymbolicLink; - } - - @Override - public long getSize() throws IOException { - return attributes.size(); - } - - @Override - public long getLastModifiedTime() throws IOException { - return attributes.lastModifiedTime().toMillis(); - } - - @Override - public long getLastChangeTime() { - // This is the best we can do with Java NIO... - return attributes.lastModifiedTime().toMillis(); - } - - @Override - public long getNodeId() { - // TODO(bazel-team): Consider making use of attributes.fileKey(). - return -1; - } - }; + FileStatus status = + new FileStatus() { + @Override + public boolean isFile() { + return attributes.isRegularFile() || (isSpecialFile() && !isDirectory()); + } + + @Override + public boolean isSpecialFile() { + return attributes.isOther(); + } + + @Override + public boolean isDirectory() { + return attributes.isDirectory(); + } + + @Override + public boolean isSymbolicLink() { + return isSymbolicLink; + } + + @Override + public long getSize() throws IOException { + return attributes.size(); + } + + @Override + public long getLastModifiedTime() throws IOException { + return attributes.lastModifiedTime().toMillis(); + } + + @Override + public long getLastChangeTime() { + // This is the best we can do with Java NIO... + return attributes.lastModifiedTime().toMillis(); + } + + @Override + public long getNodeId() { + // TODO(bazel-team): Consider making use of attributes.fileKey(). + return -1; + } + }; return status; } @@ -173,10 +174,43 @@ public class WindowsFileSystem extends JavaIoFileSystem { return super.isDirectory(path, followSymlinks); } + /** + * Returns true if the path refers to a directory junction, directory symlink, or regular symlink. + * + * <p>Directory junctions are symbolic links created with "mklink /J" where the target is a + * directory or another directory junction. Directory junctions can be created without any user + * privileges. + * + * <p>Directory symlinks are symbolic links created with "mklink /D" where the target is a + * directory or another directory symlink. Note that directory symlinks can only be created by + * Administrators. + * + * <p>Normal symlinks are symbolic links created with "mklink". Normal symlinks should not point + * at directories, because even though "mklink" can create the link, it will not be a functional + * one (the linked directory's contents cannot be listed). Only Administrators may create regular + * symlinks. + * + * <p>This method returns true for all three types as long as their target is a directory (even if + * they are dangling), though only directory junctions and directory symlinks are useful. + */ // TODO(laszlocsomor): fix https://github.com/bazelbuild/bazel/issues/1735 and use the JNI method // in WindowsFileOperations. - private static boolean isJunction(java.nio.file.Path p) throws IOException { - // Jury-rigged - return p.compareTo(p.toRealPath()) != 0; + @VisibleForTesting + static boolean isJunction(java.nio.file.Path p) throws IOException { + if (Files.exists(p, symlinkOpts(/* followSymlinks */ false))) { + DosFileAttributes attributes = + Files.readAttributes(p, DosFileAttributes.class, symlinkOpts(/* followSymlinks */ false)); + + if (attributes.isRegularFile()) { + return false; + } + + if (attributes.isDirectory()) { + return attributes.isOther(); + } else { + return attributes.isSymbolicLink(); + } + } + return false; } } diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 7adaa54b95..b79cee5e58 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -13,7 +13,9 @@ CROSS_PLATFORM_WINDOWS_TESTS = [ WINDOWS_ON_WINDOWS_TESTS = glob( ["windows/*.java"], exclude = ["windows/MockSubprocess.java"], -) +) + [ + "vfs/WindowsFileSystemTest.java", +] # All Windows-specific tests. Use this to exclude Windows tests from globs. ALL_WINDOWS_TESTS = CROSS_PLATFORM_WINDOWS_TESTS + WINDOWS_ON_WINDOWS_TESTS @@ -192,6 +194,7 @@ java_library( "windows/util/WindowsTestUtil.java", ], deps = [ + "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib:windows", "//third_party:guava", "//third_party:guava-testlib", @@ -219,6 +222,7 @@ java_test( ":testutil", ":windows_testutil", "//src/main/java/com/google/devtools/build/lib:os_util", + "//src/main/java/com/google/devtools/build/lib:vfs", "//src/main/java/com/google/devtools/build/lib:windows", "//third_party:guava", "//third_party:truth", 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 new file mode 100644 index 0000000000..48a287e1be --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/vfs/WindowsFileSystemTest.java @@ -0,0 +1,203 @@ +// Copyright 2016 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; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableMap; +import com.google.devtools.build.lib.testutil.TestSpec; +import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.windows.util.WindowsTestUtil; +import java.io.File; +import java.nio.file.Files; +import java.util.Arrays; +import java.util.HashMap; +import java.util.Map; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Unit tests for {@link WindowsFileSystem}. */ +@RunWith(JUnit4.class) +@TestSpec(localOnly = true, supportedOs = OS.WINDOWS) +public class WindowsFileSystemTest { + + private String scratchRoot; + private WindowsTestUtil testUtil; + private WindowsFileSystem fs; + + @Before + public void loadJni() throws Exception { + WindowsTestUtil.loadJni(); + scratchRoot = new File(System.getenv("TEST_TMPDIR")).getAbsolutePath() + "/x"; + testUtil = new WindowsTestUtil(scratchRoot); + fs = new WindowsFileSystem(); + cleanupScratchDir(); + } + + @After + public void cleanupScratchDir() throws Exception { + testUtil.deleteAllUnder(""); + } + + @Test + public void testCanWorkWithJunctionSymlinks() throws Exception { + testUtil.scratchFile("dir\\hello.txt", "hello"); + testUtil.scratchDir("non_existent"); + testUtil.createJunctions(ImmutableMap.of("junc", "dir", "junc_bad", "non_existent")); + + Path juncPath = testUtil.createVfsPath(fs, "junc"); + Path dirPath = testUtil.createVfsPath(fs, "dir"); + Path juncBadPath = testUtil.createVfsPath(fs, "junc_bad"); + Path nonExistentPath = testUtil.createVfsPath(fs, "non_existent"); + + // Test junction creation. + assertThat(fs.exists(juncPath, /* followSymlinks */ false)).isTrue(); + assertThat(fs.exists(dirPath, /* followSymlinks */ false)).isTrue(); + assertThat(fs.exists(juncBadPath, /* followSymlinks */ false)).isTrue(); + assertThat(fs.exists(nonExistentPath, /* followSymlinks */ false)).isTrue(); + + // Test recognizing and dereferencing a directory junction. + assertThat(fs.isSymbolicLink(juncPath)).isTrue(); + assertThat(fs.isDirectory(juncPath, /* followSymlinks */ true)).isTrue(); + assertThat(fs.isDirectory(juncPath, /* followSymlinks */ false)).isFalse(); + assertThat(fs.getDirectoryEntries(juncPath)) + .containsExactly(testUtil.createVfsPath(fs, "junc\\hello.txt")); + + // Test deleting a directory junction. + assertThat(fs.delete(juncPath)).isTrue(); + assertThat(fs.exists(juncPath, /* followSymlinks */ false)).isFalse(); + + // Test recognizing a dangling directory junction. + assertThat(fs.delete(nonExistentPath)).isTrue(); + assertThat(fs.exists(nonExistentPath, /* followSymlinks */ false)).isFalse(); + assertThat(fs.exists(juncBadPath, /* followSymlinks */ false)).isTrue(); + // TODO(bazel-team): fix https://github.com/bazelbuild/bazel/issues/1690 and uncomment the + // assertion below. + //assertThat(fs.isSymbolicLink(juncBadPath)).isTrue(); + assertThat(fs.isDirectory(juncBadPath, /* followSymlinks */ true)).isFalse(); + assertThat(fs.isDirectory(juncBadPath, /* followSymlinks */ false)).isFalse(); + + // Test deleting a dangling junction. + assertThat(fs.delete(juncBadPath)).isTrue(); + assertThat(fs.exists(juncBadPath, /* followSymlinks */ false)).isFalse(); + } + + @Test + public void testMockJunctionCreation() throws Exception { + String root = testUtil.scratchDir("dir").getParent().toString(); + testUtil.scratchFile("dir/file.txt", "hello"); + testUtil.createJunctions(ImmutableMap.of("junc", "dir")); + String[] children = new File(root + "/junc").list(); + assertThat(children).isNotNull(); + assertThat(children).hasLength(1); + assertThat(Arrays.asList(children)).containsExactly("file.txt"); + } + + @Test + public void testIsJunction() throws Exception { + final Map<String, String> junctions = new HashMap<>(); + junctions.put("shrtpath/a", "shrttrgt"); + junctions.put("shrtpath/b", "longtargetpath"); + junctions.put("shrtpath/c", "longta~1"); + junctions.put("longlinkpath/a", "shrttrgt"); + junctions.put("longlinkpath/b", "longtargetpath"); + junctions.put("longlinkpath/c", "longta~1"); + junctions.put("abbrev~1/a", "shrttrgt"); + junctions.put("abbrev~1/b", "longtargetpath"); + junctions.put("abbrev~1/c", "longta~1"); + + String root = testUtil.scratchDir("shrtpath").getParent().toAbsolutePath().toString(); + testUtil.scratchDir("longlinkpath"); + testUtil.scratchDir("abbreviated"); + testUtil.scratchDir("control/a"); + testUtil.scratchDir("control/b"); + testUtil.scratchDir("control/c"); + + testUtil.scratchFile("shrttrgt/file1.txt", "hello"); + testUtil.scratchFile("longtargetpath/file2.txt", "hello"); + + testUtil.createJunctions(junctions); + + assertThat(WindowsFileSystem.isJunction(new File(root, "shrtpath/a").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "shrtpath/b").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "shrtpath/c").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longlinkpath/a").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longlinkpath/b").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longlinkpath/c").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longli~1/a").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longli~1/b").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longli~1/c").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbreviated/a").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbreviated/b").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbreviated/c").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbrev~1/a").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbrev~1/b").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "abbrev~1/c").toPath())).isTrue(); + assertThat(WindowsFileSystem.isJunction(new File(root, "control/a").toPath())).isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "control/b").toPath())).isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "control/c").toPath())).isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "shrttrgt/file1.txt").toPath())) + .isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longtargetpath/file2.txt").toPath())) + .isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "longta~1/file2.txt").toPath())) + .isFalse(); + assertThat(WindowsFileSystem.isJunction(new File(root, "non-existent").toPath())).isFalse(); + + assertThat(Arrays.asList(new File(root + "/shrtpath/a").list())).containsExactly("file1.txt"); + assertThat(Arrays.asList(new File(root + "/shrtpath/b").list())).containsExactly("file2.txt"); + assertThat(Arrays.asList(new File(root + "/shrtpath/c").list())).containsExactly("file2.txt"); + assertThat(Arrays.asList(new File(root + "/longlinkpath/a").list())) + .containsExactly("file1.txt"); + assertThat(Arrays.asList(new File(root + "/longlinkpath/b").list())) + .containsExactly("file2.txt"); + assertThat(Arrays.asList(new File(root + "/longlinkpath/c").list())) + .containsExactly("file2.txt"); + assertThat(Arrays.asList(new File(root + "/abbreviated/a").list())) + .containsExactly("file1.txt"); + assertThat(Arrays.asList(new File(root + "/abbreviated/b").list())) + .containsExactly("file2.txt"); + assertThat(Arrays.asList(new File(root + "/abbreviated/c").list())) + .containsExactly("file2.txt"); + } + + @Test + public void testIsJunctionIsTrueForDanglingJunction() throws Exception { + java.nio.file.Path helloPath = testUtil.scratchFile("target\\hello.txt", "hello"); + testUtil.createJunctions(ImmutableMap.of("link", "target")); + + File linkPath = new File(helloPath.getParent().getParent().toFile(), "link"); + assertThat(Arrays.asList(linkPath.list())).containsExactly("hello.txt"); + assertThat(WindowsFileSystem.isJunction(linkPath.toPath())).isTrue(); + + assertThat(helloPath.toFile().delete()).isTrue(); + assertThat(helloPath.getParent().toFile().delete()).isTrue(); + assertThat(helloPath.getParent().toFile().exists()).isFalse(); + assertThat(Arrays.asList(linkPath.getParentFile().list())).containsExactly("link"); + + assertThat(WindowsFileSystem.isJunction(linkPath.toPath())).isTrue(); + assertThat( + Files.exists( + linkPath.toPath(), WindowsFileSystem.symlinkOpts(/* followSymlinks */ false))) + .isTrue(); + assertThat( + Files.exists( + linkPath.toPath(), WindowsFileSystem.symlinkOpts(/* followSymlinks */ true))) + .isFalse(); + } +} diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java index 44cd07b2e8..c899f4d369 100644 --- a/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java +++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsFileOperationsTest.java @@ -20,9 +20,11 @@ import static org.junit.Assert.fail; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.testutil.TestSpec; import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.vfs.WindowsFileSystem; import com.google.devtools.build.lib.windows.util.WindowsTestUtil; import java.io.File; import java.io.IOException; +import java.nio.file.Files; import java.util.Arrays; import java.util.HashMap; import java.util.Map; @@ -132,4 +134,29 @@ public class WindowsFileOperationsTest { assertThat(Arrays.asList(new File(root + "/abbreviated/c").list())) .containsExactly("file2.txt"); } + + @Test + public void testIsJunctionIsTrueForDanglingJunction() throws Exception { + java.nio.file.Path helloPath = testUtil.scratchFile("target\\hello.txt", "hello"); + testUtil.createJunctions(ImmutableMap.of("link", "target")); + + File linkPath = new File(helloPath.getParent().getParent().toFile(), "link"); + assertThat(Arrays.asList(linkPath.list())).containsExactly("hello.txt"); + assertThat(WindowsFileOperations.isJunction(linkPath.getAbsolutePath())).isTrue(); + + assertThat(helloPath.toFile().delete()).isTrue(); + assertThat(helloPath.getParent().toFile().delete()).isTrue(); + assertThat(helloPath.getParent().toFile().exists()).isFalse(); + assertThat(Arrays.asList(linkPath.getParentFile().list())).containsExactly("link"); + + assertThat(WindowsFileOperations.isJunction(linkPath.getAbsolutePath())).isTrue(); + assertThat( + Files.exists( + linkPath.toPath(), WindowsFileSystem.symlinkOpts(/* followSymlinks */ false))) + .isTrue(); + assertThat( + Files.exists( + linkPath.toPath(), WindowsFileSystem.symlinkOpts(/* followSymlinks */ true))) + .isFalse(); + } } 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 0a5d5436b5..c68d5199fa 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 @@ -19,6 +19,8 @@ import static org.junit.Assert.fail; import com.google.common.base.Joiner; 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.WindowsJniLoader; import java.io.BufferedReader; import java.io.File; @@ -29,7 +31,6 @@ import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.Charset; import java.nio.file.Files; -import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -55,13 +56,13 @@ public final class WindowsTestUtil { } /** - * Create directory junctions. + * Create directory junctions then assert their existence. * * <p>Each key in the map is a junction path, relative to {@link #scratchRoot}. These are the link * names. * - * <p>Each value in the map is a directory or junction path, also relative to - * {@link #scratchRoot}. These are the link targets. + * <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". */ @@ -87,6 +88,13 @@ public final class WindowsTestUtil { "mklink /j \"%s/%s\" \"%s/%s\"", scratchRoot, e.getKey(), scratchRoot, e.getValue())); } runCommand(args); + + for (Map.Entry<String, String> e : links.entrySet()) { + assertWithMessage( + String.format("Could not create junction '%s' -> '%s'", e.getKey(), e.getValue())) + .that(new File(scratchRoot, e.getKey()).exists()) + .isTrue(); + } } /** Delete everything under {@link #scratchRoot}/path. */ @@ -102,12 +110,12 @@ public final class WindowsTestUtil { } /** Create a directory under `path`, relative to {@link #scratchRoot}. */ - public Path scratchDir(String path) throws IOException { + public java.nio.file.Path scratchDir(String path) throws IOException { return Files.createDirectories(new File(scratchRoot, path).toPath()); } /** Create a file with the given contents under `path`, relative to {@link #scratchRoot}. */ - public void scratchFile(String path, String... contents) throws IOException { + public java.nio.file.Path scratchFile(String path, String... contents) throws IOException { File fd = new File(scratchRoot, path); Files.createDirectories(fd.toPath().getParent()); try (FileWriter w = new FileWriter(fd)) { @@ -116,6 +124,7 @@ public final class WindowsTestUtil { w.write('\n'); } } + return fd.toPath(); } /** Run a Command Prompt command. */ @@ -160,4 +169,8 @@ public final class WindowsTestUtil { } } } + + public Path createVfsPath(FileSystem fs, String path) throws IOException { + return fs.getPath(scratchRoot + "/" + path); + } } |