Add new skyframe function to lookup the repository given a path, and use that

to report invalid package references. Fixes #1592.

--
MOS_MIGRATED_REVID=137164164

7 files changed, 345 insertions, 15 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
index ea1fc818db..fa5f891cff 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/ContainingPackageLookupFunctionTest.java
@@ -77,6 +77,11 @@ public class ContainingPackageLookupFunctionTest extends FoundationTestCase {
     skyFunctions.put(SkyFunctions.FILE_STATE, new FileStateFunction(
         new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper));
     skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator));
+    skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction());
+    skyFunctions.put(
+        SkyFunctions.DIRECTORY_LISTING_STATE,
+        new DirectoryListingStateFunction(externalFilesHelper));
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
     RecordingDifferencer differencer = new RecordingDifferencer();
     evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
     driver = new SequentialBuildDriver(evaluator);
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
index 131dbbbc95..6de849a0cd 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FileFunctionTest.java
@@ -157,6 +157,7 @@ public class FileFunctionTest {
                             TestRuleClassProvider.getRuleClassProvider(), fs),
                         directories))
                 .put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction())
+                .put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction())
                 .build(),
             differencer);
     PrecomputedValue.BUILD_ID.set(differencer, UUID.randomUUID());
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
index 3b66ae153f..e030cefd3d 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/FilesetEntryFunctionTest.java
@@ -105,6 +105,7 @@ public final class FilesetEntryFunctionTest extends FoundationTestCase {
     skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES,
         new BlacklistedPackagePrefixesFunction());
     skyFunctions.put(SkyFunctions.FILESET_ENTRY, new FilesetEntryFunction());
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
 
     differencer = new RecordingDifferencer();
     evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
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 020934314f..4ee3a8efda 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
@@ -144,6 +144,11 @@ public abstract class GlobFunctionTest {
         new FileStateFunction(
             new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper));
     skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator));
+    skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction());
+    skyFunctions.put(
+        SkyFunctions.DIRECTORY_LISTING_STATE,
+        new DirectoryListingStateFunction(externalFilesHelper));
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
     return skyFunctions;
   }
 
@@ -692,7 +697,7 @@ public abstract class GlobFunctionTest {
     assertGlobMatches("symlinks/*.txt", "symlinks/existing.txt");
   }
 
