RELNOTES: The string list returned by the skylark 'glob' function is now sorted. Previously, it would return a list formed by concatenating the sorted results of each pattern in the 'includes' list.

A bunch of cleanups and one bug fix:
-Remove the unused-except-for tests GlobCache#globsUpToDate. This code has been dead for a very very long time, ever since we switched to using Skyframe.
-Change the semantics of the 'glob' function as described above.
-Change UnixGlob to return unsorted results. Document this in UnixGlob and GlobCache.
-Change LegacyGlobber to conditionally return sorted results. Have users other than PackageFunction get sorted results (as described above). Have PackageFunction's use case get completely unsorted results, and have PackageFunction do the sorting itself.
-Have PackageFunction's HybridGlobber unconditionally sort the glob result list. This ensure deterministic glob results, fixing a bug where the order of the elements of the result depended on the contents of the Skyframe graph, which of course depends on the sequence of incremental Blaze commands.

--
MOS_MIGRATED_REVID=130540152

1 files changed, 12 insertions, 5 deletions

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 6da3925e96..1ea067d20f 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
@@ -23,9 +23,9 @@ import com.google.common.base.Throwables;
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.cache.CacheLoader;
+import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import com.google.common.collect.Ordering;
 import com.google.common.collect.Sets;
 import com.google.common.util.concurrent.ForwardingListenableFuture;
 import com.google.common.util.concurrent.Futures;
@@ -55,6 +55,8 @@ import java.util.regex.Pattern;
  *
  * <p><code>**</code> gets special treatment in include patterns. If it is used as a complete path
  * segment it matches the filenames in subdirectories recursively.
+ *
+ * <p>Importantly, note that the glob matches are in an unspecified order.
  */
 public final class UnixGlob {
   private UnixGlob() {}
@@ -396,7 +398,7 @@ public final class UnixGlob {
     }
 
     /**
-     * Executes the glob.
+     * Executes the glob and returns the result.
      *
      * @throws InterruptedException if the thread is interrupted.
      */
@@ -499,9 +501,10 @@ public final class UnixGlob {
     }
 
     /**
-     * Performs wildcard globbing: returns the sorted list of filenames that match any of
+     * Performs wildcard globbing: returns the list of filenames that match any of
      * {@code patterns} relative to {@code base}. Directories are traversed if and only if they
-     * match {@code dirPred}. The predicate is also called for the root of the traversal.
+     * match {@code dirPred}. The predicate is also called for the root of the traversal. The order
+     * of the returned list is unspecified.
      *
      * <p>Patterns may include "*" and "?", but not "[a-z]".
      *
@@ -530,6 +533,10 @@ public final class UnixGlob {
       return "**".equals(pattern);
     }
 
+    /**
+     * Same as {@link #glob}, except does so asynchronously and returns a {@link Future} for the
+     * result.
+     */
     public Future<List<Path>> globAsync(Path base, Collection<String> patterns,
         boolean excludeDirectories, Predicate<Path> dirPred, FilesystemCalls syscalls) {
 
@@ -635,7 +642,7 @@ public final class UnixGlob {
         } else if (failure.get() != null) {
           result.setException(failure.get());
         } else {
-          result.set(Ordering.<Path>natural().immutableSortedCopy(results));
+          result.set(ImmutableList.copyOf(results));
         }
       }
     }