diff --git a/src/main/java/com/google/devtools/build/lib/actions/BUILD b/src/main/java/com/google/devtools/build/lib/actions/BUILD index 6d292df26fe5ba..015352cee07104 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/BUILD +++ b/src/main/java/com/google/devtools/build/lib/actions/BUILD @@ -111,6 +111,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:var_int", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/main/java/com/google/devtools/build/skyframe", diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD index 1f204fe5c49e4e..6c6d268f80c829 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD +++ b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD @@ -64,6 +64,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/util", "//src/main/java/com/google/devtools/build/lib/util:string", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:auto_value", "//third_party:caffeine", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD index 31d9f96b0108fb..71aed181962d57 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/BUILD @@ -112,6 +112,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/util:string", "//src/main/java/com/google/devtools/build/lib/util/io", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/skyframe", "//src/main/java/com/google/devtools/build/skyframe:skyframe-objects", diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java index 0e0b319ad7157d..a76721ca5862a7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppFileTypes.java @@ -18,12 +18,14 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.util.FileType; import com.google.devtools.build.lib.util.FileTypeSet; +import com.google.devtools.build.lib.vfs.OsPathPolicy; import java.util.regex.Pattern; - /** * C++-related file type definitions. */ public final class CppFileTypes { + private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs(); + // .cu and .cl are CUDA and OpenCL source extensions, respectively. They are expected to only be // supported with clang. Bazel is not officially supporting these targets, and the extensions are // listed only as long as they work with the existing C++ actions. @@ -64,7 +66,7 @@ public final class CppFileTypes { @Override public boolean apply(String path) { - return path.endsWith(ext) && !PIC_PREPROCESSED_C.matches(path); + return OS.endsWith(path, ext) && !PIC_PREPROCESSED_C.matches(path); } @Override @@ -79,7 +81,7 @@ public ImmutableList getExtensions() { @Override public boolean apply(String path) { - return path.endsWith(ext) && !PIC_PREPROCESSED_CPP.matches(path); + return OS.endsWith(path, ext) && !PIC_PREPROCESSED_CPP.matches(path); } @Override @@ -96,7 +98,7 @@ public ImmutableList getExtensions() { @Override public boolean apply(String path) { - return (path.endsWith(ext) && !PIC_ASSEMBLER.matches(path)) || path.endsWith(".asm"); + return (OS.endsWith(path, ext) && !PIC_ASSEMBLER.matches(path)) || OS.endsWith(path, ".asm"); } @Override @@ -114,11 +116,11 @@ public ImmutableList getExtensions() { public boolean apply(String path) { if (PIC_ARCHIVE.matches(path) || ALWAYS_LINK_LIBRARY.matches(path) - || path.endsWith(".if.lib")) { + || OS.endsWith(path, ".if.lib")) { return false; } for (String ext : extensions) { - if (path.endsWith(ext)) { + if (OS.endsWith(path, ext)) { return true; } } @@ -138,8 +140,8 @@ public ImmutableList getExtensions() { @Override public boolean apply(String path) { - return (path.endsWith(ext) && !ALWAYS_LINK_PIC_LIBRARY.matches(path)) - || path.endsWith(".lo.lib"); + return (OS.endsWith(path, ext) && !ALWAYS_LINK_PIC_LIBRARY.matches(path)) + || OS.endsWith(path, ".lo.lib"); } @Override @@ -155,7 +157,7 @@ public ImmutableList getExtensions() { @Override public boolean apply(String path) { - return (path.endsWith(ext) && !PIC_OBJECT_FILE.matches(path)) || path.endsWith(".obj"); + return (OS.endsWith(path, ext) && !PIC_OBJECT_FILE.matches(path)) || OS.endsWith(path, ".obj"); } @Override @@ -220,7 +222,7 @@ public boolean apply(String path) { // The current clang (clang-600.0.57) on Darwin doesn't support 'textual', so we can't // have '.inc' files in the module map (since they're implictly textual). // TODO(bazel-team): Use HEADERS file type once clang-700 is the base clang we support. - return artifact.getFilename().endsWith(".h"); + return OS.endsWith(artifact.getFilename(), ".h"); } }; diff --git a/src/main/java/com/google/devtools/build/lib/util/BUILD b/src/main/java/com/google/devtools/build/lib/util/BUILD index 497deb8b5939d4..7e191e59b83df7 100644 --- a/src/main/java/com/google/devtools/build/lib/util/BUILD +++ b/src/main/java/com/google/devtools/build/lib/util/BUILD @@ -206,6 +206,7 @@ java_library( deps = [ ":string", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//third_party:guava", "//third_party:jsr305", ], diff --git a/src/main/java/com/google/devtools/build/lib/util/FileType.java b/src/main/java/com/google/devtools/build/lib/util/FileType.java index cdbd187238af0a..9f2121e6f02987 100644 --- a/src/main/java/com/google/devtools/build/lib/util/FileType.java +++ b/src/main/java/com/google/devtools/build/lib/util/FileType.java @@ -21,6 +21,8 @@ import com.google.common.collect.Iterables; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec.VisibleForSerialization; +import com.google.devtools.build.lib.vfs.OsPathPolicy; + import java.util.ArrayList; import java.util.List; import javax.annotation.concurrent.Immutable; @@ -28,6 +30,8 @@ /** A base class for FileType matchers. */ @Immutable public abstract class FileType implements Predicate { + private static final OsPathPolicy OS = OsPathPolicy.getFilePathOs(); + // A special file type @AutoCodec @VisibleForSerialization public static final FileType NO_EXTENSION = @@ -63,7 +67,7 @@ static final class SingletonFileType extends FileType { @Override public boolean apply(String path) { - return path.endsWith(ext); + return OS.endsWith(path, ext); } @Override @@ -86,7 +90,7 @@ static final class ListFileType extends FileType { public boolean apply(String path) { // Do not use an iterator based for loop here as that creates excessive garbage. for (int i = 0; i < extensions.size(); i++) { - if (path.endsWith(extensions.get(i))) { + if (OS.endsWith(path, extensions.get(i))) { return true; } } diff --git a/src/main/java/com/google/devtools/build/lib/vfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/BUILD index aa14b9f25fbfc7..361d0614f491ae 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/BUILD +++ b/src/main/java/com/google/devtools/build/lib/vfs/BUILD @@ -8,30 +8,42 @@ filegroup( visibility = ["//src:__subpackages__"], ) +OS_PATH_POLICY_SOURCES = [ + "OsPathPolicy.java", + "UnixOsPathPolicy.java", + "WindowsOsPathPolicy.java", +] + PATH_FRAGMENT_SOURCES = [ "PathFragment.java", "PathFragmentSerializationProxy.java", "PathSegmentIterator.java", - "OsPathPolicy.java", - "UnixOsPathPolicy.java", - "WindowsOsPathPolicy.java", ] OUTPUT_SERVICE_SOURCES = [ "OutputService.java", ] +java_library( + name = "ospathpolicy", + srcs = OS_PATH_POLICY_SOURCES, + deps = [ + "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/main/java/com/google/devtools/build/lib/windows:file", + "//src/main/java/com/google/devtools/build/lib/windows:windows_short_path", + "//third_party:guava", + ], +) + java_library( name = "pathfragment", srcs = PATH_FRAGMENT_SOURCES, deps = [ + ":ospathpolicy", "//src/main/java/com/google/devtools/build/lib/actions:commandline_item", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant", "//src/main/java/com/google/devtools/build/lib/util:filetype", - "//src/main/java/com/google/devtools/build/lib/util:os", - "//src/main/java/com/google/devtools/build/lib/windows:file", - "//src/main/java/com/google/devtools/build/lib/windows:windows_short_path", "//third_party:error_prone_annotations", "//third_party:guava", "//third_party:jsr305", @@ -45,7 +57,7 @@ java_library( [ "*.java", ], - exclude = PATH_FRAGMENT_SOURCES + OUTPUT_SERVICE_SOURCES, + exclude = PATH_FRAGMENT_SOURCES + OUTPUT_SERVICE_SOURCES + OS_PATH_POLICY_SOURCES, ), deps = [ ":pathfragment", diff --git a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/BUILD b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/BUILD index df9105bc66f8d8..d30d154849b226 100644 --- a/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/BUILD +++ b/src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs/BUILD @@ -18,6 +18,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/concurrent", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:auto_value", "//third_party:guava", diff --git a/src/test/java/com/google/devtools/build/lib/util/BUILD b/src/test/java/com/google/devtools/build/lib/util/BUILD index dee5395ae3c615..93dd2d05895b63 100644 --- a/src/test/java/com/google/devtools/build/lib/util/BUILD +++ b/src/test/java/com/google/devtools/build/lib/util/BUILD @@ -88,6 +88,37 @@ java_test( ], ) +# Tests windows specific filetype handling on Unix. +java_library( + name = "FileTypeTests_lib", + srcs = ["FileTypeTest.java"], + + deps = [ + "//src/main/java/com/google/devtools/build/lib/util:filetype", + "//src/main/java/com/google/devtools/build/lib/util:os", + "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", + "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//src/main/java/com/google/devtools/build/lib/vfs", + "//third_party:guava", + "//third_party:junit4", + "//third_party:truth", + ], +) + +java_test( + name = "FileTypeWindowsTests", + size = "small", + jvm_flags = [ + "-Dblaze.os=Windows", + "-Dbazel.windows_unix_root=C:/fake/msys", + ], + test_class = "com.google.devtools.build.lib.AllTests", + runtime_deps = [ + ":FileTypeTests_lib", + "//src/test/java/com/google/devtools/build/lib:test_runner", + ], +) + # Tests windows specific path handling on Unix. java_library( name = "UtilWindowsTests_lib", diff --git a/src/test/java/com/google/devtools/build/lib/util/FileTypeTest.java b/src/test/java/com/google/devtools/build/lib/util/FileTypeTest.java index 0cf7562de196bd..a2989f42bf090f 100644 --- a/src/test/java/com/google/devtools/build/lib/util/FileTypeTest.java +++ b/src/test/java/com/google/devtools/build/lib/util/FileTypeTest.java @@ -23,6 +23,7 @@ import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import com.google.devtools.build.lib.util.OS; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -56,6 +57,13 @@ public String toString() { } } + private static void assertTrueOnWindows(boolean condition) { + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(condition).isTrue(); + } else { + assertThat(condition).isFalse(); + } + } @Test public void simpleDotMatch() { assertThat(TEXT.matches("readme.txt")).isTrue(); @@ -80,18 +88,25 @@ public void picksLastExtension() { @Test public void onlyExtensionStillMatches() { assertThat(TEXT.matches(".txt")).isTrue(); + assertTrueOnWindows(TEXT.matches(".TXT")); } @Test public void handlesPathObjects() { Path readme = new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/readme.txt"); + Path readme_uppercase = new InMemoryFileSystem(DigestHashFunction.SHA256).getPath("/readme.TXT"); + assertThat(TEXT.matches(readme)).isTrue(); + assertTrueOnWindows(TEXT.matches(readme_uppercase)); } @Test public void handlesPathFragmentObjects() { PathFragment readme = PathFragment.create("some/where/readme.txt"); + PathFragment readme_uppercase = PathFragment.create("some/where/readme.TXT"); + assertThat(TEXT.matches(readme)).isTrue(); + assertTrueOnWindows(TEXT.matches(readme_uppercase)); } @Test @@ -100,13 +115,15 @@ public void fileTypeSetContains() { assertThat(allowedTypes.matches("readme.txt")).isTrue(); assertThat(allowedTypes.matches("style.css")).isFalse(); + assertTrueOnWindows(allowedTypes.matches("readme.TXT")); } private List getArtifacts() { return Lists.newArrayList( new HasFileTypeImpl("Foo.java"), new HasFileTypeImpl("bar.cc"), - new HasFileTypeImpl("baz.py")); + new HasFileTypeImpl("baz.py"), + new HasFileTypeImpl("Foobar.CC")); } private String filterAll(FileType... fileTypes) { @@ -120,13 +137,22 @@ public void justJava() { @Test public void javaAndCpp() { - assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE)).isEqualTo("Foo.java bar.cc"); + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE)).isEqualTo("Foo.java bar.cc Foobar.CC"); + } else { + assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE)).isEqualTo("Foo.java bar.cc"); + } } @Test public void allThree() { - assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE, PYTHON_SOURCE)) - .isEqualTo("Foo.java bar.cc baz.py"); + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE, PYTHON_SOURCE)) + .isEqualTo("Foo.java bar.cc baz.py Foobar.CC"); + } else { + assertThat(filterAll(JAVA_SOURCE, CPP_SOURCE, PYTHON_SOURCE)) + .isEqualTo("Foo.java bar.cc baz.py"); + } } private HasFileType filename(final String name) { @@ -136,18 +162,21 @@ private HasFileType filename(final String name) { @Test public void checkingSingleWithTypePredicate() throws Exception { HasFileType item = filename("config.txt"); + HasFileType item_uppercase = filename("config.TXT"); assertThat(FileType.contains(item, TEXT)).isTrue(); assertThat(FileType.contains(item, CFG)).isFalse(); + assertTrueOnWindows(FileType.contains(item_uppercase, TEXT)); } @Test public void checkingListWithTypePredicate() throws Exception { ImmutableList unfiltered = - ImmutableList.of(filename("config.txt"), filename("index.html"), filename("README.txt")); + ImmutableList.of(filename("config.txt"), filename("index.HTML"), filename("README.txt")); assertThat(FileType.contains(unfiltered, TEXT)).isTrue(); assertThat(FileType.contains(unfiltered, CFG)).isFalse(); + assertTrueOnWindows(FileType.contains(unfiltered, HTML)); } @Test @@ -157,10 +186,18 @@ public void filteringWithTypePredicate() throws Exception { filename("config.txt"), filename("index.html"), filename("README.txt"), - filename("archive.zip")); - - assertThat(FileType.filter(unfiltered, TEXT)).containsExactly(unfiltered.get(0), - unfiltered.get(2)).inOrder(); + filename("archive.zip"), + filename("INFO.TXT")); + + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(FileType.filter(unfiltered, TEXT)) + .containsExactly(unfiltered.get(0), unfiltered.get(2), unfiltered.get(4)) + .inOrder(); + } else { + assertThat(FileType.filter(unfiltered, TEXT)) + .containsExactly(unfiltered.get(0), unfiltered.get(2)) + .inOrder(); + } } @Test @@ -170,11 +207,18 @@ public void filteringWithMatcherPredicate() throws Exception { filename("config.txt"), filename("index.html"), filename("README.txt"), - filename("archive.zip")); - - assertThat(FileType.filter(unfiltered, TEXT::matches)) - .containsExactly(unfiltered.get(0), unfiltered.get(2)) - .inOrder(); + filename("archive.zip"), + filename("INFO.TXT")); + + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(FileType.filter(unfiltered, TEXT::matches)) + .containsExactly(unfiltered.get(0), unfiltered.get(2), unfiltered.get(4)) + .inOrder(); + } else { + assertThat(FileType.filter(unfiltered, TEXT::matches)) + .containsExactly(unfiltered.get(0), unfiltered.get(2)) + .inOrder(); + } } @Test @@ -184,7 +228,8 @@ public void filteringWithAlwaysFalse() throws Exception { filename("config.txt"), filename("index.html"), filename("binary"), - filename("archive.zip")); + filename("archive.zip"), + filename("INFO.TXT")); assertThat(FileType.filter(unfiltered, FileTypeSet.NO_FILE)).isEmpty(); } @@ -196,10 +241,12 @@ public void filteringWithAlwaysTrue() throws Exception { filename("config.txt"), filename("index.html"), filename("binary"), - filename("archive.zip")); + filename("archive.zip"), + filename("INFO.TXT")); - assertThat(FileType.filter(unfiltered, FileTypeSet.ANY_FILE)).containsExactly(unfiltered.get(0), - unfiltered.get(1), unfiltered.get(2), unfiltered.get(3)).inOrder(); + assertThat(FileType.filter(unfiltered, FileTypeSet.ANY_FILE)).containsExactly( + unfiltered.get(0), unfiltered.get(1), unfiltered.get(2), unfiltered.get(3), + unfiltered.get(4)).inOrder(); } @Test @@ -209,10 +256,18 @@ public void exclusionWithTypePredicate() throws Exception { filename("config.txt"), filename("index.html"), filename("README.txt"), - filename("server.cfg")); - - assertThat(FileType.except(unfiltered, TEXT)).containsExactly(unfiltered.get(1), - unfiltered.get(3)).inOrder(); + filename("server.cfg"), + filename("INFO.TXT")); + + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(FileType.except(unfiltered, TEXT)) + .containsExactly(unfiltered.get(1), unfiltered.get(3)) + .inOrder(); + } else { + assertThat(FileType.except(unfiltered, TEXT)) + .containsExactly(unfiltered.get(1), unfiltered.get(3), unfiltered.get(4)) + .inOrder(); + } } @Test @@ -222,10 +277,18 @@ public void listFiltering() throws Exception { filename("config.txt"), filename("index.html"), filename("README.txt"), - filename("server.cfg")); + filename("server.cfg"), + filename("CLIENT.CFG")); FileTypeSet filter = FileTypeSet.of(HTML, CFG); - assertThat(FileType.filterList(unfiltered, filter)).containsExactly(unfiltered.get(1), - unfiltered.get(3)).inOrder(); + if (OS.getCurrent() == OS.WINDOWS) { + assertThat(FileType.filterList(unfiltered, filter)) + .containsExactly(unfiltered.get(1), unfiltered.get(3), unfiltered.get(4)) + .inOrder(); + } else { + assertThat(FileType.filterList(unfiltered, filter)) + .containsExactly(unfiltered.get(1), unfiltered.get(3)) + .inOrder(); + } } } diff --git a/src/test/java/com/google/devtools/build/lib/vfs/BUILD b/src/test/java/com/google/devtools/build/lib/vfs/BUILD index 662b976e09aa17..97e90c0ae5c558 100644 --- a/src/test/java/com/google/devtools/build/lib/vfs/BUILD +++ b/src/test/java/com/google/devtools/build/lib/vfs/BUILD @@ -35,6 +35,7 @@ java_library( ":testutil", "//src/main/java/com/google/devtools/build/lib/util:os", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//third_party:junit4", "//third_party:truth", @@ -60,6 +61,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec", "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/testutils", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//src/test/java/com/google/devtools/build/lib/testutil", @@ -109,6 +111,7 @@ java_library( deps = [ "//src/main/java/com/google/devtools/build/lib/clock", "//src/main/java/com/google/devtools/build/lib/vfs", + "//src/main/java/com/google/devtools/build/lib/vfs:ospathpolicy", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", "//third_party:guava",