Don't keep packages in the default repository around after loading.

Previously, this would get thrown when referring to the same package
from both the main and default repositories:
java.lang.IllegalArgumentException: Multiple entries with same key: tools/cpp=/home/brian/971-Robot-Code and tools/cpp=/home/brian/971-Robot-Code
	at com.google.common.collect.ImmutableMap.checkNoConflict(ImmutableMap.java:136)
	at com.google.common.collect.RegularImmutableMap.checkNoConflictInKeyBucket(RegularImmutableMap.java:98)
	at com.google.common.collect.RegularImmutableMap.fromEntryArray(RegularImmutableMap.java:84)
	at com.google.common.collect.ImmutableMap$Builder.build(ImmutableMap.java:295)
	at com.google.devtools.build.lib.buildtool.BuildTool.transformPackageRoots(BuildTool.java:301)
	at com.google.devtools.build.lib.buildtool.BuildTool.buildTargets(BuildTool.java:209)
	at com.google.devtools.build.lib.buildtool.BuildTool.processRequest(BuildTool.java:334)
	at com.google.devtools.build.lib.runtime.commands.TestCommand.doTest(TestCommand.java:119)
	at com.google.devtools.build.lib.runtime.commands.TestCommand.exec(TestCommand.java:104)
	at com.google.devtools.build.lib.runtime.BlazeCommandDispatcher.exec(BlazeCommandDispatcher.java:371)
	at com.google.devtools.build.lib.runtime.BlazeRuntime$3.exec(BlazeRuntime.java:1016)
	at com.google.devtools.build.lib.server.RPCService.executeRequest(RPCService.java:65)
	at com.google.devtools.build.lib.server.RPCServer.executeRequest(RPCServer.java:434)
	at com.google.devtools.build.lib.server.RPCServer.serve(RPCServer.java:229)
	at com.google.devtools.build.lib.runtime.BlazeRuntime.serverMain(BlazeRuntime.java:975)
	at com.google.devtools.build.lib.runtime.BlazeRuntime.main(BlazeRuntime.java:772)
	at com.google.devtools.build.lib.bazel.BazelMain.main(BazelMain.java:55)

And this would get thrown for any packages in the main repository loaded
from other repositories:
java.lang.RuntimeException: Unrecoverable error while evaluating node 'PACKAGE:@//tools/build_rules/go/toolchain' (requested by nodes )
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:982)
	at com.google.devtools.build.lib.concurrent.AbstractQueueVisitor$WrappedRunnable.run(AbstractQueueVisitor.java:499)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
	at java.lang.Thread.run(Thread.java:745)
Caused by: java.lang.IllegalArgumentException: Invalid BUILD file name for package '@//tools/build_rules/go/toolchain': /home/brian/bazel/tools/build_rules/go/toolchain/BUILD
	at com.google.devtools.build.lib.packages.Package.finishInit(Package.java:299)
	at com.google.devtools.build.lib.packages.Package$Builder.finishBuild(Package.java:1308)
	at com.google.devtools.build.lib.skyframe.PackageFunction.compute(PackageFunction.java:501)
	at com.google.devtools.build.skyframe.ParallelEvaluator$Evaluate.run(ParallelEvaluator.java:933)
	... 4 more

Sponsor's comment: note the abundance of new Label.resolveRepositoryRelative() calls. They are ugly, but it's only making existing ugliness explicit. Yes, we should fix it, especially in the implementation of configurable attributes.

Refs #940

--
Change-Id: I8bd7f7b00bec58a7157507595421bc50c81b404c
Reviewed-on: https://bazel-review.googlesource.com/#/c/2591
MOS_MIGRATED_REVID=117429733

1 files changed, 11 insertions, 11 deletions

diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunctionTest.java
index 4915f3ff9f..207465cda8 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfTargetsUnderDirectoryFunctionTest.java
@@ -59,7 +59,7 @@ public class PrepareDepsOfTargetsUnderDirectoryFunctionTest extends BuildViewTes
       Path root, PathFragment rootRelativePath, ImmutableSet<PathFragment> excludedPaths) {
     RootedPath rootedPath = RootedPath.toRootedPath(root, rootRelativePath);
     return CollectPackagesUnderDirectoryValue.key(
-        PackageIdentifier.DEFAULT_REPOSITORY_NAME, rootedPath, excludedPaths);
+        PackageIdentifier.MAIN_REPOSITORY_NAME, rootedPath, excludedPaths);
   }
 
   private SkyKey createPrepDepsKey(Path root, PathFragment rootRelativePath) {
@@ -70,14 +70,14 @@ public class PrepareDepsOfTargetsUnderDirectoryFunctionTest extends BuildViewTes
       ImmutableSet<PathFragment> excludedPaths) {
     RootedPath rootedPath = RootedPath.toRootedPath(root, rootRelativePath);
     return PrepareDepsOfTargetsUnderDirectoryValue.key(
-        PackageIdentifier.DEFAULT_REPOSITORY_NAME, rootedPath, excludedPaths);
+        PackageIdentifier.MAIN_REPOSITORY_NAME, rootedPath, excludedPaths);
   }
 
   private SkyKey createPrepDepsKey(Path root, PathFragment rootRelativePath,
       ImmutableSet<PathFragment> excludedPaths, FilteringPolicy filteringPolicy) {
     RootedPath rootedPath = RootedPath.toRootedPath(root, rootRelativePath);
     return PrepareDepsOfTargetsUnderDirectoryValue.key(
-        PackageIdentifier.DEFAULT_REPOSITORY_NAME, rootedPath, excludedPaths, filteringPolicy);
+        PackageIdentifier.MAIN_REPOSITORY_NAME, rootedPath, excludedPaths, filteringPolicy);
   }
 
   private EvaluationResult<?> getEvaluationResult(SkyKey... keys) throws InterruptedException {
@@ -102,14 +102,14 @@ public class PrepareDepsOfTargetsUnderDirectoryFunctionTest extends BuildViewTes
     EvaluationResult<?> evaluationResult = getEvaluationResult(key);
     WalkableGraph graph = Preconditions.checkNotNull(evaluationResult.getWalkableGraph());
 
-    // Then the TransitiveTraversalValue for "a:a" is evaluated,
-    SkyKey aaKey = TransitiveTraversalValue.key(Label.create("a", "a"));
+    // Then the TransitiveTraversalValue for "@//a:a" is evaluated,
+    SkyKey aaKey = TransitiveTraversalValue.key(Label.create("@//a", "a"));
     assertThat(graph.exists(aaKey)).isTrue();
 
-    // And that TransitiveTraversalValue depends on "b:b.txt".
+    // And that TransitiveTraversalValue depends on "@//b:b.txt".
     Iterable<SkyKey> depsOfAa =
         Iterables.getOnlyElement(graph.getDirectDeps(ImmutableList.of(aaKey)).values());
-    SkyKey bTxtKey = TransitiveTraversalValue.key(Label.create("b", "b.txt"));
+    SkyKey bTxtKey = TransitiveTraversalValue.key(Label.create("@//b", "b.txt"));
     assertThat(depsOfAa).contains(bTxtKey);
 
     // And the TransitiveTraversalValue for "b:b.txt" is evaluated.
@@ -128,12 +128,12 @@ public class PrepareDepsOfTargetsUnderDirectoryFunctionTest extends BuildViewTes
     EvaluationResult<?> evaluationResult = getEvaluationResult(key);
     WalkableGraph graph = Preconditions.checkNotNull(evaluationResult.getWalkableGraph());
 
-    // Then the TransitiveTraversalValue for "a:a" is not evaluated,
-    SkyKey aaKey = TransitiveTraversalValue.key(Label.create("a", "a"));
+    // Then the TransitiveTraversalValue for "@//a:a" is not evaluated,
+    SkyKey aaKey = TransitiveTraversalValue.key(Label.create("@//a", "a"));
     assertThat(graph.exists(aaKey)).isFalse();
 
-    // But the TransitiveTraversalValue for "a:aTest" is.
-    SkyKey aaTestKey = TransitiveTraversalValue.key(Label.create("a", "aTest"));
+    // But the TransitiveTraversalValue for "@//a:aTest" is.
+    SkyKey aaTestKey = TransitiveTraversalValue.key(Label.create("@//a", "aTest"));
     assertThat(graph.exists(aaTestKey)).isTrue();
   }