Reduce parameters in `RuleContext` and its builder in favor of using enclosing objects `AnalysisEnvironment` and `ConfiguredRuleClassProvider`.
Inspired by `TODO(bazel-team): I get the feeling we could delete much of the boilerplate by replacing some of these fields with a RuleClassProvider -- both in the builder and in the RuleContext itself.` (written by brandjon@). I'm working on a change that will need yet another `ConfiguredRuleClassProvider` field in `RuleContext`, so this will be helpful.
Reduce `RuleContext.Builder` constructor parameters to only those that are needed up-front.
PiperOrigin-RevId: 401338523
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 fe2e141..8f1baf4 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
@@ -14,7 +14,6 @@
package com.google.devtools.build.lib.analysis;
-import static com.google.common.base.MoreObjects.firstNonNull;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.devtools.build.lib.packages.ExecGroup.DEFAULT_EXEC_GROUP_NAME;
@@ -51,11 +50,9 @@
import com.google.devtools.build.lib.analysis.config.ConfigConditions;
import com.google.devtools.build.lib.analysis.config.ConfigMatchingProvider;
import com.google.devtools.build.lib.analysis.config.Fragment;
-import com.google.devtools.build.lib.analysis.config.FragmentClassSet;
import com.google.devtools.build.lib.analysis.config.FragmentCollection;
import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
-import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics;
import com.google.devtools.build.lib.analysis.constraints.RuleContextConstraintSemantics;
import com.google.devtools.build.lib.analysis.platform.ConstraintValueInfo;
import com.google.devtools.build.lib.analysis.platform.PlatformInfo;
@@ -66,7 +63,6 @@
import com.google.devtools.build.lib.collect.ImmutableSortedKeyListMultimap;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.packages.Aspect;
import com.google.devtools.build.lib.packages.AspectDescriptor;
import com.google.devtools.build.lib.packages.Attribute;
@@ -131,10 +127,6 @@
public final class RuleContext extends TargetContext
implements ActionConstructionContext, ActionRegistry, RuleErrorConsumer, AutoCloseable {
- public boolean isAllowTagsPropagation() {
- return starlarkSemantics.getBool(BuildLanguageOptions.EXPERIMENTAL_ALLOW_TAGS_PROPAGATION);
- }
-
/** Custom dependency validation logic. */
public interface PrerequisiteValidator {
/**
@@ -168,11 +160,10 @@
private final String ruleClassNameForLogging;
private final BuildConfiguration hostConfiguration;
private final ConfigurationFragmentPolicy configurationFragmentPolicy;
- private final FragmentClassSet universalFragments;
+ private final ConfiguredRuleClassProvider ruleClassProvider;
private final RuleErrorConsumer reporter;
@Nullable private final ToolchainCollection<ResolvedToolchainContext> toolchainContexts;
private final ExecGroupCollection execGroupCollection;
- private final ConstraintSemantics<RuleContext> constraintSemantics;
@Nullable private final RequiredConfigFragmentsProvider requiredConfigFragments;
private final List<Expander> makeVariableExpanders = new ArrayList<>();
@@ -185,13 +176,6 @@
private transient ConfigurationMakeVariableContext configurationMakeVariableContext = null;
/**
- * The StarlarkSemantics to use for rule analysis. Should be the same as what's reported by the
- * AnalysisEnvironment, but saving it here is more convenient since {@link
- * AnalysisEnvironment#getStarlarkSemantics()} can throw InterruptedException.
- */
- private final StarlarkSemantics starlarkSemantics;
-
- /**
* Thread used for any Starlark evaluation during analysis, e.g. rule implementation function for
* a Starlark-defined rule, or Starlarkified helper logic for native rules that have been
* partially migrated to {@code @_builtins}.
@@ -211,18 +195,7 @@
Builder builder,
AttributeMap attributes,
ListMultimap<String, ConfiguredTargetAndData> targetMap,
- ImmutableMap<Label, ConfigMatchingProvider> configConditions,
- FragmentClassSet universalFragments,
- String ruleClassNameForLogging,
- ActionLookupKey actionLookupKey,
- ImmutableMap<String, Attribute> aspectAttributes,
- @Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
- ExecGroupCollection execGroupCollection,
- ConstraintSemantics<RuleContext> constraintSemantics,
- @Nullable RequiredConfigFragmentsProvider requiredConfigFragments,
- String toolsRepository,
- StarlarkSemantics starlarkSemantics,
- Mutability mutability) {
+ ExecGroupCollection execGroupCollection) {
super(
builder.env,
builder.target.getAssociatedRule(),
@@ -231,28 +204,25 @@
builder.visibility);
this.rule = builder.target.getAssociatedRule();
this.aspects = builder.aspects;
- this.aspectDescriptors =
- builder.aspects.stream().map(Aspect::getDescriptor).collect(toImmutableList());
+ this.aspectDescriptors = aspects.stream().map(Aspect::getDescriptor).collect(toImmutableList());
this.configurationFragmentPolicy = builder.configurationFragmentPolicy;
- this.universalFragments = universalFragments;
+ this.ruleClassProvider = builder.ruleClassProvider;
this.targetMap = targetMap;
- this.configConditions = configConditions;
- this.attributes = new AspectAwareAttributeMapper(attributes, aspectAttributes);
+ this.configConditions = builder.configConditions.asProviders();
+ this.attributes = new AspectAwareAttributeMapper(attributes, builder.aspectAttributes);
Set<String> allEnabledFeatures = new HashSet<>();
Set<String> allDisabledFeatures = new HashSet<>();
getAllFeatures(allEnabledFeatures, allDisabledFeatures);
this.enabledFeatures = ImmutableSortedSet.copyOf(allEnabledFeatures);
this.disabledFeatures = ImmutableSortedSet.copyOf(allDisabledFeatures);
- this.ruleClassNameForLogging = ruleClassNameForLogging;
+ this.ruleClassNameForLogging = builder.getRuleClassNameForLogging();
this.hostConfiguration = builder.hostConfiguration;
- this.actionOwnerSymbolGenerator = new SymbolGenerator<>(actionLookupKey);
- reporter = builder.reporter;
- this.toolchainContexts = toolchainContexts;
+ this.actionOwnerSymbolGenerator = new SymbolGenerator<>(builder.actionOwnerSymbol);
+ this.reporter = builder.reporter;
+ this.toolchainContexts = builder.toolchainContexts;
this.execGroupCollection = execGroupCollection;
- this.constraintSemantics = constraintSemantics;
- this.requiredConfigFragments = requiredConfigFragments;
- this.starlarkSemantics = starlarkSemantics;
- this.starlarkThread = createStarlarkThread(toolsRepository, mutability); // uses above state
+ this.requiredConfigFragments = builder.requiredConfigFragments;
+ this.starlarkThread = createStarlarkThread(builder.mutability); // uses above state
}
private void getAllFeatures(Set<String> allEnabledFeatures, Set<String> allDisabledFeatures) {
@@ -294,6 +264,12 @@
}
}
+ public boolean isAllowTagsPropagation() {
+ return getAnalysisEnvironment()
+ .getStarlarkSemantics()
+ .getBool(BuildLanguageOptions.EXPERIMENTAL_ALLOW_TAGS_PROPAGATION);
+ }
+
public RepositoryName getRepository() {
return rule.getRepository();
}
@@ -527,7 +503,7 @@
public <T extends Fragment> boolean isLegalFragment(
Class<T> fragment, ConfigurationTransition transition) {
- return universalFragments.contains(fragment)
+ return ruleClassProvider.getFragmentRegistry().getUniversalFragments().contains(fragment)
|| configurationFragmentPolicy.isLegalConfigurationFragment(fragment, transition);
}
@@ -1143,17 +1119,13 @@
return configurationMakeVariableContext;
}
- public StarlarkSemantics getStarlarkSemantics() {
- return starlarkSemantics;
- }
-
- private StarlarkThread createStarlarkThread(String toolsRepository, Mutability mutability) {
+ private StarlarkThread createStarlarkThread(Mutability mutability) {
AnalysisEnvironment env = getAnalysisEnvironment();
- StarlarkThread thread = new StarlarkThread(mutability, starlarkSemantics);
+ StarlarkThread thread = new StarlarkThread(mutability, env.getStarlarkSemantics());
thread.setPrintHandler(Event.makeDebugPrintHandler(env.getEventHandler()));
new BazelStarlarkContext(
BazelStarlarkContext.Phase.ANALYSIS,
- toolsRepository,
+ ruleClassProvider.getToolsRepository(),
/*fragmentNameToClass=*/ null,
getTarget().getPackage().getRepositoryMapping(),
/*convertedLabelsInPackage=*/ new HashMap<>(),
@@ -1265,8 +1237,8 @@
return toolchainContexts.getTargetPlatform().constraints().hasConstraintValue(constraintValue);
}
- public ConstraintSemantics<RuleContext> getConstraintSemantics() {
- return constraintSemantics;
+ public ConfiguredRuleClassProvider getRuleClassProvider() {
+ return ruleClassProvider;
}
/** Returns the configuration fragments this rule uses. */
@@ -1643,30 +1615,24 @@
}
/** Builder class for a RuleContext. */
- // TODO(bazel-team): I get the feeling we could delete much of the boilerplate by replacing some
- // of these fields with a RuleClassProvider -- both in the builder and in the RuleContext itself.
public static final class Builder implements RuleErrorConsumer {
private final AnalysisEnvironment env;
private final Target target;
- private final ConfigurationFragmentPolicy configurationFragmentPolicy;
- private FragmentClassSet universalFragments;
+ private final ImmutableList<Aspect> aspects;
private final BuildConfiguration configuration;
- private final BuildConfiguration hostConfiguration;
- private final ActionLookupKey actionOwnerSymbol;
- private final PrerequisiteValidator prerequisiteValidator;
private final RuleErrorConsumer reporter;
+ private ConfiguredRuleClassProvider ruleClassProvider;
+ private BuildConfiguration hostConfiguration;
+ private ConfigurationFragmentPolicy configurationFragmentPolicy;
+ private ActionLookupKey actionOwnerSymbol;
private OrderedSetMultimap<Attribute, ConfiguredTargetAndData> prerequisiteMap;
private ConfigConditions configConditions;
- private String toolsRepository;
- private StarlarkSemantics starlarkSemantics;
private Mutability mutability;
private NestedSet<PackageGroupContents> visibility;
- private ImmutableMap<String, Attribute> aspectAttributes;
- private final ImmutableList<Aspect> aspects;
+ private ImmutableMap<String, Attribute> aspectAttributes = ImmutableMap.of();
private ToolchainCollection<ResolvedToolchainContext> toolchainContexts;
private ExecGroupCollection.Builder execGroupCollectionBuilder;
private ImmutableMap<String, String> rawExecProperties;
- private ConstraintSemantics<RuleContext> constraintSemantics;
@Nullable private RequiredConfigFragmentsProvider requiredConfigFragments;
@VisibleForTesting
@@ -1674,19 +1640,11 @@
AnalysisEnvironment env,
Target target,
ImmutableList<Aspect> aspects,
- BuildConfiguration configuration,
- BuildConfiguration hostConfiguration,
- PrerequisiteValidator prerequisiteValidator,
- ConfigurationFragmentPolicy configurationFragmentPolicy,
- ActionLookupKey actionOwnerSymbol) {
+ BuildConfiguration configuration) {
this.env = Preconditions.checkNotNull(env);
this.target = Preconditions.checkNotNull(target);
- this.aspects = aspects;
- this.configurationFragmentPolicy = Preconditions.checkNotNull(configurationFragmentPolicy);
+ this.aspects = Preconditions.checkNotNull(aspects);
this.configuration = Preconditions.checkNotNull(configuration);
- this.hostConfiguration = Preconditions.checkNotNull(hostConfiguration);
- this.prerequisiteValidator = prerequisiteValidator;
- this.actionOwnerSymbol = Preconditions.checkNotNull(actionOwnerSymbol);
if (configuration.allowAnalysisFailures()) {
reporter = new SuppressingErrorReporter();
} else {
@@ -1698,13 +1656,14 @@
@VisibleForTesting
public RuleContext build() throws InvalidExecGroupException {
+ Preconditions.checkNotNull(ruleClassProvider);
+ Preconditions.checkNotNull(hostConfiguration);
+ Preconditions.checkNotNull(configurationFragmentPolicy);
+ Preconditions.checkNotNull(actionOwnerSymbol);
Preconditions.checkNotNull(prerequisiteMap);
Preconditions.checkNotNull(configConditions);
- Preconditions.checkNotNull(toolsRepository);
- Preconditions.checkNotNull(starlarkSemantics);
Preconditions.checkNotNull(mutability);
Preconditions.checkNotNull(visibility);
- Preconditions.checkNotNull(constraintSemantics);
AttributeMap attributes =
ConfiguredAttributeMapper.of(
target.getAssociatedRule(), configConditions.asProviders(), configuration.checksum());
@@ -1725,18 +1684,7 @@
this,
attributes,
targetMap,
- configConditions.asProviders(),
- universalFragments,
- getRuleClassNameForLogging(),
- actionOwnerSymbol,
- firstNonNull(aspectAttributes, ImmutableMap.of()),
- toolchainContexts,
- createExecGroupCollection(execGroupCollectionBuilder, attributes),
- constraintSemantics,
- requiredConfigFragments,
- toolsRepository,
- starlarkSemantics,
- mutability);
+ createExecGroupCollection(execGroupCollectionBuilder, attributes));
}
private ExecGroupCollection createExecGroupCollection(
@@ -1775,15 +1723,23 @@
}
}
- // TODO(bazel-team): This field is only used by BazelStarlarkContext. Investigate whether that's
- // even needed in the analysis phase, and delete it if not.
- public Builder setToolsRepository(String toolsRepository) {
- this.toolsRepository = toolsRepository;
+ public Builder setRuleClassProvider(ConfiguredRuleClassProvider ruleClassProvider) {
+ this.ruleClassProvider = ruleClassProvider;
return this;
}
- public Builder setStarlarkSemantics(StarlarkSemantics starlarkSemantics) {
- this.starlarkSemantics = starlarkSemantics;
+ public Builder setHostConfiguration(BuildConfiguration hostConfiguration) {
+ this.hostConfiguration = hostConfiguration;
+ return this;
+ }
+
+ public Builder setConfigurationFragmentPolicy(ConfigurationFragmentPolicy policy) {
+ this.configurationFragmentPolicy = policy;
+ return this;
+ }
+
+ public Builder setActionOwnerSymbol(ActionLookupKey actionOwnerSymbol) {
+ this.actionOwnerSymbol = actionOwnerSymbol;
return this;
}
@@ -1822,16 +1778,6 @@
return this;
}
- /** Sets the fragment that can be legally accessed even when not explicitly declared. */
- public Builder setUniversalFragments(FragmentClassSet fragments) {
- // TODO(bazel-team): Add this directly to ConfigurationFragmentPolicy, so we
- // don't need separate logic specifically for checking this fragment. The challenge is
- // that we need RuleClassProvider to figure out what this fragment is, and not every
- // call state that creates ConfigurationFragmentPolicy has access to that.
- this.universalFragments = fragments;
- return this;
- }
-
/** Sets the {@link ResolvedToolchainContext} used to access toolchains used by this rule. */
public Builder setToolchainContext(ResolvedToolchainContext toolchainContext) {
Preconditions.checkState(
@@ -1870,11 +1816,6 @@
return this;
}
- public Builder setConstraintSemantics(ConstraintSemantics<RuleContext> constraintSemantics) {
- this.constraintSemantics = constraintSemantics;
- return this;
- }
-
public Builder setRequiredConfigFragments(
@Nullable RequiredConfigFragmentsProvider requiredConfigFragments) {
this.requiredConfigFragments = requiredConfigFragments;
@@ -1919,15 +1860,6 @@
return mapBuilder.build();
}
- /**
- * Post a raw event to the analysis environment's event handler. This circumvents normal error
- * and warning reporting functionality to post events, and should only be used in rare cases
- * where a custom event needs to be handled.
- */
- public void post(Postable event) {
- env.getEventHandler().post(event);
- }
-
@Override
public void ruleError(String message) {
reporter.ruleError(message);
@@ -2039,17 +1971,17 @@
}
/**
- * Expose the Starlark semantics that governs the building of this rule (and the rest of the
- * build)
+ * Returns the {@link StarlarkSemantics} governs the building of this rule (and the rest of the
+ * build).
*/
- public StarlarkSemantics getStarlarkSemantics() throws InterruptedException {
+ public StarlarkSemantics getStarlarkSemantics() {
return env.getStarlarkSemantics();
}
/**
* Returns a rule class name suitable for log messages, including an aspect name if applicable.
*/
- public String getRuleClassNameForLogging() {
+ private String getRuleClassNameForLogging() {
if (aspects.isEmpty()) {
return target.getAssociatedRule().getRuleClass();
}
@@ -2240,7 +2172,7 @@
validateDirectPrerequisiteType(prerequisite, attribute);
validateDirectPrerequisiteFileTypes(prerequisite, attribute);
if (attribute.performPrereqValidatorCheck()) {
- prerequisiteValidator.validate(this, prerequisite, attribute);
+ ruleClassProvider.getPrerequisiteValidator().validate(this, prerequisite, attribute);
}
}
}
