Clean up Label validation, and introduce a factory method for constructing a Label without validation.
RELNOTES: None
PiperOrigin-RevId: 164479651
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Package.java b/src/main/java/com/google/devtools/build/lib/packages/Package.java
index ec58068..9e4ab19 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Package.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Package.java
@@ -444,7 +444,7 @@
return events;
}
- /** Returns an (immutable, unordered) view of all the targets belonging to this package. */
+ /** Returns an (immutable, ordered) view of all the targets belonging to this package. */
public ImmutableSortedKeyMap<String, Target> getTargets() {
return targets;
}
@@ -457,7 +457,7 @@
}
/**
- * Returns a (read-only, unordered) iterator of all the targets belonging
+ * Returns a (read-only, ordered) iterable of all the targets belonging
* to this package which are instances of the specified class.
*/
public <T extends Target> Iterable<T> getTargets(Class<T> targetClass) {
@@ -1312,12 +1312,14 @@
// current instance here.
buildFile = (InputFile) Preconditions.checkNotNull(targets.get(buildFileLabel.getName()));
- List<Rule> rules = Lists.newArrayList(getTargets(Rule.class));
+ // The Iterable returned by getTargets is sorted, so when we build up the list of tests by
+ // processing it in order below, that list will be sorted too.
+ Iterable<Rule> sortedRules = Lists.newArrayList(getTargets(Rule.class));
if (discoverAssumedInputFiles) {
// All labels mentioned in a rule that refer to an unknown target in the
// current package are assumed to be InputFiles, so let's create them:
- for (final Rule rule : rules) {
+ for (final Rule rule : sortedRules) {
AggregatingAttributeMapper.of(rule).visitLabels(new AcceptsLabelAttribute() {
@Override
public void acceptLabelAttribute(Label label, Attribute attribute) {
@@ -1332,18 +1334,17 @@
// Note, we implement this here when the Package is fully constructed,
// since clearly this information isn't available at Rule construction
// time, as forward references are permitted.
- List<Label> allTests = new ArrayList<>();
- for (Rule rule : rules) {
+ List<Label> sortedTests = new ArrayList<>();
+ for (Rule rule : sortedRules) {
if (TargetUtils.isTestRule(rule) && !TargetUtils.hasManualTag(rule)) {
- allTests.add(rule.getLabel());
+ sortedTests.add(rule.getLabel());
}
}
- Collections.sort(allTests);
- for (Rule rule : rules) {
+ for (Rule rule : sortedRules) {
AttributeMap attributes = NonconfigurableAttributeMapper.of(rule);
if (rule.getRuleClass().equals("test_suite")
&& attributes.get("tests", BuildType.LABEL_LIST).isEmpty()) {
- rule.setAttributeValueByName("$implicit_tests", allTests);
+ rule.setAttributeValueByName("$implicit_tests", sortedTests);
}
}
return this;
diff --git a/src/main/java/com/google/devtools/build/lib/packages/Rule.java b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
index 1a7cc36d..b751f87 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/Rule.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/Rule.java
@@ -473,13 +473,29 @@
*/
void populateOutputFiles(EventHandler eventHandler, Package.Builder pkgBuilder)
throws LabelSyntaxException, InterruptedException {
+ populateOutputFilesInternal(eventHandler, pkgBuilder, /*performChecks=*/ true);
+ }
+
+ void populateOutputFilesUnchecked(EventHandler eventHandler, Package.Builder pkgBuilder)
+ throws InterruptedException {
+ try {
+ populateOutputFilesInternal(eventHandler, pkgBuilder, /*performChecks=*/ false);
+ } catch (LabelSyntaxException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ void populateOutputFilesInternal(
+ EventHandler eventHandler, Package.Builder pkgBuilder, boolean performChecks)
+ throws LabelSyntaxException, InterruptedException {
Preconditions.checkState(outputFiles == null);
// Order is important here: implicit before explicit
ImmutableList.Builder<OutputFile> outputFilesBuilder = ImmutableList.builder();
ImmutableListMultimap.Builder<String, OutputFile> outputFileMapBuilder =
ImmutableListMultimap.builder();
- populateImplicitOutputFiles(eventHandler, pkgBuilder, outputFilesBuilder);
- populateExplicitOutputFiles(eventHandler, outputFilesBuilder, outputFileMapBuilder);
+ populateImplicitOutputFiles(eventHandler, pkgBuilder, outputFilesBuilder, performChecks);
+ populateExplicitOutputFiles(
+ eventHandler, outputFilesBuilder, outputFileMapBuilder, performChecks);
outputFiles = outputFilesBuilder.build();
outputFileMap = outputFileMapBuilder.build();
}
@@ -488,7 +504,8 @@
private void populateExplicitOutputFiles(
EventHandler eventHandler,
ImmutableList.Builder<OutputFile> outputFilesBuilder,
- ImmutableListMultimap.Builder<String, OutputFile> outputFileMapBuilder)
+ ImmutableListMultimap.Builder<String, OutputFile> outputFileMapBuilder,
+ boolean performChecks)
throws LabelSyntaxException {
NonconfigurableAttributeMapper nonConfigurableAttributes =
NonconfigurableAttributeMapper.of(this);
@@ -499,11 +516,22 @@
Label outputLabel = nonConfigurableAttributes.get(name, BuildType.OUTPUT);
if (outputLabel != null) {
addLabelOutput(
- attribute, outputLabel, eventHandler, outputFilesBuilder, outputFileMapBuilder);
+ attribute,
+ outputLabel,
+ eventHandler,
+ outputFilesBuilder,
+ outputFileMapBuilder,
+ performChecks);
}
} else if (type == BuildType.OUTPUT_LIST) {
for (Label label : nonConfigurableAttributes.get(name, BuildType.OUTPUT_LIST)) {
- addLabelOutput(attribute, label, eventHandler, outputFilesBuilder, outputFileMapBuilder);
+ addLabelOutput(
+ attribute,
+ label,
+ eventHandler,
+ outputFilesBuilder,
+ outputFileMapBuilder,
+ performChecks);
}
}
}
@@ -516,23 +544,31 @@
private void populateImplicitOutputFiles(
EventHandler eventHandler,
Package.Builder pkgBuilder,
- ImmutableList.Builder<OutputFile> outputFilesBuilder)
+ ImmutableList.Builder<OutputFile> outputFilesBuilder,
+ boolean performChecks)
throws InterruptedException {
try {
RawAttributeMapper attributeMap = RawAttributeMapper.of(this);
for (String out : implicitOutputsFunction.getImplicitOutputs(attributeMap)) {
- try {
- addOutputFile(pkgBuilder.createLabel(out), eventHandler, outputFilesBuilder);
- } catch (LabelSyntaxException e) {
- reportError(
- "illegal output file name '"
- + out
- + "' in rule "
- + getLabel()
- + " due to: "
- + e.getMessage(),
- eventHandler);
+ Label label;
+ if (performChecks) {
+ try {
+ label = pkgBuilder.createLabel(out);
+ } catch (LabelSyntaxException e) {
+ reportError(
+ "illegal output file name '"
+ + out
+ + "' in rule "
+ + getLabel()
+ + " due to: "
+ + e.getMessage(),
+ eventHandler);
+ continue;
+ }
+ } else {
+ label = Label.createUnvalidated(pkgBuilder.getPackageIdentifier(), out);
}
+ addOutputFile(label, eventHandler, outputFilesBuilder);
}
} catch (EvalException e) {
reportError(String.format("In rule %s: %s", getLabel(), e.print()), eventHandler);
@@ -544,16 +580,19 @@
Label label,
EventHandler eventHandler,
ImmutableList.Builder<OutputFile> outputFilesBuilder,
- ImmutableListMultimap.Builder<String, OutputFile> outputFileMapBuilder)
+ ImmutableListMultimap.Builder<String, OutputFile> outputFileMapBuilder,
+ boolean performChecks)
throws LabelSyntaxException {
- if (!label.getPackageIdentifier().equals(pkg.getPackageIdentifier())) {
- throw new IllegalStateException("Label for attribute " + attribute
- + " should refer to '" + pkg.getName()
- + "' but instead refers to '" + label.getPackageFragment()
- + "' (label '" + label.getName() + "')");
- }
- if (label.getName().equals(".")) {
- throw new LabelSyntaxException("output file name can't be equal '.'");
+ if (performChecks) {
+ if (!label.getPackageIdentifier().equals(pkg.getPackageIdentifier())) {
+ throw new IllegalStateException("Label for attribute " + attribute
+ + " should refer to '" + pkg.getName()
+ + "' but instead refers to '" + label.getPackageFragment()
+ + "' (label '" + label.getName() + "')");
+ }
+ if (label.getName().equals(".")) {
+ throw new LabelSyntaxException("output file name can't be equal '.'");
+ }
}
OutputFile outputFile = addOutputFile(label, eventHandler, outputFilesBuilder);
outputFileMapBuilder.put(attribute.getName(), outputFile);
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 625130f..efc9d3c 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
@@ -1498,7 +1498,7 @@
Location location,
AttributeContainer attributeContainer,
ImplicitOutputsFunction implicitOutputsFunction)
- throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException {
+ throws InterruptedException, CannotPrecomputeDefaultsException {
Rule rule = pkgBuilder.createRule(
ruleLabel,
this,
@@ -1506,7 +1506,7 @@
attributeContainer,
implicitOutputsFunction);
populateRuleAttributeValues(rule, pkgBuilder, attributeValues, NullEventHandler.INSTANCE);
- rule.populateOutputFiles(NullEventHandler.INSTANCE, pkgBuilder);
+ rule.populateOutputFilesUnchecked(NullEventHandler.INSTANCE, pkgBuilder);
return rule;
}
