Make TargetPatternResolver#{isPackage, getTargetsInPackage} take a PackageIdentifier instead of a String.
This remarkably fiddly CL is a step towards making wildcards pattern work with remote repositories. I originally wanted to refactor findTargetsBeneathDirectory(), too, but it turns out that it's a much more complicated affair.
--
MOS_MIGRATED_REVID=103622420
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 b127009..6dc2385 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
@@ -249,7 +249,7 @@
Preconditions.checkArgument(excludedSubdirectories.isEmpty(),
"Target pattern \"%s\" of type %s cannot be evaluated with excluded subdirectories: %s.",
getOriginalPattern(), getType(), excludedSubdirectories);
- if (resolver.isPackage(path)) {
+ if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(path))) {
// User has specified a package name. lookout for default target.
return resolver.getExplicitTarget("//" + path);
}
@@ -260,7 +260,7 @@
// first BUILD file is found (i.e. longest prefix match).
for (int i = pieces.size() - 1; i > 0; i--) {
String packageName = SLASH_JOINER.join(pieces.subList(0, i));
- if (resolver.isPackage(packageName)) {
+ if (resolver.isPackage(PackageIdentifier.createInDefaultRepo(packageName))) {
String targetName = SLASH_JOINER.join(pieces.subList(i, pieces.size()));
return resolver.getExplicitTarget("//" + packageName + ":" + targetName);
}
@@ -334,8 +334,18 @@
return targets;
}
}
- return resolver.getTargetsInPackage(getOriginalPattern(), removeSuffix(pattern, suffix),
- rulesOnly);
+
+ String packageName = removeSuffix(pattern, suffix);
+
+ try {
+ PackageIdentifier.parse(packageName);
+ } catch (LabelSyntaxException e) {
+ throw new TargetParsingException(
+ "Invalid package name '" + packageName + "': " + e.getMessage());
+ }
+
+ return resolver.getTargetsInPackage(getOriginalPattern(),
+ PackageIdentifier.createInDefaultRepo(packageName), rulesOnly);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
index 4234ad5..d106692 100644
--- a/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/cmdline/TargetPatternResolver.java
@@ -49,11 +49,12 @@
* only return rules in the given package.
*
* @param originalPattern the original target pattern for error reporting purposes
- * @param packageName the name of the package
+ * @param packageIdentifier the identifier of the package
* @param rulesOnly whether to return rules only
*/
- ResolvedTargets<T> getTargetsInPackage(String originalPattern, String packageName,
- boolean rulesOnly) throws TargetParsingException, InterruptedException;
+ ResolvedTargets<T> getTargetsInPackage(String originalPattern,
+ PackageIdentifier packageIdentifier, boolean rulesOnly)
+ throws TargetParsingException, InterruptedException;
/**
* Returns the set containing the targets found below the given {@code directory}. Conceptually,
@@ -85,7 +86,7 @@
* Returns true, if and only if the given name corresponds to a package, i.e., a file with the
* name {@code packageName/BUILD} exists.
*/
- boolean isPackage(String packageName);
+ boolean isPackage(PackageIdentifier packageIdentifier);
/**
* Returns the target kind of the given target, for example {@code cc_library rule}.
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
index 49b7548..60a0d64 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/TargetPatternResolverUtil.java
@@ -17,6 +17,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.ResolvedTargets;
import com.google.devtools.build.lib.cmdline.TargetParsingException;
import com.google.devtools.build.lib.cmdline.TargetPatternResolver;
@@ -83,7 +84,7 @@
if (LabelValidator.validatePackageName(packageName) != null) {
throw new TargetParsingException("'" + packageName + "' is not a valid package name");
}
- if (!resolver.isPackage(packageName)) {
+ if (!resolver.isPackage(PackageIdentifier.createInDefaultRepo(packageName))) {
throw new TargetParsingException(
TargetPatternResolverUtil.getParsingErrorMessage(
"no such package '" + packageName + "': BUILD file not found on package path",
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
index b116595..a48117f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternFunction.java
@@ -160,20 +160,18 @@
}
@Override
- public ResolvedTargets<Void> getTargetsInPackage(String originalPattern, String packageName,
- boolean rulesOnly) throws TargetParsingException {
+ public ResolvedTargets<Void> getTargetsInPackage(String originalPattern,
+ PackageIdentifier packageIdentifier, boolean rulesOnly) throws TargetParsingException {
FilteringPolicy policy =
rulesOnly ? FilteringPolicies.RULES_ONLY : FilteringPolicies.NO_FILTER;
- return getTargetsInPackage(originalPattern, new PathFragment(packageName), policy);
+ return getTargetsInPackage(originalPattern, packageIdentifier, policy);
}
private ResolvedTargets<Void> getTargetsInPackage(String originalPattern,
- PathFragment packageNameFragment, FilteringPolicy policy)
+ PackageIdentifier packageIdentifier, FilteringPolicy policy)
throws TargetParsingException {
- TargetPatternResolverUtil.validatePatternPackage(originalPattern, packageNameFragment, this);
try {
- PackageIdentifier packageId = PackageIdentifier.createInDefaultRepo(packageNameFragment);
- Package pkg = packageProvider.getPackage(env.getListener(), packageId);
+ Package pkg = packageProvider.getPackage(env.getListener(), packageIdentifier);
ResolvedTargets<Target> packageTargets =
TargetPatternResolverUtil.resolvePackageTargets(pkg, policy);
ImmutableList.Builder<SkyKey> builder = ImmutableList.builder();
@@ -194,11 +192,8 @@
}
@Override
- public boolean isPackage(String packageName) {
- // TODO(bazel-team): this should get the whole PackageIdentifier. Using only the package name
- // makes it impossible to use wildcards to refer to targets in remote repositories.
- return packageProvider.isPackage(env.getListener(),
- PackageIdentifier.createInDefaultRepo(packageName));
+ public boolean isPackage(PackageIdentifier packageIdentifier) {
+ return packageProvider.isPackage(env.getListener(), packageIdentifier);
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
index 69cf3ba..147a4e7 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/RecursivePackageProviderBackedTargetPatternResolver.java
@@ -76,7 +76,7 @@
public Target getTargetOrNull(String targetName) throws InterruptedException {
try {
Label label = Label.parseAbsolute(targetName);
- if (!isPackage(label.getPackageName())) {
+ if (!isPackage(label.getPackageIdentifier())) {
return null;
}
return recursivePackageProvider.getTarget(eventHandler, label);
@@ -100,35 +100,31 @@
}
@Override
- public ResolvedTargets<Target> getTargetsInPackage(String originalPattern, String packageName,
- boolean rulesOnly)
+ public ResolvedTargets<Target> getTargetsInPackage(
+ String originalPattern, PackageIdentifier packageIdentifier, boolean rulesOnly)
throws TargetParsingException, InterruptedException {
FilteringPolicy actualPolicy = rulesOnly
? FilteringPolicies.and(FilteringPolicies.RULES_ONLY, policy)
: policy;
- return getTargetsInPackage(originalPattern, new PathFragment(packageName), actualPolicy);
+ return getTargetsInPackage(originalPattern, packageIdentifier, actualPolicy);
}
private ResolvedTargets<Target> getTargetsInPackage(String originalPattern,
- PathFragment packageNameFragment, FilteringPolicy policy)
+ PackageIdentifier packageIdentifier, FilteringPolicy policy)
throws TargetParsingException, InterruptedException {
- TargetPatternResolverUtil.validatePatternPackage(originalPattern, packageNameFragment, this);
try {
- Package pkg = getPackage(PackageIdentifier.createInDefaultRepo(packageNameFragment));
+ Package pkg = getPackage(packageIdentifier);
return TargetPatternResolverUtil.resolvePackageTargets(pkg, policy);
} catch (NoSuchThingException e) {
String message = TargetPatternResolverUtil.getParsingErrorMessage(
- "package contains errors", originalPattern);
+ e.getMessage(), originalPattern);
throw new TargetParsingException(message, e);
}
}
@Override
- public boolean isPackage(String packageName) {
- // TODO(bazel-team): this should get the whole PackageIdentifier. Using only the package name
- // makes it impossible to use the //... wildcard to refer to targets in remote repositories.
- return recursivePackageProvider.isPackage(
- eventHandler, PackageIdentifier.createInDefaultRepo(packageName));
+ public boolean isPackage(PackageIdentifier packageIdentifier) {
+ return recursivePackageProvider.isPackage(eventHandler, packageIdentifier);
}
@Override
@@ -156,7 +152,9 @@
recursivePackageProvider.getPackagesUnderDirectory(
repository, rootedPath, excludedPathFragments);
for (PathFragment pkg : packagesUnderDirectory) {
- targetBuilder.merge(getTargetsInPackage(originalPattern, pkg, FilteringPolicies.NO_FILTER));
+ targetBuilder.merge(getTargetsInPackage(originalPattern,
+ PackageIdentifier.createInDefaultRepo(pkg),
+ FilteringPolicies.NO_FILTER));
}
}
