Automated rollback of commit 16465d7613348e39e0bcdb22697cf67d257a3d91.
*** Reason for rollback ***
We need to reconsider this.
https://devblogs.microsoft.com/commandline/per-directory-case-sensitivity-and-wsl/ says case-sensitivity can be set per-directory. Requiring correct casing for every directory would make Bazel behave the same on Linux and Windows, and work with WSL-created paths.
*** Original change description ***
glob() now supports case-insensitive mode
The behavior is triggered by
FileSystem.isGlobCaseSensitive() returning false.
None of the production FileSystem implementaitions
return false yet, only some test implementations.
Motivation is to support case-insensitive glob()
on Windows.
See https://github.com/bazelbuild/bazel/issues/8705 and https://github.com/bazelbuild/bazel/issues/8759.
Next we need to add an incompatible flag that
enables this behavior, and add a relevant bit to
the WindowsFileSystem. See https://github.com/bazelbuild/bazel/issues/8767
We must also warn the user somehow if enabling
this feature would change the result of some
globs. A potential approach would be to glob
case-sensitively and case-insensitively at the
same time and warn the user if the results are
different.
PiperOrigin-RevId: 256556254
diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
index db58cfc..8d1bb74 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
@@ -255,13 +255,7 @@
}
results.addAll(items);
}
-
- // TODO(laszlocsomor): set `caseSensitive` from the value of
- // `--incompatible_windows_case_insensitive_glob` or from FileSystem.isGlobCaseSensitive()
- // See https://github.com/bazelbuild/bazel/issues/8767
- final boolean caseSensitive = true;
-
- UnixGlob.removeExcludes(results, excludes, caseSensitive);
+ UnixGlob.removeExcludes(results, excludes);
if (!allowEmpty && results.isEmpty()) {
throw new BadGlobException(
"all files in the glob have been excluded, but allow_empty is set to False.");
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index e40c76f..cabb54f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -43,10 +43,6 @@
*/
public final class GlobFunction implements SkyFunction {
- // TODO(laszlocsomor): we might need to create another regexPatternCache for case-insensitive
- // glob() mode to avoid reading wrong cache results when the user changes the
- // --incompatible_windows_case_ignoring_glob flag value between builds.
- // See https://github.com/bazelbuild/bazel/issues/8767.
private final ConcurrentHashMap<String, Pattern> regexPatternCache = new ConcurrentHashMap<>();
private final boolean alwaysUseDirListing;
@@ -156,11 +152,6 @@
}
}
- // TODO(laszlocsomor): set `caseSensitive` from the value of
- // `--incompatible_windows_case_insensitive_glob` or from FileSystem.isGlobCaseSensitive()
- // See https://github.com/bazelbuild/bazel/issues/8767
- final boolean caseSensitive = true;
-
// Now that we have the directory listing, we do three passes over it so as to maximize
// skyframe batching:
// (1) Process every dirent, keeping track of values we need to request if the dirent cannot
@@ -178,7 +169,7 @@
for (Dirent dirent : listingValue.getDirents()) {
Dirent.Type direntType = dirent.getType();
String fileName = dirent.getName();
- if (!UnixGlob.matches(patternHead, fileName, regexPatternCache, caseSensitive)) {
+ if (!UnixGlob.matches(patternHead, fileName, regexPatternCache)) {
continue;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
index cfbe006..23a6c77 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageFunction.java
@@ -1011,13 +1011,7 @@
if (legacyIncludesToken != null) {
matches.addAll(delegate.fetch(legacyIncludesToken));
}
-
- // TODO(laszlocsomor): set `caseSensitive` from the value of
- // `--incompatible_windows_case_insensitive_glob` or from FileSystem.isGlobCaseSensitive()
- // See https://github.com/bazelbuild/bazel/issues/8767
- final boolean caseSensitive = true;
-
- UnixGlob.removeExcludes(matches, excludes, caseSensitive);
+ UnixGlob.removeExcludes(matches, excludes);
List<String> result = new ArrayList<>(matches);
// Skyframe glob results are unsorted. And we used a LegacyGlobber that doesn't sort.
// Therefore, we want to unconditionally sort here.
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
index 7f6b6e0..abaa6b1 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/FileSystem.java
@@ -138,22 +138,6 @@
public abstract boolean isFilePathCaseSensitive();
/**
- * Returns true if glob() is case-sensitive.
- *
- * <p>When glob() is case-sensitive, it will only match (or exclude) file "Foo" if the include (or
- * exclude) pattern uses the same upper-case and lower-case letters.
- *
- * <p>When glob() is case-insensitive, it will match (or exclude) the file "Foo" even if the
- * include (or exclude) pattern uses a different casing such as "foO".
- */
- // TODO(laszlocsomor): After `--incompatible_windows_case_insensitive_glob` is flipped to true,
- // remove this method and all references to it and replace call sites with
- // isFilePathCaseSensitive(). See https://github.com/bazelbuild/bazel/issues/8767
- public boolean isGlobCaseSensitive() {
- return true;
- }
-
- /**
* Returns the type of the file system path belongs to.
*
* <p>The string returned is obtained directly from the operating system, so
diff --git a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
index a6355ef..d9175a2 100644
--- a/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
+++ b/src/main/java/com/google/devtools/build/lib/vfs/UnixGlob.java
@@ -15,7 +15,6 @@
package com.google.devtools.build.lib.vfs;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Ascii;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.base.Predicate;
@@ -156,9 +155,9 @@
return null;
}
- /** Calls {@link #matches(String, String, Map) matches(pattern, str, null, boolean)} */
- public static boolean matches(String pattern, String str, boolean caseSensitive) {
- return matches(pattern, str, null, caseSensitive);
+ /** Calls {@link #matches(String, String, Map) matches(pattern, str, null)} */
+ public static boolean matches(String pattern, String str) {
+ return matches(pattern, str, null);
}
/**
@@ -170,8 +169,7 @@
* @param patternCache a cache from patterns to compiled Pattern objects, or {@code null} to skip
* caching
*/
- public static boolean matches(
- String pattern, String str, Map<String, Pattern> patternCache, boolean caseSensitive) {
+ public static boolean matches(String pattern, String str, Map<String, Pattern> patternCache) {
if (pattern.length() == 0 || str.length() == 0) {
return false;
}
@@ -193,30 +191,30 @@
// Common case: *.xyz
if (pattern.charAt(0) == '*' && pattern.lastIndexOf('*') == 0) {
- return endsWithCase(str, pattern.substring(1), caseSensitive);
+ return str.endsWith(pattern.substring(1));
}
// Common case: xyz*
int lastIndex = pattern.length() - 1;
// The first clause of this if statement is unnecessary, but is an
// optimization--charAt runs faster than indexOf.
if (pattern.charAt(lastIndex) == '*' && pattern.indexOf('*') == lastIndex) {
- return startsWithCase(str, pattern.substring(0, lastIndex), caseSensitive);
+ return str.startsWith(pattern.substring(0, lastIndex));
}
Pattern regex =
patternCache == null
- ? makePatternFromWildcard(pattern, caseSensitive)
- : patternCache.computeIfAbsent(pattern, p -> makePatternFromWildcard(p, caseSensitive));
+ ? makePatternFromWildcard(pattern)
+ : patternCache.computeIfAbsent(pattern, p -> makePatternFromWildcard(p));
return regex.matcher(str).matches();
}
/**
- * Returns a regular expression implementing a matcher for "pattern", in which "*" and "?" are
- * wildcards.
+ * Returns a regular expression implementing a matcher for "pattern", in which
+ * "*" and "?" are wildcards.
*
* <p>e.g. "foo*bar?.java" -> "foo.*bar.\\.java"
*/
- private static Pattern makePatternFromWildcard(String pattern, boolean caseSensitive) {
+ private static Pattern makePatternFromWildcard(String pattern) {
StringBuilder regexp = new StringBuilder();
for (int i = 0, len = pattern.length(); i < len; i++) {
char c = pattern.charAt(i);
@@ -249,15 +247,7 @@
regexp.append(c);
break;
default:
- if (caseSensitive || !isAlphaAscii(c)) {
- regexp.append(c);
- } else {
- regexp
- .append('[')
- .append(Ascii.toUpperCase(c))
- .append(Ascii.toLowerCase(c))
- .append(']');
- }
+ regexp.append(c);
break;
}
}
@@ -568,6 +558,7 @@
if (baseStat == null || patterns.isEmpty()) {
return Futures.immediateFuture(Collections.<Path>emptyList());
}
+
List<String[]> splitPatterns = checkAndSplitPatterns(patterns);
// We do a dumb loop, even though it will likely duplicate logical work (note that the
@@ -576,7 +567,6 @@
// glob [*/*.java, sub/*.java, */*.txt]).
pendingOps.incrementAndGet();
try {
- final boolean caseSensitive = base.getFileSystem().isGlobCaseSensitive();
for (String[] splitPattern : splitPatterns) {
int numRecursivePatterns = 0;
for (String pattern : splitPattern) {
@@ -584,12 +574,9 @@
++numRecursivePatterns;
}
}
- GlobTaskContext context =
- numRecursivePatterns > 1
- ? new RecursiveGlobTaskContext(
- splitPattern, excludeDirectories, caseSensitive, dirPred, syscalls)
- : new GlobTaskContext(
- splitPattern, excludeDirectories, caseSensitive, dirPred, syscalls);
+ GlobTaskContext context = numRecursivePatterns > 1
+ ? new RecursiveGlobTaskContext(splitPattern, excludeDirectories, dirPred, syscalls)
+ : new GlobTaskContext(splitPattern, excludeDirectories, dirPred, syscalls);
context.queueGlob(base, baseStat.isDirectory(), 0);
}
} finally {
@@ -698,19 +685,16 @@
private class GlobTaskContext {
private final String[] patternParts;
private final boolean excludeDirectories;
- private final boolean caseSensitive;
private final Predicate<Path> dirPred;
private final FilesystemCalls syscalls;
GlobTaskContext(
String[] patternParts,
boolean excludeDirectories,
- boolean caseSensitive,
Predicate<Path> dirPred,
FilesystemCalls syscalls) {
this.patternParts = patternParts;
this.excludeDirectories = excludeDirectories;
- this.caseSensitive = caseSensitive;
this.dirPred = dirPred;
this.syscalls = syscalls;
}
@@ -760,10 +744,9 @@
private RecursiveGlobTaskContext(
String[] patternParts,
boolean excludeDirectories,
- boolean caseSensitive,
Predicate<Path> dirPred,
FilesystemCalls syscalls) {
- super(patternParts, excludeDirectories, caseSensitive, dirPred, syscalls);
+ super(patternParts, excludeDirectories, dirPred, syscalls);
}
@Override
@@ -837,7 +820,7 @@
// The file is a special file (fifo, etc.). No need to even match against the pattern.
continue;
}
- if (matches(pattern, dent.getName(), cache, context.caseSensitive)) {
+ if (matches(pattern, dent.getName(), cache)) {
Path child = base.getChild(dent.getName());
if (childType == Dirent.Type.SYMLINK) {
@@ -885,17 +868,12 @@
* Filters out exclude patterns from a Set of paths. Common cases such as wildcard-free patterns
* or suffix patterns are special-cased to make this function efficient.
*/
- public static void removeExcludes(
- Set<String> paths, Collection<String> excludes, boolean caseSensitive) {
+ public static void removeExcludes(Set<String> paths, Collection<String> excludes) {
ArrayList<String> complexPatterns = new ArrayList<>(excludes.size());
Map<String, List<String>> starstarSlashStarHeadTailPairs = new HashMap<>();
for (String exclude : excludes) {
if (isWildcardFree(exclude)) {
- if (caseSensitive) {
- paths.remove(exclude);
- } else {
- paths.removeIf(p -> Ascii.equalsIgnoreCase(p, exclude));
- }
+ paths.remove(exclude);
continue;
}
int patternPos = exclude.indexOf("**/*");
@@ -912,9 +890,9 @@
for (Map.Entry<String, List<String>> headTailPair : starstarSlashStarHeadTailPairs.entrySet()) {
paths.removeIf(
path -> {
- if (startsWithCase(path, headTailPair.getKey(), caseSensitive)) {
+ if (path.startsWith(headTailPair.getKey())) {
for (String tail : headTailPair.getValue()) {
- if (endsWithCase(path, tail, caseSensitive)) {
+ if (path.endsWith(tail)) {
return true;
}
}
@@ -931,7 +909,7 @@
path -> {
String[] segments = Iterables.toArray(Splitter.on('/').split(path), String.class);
for (String[] splitPattern : splitPatterns) {
- if (matchesPattern(splitPattern, segments, 0, 0, patternCache, caseSensitive)) {
+ if (matchesPattern(splitPattern, segments, 0, 0, patternCache)) {
return true;
}
}
@@ -941,25 +919,19 @@
/** Returns true if {@code pattern} matches {@code path} starting from the given segments. */
private static boolean matchesPattern(
- String[] pattern,
- String[] path,
- int i,
- int j,
- Map<String, Pattern> patternCache,
- boolean caseSensitive) {
+ String[] pattern, String[] path, int i, int j, Map<String, Pattern> patternCache) {
if (i == pattern.length) {
return j == path.length;
}
if (pattern[i].equals("**")) {
- return matchesPattern(pattern, path, i + 1, j, patternCache, caseSensitive)
- || (j < path.length
- && matchesPattern(pattern, path, i, j + 1, patternCache, caseSensitive));
+ return matchesPattern(pattern, path, i + 1, j, patternCache)
+ || (j < path.length && matchesPattern(pattern, path, i, j + 1, patternCache));
}
if (j == path.length) {
return false;
}
- if (matches(pattern[i], path[j], patternCache, caseSensitive)) {
- return matchesPattern(pattern, path, i + 1, j + 1, patternCache, caseSensitive);
+ if (matches(pattern[i], path[j], patternCache)) {
+ return matchesPattern(pattern, path, i + 1, j + 1, patternCache);
}
return false;
}
@@ -967,28 +939,4 @@
private static boolean isWildcardFree(String pattern) {
return !pattern.contains("*") && !pattern.contains("?");
}
-
- @VisibleForTesting
- static boolean startsWithCase(String s, String p, boolean caseSensitive) {
- if (caseSensitive) {
- return s.startsWith(p);
- } else {
- return s.length() >= p.length() && s.regionMatches(true, 0, p, 0, p.length());
- }
- }
-
- @VisibleForTesting
- static boolean endsWithCase(String s, String p, boolean caseSensitive) {
- if (caseSensitive) {
- return s.endsWith(p);
- } else {
- return s.length() >= p.length()
- && s.regionMatches(true, s.length() - p.length(), p, 0, p.length());
- }
- }
-
- @VisibleForTesting
- static boolean isAlphaAscii(char c) {
- return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z');
- }
}
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 abb7c63..5b3669b 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
@@ -106,13 +106,6 @@
}
@Override
- public boolean isGlobCaseSensitive() {
- // TODO(laszlocsomor): return the opposite of `--incompatible_windows_case_insensitive_glob`
- // here. See https://github.com/bazelbuild/bazel/issues/8767
- return true;
- }
-
- @Override
protected boolean fileIsSymbolicLink(File file) {
try {
if (isSymlinkOrJunction(file)) {
