Part 1 of the Implementation for new 'subpackages()` built-in helper function.
Design proposal: https://docs.google.com/document/d/13UOT0GoQofxDW40ILzH2sWpUOmuYy6QZ7CUmhej9vgk/edit#
This CL modifies the globber infrastructure to support an additional mode of listing sub-directories.
* Add new Globber Operation enum allowing, Globber implementations to
discriminate between glob, glob w/directories and the future sub-packages
use-case.
* Modify UnixGlob to replace Predicate and bools with UnixGlobPathDiscriminator interface for:
a) Determining whether to traverse a sub-directory (previously was lambda)
b) function for determing what entries to include in the List<Path> produced by UnixGlob.globAsync.
These allow relatively simple re-use of the same logic for both subpackages and glob
4) Add a few tests for UnixGlob to ensure both cases continue to work as expected.
PiperOrigin-RevId: 421125424
diff --git a/src/test/java/com/google/devtools/build/lib/packages/BUILD b/src/test/java/com/google/devtools/build/lib/packages/BUILD
index 61ef029..12a0a08 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/packages/BUILD
@@ -70,6 +70,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:test_policy",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages:exec_group",
+ "//src/main/java/com/google/devtools/build/lib/packages:globber",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/runtime/commands",
"//src/main/java/com/google/devtools/build/lib/skyframe:tests_for_target_pattern_value",
diff --git a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
index b830b661..786942f 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/GlobCacheTest.java
@@ -142,7 +142,7 @@
@Test
public void testIgnoredDirectory() throws Exception {
createCache(PathFragment.create("isolated/foo"));
- List<Path> paths = cache.safeGlobUnsorted("**/*.js", true).get();
+ List<Path> paths = cache.safeGlobUnsorted("**/*.js", Globber.Operation.FILES).get();
assertPathsAre(
paths,
"/workspace/isolated/first.js",
@@ -153,7 +153,7 @@
@Test
public void testSafeGlob() throws Exception {
- List<Path> paths = cache.safeGlobUnsorted("*.js", false).get();
+ List<Path> paths = cache.safeGlobUnsorted("*.js", Globber.Operation.FILES_AND_DIRS).get();
assertPathsAre(paths,
"/workspace/isolated/first.js", "/workspace/isolated/second.js");
}
@@ -161,7 +161,9 @@
@Test
public void testSafeGlobInvalidPattern() throws Exception {
String invalidPattern = "Foo?.txt";
- assertThrows(BadGlobException.class, () -> cache.safeGlobUnsorted(invalidPattern, false).get());
+ assertThrows(
+ BadGlobException.class,
+ () -> cache.safeGlobUnsorted(invalidPattern, Globber.Operation.FILES_AND_DIRS).get());
}
@Test
@@ -181,38 +183,53 @@
assertThat(cache.getKeySet()).isEmpty();
cache.getGlobUnsorted("*.java");
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false));
+ assertThat(cache.getKeySet())
+ .containsExactly(Pair.of("*.java", Globber.Operation.FILES_AND_DIRS));
cache.getGlobUnsorted("*.java");
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false));
+ assertThat(cache.getKeySet())
+ .containsExactly(Pair.of("*.java", Globber.Operation.FILES_AND_DIRS));
cache.getGlobUnsorted("*.js");
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false), Pair.of("*.js", false));
+ assertThat(cache.getKeySet())
+ .containsExactly(
+ Pair.of("*.java", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.js", Globber.Operation.FILES_AND_DIRS));
- cache.getGlobUnsorted("*.java", true);
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false), Pair.of("*.js", false),
- Pair.of("*.java", true));
+ cache.getGlobUnsorted("*.java", Globber.Operation.FILES);
+ assertThat(cache.getKeySet())
+ .containsExactly(
+ Pair.of("*.java", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.js", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.java", Globber.Operation.FILES));
assertThrows(BadGlobException.class, () -> cache.getGlobUnsorted("invalid?"));
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false), Pair.of("*.js", false),
- Pair.of("*.java", true));
+ assertThat(cache.getKeySet())
+ .containsExactly(
+ Pair.of("*.java", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.js", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.java", Globber.Operation.FILES));
cache.getGlobUnsorted("foo/first.*");
- assertThat(cache.getKeySet()).containsExactly(Pair.of("*.java", false), Pair.of("*.java", true),
- Pair.of("*.js", false), Pair.of("foo/first.*", false));
+ assertThat(cache.getKeySet())
+ .containsExactly(
+ Pair.of("*.java", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("*.java", Globber.Operation.FILES),
+ Pair.of("*.js", Globber.Operation.FILES_AND_DIRS),
+ Pair.of("foo/first.*", Globber.Operation.FILES_AND_DIRS));
}
@Test
public void testGlob() throws Exception {
- assertEmpty(cache.globUnsorted(list("*.java"), NONE, false, true));
+ assertEmpty(cache.globUnsorted(list("*.java"), NONE, Globber.Operation.FILES, true));
- assertThat(cache.globUnsorted(list("*.*"), NONE, false, true))
+ assertThat(cache.globUnsorted(list("*.*"), NONE, Globber.Operation.FILES, true))
.containsExactly("first.js", "first.txt", "second.js", "second.txt");
- assertThat(cache.globUnsorted(list("*.*"), list("first.js"), false, true))
+ assertThat(cache.globUnsorted(list("*.*"), list("first.js"), Globber.Operation.FILES, true))
.containsExactly("first.txt", "second.js", "second.txt");
- assertThat(cache.globUnsorted(list("*.txt", "first.*"), NONE, false, true))
+ assertThat(cache.globUnsorted(list("*.txt", "first.*"), NONE, Globber.Operation.FILES, true))
.containsExactly("first.txt", "second.txt", "first.js");
}
@@ -225,13 +242,17 @@
@Test
public void testSingleFileExclude_star() throws Exception {
- assertThat(cache.globUnsorted(list("*"), list("first.txt"), false, true))
+ assertThat(
+ cache.globUnsorted(
+ list("*"), list("first.txt"), Globber.Operation.FILES_AND_DIRS, true))
.containsExactly("BUILD", "bar", "first.js", "foo", "second.js", "second.txt");
}
@Test
public void testSingleFileExclude_starStar() throws Exception {
- assertThat(cache.globUnsorted(list("**"), list("first.txt"), false, true))
+ assertThat(
+ cache.globUnsorted(
+ list("**"), list("first.txt"), Globber.Operation.FILES_AND_DIRS, true))
.containsExactly(
"BUILD",
"bar",
@@ -247,48 +268,78 @@
@Test
public void testExcludeAll_star() throws Exception {
- assertThat(cache.globUnsorted(list("*"), list("*"), false, true)).isEmpty();
+ assertThat(cache.globUnsorted(list("*"), list("*"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
}
@Test
public void testExcludeAll_star_noMatchesAnyway() throws Exception {
- assertThat(cache.globUnsorted(list("nope"), list("*"), false, true)).isEmpty();
+ assertThat(cache.globUnsorted(list("nope"), list("*"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
}
@Test
public void testExcludeAll_starStar() throws Exception {
- assertThat(cache.globUnsorted(list("**"), list("**"), false, true)).isEmpty();
+ assertThat(cache.globUnsorted(list("**"), list("**"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
}
@Test
public void testExcludeAll_manual() throws Exception {
- assertThat(cache.globUnsorted(list("**"), list("*", "*/*", "*/*/*"), false, true)).isEmpty();
+ assertThat(
+ cache.globUnsorted(
+ list("**"), list("*", "*/*", "*/*/*"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
}
@Test
public void testSingleFileExcludeDoesntMatch() throws Exception {
- assertThat(cache.globUnsorted(list("first.txt"), list("nope.txt"), false, true))
+ assertThat(
+ cache.globUnsorted(
+ list("first.txt"), list("nope.txt"), Globber.Operation.FILES_AND_DIRS, true))
.containsExactly("first.txt");
}
@Test
public void testExcludeDirectory() throws Exception {
- assertThat(cache.globUnsorted(list("foo/*"), NONE, true, true))
+ assertThat(cache.globUnsorted(list("foo/*"), NONE, Globber.Operation.FILES, true))
.containsExactly("foo/first.js", "foo/second.js");
- assertThat(cache.globUnsorted(list("foo/*"), list("foo"), false, true))
+ assertThat(
+ cache.globUnsorted(list("foo/*"), list("foo"), Globber.Operation.FILES_AND_DIRS, true))
.containsExactly("foo/first.js", "foo/second.js");
}
@Test
public void testChildGlobWithChildExclude() throws Exception {
- assertThat(cache.globUnsorted(list("foo/*"), list("foo/*"), false, true)).isEmpty();
assertThat(
- cache.globUnsorted(list("foo/first.js", "foo/second.js"), list("foo/*"), false, true))
+ cache.globUnsorted(
+ list("foo/*"), list("foo/*"), Globber.Operation.FILES_AND_DIRS, true))
.isEmpty();
- assertThat(cache.globUnsorted(list("foo/first.js"), list("foo/first.js"), false, true))
+ assertThat(
+ cache.globUnsorted(
+ list("foo/first.js", "foo/second.js"),
+ list("foo/*"),
+ Globber.Operation.FILES_AND_DIRS,
+ true))
.isEmpty();
- assertThat(cache.globUnsorted(list("foo/first.js"), list("*/first.js"), false, true)).isEmpty();
- assertThat(cache.globUnsorted(list("foo/first.js"), list("*/*"), false, true)).isEmpty();
+ assertThat(
+ cache.globUnsorted(
+ list("foo/first.js"), list("foo/first.js"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
+ assertThat(
+ cache.globUnsorted(
+ list("foo/first.js"), list("*/first.js"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
+ assertThat(
+ cache.globUnsorted(
+ list("foo/first.js"), list("*/*"), Globber.Operation.FILES_AND_DIRS, true))
+ .isEmpty();
+ }
+
+ @Test
+ public void testSubpackages() throws Exception {
+ assertThat(cache.globUnsorted(list("**"), list(), Globber.Operation.SUBPACKAGES, true))
+ .containsExactly("sub");
}
private void assertEmpty(Collection<?> glob) {
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index d3df2e4..ee56bb8 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -1264,7 +1264,7 @@
executorService,
-1,
ThreadStateReceiver.NULL_INSTANCE);
- assertThat(globCache.globUnsorted(include, exclude, false, true))
+ assertThat(globCache.globUnsorted(include, exclude, Globber.Operation.FILES_AND_DIRS, true))
.containsExactlyElementsIn(expected);
} finally {
executorService.shutdownNow();
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
index 9a591cf..969be1e 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BUILD
@@ -170,6 +170,7 @@
"//src/main/java/com/google/devtools/build/lib/exec:single_build_file_cache",
"//src/main/java/com/google/devtools/build/lib/io:inconsistent_filesystem_exception",
"//src/main/java/com/google/devtools/build/lib/packages",
+ "//src/main/java/com/google/devtools/build/lib/packages:globber",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/pkgcache",
"//src/main/java/com/google/devtools/build/lib/pkgcache:QueryTransitivePackagePreloader",
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java
index 7eac47f..d2e7753 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobDescriptorTest.java
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.packages.Globber;
import com.google.devtools.build.lib.skyframe.serialization.testutils.FsUtils;
import com.google.devtools.build.lib.skyframe.serialization.testutils.SerializationTester;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -38,13 +39,13 @@
Root.fromPath(FsUtils.TEST_FILESYSTEM.getPath("/packageRoot")),
PathFragment.create("subdir"),
"pattern",
- /*excludeDirs=*/ false),
+ Globber.Operation.FILES_AND_DIRS),
GlobDescriptor.create(
PackageIdentifier.create("@bar", PathFragment.create("//foo")),
Root.fromPath(FsUtils.TEST_FILESYSTEM.getPath("/anotherPackageRoot")),
PathFragment.create("anotherSubdir"),
"pattern",
- /*excludeDirs=*/ true))
+ Globber.Operation.FILES))
.setVerificationFunction(GlobDescriptorTest::verifyEquivalent);
FsUtils.addDependencies(serializationTester);
serializationTester.runTests();
@@ -62,22 +63,24 @@
Root.fromPath(FsUtils.TEST_FILESYSTEM.getPath("/packageRoot")),
PathFragment.create("subdir"),
"pattern",
- /*excludeDirs=*/ false);
+ Globber.Operation.FILES_AND_DIRS);
- GlobDescriptor sameCopy = GlobDescriptor.create(
- original.getPackageId(),
- original.getPackageRoot(),
- original.getSubdir(),
- original.getPattern(),
- original.excludeDirs());
+ GlobDescriptor sameCopy =
+ GlobDescriptor.create(
+ original.getPackageId(),
+ original.getPackageRoot(),
+ original.getSubdir(),
+ original.getPattern(),
+ original.globberOperation());
assertThat(sameCopy).isSameInstanceAs(original);
- GlobDescriptor diffCopy = GlobDescriptor.create(
- original.getPackageId(),
- original.getPackageRoot(),
- original.getSubdir(),
- original.getPattern(),
- !original.excludeDirs());
+ GlobDescriptor diffCopy =
+ GlobDescriptor.create(
+ original.getPackageId(),
+ original.getPackageRoot(),
+ original.getSubdir(),
+ original.getPattern(),
+ Globber.Operation.FILES);
assertThat(diffCopy).isNotEqualTo(original);
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
index e451622..c66ec8a 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/GlobFunctionTest.java
@@ -32,6 +32,7 @@
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.NullEventHandler;
import com.google.devtools.build.lib.io.InconsistentFilesystemException;
+import com.google.devtools.build.lib.packages.Globber;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.WorkspaceFileValue;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
@@ -332,12 +333,17 @@
// Each "equality group" forms a set of elements that are all equals() to one another,
// and also produce the same hashCode.
new EqualsTester()
- .addEqualityGroup(runGlob(false, "no-such-file")) // Matches nothing.
- .addEqualityGroup(runGlob(false, "BUILD"), runGlob(true, "BUILD")) // Matches BUILD.
- .addEqualityGroup(runGlob(false, "**")) // Matches lots of things.
.addEqualityGroup(
- runGlob(false, "f*o/bar*"),
- runGlob(false, "foo/bar*")) // Matches foo/bar and foo/barnacle.
+ runGlob("no-such-file", Globber.Operation.FILES_AND_DIRS)) // Matches nothing.
+ .addEqualityGroup(
+ runGlob("BUILD", Globber.Operation.FILES_AND_DIRS),
+ runGlob("BUILD", Globber.Operation.FILES)) // Matches BUILD.
+ .addEqualityGroup(
+ runGlob("**", Globber.Operation.FILES_AND_DIRS)) // Matches lots of things.
+ .addEqualityGroup(
+ runGlob("f*o/bar*", Globber.Operation.FILES_AND_DIRS),
+ runGlob(
+ "foo/bar*", Globber.Operation.FILES_AND_DIRS)) // Matches foo/bar and foo/barnacle.
.testEquals();
}
@@ -417,11 +423,11 @@
}
private void assertGlobMatches(String pattern, String... expecteds) throws Exception {
- assertGlobMatches(false, pattern, expecteds);
+ assertGlobMatches(pattern, Globber.Operation.FILES_AND_DIRS, expecteds);
}
- private void assertGlobMatches(boolean excludeDirs, String pattern, String... expecteds)
- throws Exception {
+ private void assertGlobMatches(
+ String pattern, Globber.Operation globberOperation, String... expecteds) throws Exception {
// The order requirement is not strictly necessary -- a change to GlobFunction semantics that
// changes the output order is fine, but we require that the order be the same here to detect
// potential non-determinism in output order, which would be bad.
@@ -430,27 +436,28 @@
// directories.
assertThat(
Iterables.transform(
- runGlob(excludeDirs, pattern).getMatches().toList(), Functions.toStringFunction()))
+ runGlob(pattern, globberOperation).getMatches().toList(),
+ Functions.toStringFunction()))
.containsExactlyElementsIn(ImmutableList.copyOf(expecteds))
.inOrder();
}
private void assertGlobWithoutDirsMatches(String pattern, String... expecteds) throws Exception {
- assertGlobMatches(true, pattern, expecteds);
+ assertGlobMatches(pattern, Globber.Operation.FILES, expecteds);
}
private void assertGlobsEqual(String pattern1, String pattern2) throws Exception {
- GlobValue value1 = runGlob(false, pattern1);
- GlobValue value2 = runGlob(false, pattern2);
+ GlobValue value1 = runGlob(pattern1, Globber.Operation.FILES_AND_DIRS);
+ GlobValue value2 = runGlob(pattern2, Globber.Operation.FILES_AND_DIRS);
new EqualsTester()
.addEqualityGroup(value1, value2)
.testEquals();
}
- private GlobValue runGlob(boolean excludeDirs, String pattern) throws Exception {
+ private GlobValue runGlob(String pattern, Globber.Operation globberOperation) throws Exception {
SkyKey skyKey =
GlobValue.key(
- PKG_ID, Root.fromPath(root), pattern, excludeDirs, PathFragment.EMPTY_FRAGMENT);
+ PKG_ID, Root.fromPath(root), pattern, globberOperation, PathFragment.EMPTY_FRAGMENT);
EvaluationResult<SkyValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), EVALUATION_OPTIONS);
if (result.hasError()) {
@@ -533,7 +540,11 @@
InvalidGlobPatternException.class,
() ->
GlobValue.key(
- PKG_ID, Root.fromPath(root), pattern, false, PathFragment.EMPTY_FRAGMENT));
+ PKG_ID,
+ Root.fromPath(root),
+ pattern,
+ Globber.Operation.FILES_AND_DIRS,
+ PathFragment.EMPTY_FRAGMENT));
}
/**
@@ -681,7 +692,12 @@
differencer.inject(ImmutableMap.of(FileValue.key(pkgRootedPath), pkgDirValue));
String expectedMessage = "/root/workspace/pkg is no longer an existing directory";
SkyKey skyKey =
- GlobValue.key(PKG_ID, Root.fromPath(root), "*/foo", false, PathFragment.EMPTY_FRAGMENT);
+ GlobValue.key(
+ PKG_ID,
+ Root.fromPath(root),
+ "*/foo",
+ Globber.Operation.FILES_AND_DIRS,
+ PathFragment.EMPTY_FRAGMENT);
EvaluationResult<GlobValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), EVALUATION_OPTIONS);
assertThat(result.hasError()).isTrue();
@@ -705,7 +721,12 @@
DirectoryListingStateValue.key(fooBarDirRootedPath), fooBarDirListingValue));
String expectedMessage = "/root/workspace/pkg/foo/bar/wiz is no longer an existing directory.";
SkyKey skyKey =
- GlobValue.key(PKG_ID, Root.fromPath(root), "**/wiz", false, PathFragment.EMPTY_FRAGMENT);
+ GlobValue.key(
+ PKG_ID,
+ Root.fromPath(root),
+ "**/wiz",
+ Globber.Operation.FILES_AND_DIRS,
+ PathFragment.EMPTY_FRAGMENT);
EvaluationResult<GlobValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), EVALUATION_OPTIONS);
assertThat(result.hasError()).isTrue();
@@ -776,7 +797,11 @@
"readdir and stat disagree about whether " + fileRootedPath.asPath() + " is a symlink";
SkyKey skyKey =
GlobValue.key(
- PKG_ID, Root.fromPath(root), "foo/bar/wiz/*", false, PathFragment.EMPTY_FRAGMENT);
+ PKG_ID,
+ Root.fromPath(root),
+ "foo/bar/wiz/*",
+ Globber.Operation.FILES_AND_DIRS,
+ PathFragment.EMPTY_FRAGMENT);
EvaluationResult<GlobValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), EVALUATION_OPTIONS);
assertThat(result.hasError()).isTrue();
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 76fdd0d..f764760 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/vfs/BUILD
@@ -67,6 +67,7 @@
"//src/test/java/com/google/devtools/build/lib/testutil:TestThread",
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
"//src/test/java/com/google/devtools/build/lib/vfs/util",
+ "//src/test/java/com/google/devtools/build/lib/vfs/util:test_glob_path_discriminator",
"//third_party:guava",
"//third_party:guava-testlib",
"//third_party:junit4",
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
index 28f6ab5..0463f57 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/GlobTest.java
@@ -16,12 +16,12 @@
import static com.google.common.truth.Truth.assertThat;
import static org.junit.Assert.assertThrows;
-import com.google.common.base.Predicate;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.common.util.concurrent.Uninterruptibles;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.lib.vfs.util.TestUnixGlobPathDiscriminator;
import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.ArrayList;
@@ -37,15 +37,14 @@
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;
+import java.util.function.Predicate;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Tests {@link UnixGlob}
- */
+/** Tests {@link UnixGlob} */
@RunWith(JUnit4.class)
public class GlobTest {
@@ -55,7 +54,7 @@
private Path throwOnStat = null;
@Before
- public final void initializeFileSystem() throws Exception {
+ public final void initializeFileSystem() throws Exception {
fs =
new InMemoryFileSystem(DigestHashFunction.SHA256) {
@Override
@@ -77,10 +76,12 @@
}
};
tmpPath = fs.getPath("/globtmp");
- for (String dir : ImmutableList.of("foo/bar/wiz",
- "foo/barnacle/wiz",
- "food/barnacle/wiz",
- "fool/barnacle/wiz")) {
+
+ final ImmutableList<String> directories =
+ ImmutableList.of(
+ "foo/bar/wiz", "foo/barnacle/wiz", "food/barnacle/wiz", "fool/barnacle/wiz");
+
+ for (String dir : directories) {
FileSystemUtils.createDirectoryAndParents(tmpPath.getRelative(dir));
}
FileSystemUtils.createEmptyFile(tmpPath.getRelative("foo/bar/wiz/file"));
@@ -93,7 +94,7 @@
@Test
public void testQuestionMarkMatch() throws Exception {
- assertGlobMatches("foo?", /* => */"food", "fool");
+ assertGlobMatches("foo?", /* => */ "food", "fool");
}
@Test
@@ -103,48 +104,48 @@
@Test
public void testStartsWithStar() throws Exception {
- assertGlobMatches("*oo", /* => */"foo");
+ assertGlobMatches("*oo", /* => */ "foo");
}
@Test
public void testStartsWithStarWithMiddleStar() throws Exception {
- assertGlobMatches("*f*o", /* => */"foo");
+ assertGlobMatches("*f*o", /* => */ "foo");
}
@Test
public void testEndsWithStar() throws Exception {
- assertGlobMatches("foo*", /* => */"foo", "food", "fool");
+ assertGlobMatches("foo*", /* => */ "foo", "food", "fool");
}
@Test
public void testEndsWithStarWithMiddleStar() throws Exception {
- assertGlobMatches("f*oo*", /* => */"foo", "food", "fool");
+ assertGlobMatches("f*oo*", /* => */ "foo", "food", "fool");
}
@Test
public void testMiddleStar() throws Exception {
- assertGlobMatches("f*o", /* => */"foo");
+ assertGlobMatches("f*o", /* => */ "foo");
}
@Test
public void testTwoMiddleStars() throws Exception {
- assertGlobMatches("f*o*o", /* => */"foo");
+ assertGlobMatches("f*o*o", /* => */ "foo");
}
@Test
public void testSingleStarPatternWithNamedChild() throws Exception {
- assertGlobMatches("*/bar", /* => */"foo/bar");
+ assertGlobMatches("*/bar", /* => */ "foo/bar");
}
@Test
public void testSingleStarPatternWithChildGlob() throws Exception {
- assertGlobMatches("*/bar*", /* => */
- "foo/bar", "foo/barnacle", "food/barnacle", "fool/barnacle");
+ assertGlobMatches(
+ "*/bar*", /* => */ "foo/bar", "foo/barnacle", "food/barnacle", "fool/barnacle");
}
@Test
public void testSingleStarAsChildGlob() throws Exception {
- assertGlobMatches("foo/*/wiz", /* => */"foo/bar/wiz", "foo/barnacle/wiz");
+ assertGlobMatches("foo/*/wiz", /* => */ "foo/bar/wiz", "foo/barnacle/wiz");
}
@Test
@@ -160,10 +161,93 @@
}
@Test
+ public void testFilteredResults_noDirs() throws Exception {
+
+ assertThat(
+ new UnixGlob.Builder(tmpPath)
+ .addPatterns("**")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(
+ p -> /*traversalPredicate=*/ true,
+ /*resultPredicate=*/ (p, isDir) -> !isDir))
+ .globInterruptible())
+ .containsExactlyElementsIn(resolvePaths("foo/bar/wiz/file"));
+ }
+
+ @Test
+ public void testFilteredResults_noFiles() throws Exception {
+ assertThat(
+ new UnixGlob.Builder(tmpPath)
+ .addPatterns("**")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(
+ /*traversalPredicate=*/ p -> true,
+ /*resultPredicate=*/ (p, isDir) -> isDir))
+ .globInterruptible())
+ .containsExactlyElementsIn(
+ resolvePaths(
+ "",
+ "foo",
+ "foo/bar",
+ "foo/bar/wiz",
+ "foo/barnacle",
+ "foo/barnacle/wiz",
+ "food",
+ "food/barnacle",
+ "food/barnacle/wiz",
+ "fool",
+ "fool/barnacle",
+ "fool/barnacle/wiz"));
+ }
+
+ @Test
+ public void testFilteredResults_pathMatch() throws Exception {
+
+ Path wanted = tmpPath.getRelative("food/barnacle/wiz");
+
+ assertThat(
+ new UnixGlob.Builder(tmpPath)
+ .addPatterns("**")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(
+ /*traversalPredicate=*/ p -> true,
+ /*resultPredicate=*/ (path, isDir) -> path.equals(wanted)))
+ .globInterruptible())
+ .containsExactly(wanted);
+ }
+
+ @Test
+ public void testTraversal_onlyFoo() throws Exception {
+ // Use a directory traversal filter to only walk the root dir and "foo", but not "fool or "food"
+ // So we'll end up the directories, "fool" and "food", but not sub-dirs.
+ assertThat(
+ new UnixGlob.Builder(tmpPath)
+ .addPatterns("**")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(
+ /*traversalPredicate=*/ path ->
+ path.equals(tmpPath)
+ || path.getPathString().contains("foo/")
+ || path.getPathString().endsWith("foo"),
+ /*resultPredicate=*/ (x, isDir) -> true))
+ .globInterruptible())
+ .containsExactlyElementsIn(
+ resolvePaths(
+ "",
+ "foo",
+ "foo/bar",
+ "foo/bar/wiz",
+ "foo/bar/wiz/file",
+ "foo/barnacle",
+ "foo/barnacle/wiz",
+ "fool",
+ "food"));
+ }
+
+ @Test
public void testGlobWithNonExistentBase() throws Exception {
- Collection<Path> globResult = UnixGlob.forPath(fs.getPath("/does/not/exist"))
- .addPattern("*.txt")
- .globInterruptible();
+ Collection<Path> globResult =
+ UnixGlob.forPath(fs.getPath("/does/not/exist")).addPattern("*.txt").globInterruptible();
assertThat(globResult).isEmpty();
}
@@ -172,27 +256,19 @@
assertGlobMatches("foo/bar/wiz/file/*" /* => nothing */);
}
- private void assertGlobMatches(String pattern, String... expecteds)
- throws Exception {
+ private void assertGlobMatches(String pattern, String... expecteds) throws Exception {
assertGlobMatches(Collections.singleton(pattern), expecteds);
}
- private void assertGlobMatches(Collection<String> pattern,
- String... expecteds)
- throws Exception {
- assertThat(
- new UnixGlob.Builder(tmpPath)
- .addPatterns(pattern)
- .globInterruptible())
- .containsExactlyElementsIn(resolvePaths(expecteds));
+ private void assertGlobMatches(Collection<String> pattern, String... expecteds) throws Exception {
+ assertThat(new UnixGlob.Builder(tmpPath).addPatterns(pattern).globInterruptible())
+ .containsExactlyElementsIn(resolvePaths(expecteds));
}
private Set<Path> resolvePaths(String... relativePaths) {
Set<Path> expectedFiles = new HashSet<>();
for (String expected : relativePaths) {
- Path file = expected.equals(".")
- ? tmpPath
- : tmpPath.getRelative(expected);
+ Path file = expected.equals(".") ? tmpPath : tmpPath.getRelative(expected);
expectedFiles.add(file);
}
return expectedFiles;
@@ -270,9 +346,7 @@
assertIllegalPattern("foo//bar");
}
- /**
- * Tests that globs can contain Java regular expression special characters
- */
+ /** Tests that globs can contain Java regular expression special characters */
@Test
public void testSpecialRegexCharacter() throws Exception {
Path tmpPath2 = fs.getPath("/globtmp2");
@@ -357,19 +431,18 @@
@Test
public void testMultiplePatternsWithOverlap() throws Exception {
- assertGlobMatchesAnyOrder(Lists.newArrayList("food", "foo?"),
- "food", "fool");
- assertGlobMatchesAnyOrder(Lists.newArrayList("food", "?ood", "f??d"),
- "food");
- assertThat(resolvePaths("food", "fool", "foo")).containsExactlyElementsIn(
- new UnixGlob.Builder(tmpPath).addPatterns("food", "xxx", "*").glob());
-
+ assertGlobMatchesAnyOrder(Lists.newArrayList("food", "foo?"), "food", "fool");
+ assertGlobMatchesAnyOrder(Lists.newArrayList("food", "?ood", "f??d"), "food");
+ assertThat(resolvePaths("food", "fool", "foo"))
+ .containsExactlyElementsIn(
+ new UnixGlob.Builder(tmpPath).addPatterns("food", "xxx", "*").glob());
}
- private void assertGlobMatchesAnyOrder(ArrayList<String> patterns,
- String... paths) throws Exception {
- assertThat(resolvePaths(paths)).containsExactlyElementsIn(
- new UnixGlob.Builder(tmpPath).addPatterns(patterns).globInterruptible());
+ private void assertGlobMatchesAnyOrder(ArrayList<String> patterns, String... paths)
+ throws Exception {
+ assertThat(resolvePaths(paths))
+ .containsExactlyElementsIn(
+ new UnixGlob.Builder(tmpPath).addPatterns(patterns).globInterruptible());
}
private void assertIllegalPattern(String pattern) throws Exception {
@@ -419,16 +492,20 @@
Predicate<Path> interrupterPredicate =
new Predicate<Path>() {
@Override
- public boolean apply(Path input) {
+ public boolean test(Path input) {
mainThread.interrupt();
return true;
}
};
+ UnixGlobPathDiscriminator interrupterDiscriminator =
+ new TestUnixGlobPathDiscriminator(
+ /*traversalPredicate=*/ interrupterPredicate, /*resultPredicate=*/ (x, isDir) -> true);
+
Future<?> globResult =
new UnixGlob.Builder(tmpPath)
.addPattern("**")
- .setDirectoryFilter(interrupterPredicate)
+ .setPathDiscriminator(interrupterDiscriminator)
.setExecutor(executor)
.globAsync();
assertThrows(InterruptedException.class, () -> globResult.get());
@@ -450,20 +527,25 @@
final ThreadPoolExecutor executor = (ThreadPoolExecutor) Executors.newFixedThreadPool(10);
final AtomicBoolean sentInterrupt = new AtomicBoolean(false);
- Predicate<Path> interrupterPredicate = new Predicate<Path>() {
- @Override
- public boolean apply(Path input) {
- if (!sentInterrupt.getAndSet(true)) {
- mainThread.interrupt();
- }
- return true;
- }
- };
+ Predicate<Path> interrupterPredicate =
+ new Predicate<Path>() {
+ @Override
+ public boolean test(Path input) {
+ if (!sentInterrupt.getAndSet(true)) {
+ mainThread.interrupt();
+ }
+ return true;
+ }
+ };
+
+ UnixGlobPathDiscriminator interrupterDiscriminator =
+ new TestUnixGlobPathDiscriminator(
+ /*traversalPredicate=*/ interrupterPredicate, /*resultPredicate=*/ (x, isDir) -> true);
List<Path> result =
new UnixGlob.Builder(tmpPath)
.addPatterns("**", "*")
- .setDirectoryFilter(interrupterPredicate)
+ .setPathDiscriminator(interrupterDiscriminator)
.setExecutor(executor)
.glob();
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java b/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
index ab515c0..d7e0bd3 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/NativePathTest.java
@@ -20,6 +20,7 @@
import com.google.common.testing.EqualsTester;
import com.google.devtools.build.lib.testutil.TestUtils;
import com.google.devtools.build.lib.vfs.util.FileSystems;
+import com.google.devtools.build.lib.vfs.util.TestUnixGlobPathDiscriminator;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
@@ -133,16 +134,17 @@
Collection<Path> onlyFiles =
UnixGlob.forPath(fs.getPath(tmpDir.getPath()))
- .addPattern("*")
- .setExcludeDirectories(true)
- .globInterruptible();
+ .addPattern("*")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(p -> true, (p, isDir) -> !isDir))
+ .globInterruptible();
assertPathSet(onlyFiles, aFile.getPath());
Collection<Path> directoriesToo =
UnixGlob.forPath(fs.getPath(tmpDir.getPath()))
- .addPattern("*")
- .setExcludeDirectories(false)
- .globInterruptible();
+ .addPattern("*")
+ .setPathDiscriminator(new TestUnixGlobPathDiscriminator(p -> true, (p, isDir) -> true))
+ .globInterruptible();
assertPathSet(directoriesToo, aFile.getPath(), aDirectory.getPath());
}
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/RecursiveGlobTest.java b/src/test/java/com/google/devtools/build/lib/vfs/RecursiveGlobTest.java
index 1c2ac8a..07abb57 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/RecursiveGlobTest.java
+++ b/src/test/java/com/google/devtools/build/lib/vfs/RecursiveGlobTest.java
@@ -20,6 +20,7 @@
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.clock.BlazeClock;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.lib.vfs.util.TestUnixGlobPathDiscriminator;
import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
@@ -136,10 +137,12 @@
@Test
public void testRecursiveGlobsAreOptimized() throws Exception {
- long numGlobTasks = new UnixGlob.Builder(tmpPath)
- .addPattern("**")
- .setExcludeDirectories(false)
- .globInterruptibleAndReturnNumGlobTasksForTesting();
+ long numGlobTasks =
+ new UnixGlob.Builder(tmpPath)
+ .addPattern("**")
+ .setPathDiscriminator(
+ new TestUnixGlobPathDiscriminator(p -> true, (p, isDir) -> !isDir))
+ .globInterruptibleAndReturnNumGlobTasksForTesting();
// The old glob implementation used to use 41 total glob tasks.
// Yes, checking for an exact value here is super brittle, but it lets us catch performance
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/util/BUILD b/src/test/java/com/google/devtools/build/lib/vfs/util/BUILD
index 379d2ea..30ef7d8 100644
--- a/src/test/java/com/google/devtools/build/lib/vfs/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/vfs/util/BUILD
@@ -26,7 +26,10 @@
java_library(
name = "util_internal",
testonly = 1,
- srcs = glob(["*.java"]),
+ srcs = [
+ "FileSystems.java",
+ "FsApparatus.java",
+ ],
deps = [
"//src/main/java/com/google/devtools/build/lib/clock",
"//src/main/java/com/google/devtools/build/lib/util:os",
@@ -38,3 +41,13 @@
"//src/test/java/com/google/devtools/build/lib/testutil:TestUtils",
],
)
+
+java_library(
+ name = "test_glob_path_discriminator",
+ testonly = 1,
+ srcs = ["TestUnixGlobPathDiscriminator.java"],
+ deps = [
+ "//src/main/java/com/google/devtools/build/lib/vfs",
+ "//third_party:error_prone_annotations",
+ ],
+)
diff --git a/src/test/java/com/google/devtools/build/lib/vfs/util/TestUnixGlobPathDiscriminator.java b/src/test/java/com/google/devtools/build/lib/vfs/util/TestUnixGlobPathDiscriminator.java
new file mode 100644
index 0000000..50d81d6
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/vfs/util/TestUnixGlobPathDiscriminator.java
@@ -0,0 +1,47 @@
+// Copyright 2022 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+package com.google.devtools.build.lib.vfs.util;
+
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.UnixGlobPathDiscriminator;
+import com.google.errorprone.annotations.CheckReturnValue;
+import java.util.function.BiPredicate;
+import java.util.function.Predicate;
+
+/**
+ * Test version of UnixGlobPathDiscriminator that accepts predicate/bipredicate for handling
+ * specific use-cases without creating a new class.
+ */
+@CheckReturnValue
+public final class TestUnixGlobPathDiscriminator implements UnixGlobPathDiscriminator {
+
+ private final Predicate<Path> traversalPredicate;
+ private final BiPredicate<Path, Boolean> resultPredicate;
+
+ public TestUnixGlobPathDiscriminator(
+ Predicate<Path> traversalPredicate, BiPredicate<Path, Boolean> resultPredicate) {
+ this.traversalPredicate = traversalPredicate;
+ this.resultPredicate = resultPredicate;
+ }
+
+ @Override
+ public boolean shouldTraverseDirectory(Path path) {
+ return traversalPredicate.test(path);
+ }
+
+ @Override
+ public boolean shouldIncludePathInResult(Path path, boolean isDirectory) {
+ return resultPredicate.test(path, isDirectory);
+ }
+}
