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/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
index 993113a..96a20fb 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AggregatingAttributeMapper.java
@@ -114,7 +114,7 @@
for (Object value : visitAttribute(attribute.getName(), attribute.getType())) {
if (value != null) {
for (Label label : extractLabels(type, value)) {
- observer.acceptLabelAttribute(label, attribute);
+ observer.acceptLabelAttribute(getLabel().resolveRepositoryRelative(label), attribute);
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
index 875d9dc..9d2af18 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/AspectDefinition.java
@@ -29,7 +29,6 @@
import java.util.Collection;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
-import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -196,13 +195,15 @@
continue;
}
if (aspectAttribute.getType() == BuildType.LABEL) {
- Label label = BuildType.LABEL.cast(aspectAttribute.getDefaultValue(from));
+ Label label = from.getLabel().resolveRepositoryRelative(
+ BuildType.LABEL.cast(aspectAttribute.getDefaultValue(from)));
if (label != null) {
labelBuilder.put(aspectAttribute, label);
}
} else if (aspectAttribute.getType() == BuildType.LABEL_LIST) {
- List<Label> labelList = BuildType.LABEL_LIST.cast(aspectAttribute.getDefaultValue(from));
- labelBuilder.putAll(aspectAttribute, labelList);
+ for (Label label : BuildType.LABEL_LIST.cast(aspectAttribute.getDefaultValue(from))) {
+ labelBuilder.put(aspectAttribute, from.getLabel().resolveRepositoryRelative(label));
+ }
}
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 9181d76..d2f7090 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -699,7 +699,7 @@
inputFile.getName()));
}
if (license == null && pkgBuilder.getDefaultLicense() == License.NO_LICENSE
- && pkgBuilder.getBuildFileLabel().toString().startsWith("//third_party/")) {
+ && RuleClass.isThirdPartyPackage(pkgBuilder.getPackageIdentifier())) {
throw new EvalException(ast.getLocation(),
"third-party file '" + inputFile.getName() + "' lacks a license declaration "
+ "with one of the following types: notice, reciprocal, permissive, "
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RelativePackageNameResolver.java b/src/main/java/com/google/devtools/build/lib/packages/RelativePackageNameResolver.java
index f7e3aed..144dc89 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RelativePackageNameResolver.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RelativePackageNameResolver.java
@@ -58,7 +58,7 @@
relativePkg = pkg.substring(2);
} else if (pkg.startsWith("/")) {
throw new InvalidPackageNameException(
- PackageIdentifier.createInDefaultRepo(pkg),
+ PackageIdentifier.createInMainRepo(pkg),
"package name cannot start with a single slash");
} else {
isAbsolute = false;
@@ -75,7 +75,7 @@
result = result.normalize();
if (result.containsUplevelReferences()) {
throw new InvalidPackageNameException(
- PackageIdentifier.createInDefaultRepo(pkg),
+ PackageIdentifier.createInMainRepo(pkg),
"package name contains too many '..' segments");
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
index 8064936..9accac3 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClass.java
@@ -30,6 +30,7 @@
import com.google.common.collect.Ordering;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.events.Location;
import com.google.devtools.build.lib.packages.BuildType.SelectorList;
@@ -103,6 +104,9 @@
public final class RuleClass {
public static final Function<? super Rule, Map<String, Label>> NO_EXTERNAL_BINDINGS =
Functions.<Map<String, Label>>constant(ImmutableMap.<String, Label>of());
+
+ private static final PathFragment THIRD_PARTY_PREFIX = new PathFragment("third_party");
+
/**
* A constraint for the package name of the Rule instances.
*/
@@ -1540,7 +1544,7 @@
*/
private static void checkThirdPartyRuleHasLicense(Rule rule,
Package.Builder pkgBuilder, EventHandler eventHandler) {
- if (rule.getLabel().getPackageName().startsWith("third_party/")) {
+ if (isThirdPartyPackage(rule.getLabel().getPackageIdentifier())) {
License license = rule.getLicense();
if (license == null) {
license = pkgBuilder.getDefaultLicense();
@@ -1778,4 +1782,20 @@
public boolean outputsDefaultExecutable() {
return outputsDefaultExecutable;
}
+
+ public static boolean isThirdPartyPackage(PackageIdentifier packageIdentifier) {
+ if (!packageIdentifier.getRepository().isMain()) {
+ return false;
+ }
+
+ if (!packageIdentifier.getPackageFragment().startsWith(THIRD_PARTY_PREFIX)) {
+ return false;
+ }
+
+ if (packageIdentifier.getPackageFragment().segmentCount() <= 1) {
+ return false;
+ }
+
+ return true;
+ }
}
