Sort the results returned by the HybridGlobber if we are using results from Skyframe globbing. This adds a log(n) factor to uses of globs, but getting globs to be returned in a reasonable order that can be emulated by legacy globbing is hard and bug-prone right now, and we must sort anyway if we are merging legacy and Skyframe globs.
Note that this log(n) factor is already present on clean builds with legacy globbing. If we end up seeing performance issues on incremental loading, we can investigate making GlobFunction efficiently return elements in sorted order. (We would still need to sort if merging legacy and Skyframe globs, but that should be a relatively rare occurrence, and can be dealt with by a more efficient merge sort if necessary.)
--
MOS_MIGRATED_REVID=127752554
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
index cf95adb..119205a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageFunctionTest.java
@@ -330,6 +330,109 @@
Label.parseAbsolute("//bar:a"), Label.parseAbsolute("//baz:c"));
}
+ @SuppressWarnings("unchecked") // Cast of srcs attribute to Iterable<Label>.
+ @Test
+ public void testGlobOrderStable() throws Exception {
+ scratch.file("foo/BUILD", "sh_library(name = 'foo', srcs = glob(['**/*.txt']))");
+ scratch.file("foo/b.txt");
+ scratch.file("foo/c/c.txt");
+ preparePackageLoading(rootDirectory);
+ SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
+ PackageValue value = validPackage(skyKey);
+ assertThat(
+ (Iterable<Label>)
+ value
+ .getPackage()
+ .getTarget("foo")
+ .getAssociatedRule()
+ .getAttributeContainer()
+ .getAttr("srcs"))
+ .containsExactly(
+ Label.parseAbsoluteUnchecked("//foo:b.txt"),
+ Label.parseAbsoluteUnchecked("//foo:c/c.txt"))
+ .inOrder();
+ scratch.file("foo/d.txt");
+ getSkyframeExecutor()
+ .invalidateFilesUnderPathForTesting(
+ reporter,
+ ModifiedFileSet.builder().modify(new PathFragment("foo/d.txt")).build(),
+ rootDirectory);
+ value = validPackage(skyKey);
+ assertThat(
+ (Iterable<Label>)
+ value
+ .getPackage()
+ .getTarget("foo")
+ .getAssociatedRule()
+ .getAttributeContainer()
+ .getAttr("srcs"))
+ .containsExactly(
+ Label.parseAbsoluteUnchecked("//foo:b.txt"),
+ Label.parseAbsoluteUnchecked("//foo:c/c.txt"),
+ Label.parseAbsoluteUnchecked("//foo:d.txt"))
+ .inOrder();
+ }
+
+ @SuppressWarnings("unchecked") // Cast of srcs attribute to Iterable<Label>.
+ @Test
+ public void testGlobOrderStableWithLegacyAndSkyframeComponents() throws Exception {
+ scratch.file("foo/BUILD", "sh_library(name = 'foo', srcs = glob(['*.txt']))");
+ scratch.file("foo/b.txt");
+ scratch.file("foo/a.config");
+ preparePackageLoading(rootDirectory);
+ SkyKey skyKey = PackageValue.key(PackageIdentifier.parse("@//foo"));
+ PackageValue value = validPackage(skyKey);
+ assertThat(
+ (Iterable<Label>)
+ value
+ .getPackage()
+ .getTarget("foo")
+ .getAssociatedRule()
+ .getAttributeContainer()
+ .getAttr("srcs"))
+ .containsExactly(Label.parseAbsoluteUnchecked("//foo:b.txt"));
+ scratch.overwriteFile(
+ "foo/BUILD", "sh_library(name = 'foo', srcs = glob(['*.txt', '*.config']))");
+ getSkyframeExecutor()
+ .invalidateFilesUnderPathForTesting(
+ reporter,
+ ModifiedFileSet.builder().modify(new PathFragment("foo/BUILD")).build(),
+ rootDirectory);
+ value = validPackage(skyKey);
+ assertThat(
+ (Iterable<Label>)
+ value
+ .getPackage()
+ .getTarget("foo")
+ .getAssociatedRule()
+ .getAttributeContainer()
+ .getAttr("srcs"))
+ .containsExactly(
+ Label.parseAbsoluteUnchecked("//foo:a.config"),
+ Label.parseAbsoluteUnchecked("//foo:b.txt"))
+ .inOrder();
+ scratch.overwriteFile(
+ "foo/BUILD", "sh_library(name = 'foo', srcs = glob(['*.txt', '*.config'])) # comment");
+ getSkyframeExecutor()
+ .invalidateFilesUnderPathForTesting(
+ reporter,
+ ModifiedFileSet.builder().modify(new PathFragment("foo/BUILD")).build(),
+ rootDirectory);
+ value = validPackage(skyKey);
+ assertThat(
+ (Iterable<Label>)
+ value
+ .getPackage()
+ .getTarget("foo")
+ .getAssociatedRule()
+ .getAttributeContainer()
+ .getAttr("srcs"))
+ .containsExactly(
+ Label.parseAbsoluteUnchecked("//foo:a.config"),
+ Label.parseAbsoluteUnchecked("//foo:b.txt"))
+ .inOrder();
+ }
+
@Test
public void testIncludeInMainAndDefaultRepository() throws Exception {
scratch.file("foo/BUILD",