-  private class CustomInMemoryFs extends InMemoryFileSystem {
+  private static final class CustomInMemoryFs extends InMemoryFileSystem {
 
     private Map<Path, FileStatus> stubbedStats = Maps.newHashMap();
 
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java
new file mode 100644
index 0000000000..d4b51bbe15
--- /dev/null
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/LocalRepositoryLookupFunctionTest.java
@@ -0,0 +1,235 @@
+// Copyright 2016 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.skyframe;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
+import com.google.devtools.build.lib.analysis.BlazeDirectories;
+import com.google.devtools.build.lib.analysis.util.AnalysisMock;
+import com.google.devtools.build.lib.bazel.rules.BazelRulesModule;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
+import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
+import com.google.devtools.build.lib.testutil.FoundationTestCase;
+import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
+import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.skyframe.EvaluationResult;
+import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
+import com.google.devtools.build.skyframe.MemoizingEvaluator;
+import com.google.devtools.build.skyframe.RecordingDifferencer;
+import com.google.devtools.build.skyframe.SequentialBuildDriver;
+import com.google.devtools.build.skyframe.SkyFunction;
+import com.google.devtools.build.skyframe.SkyFunctionName;
+import com.google.devtools.build.skyframe.SkyKey;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicReference;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+/** Tests for {@link LocalRepositoryLookupFunction}. */
+@RunWith(JUnit4.class)
+public class LocalRepositoryLookupFunctionTest extends FoundationTestCase {
+  private AtomicReference<ImmutableSet<PackageIdentifier>> deletedPackages;
+  private MemoizingEvaluator evaluator;
+  private SequentialBuildDriver driver;
+  private RecordingDifferencer differencer;
+
+  @Before
+  public final void setUp() throws Exception {
+    AnalysisMock analysisMock = AnalysisMock.get();
+    AtomicReference<PathPackageLocator> pkgLocator =
+        new AtomicReference<>(new PathPackageLocator(outputBase, ImmutableList.of(rootDirectory)));
+    deletedPackages = new AtomicReference<>(ImmutableSet.<PackageIdentifier>of());
+    BlazeDirectories directories =
+        new BlazeDirectories(
+            rootDirectory, outputBase, rootDirectory, analysisMock.getProductName());
+    ExternalFilesHelper externalFilesHelper = new ExternalFilesHelper(
+        pkgLocator, ExternalFileAction.DEPEND_ON_EXTERNAL_PKG_FOR_EXTERNAL_REPO_PATHS, directories);
+
+    Map<SkyFunctionName, SkyFunction> skyFunctions = new HashMap<>();
+    skyFunctions.put(
+        SkyFunctions.PACKAGE_LOOKUP,
+        new PackageLookupFunction(deletedPackages, CrossRepositoryLabelViolationStrategy.ERROR));
+    skyFunctions.put(
+        SkyFunctions.FILE_STATE,
+        new FileStateFunction(
+            new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper));
+    skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator));
+    skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction());
+    skyFunctions.put(
+        SkyFunctions.DIRECTORY_LISTING_STATE,
+        new DirectoryListingStateFunction(externalFilesHelper));
+    RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider();
+    skyFunctions.put(SkyFunctions.WORKSPACE_AST, new WorkspaceASTFunction(ruleClassProvider));
+    skyFunctions.put(
+        SkyFunctions.WORKSPACE_FILE,
+        new WorkspaceFileFunction(
+            ruleClassProvider,
+            analysisMock
+                .getPackageFactoryForTesting()
+                .create(
+                    ruleClassProvider,
+                    new BazelRulesModule().getPackageEnvironmentExtension(),
+                    scratch.getFileSystem()),
+            directories));
+    skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
+    skyFunctions.put(
+        SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS, new FileSymlinkCycleUniquenessFunction());
+
+    differencer = new RecordingDifferencer();
+    evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
+    driver = new SequentialBuildDriver(evaluator);
+    PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
+  }
+
+  private SkyKey createKey(RootedPath directory) {
+    return LocalRepositoryLookupValue.key(directory);
+  }
+
+  private LocalRepositoryLookupValue lookupDirectory(RootedPath directory)
+      throws InterruptedException {
+    SkyKey key = createKey(directory);
+    return lookupDirectory(key).get(key);
+  }
+
+  private EvaluationResult<LocalRepositoryLookupValue> lookupDirectory(SkyKey directoryKey)
+      throws InterruptedException {
+    return driver.<LocalRepositoryLookupValue>evaluate(
+        ImmutableList.of(directoryKey),
+        false,
+        SkyframeExecutor.DEFAULT_THREAD_COUNT,
+        NullEventHandler.INSTANCE);
+  }
+
+  @Test
+  public void testNoPath() throws Exception {
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(RootedPath.toRootedPath(rootDirectory, PathFragment.EMPTY_FRAGMENT));
+    assertThat(repositoryLookupValue).isNotNull();
+    assertThat(repositoryLookupValue.getRepository()).isEqualTo(RepositoryName.MAIN);
+  }
+
+  @Test
+  public void testActualPackage() throws Exception {
+    scratch.file("some/path/BUILD");
+
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(RootedPath.toRootedPath(rootDirectory, new PathFragment("some/path")));
+    assertThat(repositoryLookupValue).isNotNull();
+    assertThat(repositoryLookupValue.getRepository()).isEqualTo(RepositoryName.MAIN);
+  }
+
+  @Test
+  public void testLocalRepository() throws Exception {
+    scratch.overwriteFile("WORKSPACE", "local_repository(name='local', path='local/repo')");
+    scratch.file("local/repo/WORKSPACE");
+    scratch.file("local/repo/BUILD");
+
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(RootedPath.toRootedPath(rootDirectory, new PathFragment("local/repo")));
+    assertThat(repositoryLookupValue).isNotNull();
+    assertThat(repositoryLookupValue.getRepository().getName()).isEqualTo("@local");
+  }
+
+  @Test
+  public void testLocalRepositorySubPackage() throws Exception {
+    scratch.overwriteFile("WORKSPACE", "local_repository(name='local', path='local/repo')");
+    scratch.file("local/repo/WORKSPACE");
+    scratch.file("local/repo/BUILD");
+    scratch.file("local/repo/sub/package/BUILD");
+
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(
+            RootedPath.toRootedPath(rootDirectory, new PathFragment("local/repo/sub/package")));
+    assertThat(repositoryLookupValue).isNotNull();
+    assertThat(repositoryLookupValue.getRepository().getName()).isEqualTo("@local");
+  }
+
+  @Test
+  public void testWorkspaceButNoLocalRepository() throws Exception {
+    scratch.overwriteFile("WORKSPACE", "");
+    scratch.file("local/repo/WORKSPACE");
+    scratch.file("local/repo/BUILD");
+
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(RootedPath.toRootedPath(rootDirectory, new PathFragment("local/repo")));
+    assertThat(repositoryLookupValue).isNotNull();
+    assertThat(repositoryLookupValue.getRepository()).isEqualTo(RepositoryName.MAIN);
+  }
+
+  @Test
+  public void testLocalRepository_LocalWorkspace_SymlinkCycle() throws Exception {
+    scratch.overwriteFile("WORKSPACE", "local_repository(name='local', path='local/repo')");
+    Path localRepoWorkspace = scratch.resolve("local/repo/WORKSPACE");
+    Path localRepoWorkspaceLink = scratch.resolve("local/repo/WORKSPACE.link");
+    FileSystemUtils.createDirectoryAndParents(localRepoWorkspace.getParentDirectory());
+    FileSystemUtils.createDirectoryAndParents(localRepoWorkspaceLink.getParentDirectory());
+    localRepoWorkspace.createSymbolicLink(localRepoWorkspaceLink);
+    localRepoWorkspaceLink.createSymbolicLink(localRepoWorkspace);
+    scratch.file("local/repo/BUILD");
+
+    SkyKey localRepositoryKey =
+        createKey(RootedPath.toRootedPath(rootDirectory, new PathFragment("local/repo")));
+    EvaluationResult<LocalRepositoryLookupValue> result = lookupDirectory(localRepositoryKey);
+
+    assertThatEvaluationResult(result)
+        .hasErrorEntryForKeyThat(localRepositoryKey)
+        .hasExceptionThat()
+        .hasMessage(
+            "FileSymlinkException while checking if there is a WORKSPACE file in "
+                + "/workspace/local/repo");
+  }
+
+  @Test
+  public void testLocalRepository_MainWorkspace_NotFound() throws Exception {
+    // Do not add a local_repository to WORKSPACE.
+    scratch.overwriteFile("WORKSPACE", "");
+    scratch.deleteFile("WORKSPACE");
+    scratch.file("local/repo/WORKSPACE");
+    scratch.file("local/repo/BUILD");
+
+    LocalRepositoryLookupValue repositoryLookupValue =
+        lookupDirectory(RootedPath.toRootedPath(rootDirectory, new PathFragment("local/repo")));
+    assertThat(repositoryLookupValue).isNotNull();
+    // In this case, the repository should be MAIN as we can't find any local_repository rules.
+    assertThat(repositoryLookupValue.getRepository()).isEqualTo(RepositoryName.MAIN);
+  }
+
+  // TODO(katre): Add tests for the following exceptions
+  // While reading dir/WORKSPACE:
+  // - IOException
+  // - FileSymlinkException
+  // - InconsistentFilesystemException
+  // While loading //external
+  // - BuildFileNotFoundException
+  // - InconsistentFilesystemException
+  // While reading //external:WORKSPACE
+  // - PackageFunctionException
+  // - NameConflictException
+  // - WorkspaceFileException
+}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
index 40bebb74aa..a8ead28268 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PackageLookupFunctionTest.java
@@ -14,12 +14,14 @@
 
 package com.google.devtools.build.lib.skyframe;
 
