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

1 files changed, 12 insertions, 14 deletions

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 69cf3baf70..147a4e7046 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 class RecursivePackageProviderBackedTargetPatternResolver
   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 @@ public class RecursivePackageProviderBackedTargetPatternResolver
   }
 
   @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 @@ public class RecursivePackageProviderBackedTargetPatternResolver
           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));
       }
     }