diff options
Diffstat (limited to 'src/main/java/com/google/devtools')
6 files changed, 113 insertions, 91 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index f8e5643491..9e54fc10a6 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -123,7 +123,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu try { LabelValidator.PackageAndTarget labelParts = LabelValidator.parseAbsoluteLabel(absName); PackageIdentifier pkgIdWithoutRepo = - validate(labelParts.getPackageName(), labelParts.getTargetName()); + validatePackageName(labelParts.getPackageName(), labelParts.getTargetName()); PathFragment packageFragment = pkgIdWithoutRepo.getPackageFragment(); if (repo.isEmpty() && ABSOLUTE_PACKAGE_NAMES.contains(packageFragment)) { repo = "@"; @@ -164,7 +164,7 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu * @throws LabelSyntaxException if either of the arguments was invalid. */ public static Label create(String packageName, String targetName) throws LabelSyntaxException { - return LABEL_INTERNER.intern(new Label(packageName, targetName)); + return create(validatePackageName(packageName, targetName), targetName); } /** @@ -173,7 +173,18 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu */ public static Label create(PackageIdentifier packageId, String targetName) throws LabelSyntaxException { - return LABEL_INTERNER.intern(new Label(packageId, targetName)); + return createUnvalidated(packageId, validateTargetName(packageId, targetName)); + } + + /** + * Similar factory to above, but does not perform target name validation. + * + * <p>Only call this method if you know what you're doing; in particular, don't call it on + * arbitrary {@code targetName} inputs + */ + + public static Label createUnvalidated(PackageIdentifier packageId, String targetName) { + return LABEL_INTERNER.intern(new Label(packageId, StringCanonicalizer.intern(targetName))); } /** @@ -213,13 +224,17 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu } /** - * Validates the given target name and returns a canonical String instance if it is valid. - * Otherwise it throws a SyntaxException. + * Validates the given target name and returns a normalized name if it is valid. Otherwise it + * throws a SyntaxException. */ - private static String canonicalizeTargetName(String name) throws LabelSyntaxException { + private static String validateTargetName(PackageIdentifier packageIdentifier, String name) + throws LabelSyntaxException { String error = LabelValidator.validateTargetName(name); if (error != null) { error = "invalid target name '" + StringUtilities.sanitizeControlChars(name) + "': " + error; + if (packageIdentifier.getPackageFragment().getPathString().endsWith("/" + name)) { + error += " (perhaps you meant \":" + name + "\"?)"; + } throw new LabelSyntaxException(error); } @@ -227,15 +242,14 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu if (name.endsWith("/.")) { name = name.substring(0, name.length() - 2); } - - return StringCanonicalizer.intern(name); + return name; } /** * Validates the given package name and returns a canonical {@link PackageIdentifier} instance * if it is valid. Otherwise it throws a SyntaxException. */ - private static PackageIdentifier validate(String packageIdentifier, String name) + private static PackageIdentifier validatePackageName(String packageIdentifier, String name) throws LabelSyntaxException { String error = null; try { @@ -262,34 +276,12 @@ public final class Label implements Comparable<Label>, Serializable, SkylarkValu /** Precomputed hash code. */ private final int hashCode; - /** - * Constructor from a package name, target name. Both are checked for validity - * and a SyntaxException is thrown if either is invalid. - * TODO(bazel-team): move the validation to {@link PackageIdentifier}. Unfortunately, there are a - * bazillion tests that use invalid package names (taking advantage of the fact that calling - * Label(PathFragment, String) doesn't validate the package name). - */ - private Label(String packageIdentifier, String name) throws LabelSyntaxException { - this(validate(packageIdentifier, name), name); - } - - private Label(PackageIdentifier packageIdentifier, String name) - throws LabelSyntaxException { + private Label(PackageIdentifier packageIdentifier, String name) { Preconditions.checkNotNull(packageIdentifier); Preconditions.checkNotNull(name); this.packageIdentifier = packageIdentifier; - try { - this.name = canonicalizeTargetName(name); - } catch (LabelSyntaxException e) { - // This check is just for a more helpful error message - // i.e. valid target name, invalid package name, colon-free label form - // used => probably they meant "//foo:bar.c" not "//foo/bar.c". - if (packageIdentifier.getPackageFragment().getPathString().endsWith("/" + name)) { - throw new LabelSyntaxException(e.getMessage() + " (perhaps you meant \":" + name + "\"?)"); - } - throw e; - } + this.name = name; this.hashCode = hashCode(this.name, this.packageIdentifier); } 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 ec5806869a..9e4ab1954a 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 @@ public class Package { 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 @@ public class Package { } /** - * 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 @@ public class Package { // 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 @@ public class Package { // 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 1a7cc36ded..b751f87956 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 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide */ 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 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide 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 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide 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 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide 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 @@ public final class Rule implements Target, DependencyFilter.AttributeInfoProvide 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 625130f9f9..efc9d3c0f5 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 @@ public class RuleClass { Location location, AttributeContainer attributeContainer, ImplicitOutputsFunction implicitOutputsFunction) - throws LabelSyntaxException, InterruptedException, CannotPrecomputeDefaultsException { + throws InterruptedException, CannotPrecomputeDefaultsException { Rule rule = pkgBuilder.createRule( ruleLabel, this, @@ -1506,7 +1506,7 @@ public class RuleClass { attributeContainer, implicitOutputsFunction); populateRuleAttributeValues(rule, pkgBuilder, attributeValues, NullEventHandler.INSTANCE); - rule.populateOutputFiles(NullEventHandler.INSTANCE, pkgBuilder); + rule.populateOutputFilesUnchecked(NullEventHandler.INSTANCE, pkgBuilder); return rule; } diff --git a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java index 593957330f..2781b0fcd7 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/BlazeQueryEnvironment.java @@ -19,7 +19,6 @@ import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; import com.google.common.collect.Maps; 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.cmdline.ResolvedTargets; import com.google.devtools.build.lib.cmdline.TargetParsingException; @@ -403,15 +402,11 @@ public class BlazeQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> // Also add the BUILD file of the subinclude. if (buildFiles) { - try { - addIfUniqueLabel( - getSubincludeTarget(subinclude.getLocalTargetLabel("BUILD"), pkg), - seenLabels, - dependentFiles); - - } catch (LabelSyntaxException e) { - throw new AssertionError("BUILD should always parse as a target name", e); - } + addIfUniqueLabel( + getSubincludeTarget( + Label.createUnvalidated(subinclude.getPackageIdentifier(), "BUILD"), pkg), + seenLabels, + dependentFiles); } } } diff --git a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java index 80551587c3..f68fb30465 100644 --- a/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java +++ b/src/main/java/com/google/devtools/build/lib/query2/SkyQueryEnvironment.java @@ -38,7 +38,6 @@ import com.google.common.util.concurrent.ListeningExecutorService; import com.google.common.util.concurrent.MoreExecutors; import com.google.common.util.concurrent.ThreadFactoryBuilder; 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.cmdline.TargetParsingException; import com.google.devtools.build.lib.cmdline.TargetPattern; @@ -778,15 +777,11 @@ public class SkyQueryEnvironment extends AbstractBlazeQueryEnvironment<Target> if (buildFiles) { // Also add the BUILD file of the subinclude. - try { - addIfUniqueLabel( - getSubincludeTarget(subinclude.getLocalTargetLabel("BUILD"), pkg), - seenLabels, - dependentFiles); - - } catch (LabelSyntaxException e) { - throw new AssertionError("BUILD should always parse as a target name", e); - } + addIfUniqueLabel( + getSubincludeTarget( + Label.createUnvalidated(subinclude.getPackageIdentifier(), "BUILD"), pkg), + seenLabels, + dependentFiles); } } } |