+import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.testing.EqualsTester;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
@@ -27,17 +29,25 @@ import com.google.devtools.build.lib.analysis.util.AnalysisMock;
 import com.google.devtools.build.lib.bazel.rules.BazelRulesModule;
 import com.google.devtools.build.lib.cmdline.PackageIdentifier;
 import com.google.devtools.build.lib.events.NullEventHandler;
+import com.google.devtools.build.lib.packages.BuildFileNotFoundException;
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
+import com.google.devtools.build.lib.rules.repository.LocalRepositoryFunction;
+import com.google.devtools.build.lib.rules.repository.LocalRepositoryRule;
+import com.google.devtools.build.lib.rules.repository.RepositoryDelegatorFunction;
+import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
+import com.google.devtools.build.lib.rules.repository.RepositoryLoaderFunction;
 import com.google.devtools.build.lib.skyframe.ExternalFilesHelper.ExternalFileAction;
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction.CrossRepositoryLabelViolationStrategy;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue.BuildFileName;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue.ErrorReason;
 import com.google.devtools.build.lib.testutil.FoundationTestCase;
 import com.google.devtools.build.lib.util.io.TimestampGranularityMonitor;
+import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.google.devtools.build.lib.vfs.RootedPath;
+import com.google.devtools.build.skyframe.EvaluationResult;
 import com.google.devtools.build.skyframe.InMemoryMemoizingEvaluator;
 import com.google.devtools.build.skyframe.MemoizingEvaluator;
 import com.google.devtools.build.skyframe.RecordingDifferencer;
