Automated [] rollback of commit 846a5ab98fc26d72024890fdb79a5d3bc6a5a1ba + manual rollback of []

*** Reason for rollback ***

Depot has been fixed / is in the process of being fixed. See the work tracked on []

*** Original change description ***

Automated [] rollback of commit bb5d5efb4b50710241b5b374eb67084f4bf08278.

--
MOS_MIGRATED_REVID=131095905

2 files changed, 53 insertions, 64 deletions

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 45e36ed90b..694601d1c4 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
@@ -18,7 +18,6 @@ import com.google.common.base.Predicate;
 import com.google.common.base.Throwables;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.SettableFuture;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
@@ -32,7 +31,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
-import java.util.LinkedHashSet;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -126,26 +125,26 @@ public class GlobCache {
    * @throws BadGlobException if the glob was syntactically invalid, or
    *  contained uplevel references.
    */
-  Future<List<Path>> getGlobAsync(String pattern, boolean excludeDirs)
+  Future<List<Path>> getGlobUnsortedAsync(String pattern, boolean excludeDirs)
       throws BadGlobException {
     Future<List<Path>> cached = globCache.get(Pair.of(pattern, excludeDirs));
     if (cached == null) {
-      cached = safeGlob(pattern, excludeDirs);
+      cached = safeGlobUnsorted(pattern, excludeDirs);
       setGlobPaths(pattern, excludeDirs, cached);
     }
     return cached;
   }
 
   @VisibleForTesting
-  List<String> getGlob(String pattern)
+  List<String> getGlobUnsorted(String pattern)
       throws IOException, BadGlobException, InterruptedException {
-    return getGlob(pattern, false);
+    return getGlobUnsorted(pattern, false);
   }
 
   @VisibleForTesting
-  protected List<String> getGlob(String pattern, boolean excludeDirs)
+  protected List<String> getGlobUnsorted(String pattern, boolean excludeDirs)
       throws IOException, BadGlobException, InterruptedException {
-    Future<List<Path>> futureResult = getGlobAsync(pattern, excludeDirs);
+    Future<List<Path>> futureResult = getGlobUnsortedAsync(pattern, excludeDirs);
     List<Path> globPaths = fromFuture(futureResult);
     // Replace the UnixGlob.GlobFuture with a completed future object, to allow
     // garbage collection of the GlobFuture and GlobVisitor objects.
@@ -168,11 +167,8 @@ public class GlobCache {
     return result;
   }
 
-  /**
-   * Adds glob entries to the cache, making sure they are sorted first.
-   */
-  @VisibleForTesting
-  void setGlobPaths(String pattern, boolean excludeDirectories, Future<List<Path>> result) {
+  /** Adds glob entries to the cache. */
+  private void setGlobPaths(String pattern, boolean excludeDirectories, Future<List<Path>> result) {
     globCache.put(Pair.of(pattern, excludeDirectories), result);
   }
 
@@ -181,7 +177,7 @@ public class GlobCache {
    * getGlob().
    */
   @VisibleForTesting
-  Future<List<Path>> safeGlob(String pattern, boolean excludeDirs) throws BadGlobException {
+  Future<List<Path>> safeGlobUnsorted(String pattern, boolean excludeDirs) throws BadGlobException {
     // Forbidden patterns:
     if (pattern.indexOf('?') != -1) {
       throw new BadGlobException("glob pattern '" + pattern + "' contains forbidden '?' wildcard");
@@ -217,64 +213,28 @@ public class GlobCache {
   }
 
   /**
-   * Returns true iff all this package's globs are up-to-date.  That is,
-   * re-evaluating the package's BUILD file at this moment would yield an
-   * equivalent Package instance.  (This call requires filesystem I/O to
-   * re-evaluate the globs.)
-   */
-  public boolean globsUpToDate() throws InterruptedException {
-    // Start all globs in parallel.
-    Map<Pair<String, Boolean>, Future<List<Path>>> newGlobs = new HashMap<>();
-    try {
-      for (Map.Entry<Pair<String, Boolean>, Future<List<Path>>> entry : globCache.entrySet()) {
-        Pair<String, Boolean> key = entry.getKey();
-        try {
-          newGlobs.put(key, safeGlob(key.first, key.second));
-        } catch (BadGlobException e) {
-          return false;
-        }
-      }
-
-      for (Map.Entry<Pair<String, Boolean>, Future<List<Path>>> entry : globCache.entrySet()) {
-        try {
-          Pair<String, Boolean> key = entry.getKey();
-          List<Path> newGlob = fromFuture(newGlobs.get(key));
-          List<Path> oldGlob = fromFuture(entry.getValue());
-          if (!oldGlob.equals(newGlob)) {
-            return false;
-          }
-        } catch (IOException e) {
-          return false;
-        }
-      }
-
-      return true;
-    } finally {
-      finishBackgroundTasks(newGlobs.values());
-    }
-  }
-
-  /**
-   * Evaluate the build language expression "glob(includes, excludes)" in the
+   * Helper for evaluating the build language expression "glob(includes, excludes)" in the
    * context of this package.
    *
    * <p>Called by PackageFactory via Package.
    */
-  public List<String> glob(List<String> includes, List<String> excludes, boolean excludeDirs)
-      throws IOException, BadGlobException, InterruptedException {
+  public List<String> globUnsorted(
+      List<String> includes,
+      List<String> excludes,
+      boolean excludeDirs) throws IOException, BadGlobException, InterruptedException {
     // Start globbing all patterns in parallel. The getGlob() calls below will
     // block on an individual pattern's results, but the other globs can
     // continue in the background.
     for (String pattern : Iterables.concat(includes, excludes)) {
-      getGlobAsync(pattern, excludeDirs);
+      getGlobUnsortedAsync(pattern, excludeDirs);
     }
 
-    LinkedHashSet<String> results = Sets.newLinkedHashSetWithExpectedSize(includes.size());
+    HashSet<String> results = new HashSet<>();
     for (String pattern : includes) {
-      results.addAll(getGlob(pattern, excludeDirs));
+      results.addAll(getGlobUnsorted(pattern, excludeDirs));
     }
     for (String pattern : excludes) {
-      for (String excludeMatch : getGlob(pattern, excludeDirs)) {
+      for (String excludeMatch : getGlobUnsorted(pattern, excludeDirs)) {
         results.remove(excludeMatch);
       }
     }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 9c1a5c8a0c..4b8db490f3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -70,6 +70,7 @@ import com.google.devtools.build.lib.vfs.UnixGlob;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -285,9 +286,11 @@ public final class PackageFactory {
   /** {@link Globber} that uses the legacy GlobCache. */
   public static class LegacyGlobber implements Globber {
     private final GlobCache globCache;
+    private final boolean sort;
 
-    public LegacyGlobber(GlobCache globCache) {
+    private LegacyGlobber(GlobCache globCache, boolean sort) {
       this.globCache = globCache;
+      this.sort = sort;
     }
 
     private static class Token extends Globber.Token {
@@ -306,20 +309,25 @@ public final class PackageFactory {
     public Token runAsync(List<String> includes, List<String> excludes, boolean excludeDirs)
         throws BadGlobException {
       for (String pattern : Iterables.concat(includes, excludes)) {
-        globCache.getGlobAsync(pattern, excludeDirs);
+        globCache.getGlobUnsortedAsync(pattern, excludeDirs);
       }
       return new Token(includes, excludes, excludeDirs);
     }
 
     @Override
     public List<String> fetch(Globber.Token token) throws IOException, InterruptedException {
+      List<String> result;
       Token legacyToken = (Token) token;
       try {
-        return globCache.glob(legacyToken.includes, legacyToken.excludes,
+        result = globCache.globUnsorted(legacyToken.includes, legacyToken.excludes,
             legacyToken.excludeDirs);
       } catch (BadGlobException e) {
         throw new IllegalStateException(e);
       }
+      if (sort) {
+        Collections.sort(result);
+      }
+      return result;
     }
 
     @Override
@@ -1428,12 +1436,33 @@ public final class PackageFactory {
     }
   }
 
-  public LegacyGlobber createLegacyGlobber(Path packageDirectory, PackageIdentifier packageId,
+  /** Returns a new {@link LegacyGlobber}. */
+  public LegacyGlobber createLegacyGlobber(
+      Path packageDirectory,
+      PackageIdentifier packageId,
       CachingPackageLocator locator) {
-    return new LegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
+    return createLegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
         threadPool));
   }
 
+  /** Returns a new {@link LegacyGlobber}. */
+  public static LegacyGlobber createLegacyGlobber(GlobCache globCache) {
+    return new LegacyGlobber(globCache, /*sort=*/ true);
+  }
+
+  /**
+   * Returns a new {@link LegacyGlobber}, the same as in {@link #createLegacyGlobber}, except that
+   * the implementation of {@link Globber#fetch} intentionally breaks the contract and doesn't
+   * return sorted results.
+   */
+  public LegacyGlobber createLegacyGlobberThatDoesntSort(
+      Path packageDirectory,
+      PackageIdentifier packageId,
+      CachingPackageLocator locator) {
+    return new LegacyGlobber(new GlobCache(packageDirectory, packageId, locator, syscalls,
+        threadPool), /*sort=*/ false);
+  }
+
   @Nullable
   private byte[] maybeGetBuildFileBytes(Path buildFile, EventHandler eventHandler) {
     try {