Move msys path support into DependencySet, removing it from the general Path system.
This is the only place that should actually need it.
PiperOrigin-RevId: 179054861
diff --git a/src/main/java/com/google/devtools/build/lib/util/DependencySet.java b/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
index 358610d..e2ce5fb 100644
--- a/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
+++ b/src/main/java/com/google/devtools/build/lib/util/DependencySet.java
@@ -24,6 +24,7 @@
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
+import java.util.concurrent.atomic.AtomicReference;
/**
* Representation of a set of file dependencies for a given output file. There
@@ -99,10 +100,18 @@
* Adds a given dependency to this DependencySet instance.
*/
private void addDependency(String dep) {
+ dep = translatePath(dep);
Path depPath = root.getRelative(dep);
dependencies.add(depPath);
}
+ private String translatePath(String path) {
+ if (OS.getCurrent() != OS.WINDOWS) {
+ return path;
+ }
+ return WindowsPath.translateWindowsPath(path);
+ }
+
/**
* Reads a dotd file into this DependencySet instance.
*/
@@ -241,4 +250,64 @@
public int hashCode() {
return dependencies.hashCode();
}
+
+ private static final class WindowsPath {
+ private static final AtomicReference<String> UNIX_ROOT = new AtomicReference<>(null);
+
+ private static String translateWindowsPath(String path) {
+ int n = path.length();
+ if (n == 0 || path.charAt(0) != '/') {
+ return path;
+ }
+ if (n >= 2 && isAsciiLetter(path.charAt(1)) && (n == 2 || path.charAt(2) == '/')) {
+ StringBuilder sb = new StringBuilder(path.length());
+ sb.append(Character.toUpperCase(path.charAt(1)));
+ sb.append(":/");
+ sb.append(path, 2, path.length());
+ return sb.toString();
+ } else {
+ String unixRoot = getUnixRoot();
+ return unixRoot + path;
+ }
+ }
+
+ private static boolean isAsciiLetter(char c) {
+ return (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+ }
+
+ private static String getUnixRoot() {
+ String value = UNIX_ROOT.get();
+ if (value == null) {
+ String jvmFlag = "bazel.windows_unix_root";
+ value = determineUnixRoot(jvmFlag);
+ if (value == null) {
+ throw new IllegalStateException(
+ String.format(
+ "\"%1$s\" JVM flag is not set. Use the --host_jvm_args flag. "
+ + "For example: "
+ + "\"--host_jvm_args=-D%1$s=c:/tools/msys64\".",
+ jvmFlag));
+ }
+ value = value.replace('\\', '/');
+ if (value.length() > 3 && value.endsWith("/")) {
+ value = value.substring(0, value.length() - 1);
+ }
+ UNIX_ROOT.set(value);
+ }
+ return value;
+ }
+
+ private static String determineUnixRoot(String jvmArgName) {
+ // Get the path from a JVM flag, if specified.
+ String path = System.getProperty(jvmArgName);
+ if (path == null) {
+ return null;
+ }
+ path = path.trim();
+ if (path.isEmpty()) {
+ return null;
+ }
+ return path;
+ }
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/Path.java b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
index 6fe6118..1d3947d 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/Path.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/Path.java
@@ -78,11 +78,6 @@
/**
* Makes the proper invocation of {@link FileSystem#getCachedChildPathInternal}, doing
* filesystem-specific logic if necessary.
- *
- * <p>On Unix filesystems this method merely calls through to {@code
- * FileSystem.getCachedChildPathInternal(parent, child)}, but on Windows this can be used to
- * handle the translatation of absolute Unix paths to absolute Windows paths, e.g. "/c" to "C:/"
- * or "/usr" to "C:/tools/msys64/usr".
*/
Path getCachedChildPathInternal(Path path, String childName);
}
diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
index 21de058..af97e27 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsFileSystem.java
@@ -32,7 +32,6 @@
import java.nio.file.LinkOption;
import java.nio.file.attribute.DosFileAttributes;
import java.util.Arrays;
-import java.util.concurrent.atomic.AtomicReference;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
import javax.annotation.Nullable;
@@ -106,42 +105,11 @@
private static Path getCachedChildPathInternalImpl(
Path parent, String child, Function<String, String> resolver) {
if (parent != null && parent.isRootDirectory()) {
- // This is a top-level directory. It's either a drive name ("C:" or "c") or some other
- // Unix path (e.g. "/usr").
- //
- // We need to translate it to an absolute Windows path. The correct way would be looking
- // up /etc/mtab to see if any mount point matches the prefix of the path, and change the
- // prefix to the mounted path. Looking up /etc/mtab each time we create a path however
- // would be too expensive so we use a heuristic instead.
- //
- // If the name looks like a volume name ("C:" or "c") then we treat it as such, otherwise
- // we make it relative to UNIX_ROOT, thus "/usr" becomes "C:/tools/msys64/usr".
- //
- // This heuristic ignores other mount points as well as procfs.
-
- // TODO(laszlocsomor): use GetLogicalDrives to retrieve the list of drives and only apply
- // this heuristic for the valid drives. It's possible that the user has a directory "/a"
- // but no "A:\" drive, so in that case we should prepend the MSYS root.
-
+ // This is a top-level directory. It must be a drive name ("C:" or "c").
if (WindowsPath.isWindowsVolumeName(child)) {
child = WindowsPath.getDriveLetter((WindowsPath) parent, child) + ":";
} else {
- if (UNIX_ROOT.get() == null) {
- String jvmFlag = "bazel.windows_unix_root";
- PathFragment value = determineUnixRoot(jvmFlag);
- if (value == null) {
- throw new IllegalStateException(
- String.format(
- "\"%1$s\" JVM flag is not set. Use the --host_jvm_args flag or export the "
- + "BAZEL_SH environment variable. For example "
- + "\"--host_jvm_args=-D%1$s=c:/tools/msys64\" or "
- + "\"set BAZEL_SH=c:/tools/msys64/usr/bin/bash.exe\". "
- + "parent=(%2$s) name=(%3$s)",
- jvmFlag, parent, child));
- }
- UNIX_ROOT.set(value);
- }
- parent = parent.getRelative(UNIX_ROOT.get());
+ throw new IllegalArgumentException("Cannot create Unix-style paths on Windows.");
}
}
@@ -303,8 +271,6 @@
return WindowsPathFactory.createForTesting(mockResolver);
}
- private static final AtomicReference<PathFragment> UNIX_ROOT = new AtomicReference<>(null);
-
public static final LinkOption[] NO_OPTIONS = new LinkOption[0];
public static final LinkOption[] NO_FOLLOW = new LinkOption[] {LinkOption.NOFOLLOW_LINKS};
@@ -478,23 +444,4 @@
return Files.readAttributes(
file.toPath(), DosFileAttributes.class, symlinkOpts(followSymlinks));
}
-
- private static PathFragment determineUnixRoot(String jvmArgName) {
- // Get the path from a JVM flag, if specified.
- String path = System.getProperty(jvmArgName);
- if (path == null) {
- return null;
- }
-
- path = path.trim();
- if (path.isEmpty()) {
- return null;
- }
-
- PathFragment result = PathFragment.create(path);
- if (result.getDriveLetter() == '\0' || !result.isAbsolute()) {
- return null;
- }
- return result;
- }
}
diff --git a/src/test/java/com/google/devtools/build/lib/util/DependencySetWindowsTest.java b/src/test/java/com/google/devtools/build/lib/util/DependencySetWindowsTest.java
index c0099d2..d4a079d 100644
--- a/src/test/java/com/google/devtools/build/lib/util/DependencySetWindowsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/util/DependencySetWindowsTest.java
@@ -79,8 +79,8 @@
Set<Path> expected = Sets.newHashSet(
root.getRelative("cpp/hello-lib.cc"),
root.getRelative("cpp/hello-lib.h"),
- fileSystem.getPath("/mingw/include/stdio.h"),
- fileSystem.getPath("/mingw/include/_mingw.h"),
+ fileSystem.getPath("C:/fake/msys/mingw/include/stdio.h"),
+ fileSystem.getPath("C:/fake/msys/mingw/include/_mingw.h"),
fileSystem.getPath("C:/Program Files (x86)/LLVM/lib/clang/3.5.0/include/stddef.h"),
fileSystem.getPath("C:/Program Files (x86)/LLVM/lib/clang/3.5.0/include/stdarg.h"));
diff --git a/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java b/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
index 1bd3779..fb3a9aa 100644
--- a/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/windows/PathWindowsTest.java
@@ -124,29 +124,10 @@
}
@Test
- public void testAbsoluteUnixPathIsRelativeToWindowsUnixRoot() {
- Path actual = root.getRelative("/foo/bar");
- Path expected = root.getRelative("C:/fake/msys/foo/bar");
- assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
- assertThat(actual).isEqualTo(expected);
- }
-
- @Test
- public void testAbsoluteUnixPathReferringToDriveIsRecognized() {
- Path actual = root.getRelative("/c/foo");
- Path expected = root.getRelative("C:/foo");
- Path weird = root.getRelative("/c:");
- assertThat(actual.getPathString()).isEqualTo(expected.getPathString());
- assertThat(actual).isEqualTo(expected);
- assertThat(weird).isNotEqualTo(expected);
- }
-
- @Test
public void testStartsWithWorksOnWindows() {
assertStartsWithReturnsOnWindows(true, "C:/first/x", "C:/first/x/y");
assertStartsWithReturnsOnWindows(true, "c:/first/x", "C:/FIRST/X/Y");
assertStartsWithReturnsOnWindows(true, "C:/FIRST/X", "c:/first/x/y");
- assertStartsWithReturnsOnWindows(true, "/", "C:/");
assertStartsWithReturnsOnWindows(false, "C:/", "/");
assertStartsWithReturnsOnWindows(false, "C:/", "D:/");
assertStartsWithReturnsOnWindows(false, "C:/", "D:/foo");
@@ -168,41 +149,6 @@
}
@Test
- public void testChildRegistrationWithTranslatedPaths() {
- // Ensure the Path to "/usr" (actually "C:/fake/msys/usr") is created, path parents/children
- // properly registered.
- WindowsPath usrPath = (WindowsPath) root.getRelative("/usr");
- root.getRelative("dummy_path");
-
- // Assert that "usr" is not registered as a child of "/".
- final List<String> children = new ArrayList<>(2);
- root.applyToChildren(
- new Predicate<Path>() {
- @Override
- public boolean apply(Path input) {
- children.add(input.getPathString());
- return true;
- }
- });
- assertThat(children).containsAllOf("C:/fake", "C:/dummy_path");
-
- // Assert that "usr" is registered as a child of "C:/fake/msys/".
- children.clear();
- ((WindowsPath) root.getRelative("C:/fake/msys"))
- .applyToChildren(
- new Predicate<Path>() {
- @Override
- public boolean apply(Path input) {
- children.add(input.getPathString());
- return true;
- }
- });
- assertThat(children).containsExactly("C:/fake/msys/usr");
-
- assertThat(usrPath).isEqualTo(root.getRelative("C:/fake/msys/usr"));
- }
-
- @Test
public void testResolvesShortenedPaths() {
shortPathResolver.resolutions.put("d:/progra~1", "program files");
shortPathResolver.resolutions.put("d:/program files/micros~1", "microsoft something");
