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
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java
index 1d90220..a21c6f5 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/PrepareDepsOfPatternsFunctionTest.java
@@ -59,11 +59,11 @@
// When PrepareDepsOfPatternsFunction successfully completes evaluation,
WalkableGraph walkableGraph = getGraphFromPatternsEvaluation(patternSequence);
- // Then the graph contains a value for the target "//foo:foo",
- assertValidValue(walkableGraph, getKeyForLabel(Label.create("foo", "foo")));
+ // Then the graph contains a value for the target "@//foo:foo",
+ assertValidValue(walkableGraph, getKeyForLabel(Label.create("@//foo", "foo")));
- // And the graph does not contain a value for the target "//foo:foo2".
- assertFalse(walkableGraph.exists(getKeyForLabel(Label.create("foo", "foo2"))));
+ // And the graph does not contain a value for the target "@//foo:foo2".
+ assertFalse(walkableGraph.exists(getKeyForLabel(Label.create("@//foo", "foo2"))));
}
@Test
@@ -79,12 +79,12 @@
WalkableGraph walkableGraph = getGraphFromPatternsEvaluation(patternSequence);
// Then the graph contains an entry for ":foo"'s dependency, ":foo2".
- assertValidValue(walkableGraph, getKeyForLabel(Label.create("foo", "foo2")));
+ assertValidValue(walkableGraph, getKeyForLabel(Label.create("@//foo", "foo2")));
}
@Test
public void testFunctionExpandsTargetPatterns() throws Exception {
- // Given a package "//foo" with independent target rules ":foo" and ":foo2",
+ // Given a package "@//foo" with independent target rules ":foo" and ":foo2",
createFooAndFoo2(/*dependent=*/ false);
// Given a target pattern sequence consisting of a pattern for "//foo:*",
@@ -94,8 +94,8 @@
WalkableGraph walkableGraph = getGraphFromPatternsEvaluation(patternSequence);
// Then the graph contains an entry for ":foo" and ":foo2".
- assertValidValue(walkableGraph, getKeyForLabel(Label.create("foo", "foo")));
- assertValidValue(walkableGraph, getKeyForLabel(Label.create("foo", "foo2")));
+ assertValidValue(walkableGraph, getKeyForLabel(Label.create("@//foo", "foo")));
+ assertValidValue(walkableGraph, getKeyForLabel(Label.create("@//foo", "foo2")));
}
@Test
@@ -108,7 +108,7 @@
WalkableGraph walkableGraph = getGraphFromPatternsEvaluation(patternSequence);
// Then the graph does not contain an entry for ":foo",
- assertFalse(walkableGraph.exists(getKeyForLabel(Label.create("foo", "foo"))));
+ assertFalse(walkableGraph.exists(getKeyForLabel(Label.create("@//foo", "foo"))));
}
@Test
@@ -126,11 +126,11 @@
// Then the graph contains an entry for ":foo",
assertValidValue(
walkableGraph,
- getKeyForLabel(Label.create("foo", "foo")),
+ getKeyForLabel(Label.create("@//foo", "foo")),
/*expectTransitiveException=*/ true);
// And an entry with a NoSuchPackageException for "//bar:bar",
- Exception e = assertException(walkableGraph, getKeyForLabel(Label.create("bar", "bar")));
+ Exception e = assertException(walkableGraph, getKeyForLabel(Label.create("@//bar", "bar")));
assertThat(e).isInstanceOf(NoSuchPackageException.class);
}
@@ -149,11 +149,11 @@
// Then the graph contains an entry for ":foo" which has both a value and an exception,
assertValidValue(
walkableGraph,
- getKeyForLabel(Label.create("foo", "foo")),
+ getKeyForLabel(Label.create("@//foo", "foo")),
/*expectTransitiveException=*/ true);
// And an entry with a NoSuchTargetException for "//bar:bar",
- Exception e = assertException(walkableGraph, getKeyForLabel(Label.create("bar", "bar")));
+ Exception e = assertException(walkableGraph, getKeyForLabel(Label.create("@//bar", "bar")));
assertThat(e).isInstanceOf(NoSuchTargetException.class);
}
@@ -189,8 +189,8 @@
// Then it skips evaluation of the malformed target pattern, but logs about it,
assertContainsEvent("Skipping '" + bogusPattern + "': ");
- // And then the graph contains a value for the legit target pattern's target "//foo:foo".
- assertTrue(walkableGraph.exists(getKeyForLabel(Label.create("foo", "foo"))));
+ // And then the graph contains a value for the legit target pattern's target "@//foo:foo".
+ assertTrue(walkableGraph.exists(getKeyForLabel(Label.create("@//foo", "foo"))));
}
// Helpers:
