Make globs work in remote repositories.

This involved quite a few changes, mainly changing a bunch of places where we refer to packages by a PathFragment to PackageIdentifier. 

The only wart is the code in PathPackageLocator: ideally, it would just call into PackageLookupFunction. Unfortunately, it is (through globbing and Parser.include) called from within a Skyframe function, and we don't want to have two eval() calls going on at the same time, so we cannot use that.

There is a potential correctness issue there: PathPackageLocator now assumes where external repositories are put and assumes that they are there when it gets control, but my understanding is that the associated RepositoryValue is always evaluated before, so it works out okay.

--
MOS_MIGRATED_REVID=97647787

21 files changed, 150 insertions, 80 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java b/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java
index f6badad9dd..3d677990f3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/CachingPackageLocator.java
@@ -40,6 +40,6 @@ public interface CachingPackageLocator {
    * <p> This method must be thread-safe.
    */
   @ThreadSafe
-  Path getBuildFileForPackage(String packageName);
+  Path getBuildFileForPackage(PackageIdentifier packageName);
 
 }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
index de9bfdfebc..43669a86bf 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/GlobCache.java
@@ -23,7 +23,6 @@ import com.google.common.util.concurrent.SettableFuture;
 import com.google.devtools.build.lib.concurrent.ThreadSafety;
 import com.google.devtools.build.lib.util.Pair;
 import com.google.devtools.build.lib.vfs.Path;
-import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.lib.vfs.UnixGlob;
 
@@ -107,7 +106,6 @@ public class GlobCache {
     this.syscalls = syscalls == null ? new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS) : syscalls;
 
     Preconditions.checkNotNull(locator);
-    final PathFragment pkgNameFrag = packageId.getPackageFragment();
     childDirectoryPredicate = new Predicate<Path>() {
       @Override
       public boolean apply(Path directory) {
@@ -115,7 +113,9 @@ public class GlobCache {
           return true;
         }
 
-        PathFragment pkgName = pkgNameFrag.getRelative(directory.relativeTo(packageDirectory));
+        PackageIdentifier subPackageId = new PackageIdentifier(
+            packageId.getRepository(),
+            packageId.getPackageFragment().getRelative(directory.relativeTo(packageDirectory)));
         UnixGlob.FilesystemCalls syscalls = GlobCache.this.syscalls.get();
         if (syscalls != UnixGlob.DEFAULT_SYSCALLS) {
           // This is needed because in case the BUILD file exists, we do not call readdir() on its
@@ -134,7 +134,7 @@ public class GlobCache {
           syscalls.statNullable(directory.getChild("BUILD"), Symlinks.FOLLOW);
         }
 
-        return locator.getBuildFileForPackage(pkgName.getPathString()) == null;
+        return locator.getBuildFileForPackage(subPackageId) == null;
       }
     };
   }
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java
index f2f5b8e0e0..803fb48855 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageIdentifier.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.packages;
 
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ComparisonChain;
+import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.syntax.Label.SyntaxException;
 import com.google.devtools.build.lib.util.StringCanonicalizer;
 import com.google.devtools.build.lib.util.StringUtilities;
@@ -217,6 +218,34 @@ public final class PackageIdentifier implements Comparable<PackageIdentifier>, S
     this.pkgName = Canonicalizer.fragments().intern(pkgName.normalize());
   }
 
+  public static PackageIdentifier parse(String input) throws SyntaxException {
+    String repo;
+    String packageName;
+    int packageStartPos = input.indexOf("//");
+    if (packageStartPos > 0) {
+      repo = input.substring(0, packageStartPos);
+      packageName = input.substring(packageStartPos + 2);
+    } else if (packageStartPos == 0) {
+      repo = PackageIdentifier.DEFAULT_REPOSITORY;
+      packageName = input.substring(2);
+    } else {
+      repo = PackageIdentifier.DEFAULT_REPOSITORY;
+      packageName = input;
+    }
+
+    String error = RepositoryName.validate(repo);
+    if (error != null) {
+      throw new SyntaxException(error);
+    }
+
+    error = LabelValidator.validatePackageName(packageName);
+    if (error != null) {
+      throw new SyntaxException(error);
+    }
+
+    return new PackageIdentifier(repo, new PathFragment(packageName));
+  }
+
   private Object writeReplace() throws ObjectStreamException {
     return new SerializationProxy(this);
   }
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
index e57fe0df61..daba8f1e88 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageCacheOptions.java
@@ -16,6 +16,7 @@ package com.google.devtools.build.lib.pkgcache;
 
 import com.google.devtools.build.lib.Constants;
 import com.google.devtools.build.lib.packages.ConstantRuleVisibility;
