From 8824d5ec1e58ca92fe04df932e7f649a6add715c Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Wed, 20 Jan 2016 21:37:05 +0000 Subject: Enforce aspect configuration fragment specification when present. If an aspect has specified its configuration fragment dependencies, use these in place of the rule's. Note that the dynamic configuration support for this is yet to come. Also in this CL: * RuleContext is constructed with a ruleClassNameForLogging, which allows error messages involving aspects to be clearer. RELNOTES[NEW]: Skylark aspects can now specify configuration fragment dependencies with fragments and host_fragments like rules can. -- MOS_MIGRATED_REVID=112614357 --- .../devtools/build/lib/analysis/BuildView.java | 5 +- .../lib/analysis/ConfiguredTargetFactory.java | 16 +++- .../devtools/build/lib/analysis/RuleContext.java | 92 ++++++++++++++++------ 3 files changed, 83 insertions(+), 30 deletions(-) (limited to 'src/main/java/com/google') diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java index 862aa0f878..166879d62c 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java @@ -960,8 +960,9 @@ public class BuildView { throws EvalException, InterruptedException { BuildConfiguration targetConfig = target.getConfiguration(); return new RuleContext.Builder( - env, (Rule) target.getTarget(), targetConfig, configurations.getHostConfiguration(), - ruleClassProvider.getPrerequisiteValidator()) + env, (Rule) target.getTarget(), null, targetConfig, configurations.getHostConfiguration(), + ruleClassProvider.getPrerequisiteValidator(), + ((Rule) target.getTarget()).getRuleClassObject().getConfigurationFragmentPolicy()) .setVisibility(NestedSetBuilder.create( Order.STABLE_ORDER, PackageSpecification.EVERYTHING)) .setPrerequisites(getPrerequisiteMapForTesting(eventHandler, target, configurations)) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java index 3565324489..71a1626100 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredTargetFactory.java @@ -221,8 +221,10 @@ public final class ConfiguredTargetFactory { ListMultimap prerequisiteMap, Set configConditions) throws InterruptedException { // Visibility computation and checking is done for every rule. - RuleContext ruleContext = new RuleContext.Builder(env, rule, configuration, hostConfiguration, - ruleClassProvider.getPrerequisiteValidator()) + RuleContext ruleContext = new RuleContext.Builder(env, rule, null, + configuration, hostConfiguration, + ruleClassProvider.getPrerequisiteValidator(), + rule.getRuleClassObject().getConfigurationFragmentPolicy()) .setVisibility(convertVisibility(prerequisiteMap, env.getEventHandler(), rule, null)) .setPrerequisites(prerequisiteMap) .setConfigConditions(configConditions) @@ -296,11 +298,19 @@ public final class ConfiguredTargetFactory { Set configConditions, BuildConfiguration hostConfiguration) throws InterruptedException { + ConfigurationFragmentPolicy aspectPolicy = + aspect.getDefinition().getConfigurationFragmentPolicy(); + ConfigurationFragmentPolicy rulePolicy = + ((Rule) associatedTarget.getTarget()).getRuleClassObject().getConfigurationFragmentPolicy(); RuleContext.Builder builder = new RuleContext.Builder(env, associatedTarget.getTarget(), + aspect.getAspectClass().getName(), associatedTarget.getConfiguration(), hostConfiguration, - ruleClassProvider.getPrerequisiteValidator()); + ruleClassProvider.getPrerequisiteValidator(), + // TODO(mstaib): When AspectDefinition can no longer have null ConfigurationFragmentPolicy, + // remove this conditional. + aspectPolicy != null ? aspectPolicy : rulePolicy); RuleContext ruleContext = builder .setVisibility( diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index 6e06ef1f91..b54e9e0ba2 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java @@ -136,6 +136,7 @@ public final class RuleContext extends TargetContext private final Set configConditions; private final AttributeMap attributes; private final ImmutableSet features; + private final String ruleClassNameForLogging; private final ImmutableMap aspectAttributes; private final BuildConfiguration hostConfiguration; private final ConfigurationFragmentPolicy configurationFragmentPolicy; @@ -153,6 +154,7 @@ public final class RuleContext extends TargetContext ListMultimap filesetEntryMap, Set configConditions, Class universalFragment, + String ruleClassNameForLogging, ImmutableMap aspectAttributes) { super(builder.env, builder.rule, builder.configuration, builder.prerequisiteMap.get(null), builder.visibility); @@ -165,6 +167,7 @@ public final class RuleContext extends TargetContext this.attributes = ConfiguredAttributeMapper.of(builder.rule, configConditions); this.features = getEnabledFeatures(); + this.ruleClassNameForLogging = ruleClassNameForLogging; this.aspectAttributes = aspectAttributes; this.hostConfiguration = builder.hostConfiguration; reporter = builder.reporter; @@ -214,6 +217,13 @@ public final class RuleContext extends TargetContext return rule; } + /** + * Returns a rule class name suitable for log messages, including an aspect name if applicable. + */ + public String getRuleClassNameForLogging() { + return ruleClassNameForLogging; + } + /** * Returns the workspace name for the rule. */ @@ -295,7 +305,7 @@ public final class RuleContext extends TargetContext Preconditions.checkArgument(isLegalFragment(fragment, config), "%s has to declare '%s' as a required fragment " + "in %s configuration in order to access it.%s", - rule.getRuleClass(), name, FragmentCollection.getConfigurationName(config), + getRuleClassNameForLogging(), name, FragmentCollection.getConfigurationName(config), additionalErrorMessage); return getConfiguration(config).getFragment(fragment); } @@ -688,8 +698,8 @@ public final class RuleContext extends TargetContext checkAttribute(attributeName, mode); List elements = targetMap.get(attributeName); if (elements.size() > 1) { - throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName - + " produces more then one prerequisites"); + throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + + " produces more than one prerequisite"); } return elements.isEmpty() ? null : elements.get(0); } @@ -731,11 +741,11 @@ public final class RuleContext extends TargetContext Attribute ruleDefinition = getAttribute(attributeName); if (ruleDefinition == null) { - throw new IllegalStateException(getRule().getRuleClass() + " attribute " + attributeName + throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not defined"); } if (!ruleDefinition.isExecutable()) { - throw new IllegalStateException(getRule().getRuleClass() + " attribute " + attributeName + throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured to be executable"); } @@ -887,36 +897,36 @@ public final class RuleContext extends TargetContext private void checkAttribute(String attributeName, Mode mode) { Attribute attributeDefinition = getAttribute(attributeName); if (attributeDefinition == null) { - throw new IllegalStateException(getRule().getLocation() + ": " + getRule().getRuleClass() + throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not defined"); } if (!(attributeDefinition.getType() == BuildType.LABEL || attributeDefinition.getType() == BuildType.LABEL_LIST)) { - throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName + throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not a label type attribute"); } if (mode == Mode.HOST) { if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.HOST) { throw new IllegalStateException(getRule().getLocation() + ": " - + getRule().getRuleClass() + " attribute " + attributeName + + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the host configuration"); } } else if (mode == Mode.TARGET) { if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.NONE) { throw new IllegalStateException(getRule().getLocation() + ": " - + getRule().getRuleClass() + " attribute " + attributeName + + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the target configuration"); } } else if (mode == Mode.DATA) { if (attributeDefinition.getConfigurationTransition() != ConfigurationTransition.DATA) { throw new IllegalStateException(getRule().getLocation() + ": " - + getRule().getRuleClass() + " attribute " + attributeName + + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for the data configuration"); } } else if (mode == Mode.SPLIT) { if (!(attributeDefinition.getConfigurationTransition() instanceof SplitTransition)) { throw new IllegalStateException(getRule().getLocation() + ": " - + getRule().getRuleClass() + " attribute " + attributeName + + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured for a split transition"); } } @@ -929,12 +939,12 @@ public final class RuleContext extends TargetContext public Mode getAttributeMode(String attributeName) { Attribute attributeDefinition = getAttribute(attributeName); if (attributeDefinition == null) { - throw new IllegalStateException(getRule().getLocation() + ": " + getRule().getRuleClass() + throw new IllegalStateException(getRule().getLocation() + ": " + getRuleClassNameForLogging() + " attribute " + attributeName + " is not defined"); } if (!(attributeDefinition.getType() == BuildType.LABEL || attributeDefinition.getType() == BuildType.LABEL_LIST)) { - throw new IllegalStateException(rule.getRuleClass() + " attribute " + attributeName + throw new IllegalStateException(getRuleClassNameForLogging() + " attribute " + attributeName + " is not a label type attribute"); } if (attributeDefinition.getConfigurationTransition() == ConfigurationTransition.HOST) { @@ -947,7 +957,7 @@ public final class RuleContext extends TargetContext return Mode.SPLIT; } throw new IllegalStateException(getRule().getLocation() + ": " - + getRule().getRuleClass() + " attribute " + attributeName + " is not configured"); + + getRuleClassNameForLogging() + " attribute " + attributeName + " is not configured"); } /** @@ -1016,7 +1026,7 @@ public final class RuleContext extends TargetContext } public Artifact getSingleSource() { - return getSingleSource(getRule().getRuleClass() + " source file"); + return getSingleSource(getRuleClassNameForLogging() + " source file"); } /** @@ -1246,22 +1256,29 @@ public final class RuleContext extends TargetContext private final BuildConfiguration configuration; private final BuildConfiguration hostConfiguration; private final PrerequisiteValidator prerequisiteValidator; + @Nullable private final String aspectName; private final ErrorReporter reporter; private ListMultimap prerequisiteMap; private Set configConditions; private NestedSet visibility; private ImmutableMap aspectAttributes; - Builder(AnalysisEnvironment env, Rule rule, BuildConfiguration configuration, + Builder( + AnalysisEnvironment env, + Rule rule, + @Nullable String aspectName, + BuildConfiguration configuration, BuildConfiguration hostConfiguration, - PrerequisiteValidator prerequisiteValidator) { + PrerequisiteValidator prerequisiteValidator, + ConfigurationFragmentPolicy configurationFragmentPolicy) { this.env = Preconditions.checkNotNull(env); this.rule = Preconditions.checkNotNull(rule); - this.configurationFragmentPolicy = rule.getRuleClassObject().getConfigurationFragmentPolicy(); + this.aspectName = aspectName; + this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy); this.configuration = Preconditions.checkNotNull(configuration); this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration); this.prerequisiteValidator = prerequisiteValidator; - reporter = new ErrorReporter(env, rule); + reporter = new ErrorReporter(env, rule, getRuleClassNameForLogging()); } RuleContext build() { @@ -1271,7 +1288,13 @@ public final class RuleContext extends TargetContext ListMultimap targetMap = createTargetMap(); ListMultimap filesetEntryMap = createFilesetEntryMap(rule, configConditions); - return new RuleContext(this, targetMap, filesetEntryMap, configConditions, universalFragment, + return new RuleContext( + this, + targetMap, + filesetEntryMap, + configConditions, + universalFragment, + getRuleClassNameForLogging(), aspectAttributes != null ? aspectAttributes : ImmutableMap.of()); } @@ -1519,6 +1542,15 @@ public final class RuleContext extends TargetContext return rule; } + /** + * Returns a rule class name suitable for log messages, including an aspect name if applicable. + */ + public String getRuleClassNameForLogging() { + return aspectName != null + ? aspectName + " aspect on " + rule.getRuleClass() + : rule.getRuleClass(); + } + public BuildConfiguration getConfiguration() { return configuration; } @@ -1569,7 +1601,7 @@ public final class RuleContext extends TargetContext } } attributeError(attribute.getName(), "'" + prerequisite.getLabel() - + "' does not produce any " + rule.getRuleClass() + " " + attribute.getName() + + "' does not produce any " + getRuleClassNameForLogging() + " " + attribute.getName() + " files (expected " + allowedFileTypes + ")"); } } @@ -1599,10 +1631,12 @@ public final class RuleContext extends TargetContext public static final class ErrorReporter implements RuleErrorConsumer { private final AnalysisEnvironment env; private final Rule rule; + private final String ruleClassNameForLogging; - ErrorReporter(AnalysisEnvironment env, Rule rule) { + ErrorReporter(AnalysisEnvironment env, Rule rule, String ruleClassNameForLogging) { this.env = env; this.rule = rule; + this.ruleClassNameForLogging = ruleClassNameForLogging; } public void reportError(Location location, String message) { @@ -1635,7 +1669,8 @@ public final class RuleContext extends TargetContext } private String prefixRuleMessage(String message) { - return String.format("in %s rule %s: %s", rule.getRuleClass(), rule.getLabel(), message); + return String.format( + "in %s rule %s: %s", getRuleClassNameForLogging(), rule.getLabel(), message); } private String maskInternalAttributeNames(String name) { @@ -1657,8 +1692,15 @@ public final class RuleContext extends TargetContext : ""; return String.format("in %s attribute of %s rule %s: %s%s", - maskInternalAttributeNames(attrName), rule.getRuleClass(), rule.getLabel(), message, - macroMessageAppendix); + maskInternalAttributeNames(attrName), getRuleClassNameForLogging(), rule.getLabel(), + message, macroMessageAppendix); + } + + /** + * Returns a rule class name suitable for log messages, including an aspect name if applicable. + */ + private String getRuleClassNameForLogging() { + return ruleClassNameForLogging; } private String getGeneratorFunction() { -- cgit v1.2.3