Be very deliberate about the concept of a TargetPattern's "directory".
--
PiperOrigin-RevId: 144494739
MOS_MIGRATED_REVID=144494739
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
index 2ca2215..e997b9f 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPattern.java
@@ -172,32 +172,62 @@
/**
* Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} and
- * {@code directory} is contained by or equals this pattern's directory. For example,
- * returns {@code true} for {@code this = TargetPattern ("//...")} and {@code directory
- * = "foo")}.
+ * {@code directory} is contained by or equals this pattern's directory.
+ *
+ * <p>For example, returns {@code true} for {@code this = TargetPattern ("//...")} and
+ * {@code directory = "foo")}.
*/
- public abstract boolean containsBelowDirectory(PackageIdentifier directory);
+ public abstract boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory);
/**
- * Shorthand for {@code containsBelowDirectory(containedPattern.getDirectory())}.
+ * Returns {@code true} iff both this pattern and {@code containedPattern} have type
+ * {@code Type.TARGETS_BELOW_DIRECTORY} and the directory in question for {@code containedPattern}
+ * is underneath the directory in question for this pattern.
+ *
+ * <p>That is, when this method returns {@code true} it means every target matched by
+ * {@code containedPattern} is also matched by this pattern.
*/
- public boolean containsBelowDirectory(TargetPattern containedPattern) {
- return containsBelowDirectory(containedPattern.getDirectory());
+ public boolean containsDirectoryOfTBDForTBD(TargetPattern containedPattern) {
+ return containedPattern.getType() != Type.TARGETS_BELOW_DIRECTORY
+ ? false
+ : containsAllTransitiveSubdirectoriesForTBD(
+ containedPattern.getDirectoryForTargetsUnderDirectory());
}
/**
- * Returns a {@link PackageIdentifier} identifying the most specific containing directory of the
- * patterns that could be matched by this pattern.
+ * For patterns of type {@link Type#TARGETS_BELOW_DIRECTORY}, returns a {@link PackageIdentifier}
+ * identifying the most specific containing directory of the patterns that could be matched by
+ * this pattern.
*
* <p>Note that we are using the {@link PackageIdentifier} type as a convenience; there may not
* actually be a package corresponding to this directory!
*
- * <p>For patterns of type {@code Type.TARGETS_BELOW_DIRECTORY}, this returns a
- * {@link PackageIdentifier} that identifies the referred-to directory. For example, for a
- * {@code Type.TARGETS_BELOW_DIRECTORY} corresponding to "//foo/bar/...", this method returns a
- * {@link PackageIdentifier} for "foo/bar".
+ * <p>This returns a {@link PackageIdentifier} that identifies the referred-to directory. For
+ * example, for a {@link Type#TARGETS_BELOW_DIRECTORY} corresponding to "//foo/bar/...", this
+ * method returns a {@link PackageIdentifier} for "foo/bar".
*/
- public abstract PackageIdentifier getDirectory();
+ public PackageIdentifier getDirectoryForTargetsUnderDirectory() {
+ throw new IllegalStateException();
+ }
+
+ /**
+ * For patterns of type {@link Type#PATH_AS_TARGET}, returns the path in question.
+ *
+ * <p>The interpretation of this path, of course, depends on the existence of packages.
+ * See {@link InterpretPathAsTarget#eval}.
+ */
+ public String getPathForPathAsTarget() {
+ throw new IllegalStateException();
+ }
+
+ /**
+ * For patterns of type {@link Type#SINGLE_TARGET} and {@link Type#TARGETS_IN_PACKAGE}, returns
+ * the {@link PackageIdentifier} corresponding to the package that would contain the target(s)
+ * matched by this {@link TargetPattern}.
+ */
+ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
+ throw new IllegalStateException();
+ }
/**
* Returns {@code true} iff this pattern has type {@code Type.TARGETS_BELOW_DIRECTORY} or
@@ -231,12 +261,12 @@
}
@Override
- public boolean containsBelowDirectory(PackageIdentifier directory) {
+ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) {
return false;
}
@Override
- public PackageIdentifier getDirectory() {
+ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
return directory;
}
@@ -307,16 +337,13 @@
}
@Override
- public boolean containsBelowDirectory(PackageIdentifier directory) {
+ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) {
return false;
}
@Override
- public PackageIdentifier getDirectory() {
- int lastSlashIndex = path.lastIndexOf('/');
- // The package name cannot be illegal because we verified it during target parsing
- return PackageIdentifier.createInMainRepo(
- lastSlashIndex < 0 ? "" : path.substring(0, lastSlashIndex));
+ public String getPathForPathAsTarget() {
+ return path;
}
@Override
@@ -386,12 +413,12 @@
}
@Override
- public boolean containsBelowDirectory(PackageIdentifier directory) {
+ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier directory) {
return false;
}
@Override
- public PackageIdentifier getDirectory() {
+ public PackageIdentifier getDirectoryForTargetOrTargetsInPackage() {
return packageIdentifier;
}
@@ -510,7 +537,7 @@
}
@Override
- public boolean containsBelowDirectory(PackageIdentifier containedDirectory) {
+ public boolean containsAllTransitiveSubdirectoriesForTBD(PackageIdentifier containedDirectory) {
// Note that merely checking to see if the directory startsWith the TargetsBelowDirectory's
// directory is insufficient. "food" begins with "foo", but "//foo/..." does not contain
// "//food/...".
@@ -519,7 +546,7 @@
}
@Override
- public PackageIdentifier getDirectory() {
+ public PackageIdentifier getDirectoryForTargetsUnderDirectory() {
return directory;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
index 337b14d..9f922da 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GraphBackedRecursivePackageProvider.java
@@ -177,7 +177,9 @@
boolean isTBD = pattern.getType().equals(Type.TARGETS_BELOW_DIRECTORY);
PackageIdentifier packageIdentifier = PackageIdentifier.create(
repository, directory);
- if (isTBD && pattern.containsBelowDirectory(packageIdentifier)) {
+ if (isTBD
+ && pattern.containsAllTransitiveSubdirectoriesForTBD(
+ packageIdentifier)) {
inUniverse = true;
break;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
index 423d07e..eb1754e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternValue.java
@@ -144,8 +144,11 @@
TargetPatternKey laterTargetPatternKey = (TargetPatternKey) laterSkyKey.argument();
TargetPattern laterParsedPattern = laterTargetPatternKey.getParsedPattern();
if (laterTargetPatternKey.isNegative()
- && targetPatternKey.getParsedPattern().containsBelowDirectory(laterParsedPattern)) {
- excludedDirectoriesBuilder.add(laterParsedPattern.getDirectory().getPackageFragment());
+ && laterParsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY
+ && targetPatternKey.getParsedPattern().containsDirectoryOfTBDForTBD(
+ laterParsedPattern)) {
+ excludedDirectoriesBuilder.add(
+ laterParsedPattern.getDirectoryForTargetsUnderDirectory().getPackageFragment());
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
index e555b7d..22b400e 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/TargetPatternValue.java
@@ -221,9 +221,10 @@
if (parsedPattern.getType() == Type.TARGETS_BELOW_DIRECTORY) {
for (PathFragment blacklistedPackagePrefix : blacklistedPackagePrefixes.get()) {
PackageIdentifier pkgIdForBlacklistedDirectorPrefix = PackageIdentifier.create(
- parsedPattern.getDirectory().getRepository(),
+ parsedPattern.getDirectoryForTargetsUnderDirectory().getRepository(),
blacklistedPackagePrefix);
- if (parsedPattern.containsBelowDirectory(pkgIdForBlacklistedDirectorPrefix)) {
+ if (parsedPattern.containsAllTransitiveSubdirectoriesForTBD(
+ pkgIdForBlacklistedDirectorPrefix)) {
excludedPathsBuilder.add(blacklistedPackagePrefix);
}
}
diff --git a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
index dd62d99..cc22cfb 100644
--- a/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
+++ b/src/test/java/com/google/devtools/build/lib/cmdline/TargetPatternTest.java
@@ -97,9 +97,9 @@
// And a nested inner pattern '//foo/bar/...',
TargetPattern innerPattern = parseAsExpectedType("//foo/bar/...", Type.TARGETS_BELOW_DIRECTORY);
// Then the outer pattern contains the inner pattern,,
- assertTrue(outerPattern.containsBelowDirectory(innerPattern));
+ assertTrue(outerPattern.containsDirectoryOfTBDForTBD(innerPattern));
// And the inner pattern does not contain the outer pattern.
- assertFalse(innerPattern.containsBelowDirectory(outerPattern));
+ assertFalse(innerPattern.containsDirectoryOfTBDForTBD(outerPattern));
}
@Test
@@ -109,8 +109,8 @@
// And a pattern '//bar/...',
TargetPattern patternBar = parseAsExpectedType("//bar/...", Type.TARGETS_BELOW_DIRECTORY);
// Then neither pattern contains the other.
- assertFalse(patternFoo.containsBelowDirectory(patternBar));
- assertFalse(patternBar.containsBelowDirectory(patternFoo));
+ assertFalse(patternFoo.containsDirectoryOfTBDForTBD(patternBar));
+ assertFalse(patternBar.containsDirectoryOfTBDForTBD(patternFoo));
}
@Test
@@ -124,16 +124,16 @@
TargetPattern singleTargetPattern = parseAsExpectedType("//foo:bar", Type.SINGLE_TARGET);
TargetPattern targetsInPackagePattern = parseAsExpectedType("foo:all", Type.TARGETS_IN_PACKAGE);
- // Then the non-TargetsBelowDirectory patterns are contained by tbdFoo, and do not contain
- // tbdFoo.
- assertTrue(tbdFoo.containsBelowDirectory(pathAsTargetPattern));
- assertFalse(pathAsTargetPattern.containsBelowDirectory(tbdFoo));
+ // Then the non-TargetsBelowDirectory patterns do not contain tbdFoo.
+ assertFalse(pathAsTargetPattern.containsDirectoryOfTBDForTBD(tbdFoo));
+ // And are not considered to be a contained directory of the TargetsBelowDirectory pattern.
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(pathAsTargetPattern));
- assertTrue(tbdFoo.containsBelowDirectory(singleTargetPattern));
- assertFalse(singleTargetPattern.containsBelowDirectory(tbdFoo));
+ assertFalse(singleTargetPattern.containsDirectoryOfTBDForTBD(tbdFoo));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(singleTargetPattern));
- assertTrue(tbdFoo.containsBelowDirectory(targetsInPackagePattern));
- assertFalse(targetsInPackagePattern.containsBelowDirectory(tbdFoo));
+ assertFalse(targetsInPackagePattern.containsDirectoryOfTBDForTBD(tbdFoo));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(targetsInPackagePattern));
}
@Test
@@ -151,10 +151,10 @@
parseAsExpectedType("food:all", Type.TARGETS_IN_PACKAGE);
// Then the non-TargetsBelowDirectory patterns are not contained by tbdFoo.
- assertFalse(tbdFoo.containsBelowDirectory(targetsBelowDirectoryPattern));
- assertFalse(tbdFoo.containsBelowDirectory(pathAsTargetPattern));
- assertFalse(tbdFoo.containsBelowDirectory(singleTargetPattern));
- assertFalse(tbdFoo.containsBelowDirectory(targetsInPackagePattern));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(targetsBelowDirectoryPattern));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(pathAsTargetPattern));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(singleTargetPattern));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(targetsInPackagePattern));
}
@Test
@@ -170,17 +170,17 @@
TargetPattern targetsInPackagePattern = parseAsExpectedType("foo:all", Type.TARGETS_IN_PACKAGE);
// Then the patterns are contained by tbdDepot, and do not contain tbdDepot.
- assertTrue(tbdDepot.containsBelowDirectory(tbdFoo));
- assertFalse(tbdFoo.containsBelowDirectory(tbdDepot));
+ assertTrue(tbdDepot.containsDirectoryOfTBDForTBD(tbdFoo));
+ assertFalse(tbdFoo.containsDirectoryOfTBDForTBD(tbdDepot));
- assertTrue(tbdDepot.containsBelowDirectory(pathAsTargetPattern));
- assertFalse(pathAsTargetPattern.containsBelowDirectory(tbdDepot));
+ assertFalse(tbdDepot.containsDirectoryOfTBDForTBD(pathAsTargetPattern));
+ assertFalse(pathAsTargetPattern.containsDirectoryOfTBDForTBD(tbdDepot));
- assertTrue(tbdDepot.containsBelowDirectory(singleTargetPattern));
- assertFalse(singleTargetPattern.containsBelowDirectory(tbdDepot));
+ assertFalse(tbdDepot.containsDirectoryOfTBDForTBD(singleTargetPattern));
+ assertFalse(singleTargetPattern.containsDirectoryOfTBDForTBD(tbdDepot));
- assertTrue(tbdDepot.containsBelowDirectory(targetsInPackagePattern));
- assertFalse(targetsInPackagePattern.containsBelowDirectory(tbdDepot));
+ assertFalse(tbdDepot.containsDirectoryOfTBDForTBD(targetsInPackagePattern));
+ assertFalse(targetsInPackagePattern.containsDirectoryOfTBDForTBD(tbdDepot));
}
private static TargetPattern parse(String pattern) throws TargetParsingException {