@@ -48,6 +58,7 @@ import com.google.devtools.build.skyframe.SkyKey;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.UUID;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Before;
 import org.junit.Test;
@@ -88,6 +99,10 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
     skyFunctions.put(SkyFunctions.FILE_STATE, new FileStateFunction(
         new AtomicReference<TimestampGranularityMonitor>(), externalFilesHelper));
     skyFunctions.put(SkyFunctions.FILE, new FileFunction(pkgLocator));
+    skyFunctions.put(SkyFunctions.DIRECTORY_LISTING, new DirectoryListingFunction());
+    skyFunctions.put(
+        SkyFunctions.DIRECTORY_LISTING_STATE,
+        new DirectoryListingStateFunction(externalFilesHelper));
     skyFunctions.put(SkyFunctions.BLACKLISTED_PACKAGE_PREFIXES,
         new BlacklistedPackagePrefixesFunction());
     RuleClassProvider ruleClassProvider = analysisMock.createRuleClassProvider();
@@ -104,6 +119,17 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
                     scratch.getFileSystem()),
             directories));
     skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
+    skyFunctions.put(
+        SkyFunctions.FILE_SYMLINK_CYCLE_UNIQUENESS, new FileSymlinkCycleUniquenessFunction());
+
+    ImmutableMap<String, RepositoryFunction> repositoryHandlers =
+        ImmutableMap.of(LocalRepositoryRule.NAME, new LocalRepositoryFunction());
+    skyFunctions.put(
+        SkyFunctions.REPOSITORY_DIRECTORY,
+        new RepositoryDelegatorFunction(repositoryHandlers, null, new AtomicBoolean(true)));
+    skyFunctions.put(SkyFunctions.REPOSITORY, new RepositoryLoaderFunction());
+
     differencer = new RecordingDifferencer();
     evaluator = new InMemoryMemoizingEvaluator(skyFunctions, differencer);
     driver = new SequentialBuildDriver(evaluator);
@@ -111,18 +137,26 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
     PrecomputedValue.PATH_PACKAGE_LOCATOR.set(differencer, pkgLocator.get());
     PrecomputedValue.BLACKLISTED_PACKAGE_PREFIXES_FILE.set(
         differencer, PathFragment.EMPTY_FRAGMENT);
+    PrecomputedValue.BLAZE_DIRECTORIES.set(differencer, directories);
   }
 
-  private PackageLookupValue lookupPackage(String packageName) throws InterruptedException {
+  protected PackageLookupValue lookupPackage(String packageName) throws InterruptedException {
     return lookupPackage(PackageIdentifier.createInMainRepo(packageName));
   }
 
-  private PackageLookupValue lookupPackage(PackageIdentifier packageId)
+  protected PackageLookupValue lookupPackage(PackageIdentifier packageId)
       throws InterruptedException {
     SkyKey key = PackageLookupValue.key(packageId);
+    return lookupPackage(key).get(key);
+  }
+
+  protected EvaluationResult<PackageLookupValue> lookupPackage(SkyKey packageIdentifierSkyKey)
+      throws InterruptedException {
     return driver.<PackageLookupValue>evaluate(
-        ImmutableList.of(key), false, SkyframeExecutor.DEFAULT_THREAD_COUNT,
-        NullEventHandler.INSTANCE).get(key);
+        ImmutableList.of(packageIdentifierSkyKey),
+        false,
+        SkyframeExecutor.DEFAULT_THREAD_COUNT,
+        NullEventHandler.INSTANCE);
   }
 
   @Test
@@ -256,35 +290,83 @@ public abstract class PackageLookupFunctionTest extends FoundationTestCase {
         .testEquals();
   }
 