+import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.syntax.CommaSeparatedPackageNameListConverter;
 import com.google.devtools.common.options.Converter;
@@ -110,7 +111,7 @@ public class PackageCacheOptions extends OptionsBase {
           + "encounters a label '//x:y/z' if that is still provided by another "
           + "package_path entry.  Specifying --deleted_packages x/y avoids this "
           + "problem.")
-  public List<String> deletedPackages;
+  public List<PackageIdentifier> deletedPackages;
 
   @Option(name = "default_visibility",
       defaultValue = "private",
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
index 5363cb5a8a..9bce541a51 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PackageProvider.java
@@ -59,5 +59,5 @@ public interface PackageProvider extends TargetProvider {
    * @param eventHandler the eventHandler on which to report warnings and errors
    * @param packageName the name of the package.
    */
-  boolean isPackage(EventHandler eventHandler, String packageName);
+  boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName);
 }
diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
index c8204c6a26..27b072f211 100644
--- a/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
+++ b/src/main/java/com/google/devtools/build/lib/pkgcache/PathPackageLocator.java
@@ -13,6 +13,8 @@
 // limitations under the License.
 package com.google.devtools.build.lib.pkgcache;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Objects;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.events.Event;
@@ -26,6 +28,9 @@ import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.Symlinks;
 import com.google.devtools.build.lib.vfs.UnixGlob;
 
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -45,33 +50,27 @@ public class PathPackageLocator implements Serializable {
 
   public static final Set<String> DEFAULT_TOP_LEVEL_EXCLUDES = ImmutableSet.of("experimental");
 
-  /**
-   * An interface which accepts {@link PathFragment}s.
-   */
-  public interface AcceptsPathFragment {
-
-    /**
-     * Accept a {@link PathFragment}.
-     *
-     * @param fragment The path fragment.
-     */
-    void accept(PathFragment fragment);
-  }
-
   private final ImmutableList<Path> pathEntries;
+  private final Path outputBase;
+
+  private PathPackageLocator(Path outputBase, List<Path> pathEntries) {
+    this.outputBase = outputBase;
+    this.pathEntries = ImmutableList.copyOf(pathEntries);
+  }
 
   /**
    * Constructs a PathPackageLocator based on the specified list of package root directories.
    */
+  @VisibleForTesting
   public PathPackageLocator(List<Path> pathEntries) {
-    this.pathEntries = ImmutableList.copyOf(pathEntries);
+    this(null, pathEntries);
   }
 
   /**
    * Constructs a PathPackageLocator based on the specified array of package root directories.
    */
   public PathPackageLocator(Path... pathEntries) {
-    this(Arrays.asList(pathEntries));
+    this(null, Arrays.asList(pathEntries));
   }
 
   /**
@@ -87,11 +86,10 @@ public class PathPackageLocator implements Serializable {
    * <p>If the same package exists beneath multiple package path entries, the
    * first path that matches always wins.
    */
-  public Path getPackageBuildFile(String packageName) throws NoSuchPackageException {
+  public Path getPackageBuildFile(PackageIdentifier packageName) throws NoSuchPackageException {
     Path buildFile  = getPackageBuildFileNullable(packageName, UnixGlob.DEFAULT_SYSCALLS_REF);
     if (buildFile == null) {
-      throw new BuildFileNotFoundException(PackageIdentifier.createInDefaultRepo(packageName),
-          "BUILD file not found on package path");
+      throw new BuildFileNotFoundException(packageName, "BUILD file not found on package path");
     }
     return buildFile;
   }
@@ -102,9 +100,25 @@ public class PathPackageLocator implements Serializable {
    * @param packageName the name of the package.
    * @param cache a filesystem-level cache of stat() calls.
    */
-  public Path getPackageBuildFileNullable(String packageName,
+  public Path getPackageBuildFileNullable(PackageIdentifier packageName,
       AtomicReference<? extends UnixGlob.FilesystemCalls> cache)  {
-    return getFilePath(new PathFragment(packageName).getRelative("BUILD"), cache);
+    if (packageName.getRepository().isDefault()) {
+      return getFilePath(packageName.getPackageFragment().getRelative("BUILD"), cache);
+    } else if (outputBase != null) {
+      // This works only to some degree, because it relies on the presence of the repository under
+      // $OUTPUT_BASE/external, which is created by the appropriate RepositoryValue. This is true
+      // for the invocation in GlobCache, but not for the locator.getBuildFileForPackage()
+      // invocation in Parser#include().
+      Path buildFile = outputBase.getRelative(packageName.getPathFragment()).getRelative("BUILD");
+      FileStatus stat = cache.get().statNullable(buildFile, Symlinks.FOLLOW);
+      if (stat != null && stat.isFile()) {
+        return buildFile;
+      } else {
+        return null;
+      }
+    } else {
+      return null;
+    }
   }
 
 
@@ -124,6 +138,7 @@ public class PathPackageLocator implements Serializable {
    * A factory of PathPackageLocators from a list of path elements.  Elements
    * may contain "%workspace%", indicating the workspace.
    *
+   * @param outputBase the output base. Can be null if remote repositories are not in use.
    * @param pathElements Each element must be an absolute path, relative path,
    *                     or some string "%workspace%" + relative, where relative is itself a
    *                     relative path.  The special symbol "%workspace%" means to interpret
@@ -135,7 +150,8 @@ public class PathPackageLocator implements Serializable {
    * @param clientWorkingDirectory The client's working directory.
    * @return a list of {@link Path}s.
    */
-  public static PathPackageLocator create(List<String> pathElements,
+  public static PathPackageLocator create(Path outputBase,
+                                          List<String> pathElements,
                                           EventHandler eventHandler,
                                           Path workspace,
                                           Path clientWorkingDirectory) {
@@ -164,7 +180,7 @@ public class PathPackageLocator implements Serializable {
         resolvedPaths.add(rootPath);
       }
     }
-    return new PathPackageLocator(resolvedPaths);
+    return new PathPackageLocator(outputBase, resolvedPaths);
   }
 
   /**
@@ -194,7 +210,7 @@ public class PathPackageLocator implements Serializable {
 
   @Override
   public int hashCode() {
-    return pathEntries.hashCode();
+    return Objects.hashCode(pathEntries, outputBase);
   }
 
   @Override
@@ -205,6 +221,15 @@ public class PathPackageLocator implements Serializable {
     if (!(other instanceof PathPackageLocator)) {
       return false;
     }
-    return this.getPathEntries().equals(((PathPackageLocator) other).getPathEntries());
+    return this.getPathEntries().equals(((PathPackageLocator) other).getPathEntries())
+        && Objects.equal(this.outputBase, ((PathPackageLocator) other).outputBase);
+  }
+
+  private void writeObject(ObjectOutputStream out) throws IOException {
+    out.writeObject(pathEntries);
+  }
+
+  private void readObject(ObjectInputStream in) throws IOException {
+    throw new IllegalStateException();
   }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
index 2aae12f91a..bbc432e7a5 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genquery/GenQuery.java
@@ -461,7 +461,7 @@ public class GenQuery implements RuleConfiguredTargetFactory {
     }
 
     @Override
-    public boolean isPackage(EventHandler eventHandler, String packageName) {
+    public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) {
       throw new UnsupportedOperationException();
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
index fca2ae918f..fcd11e5e46 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeRuntime.java
@@ -1014,7 +1014,7 @@ public final class BlazeRuntime {
     if (!skyframeExecutor.hasIncrementalState()) {
       clearSkyframeRelevantCaches();
     }
-    skyframeExecutor.sync(packageCacheOptions, getWorkingDirectory(),
+    skyframeExecutor.sync(packageCacheOptions, getOutputBase(), getWorkingDirectory(),
         defaultsPackageContents, getCommandId());
   }
 
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
index bec0deb5d1..133179f07d 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/ProjectFileSupport.java
@@ -63,8 +63,11 @@ public final class ProjectFileSupport {
       // relative to the cwd instead.
       PathFragment projectFilePath = new PathFragment(targets.get(0).substring(1));
       List<Path> packagePath = PathPackageLocator.create(
-          optionsParser.getOptions(PackageCacheOptions.class).packagePath, runtime.getReporter(),
-          runtime.getWorkspace(), runtime.getWorkingDirectory()).getPathEntries();
+          runtime.getOutputBase(),
+          optionsParser.getOptions(PackageCacheOptions.class).packagePath,
+          runtime.getReporter(),
+          runtime.getWorkspace(),
+          runtime.getWorkingDirectory()).getPathEntries();
       ProjectFile projectFile = projectFileProvider.getProjectFile(packagePath, projectFilePath);
       runtime.getReporter().handle(Event.info("Using " + projectFile.getName()));
 
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
index 9ea62103fe..ac5b6a2f1f 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/EnvironmentBackedRecursivePackageProvider.java
@@ -64,9 +64,9 @@ public final class EnvironmentBackedRecursivePackageProvider implements Recursiv
   }
 
   @Override
-  public boolean isPackage(EventHandler eventHandler, String packageName)
+  public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageId)
       throws MissingDepException {
-    SkyKey packageLookupKey = PackageLookupValue.key(new PathFragment(packageName));
+    SkyKey packageLookupKey = PackageLookupValue.key(packageId);
     try {
       PackageLookupValue packageLookupValue =
           (PackageLookupValue) env.getValueOrThrow(packageLookupKey, NoSuchPackageException.class,
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
index d1ee81cb07..942a201770 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/GlobFunction.java
@@ -18,6 +18,7 @@ import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
+import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.vfs.Dirent;
 import com.google.devtools.build.lib.vfs.Dirent.Type;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -60,8 +61,9 @@ final class GlobFunction implements SkyFunction {
     PathFragment globSubdir = glob.getSubdir();
     if (!globSubdir.equals(PathFragment.EMPTY_FRAGMENT)) {
       PackageLookupValue globSubdirPkgLookupValue = (PackageLookupValue) env.getValue(
-          PackageLookupValue.key(glob.getPackageId().getPackageFragment()
-              .getRelative(globSubdir)));
+          PackageLookupValue.key(new PackageIdentifier(
+              glob.getPackageId().getRepository(),
+              glob.getPackageId().getPackageFragment().getRelative(globSubdir))));
       if (globSubdirPkgLookupValue == null) {
         return null;
       }
@@ -220,7 +222,8 @@ final class GlobFunction implements SkyFunction {
         PathFragment directory = glob.getPackageId().getPackageFragment()
             .getRelative(glob.getSubdir()).getRelative(fileName);
         PackageLookupValue pkgLookupValue = (PackageLookupValue)
-            env.getValue(PackageLookupValue.key(directory));
+            env.getValue(PackageLookupValue.key(new PackageIdentifier(
+                glob.getPackageId().getRepository(), directory)));
         if (pkgLookupValue == null) {
           return;
         }
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 e2c83a024c..4b701dd775 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
@@ -68,8 +68,8 @@ public final class GraphBackedRecursivePackageProvider implements RecursivePacka
   }
 
   @Override
-  public boolean isPackage(EventHandler eventHandler, String packageName) {
-    SkyKey packageLookupKey = PackageLookupValue.key(new PathFragment(packageName));
+  public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) {
+    SkyKey packageLookupKey = PackageLookupValue.key(packageName);
     if (!graph.exists(packageLookupKey)) {
       // If the package lookup key does not exist in the graph, then it must not correspond to any
       // package, because the SkyQuery environment has already loaded the universe.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
index a4996a4479..c634d08d5a 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/PackageLookupFunction.java
@@ -41,9 +41,9 @@ import javax.annotation.Nullable;
  */
 class PackageLookupFunction implements SkyFunction {
 
-  private final AtomicReference<ImmutableSet<String>> deletedPackages;
+  private final AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages;
 
-  PackageLookupFunction(AtomicReference<ImmutableSet<String>> deletedPackages) {
+  PackageLookupFunction(AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages) {
     this.deletedPackages = deletedPackages;
   }
 
@@ -64,7 +64,7 @@ class PackageLookupFunction implements SkyFunction {
           + packageNameErrorMsg);
     }
 
-    if (deletedPackages.get().contains(packageKey.getPackageFragment().toString())) {
+    if (deletedPackages.get().contains(packageKey)) {
       return PackageLookupValue.deletedPackage();
     }
 
@@ -142,7 +142,7 @@ class PackageLookupFunction implements SkyFunction {
       throws PackageLookupFunctionException {
     PackageIdentifier id = (PackageIdentifier) skyKey.argument();
     SkyKey repositoryKey = RepositoryValue.key(id.getRepository());
-    RepositoryValue repositoryValue = null;
+    RepositoryValue repositoryValue;
     try {
       repositoryValue = (RepositoryValue) env.getValueOrThrow(
           repositoryKey, NoSuchPackageException.class, IOException.class, EvalException.class);
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 bf32843787..2fa6ca91a8 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
@@ -187,7 +187,10 @@ public class PrepareDepsOfPatternFunction implements SkyFunction {
 
     @Override
     public boolean isPackage(String packageName) {
-      return packageProvider.isPackage(env.getListener(), 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));
     }
 
     @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 942ed83fcb..b91362834e 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
@@ -132,7 +132,10 @@ public class RecursivePackageProviderBackedTargetPatternResolver
 
   @Override
   public boolean isPackage(String packageName) {
-    return recursivePackageProvider.isPackage(eventHandler, 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));
   }
 
   @Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
index ef22ab993b..a1ac8e2116 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SequencedSkyframeExecutor.java
@@ -211,11 +211,12 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
   }
 
   @Override
-  public void sync(PackageCacheOptions packageCacheOptions, Path workingDirectory,
+  public void sync(PackageCacheOptions packageCacheOptions, Path outputBase, Path workingDirectory,
                    String defaultsPackageContents, UUID commandId)
       throws InterruptedException, AbruptExitException {
     this.valueCacheEvictionLimit = packageCacheOptions.minLoadedPkgCountForCtNodeEviction;
-    super.sync(packageCacheOptions, workingDirectory, defaultsPackageContents, commandId);
+    super.sync(
+        packageCacheOptions, outputBase, workingDirectory, defaultsPackageContents, commandId);
     handleDiffs();
   }
 
@@ -243,11 +244,10 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
     recordingDiffer.invalidate(Iterables.filter(memoizingEvaluator.getValues().keySet(), pred));
   }
 
-  private void invalidateDeletedPackages(Iterable<String> deletedPackages) {
+  private void invalidateDeletedPackages(Iterable<PackageIdentifier> deletedPackages) {
     ArrayList<SkyKey> packagesToInvalidate = Lists.newArrayList();
-    for (String deletedPackage : deletedPackages) {
-      PathFragment pathFragment = new PathFragment(deletedPackage);
-      packagesToInvalidate.add(PackageLookupValue.key(pathFragment));
+    for (PackageIdentifier deletedPackage : deletedPackages) {
+      packagesToInvalidate.add(PackageLookupValue.key(deletedPackage));
     }
     recordingDiffer.invalidate(packagesToInvalidate);
   }
@@ -257,7 +257,7 @@ public final class SequencedSkyframeExecutor extends SkyframeExecutor {
    */
   @Override
   @VisibleForTesting  // productionVisibility = Visibility.PRIVATE
-  public void setDeletedPackages(Iterable<String> pkgs) {
+  public void setDeletedPackages(Iterable<PackageIdentifier> pkgs) {
     // Invalidate the old deletedPackages as they may exist now.
     invalidateDeletedPackages(deletedPackages.get());
     deletedPackages.set(ImmutableSet.copyOf(pkgs));
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index b2ed3c6c5a..79acbe0985 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -183,8 +183,8 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
       new AtomicReference<>(UnixGlob.DEFAULT_SYSCALLS);
   protected final AtomicReference<PathPackageLocator> pkgLocator =
       new AtomicReference<>();
-  protected final AtomicReference<ImmutableSet<String>> deletedPackages =
-      new AtomicReference<>(ImmutableSet.<String>of());
+  protected final AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages =
+      new AtomicReference<>(ImmutableSet.<PackageIdentifier>of());
   private final AtomicReference<EventBus> eventBus = new AtomicReference<>();
 
   private final ImmutableList<BuildInfoFactory> buildInfoFactories;
@@ -756,7 +756,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    * Sets the packages that should be treated as deleted and ignored.
    */
   @VisibleForTesting  // productionVisibility = Visibility.PRIVATE
-  public abstract void setDeletedPackages(Iterable<String> pkgs);
+  public abstract void setDeletedPackages(Iterable<PackageIdentifier> pkgs);
 
   /**
    * Prepares the evaluator for loading.
@@ -1302,7 +1302,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
     /**
      * Returns whether the given package should be consider deleted and thus should be ignored.
      */
-    public boolean isPackageDeleted(String packageName) {
+    public boolean isPackageDeleted(PackageIdentifier packageName) {
       return deletedPackages.get().contains(packageName);
     }
 
@@ -1352,12 +1352,13 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   @ThreadCompatible
   public abstract void updateLoadedPackageSet(Set<PackageIdentifier> loadedPackages);
 
-  public void sync(PackageCacheOptions packageCacheOptions, Path workingDirectory,
+  public void sync(PackageCacheOptions packageCacheOptions, Path outputBase, Path workingDirectory,
       String defaultsPackageContents, UUID commandId) throws InterruptedException,
       AbruptExitException{
 
     preparePackageLoading(
-        createPackageLocator(packageCacheOptions, directories.getWorkspace(), workingDirectory),
+        createPackageLocator(
+            packageCacheOptions, outputBase, directories.getWorkspace(), workingDirectory),
         packageCacheOptions.defaultVisibility, packageCacheOptions.showLoadingProgress,
         packageCacheOptions.globbingThreads, defaultsPackageContents, commandId);
     setDeletedPackages(ImmutableSet.copyOf(packageCacheOptions.deletedPackages));
@@ -1367,9 +1368,9 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
   }
 
   protected PathPackageLocator createPackageLocator(PackageCacheOptions packageCacheOptions,
-      Path workspace, Path workingDirectory) throws AbruptExitException{
+      Path outputBase, Path workspace, Path workingDirectory) throws AbruptExitException {
     return PathPackageLocator.create(
-        packageCacheOptions.packagePath, getReporter(), workspace, workingDirectory);
+        outputBase, packageCacheOptions.packagePath, getReporter(), workspace, workingDirectory);
   }
 
   private CyclesReporter createCyclesReporter() {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
index 3e9b6cad1f..7ffb79ce89 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframePackageManager.java
@@ -13,7 +13,6 @@
 // limitations under the License.
 package com.google.devtools.build.lib.skyframe;
 
-import com.google.devtools.build.lib.cmdline.LabelValidator;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.ThreadSafe;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
@@ -136,7 +135,7 @@ class SkyframePackageManager implements PackageManager {
   }
 
   @Override
-  public boolean isPackage(EventHandler eventHandler, String packageName) {
+  public boolean isPackage(EventHandler eventHandler, PackageIdentifier packageName) {
     return getBuildFileForPackage(packageName) != null;
   }
 
@@ -147,11 +146,10 @@ class SkyframePackageManager implements PackageManager {
 
   @ThreadSafe
   @Override
-  public Path getBuildFileForPackage(String packageName) {
+  public Path getBuildFileForPackage(PackageIdentifier packageName) {
     // Note that this method needs to be thread-safe, as it is currently used concurrently by
     // legacy blaze code.
-    if (packageLoader.isPackageDeleted(packageName)
-        || LabelValidator.validatePackageName(packageName) != null) {
+    if (packageLoader.isPackageDeleted(packageName)) {
       return null;
     }
     // TODO(bazel-team): Use a PackageLookupValue here [skyframe-loading]
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java b/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java
index 070e928eb9..ce7680ce3e 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/CommaSeparatedPackageNameListConverter.java
@@ -16,7 +16,7 @@ package com.google.devtools.build.lib.syntax;
 import com.google.common.base.Splitter;
 import com.google.common.base.Strings;
 import com.google.common.collect.ImmutableList;
-import com.google.devtools.build.lib.cmdline.LabelValidator;
+import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.OptionsParsingException;
 
@@ -26,22 +26,22 @@ import java.util.List;
  * A converter from strings containing comma-separated names of packages to lists of strings.
  */
 public class CommaSeparatedPackageNameListConverter
-    implements Converter<List<String>> {
+    implements Converter<List<PackageIdentifier>> {
 
   private static final Splitter SPACE_SPLITTER = Splitter.on(',');
 
   @Override
-  public List<String> convert(String input) throws OptionsParsingException {
+  public List<PackageIdentifier> convert(String input) throws OptionsParsingException {
     if (Strings.isNullOrEmpty(input)) {
       return ImmutableList.of();
     }
-    ImmutableList.Builder<String> list = ImmutableList.builder();
+    ImmutableList.Builder<PackageIdentifier> list = ImmutableList.builder();
     for (String s : SPACE_SPLITTER.split(input)) {
-      String errorMessage = LabelValidator.validatePackageName(s);
-      if (errorMessage != null) {
-        throw new OptionsParsingException(errorMessage);
+      try {
+        list.add(PackageIdentifier.parse(s));
+      } catch (Label.SyntaxException e) {
+        throw new OptionsParsingException(e.getMessage());
       }
-      list.add(s);
     }
     return list.build();
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java b/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java
index 4966fb8717..f91222c740 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/EvaluationContext.java
@@ -21,6 +21,7 @@ import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.events.EventKind;
 import com.google.devtools.build.lib.packages.CachingPackageLocator;
+import com.google.devtools.build.lib.packages.PackageIdentifier;
 import com.google.devtools.build.lib.syntax.Environment.NoSuchVariableException;
 import com.google.devtools.build.lib.vfs.Path;
 
@@ -90,7 +91,7 @@ public final class EvaluationContext {
   /** Mock package locator */
   private static final class EmptyPackageLocator implements CachingPackageLocator {
     @Override
-    public Path getBuildFileForPackage(String packageName) {
+    public Path getBuildFileForPackage(PackageIdentifier packageName) {
       return null;
     }
   }
diff --git a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
index 2a9d103beb..f2f7c2c546 100644
--- a/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
+++ b/src/main/java/com/google/devtools/build/lib/syntax/Parser.java
@@ -612,10 +612,13 @@ class Parser {
 
     try {
       Label label = Label.parseAbsolute(labelName);
-      String packageName = label.getPackageFragment().getPathString();
-      Path packagePath = locator.getBuildFileForPackage(packageName);
+      // Note that this doesn't really work if the label belongs to a different repository, because
+      // there is no guarantee that its RepositoryValue has been evaluated. In an ideal world, we
+      // could put a Skyframe dependency the appropriate PackageLookupValue, but we can't do that
+      // because package loading is not completely Skyframized.
+      Path packagePath = locator.getBuildFileForPackage(label.getPackageIdentifier());
       if (packagePath == null) {
-        reportError(location, "Package '" + packageName + "' not found");
+        reportError(location, "Package '" + label.getPackageIdentifier() + "' not found");
         list.add(mocksubincludeExpression(labelName, "", location));
         return;
       }