+  protected void createAndCheckInvalidPackageLabel(boolean expectedPackageExists) throws Exception {
+    scratch.overwriteFile("WORKSPACE", "local_repository(name='local', path='local/repo')");
+    scratch.file("local/repo/WORKSPACE");
+    scratch.file("local/repo/BUILD");
+
+    // First, use the correct label.
+    PackageLookupValue packageLookupValue =
+        lookupPackage(PackageIdentifier.create("@local", PathFragment.EMPTY_FRAGMENT));
+    assertTrue(packageLookupValue.packageExists());
+
+    // Then, use the incorrect label.
+    packageLookupValue = lookupPackage(PackageIdentifier.createInMainRepo("local/repo"));
+    assertEquals(expectedPackageExists, packageLookupValue.packageExists());
+  }
+
   /**
-   * Runs all tests in the base {@link PackageLookupFunctionTest} class with the
-   * {@link CrossRepositoryLabelViolationStrategy#IGNORE} enum set, and also additional tests
-   * specific to that setting.
+   * Runs all tests in the base {@link PackageLookupFunctionTest} class with the {@link
+   * CrossRepositoryLabelViolationStrategy#IGNORE} enum set, and also additional tests specific to
+   * that setting.
    */
   @RunWith(JUnit4.class)
-  public static class IgnoreLabelViolationsTest
-      extends PackageLookupFunctionTest {
+  public static class IgnoreLabelViolationsTest extends PackageLookupFunctionTest {
     @Override
     protected CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy() {
       return CrossRepositoryLabelViolationStrategy.IGNORE;
     }
 
     // Add any ignore-specific tests here.
+
+    @Test
+    public void testInvalidPackageLabelIsIgnored() throws Exception {
+      createAndCheckInvalidPackageLabel(true);
+    }
   }
 
   /**
-   * Runs all tests in the base {@link PackageLookupFunctionTest} class with the
-   * {@link CrossRepositoryLabelViolationStrategy#ERROR} enum set, and also additional tests
-   * specific to that setting.
+   * Runs all tests in the base {@link PackageLookupFunctionTest} class with the {@link
+   * CrossRepositoryLabelViolationStrategy#ERROR} enum set, and also additional tests specific to
+   * that setting.
    */
   @RunWith(JUnit4.class)
-  public static class ErrorLabelViolationsTest
-      extends PackageLookupFunctionTest {
+  public static class ErrorLabelViolationsTest extends PackageLookupFunctionTest {
     @Override
     protected CrossRepositoryLabelViolationStrategy crossRepositoryLabelViolationStrategy() {
       return CrossRepositoryLabelViolationStrategy.ERROR;
     }
 
     // Add any error-specific tests here.
+
+    @Test
+    public void testInvalidPackageLabelIsError() throws Exception {
+      createAndCheckInvalidPackageLabel(false);
+    }
+
+    @Test
+    public void testSymlinkCycleInWorkspace() throws Exception {
+      scratch.overwriteFile("WORKSPACE", "local_repository(name='local', path='local/repo')");
+      Path localRepoWorkspace = scratch.resolve("local/repo/WORKSPACE");
+      Path localRepoWorkspaceLink = scratch.resolve("local/repo/WORKSPACE.link");
+      FileSystemUtils.createDirectoryAndParents(localRepoWorkspace.getParentDirectory());
+      FileSystemUtils.createDirectoryAndParents(localRepoWorkspaceLink.getParentDirectory());
+      localRepoWorkspace.createSymbolicLink(localRepoWorkspaceLink);
+      localRepoWorkspaceLink.createSymbolicLink(localRepoWorkspace);
+      scratch.file("local/repo/BUILD");
+
+      SkyKey skyKey = PackageLookupValue.key(PackageIdentifier.createInMainRepo("local/repo"));
+      EvaluationResult<PackageLookupValue> result = lookupPackage(skyKey);
+      assertThatEvaluationResult(result)
+          .hasErrorEntryForKeyThat(skyKey)
+          .hasExceptionThat()
+          .isInstanceOf(BuildFileNotFoundException.class);
+      assertThatEvaluationResult(result)
+          .hasErrorEntryForKeyThat(skyKey)
+          .hasExceptionThat()
+          .hasMessage(
+              "no such package 'local/repo': Unable to determine the local repository for "
+                  + "directory /workspace/local/repo");
+    }
   }
 }
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
index b769b6dd0a..58b31e7d84 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/RecursiveFilesystemTraversalFunctionTest.java
@@ -120,6 +120,7 @@ public final class RecursiveFilesystemTraversalFunctionTest extends FoundationTe
                 .create(ruleClassProvider, scratch.getFileSystem()),
             directories));
     skyFunctions.put(SkyFunctions.EXTERNAL_PACKAGE, new ExternalPackageFunction());
+    skyFunctions.put(SkyFunctions.LOCAL_REPOSITORY_LOOKUP, new LocalRepositoryLookupFunction());
 
     progressReceiver = new RecordingEvaluationProgressReceiver();
     differencer = new RecordingDifferencer();