Skip to content

Commit

Permalink
Convert uses of RuleTransitionFactory to TransitionFactory.
Browse files Browse the repository at this point in the history
Part of #7814.

Closes #7922.

PiperOrigin-RevId: 241744479
  • Loading branch information
katre authored and copybara-github committed Apr 3, 2019
1 parent 59a8864 commit b3b3e8b
Show file tree
Hide file tree
Showing 24 changed files with 131 additions and 257 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,10 @@
import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
import com.google.devtools.build.lib.analysis.config.BuildConfiguration.Fragment;
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.ComposingRuleTransitionFactory;
import com.google.devtools.build.lib.analysis.config.ConfigurationFragmentFactory;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.ComposingTransitionFactory;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.analysis.constraints.ConstraintSemantics;
import com.google.devtools.build.lib.analysis.skylark.BazelStarlarkContext;
import com.google.devtools.build.lib.analysis.skylark.SkylarkModules;
Expand All @@ -52,7 +53,6 @@
import com.google.devtools.build.lib.packages.RuleClass.Builder.ThirdPartyLicenseExistencePolicy;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleErrorConsumer;
import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData;
import com.google.devtools.build.lib.skylarkbuildapi.Bootstrap;
Expand Down Expand Up @@ -252,7 +252,7 @@ public static class Builder implements RuleDefinitionEnvironment {
new Digraph<>();
private List<Class<? extends BuildConfiguration.Fragment>> universalFragments =
new ArrayList<>();
@Nullable private RuleTransitionFactory trimmingTransitionFactory;
@Nullable private TransitionFactory<Rule> trimmingTransitionFactory;
private OptionsDiffPredicate shouldInvalidateCacheForOptionDiff =
OptionsDiffPredicate.ALWAYS_INVALIDATE;
private PrerequisiteValidator prerequisiteValidator;
Expand Down Expand Up @@ -410,23 +410,25 @@ public Builder setThirdPartyLicenseExistencePolicy(ThirdPartyLicenseExistencePol
* feature flags, and support for this transition factory will likely be removed at some point
* in the future (whenever automatic trimming is sufficiently workable).
*/
public Builder addTrimmingTransitionFactory(RuleTransitionFactory factory) {
public Builder addTrimmingTransitionFactory(TransitionFactory<Rule> factory) {
Preconditions.checkNotNull(factory);
Preconditions.checkArgument(!factory.isSplit());
if (trimmingTransitionFactory == null) {
trimmingTransitionFactory = Preconditions.checkNotNull(factory);
trimmingTransitionFactory = factory;
} else {
trimmingTransitionFactory = new ComposingRuleTransitionFactory(
trimmingTransitionFactory, Preconditions.checkNotNull(factory));
trimmingTransitionFactory =
ComposingTransitionFactory.of(trimmingTransitionFactory, factory);
}
return this;
}

/**
* Overrides the transition factory run over all targets.
*
* @see {@link #addTrimmingTransitionFactory(RuleTransitionFactory)}
* @see {@link #addTrimmingTransitionFactory(TransitionFactory<Rule>)}
*/
@VisibleForTesting(/* for testing trimming transition factories without relying on prod use */ )
public Builder overrideTrimmingTransitionFactoryForTesting(RuleTransitionFactory factory) {
public Builder overrideTrimmingTransitionFactoryForTesting(TransitionFactory<Rule> factory) {
trimmingTransitionFactory = null;
return this.addTrimmingTransitionFactory(factory);
}
Expand Down Expand Up @@ -609,7 +611,7 @@ public String getToolsRepository() {
private final Map<String, Class<? extends Fragment>> optionsToFragmentMap;

/** The transition factory used to produce the transition that will trim targets. */
@Nullable private final RuleTransitionFactory trimmingTransitionFactory;
@Nullable private final TransitionFactory<Rule> trimmingTransitionFactory;

/** The predicate used to determine whether a diff requires the cache to be invalidated. */
private final OptionsDiffPredicate shouldInvalidateCacheForOptionDiff;
Expand Down Expand Up @@ -649,7 +651,7 @@ private ConfiguredRuleClassProvider(
ImmutableList<Class<? extends FragmentOptions>> configurationOptions,
ImmutableList<ConfigurationFragmentFactory> configurationFragments,
ImmutableList<Class<? extends BuildConfiguration.Fragment>> universalFragments,
@Nullable RuleTransitionFactory trimmingTransitionFactory,
@Nullable TransitionFactory<Rule> trimmingTransitionFactory,
OptionsDiffPredicate shouldInvalidateCacheForOptionDiff,
PrerequisiteValidator prerequisiteValidator,
ImmutableMap<String, Object> skylarkAccessibleJavaClasses,
Expand Down Expand Up @@ -769,12 +771,12 @@ public Class<? extends Fragment> getConfigurationFragmentForOption(String requir
/**
* Returns the transition factory used to produce the transition to trim targets.
*
* <p>This is a temporary measure for supporting manual trimming of feature flags, and support
* for this transition factory will likely be removed at some point in the future (whenever
* automatic trimming is sufficiently workable
* <p>This is a temporary measure for supporting manual trimming of feature flags, and support for
* this transition factory will likely be removed at some point in the future (whenever automatic
* trimming is sufficiently workable
*/
@Nullable
public RuleTransitionFactory getTrimmingTransitionFactory() {
public TransitionFactory<Rule> getTrimmingTransitionFactory() {
return trimmingTransitionFactory;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
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.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.causes.Cause;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
Expand All @@ -46,7 +47,6 @@
import com.google.devtools.build.lib.packages.PackageGroup;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
Expand Down Expand Up @@ -188,7 +188,7 @@ public final OrderedSetMultimap<DependencyKind, Dependency> dependentNodeMap(
@Nullable Aspect aspect,
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
ImmutableSet<Label> toolchainLabels,
@Nullable RuleTransitionFactory trimmingTransitionFactory)
@Nullable TransitionFactory<Rule> trimmingTransitionFactory)
throws EvalException, InterruptedException, InconsistentAspectOrderException {
NestedSetBuilder<Cause> rootCauses = NestedSetBuilder.stableOrder();
OrderedSetMultimap<DependencyKind, Dependency> outgoingEdges =
Expand Down Expand Up @@ -244,7 +244,7 @@ public final OrderedSetMultimap<DependencyKind, Dependency> dependentNodeMap(
ImmutableMap<Label, ConfigMatchingProvider> configConditions,
ImmutableSet<Label> toolchainLabels,
NestedSetBuilder<Cause> rootCauses,
@Nullable RuleTransitionFactory trimmingTransitionFactory)
@Nullable TransitionFactory<Rule> trimmingTransitionFactory)
throws EvalException, InterruptedException, InconsistentAspectOrderException {
Target target = node.getTarget();
BuildConfiguration config = node.getConfiguration();
Expand Down Expand Up @@ -366,7 +366,7 @@ private OrderedSetMultimap<DependencyKind, Dependency> fullyResolveDependencies(
OrderedSetMultimap<DependencyKind, PartiallyResolvedDependency> partiallyResolvedDeps,
Map<Label, Target> targetMap,
BuildConfiguration originalConfiguration,
@Nullable RuleTransitionFactory trimmingTransitionFactory)
@Nullable TransitionFactory<Rule> trimmingTransitionFactory)
throws InconsistentAspectOrderException {
OrderedSetMultimap<DependencyKind, Dependency> outgoingEdges = OrderedSetMultimap.create();

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.RuleClassProvider;
import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.skyframe.BuildConfigurationValue;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetFunction;
Expand Down Expand Up @@ -575,7 +574,9 @@ private static OrderedSetMultimap<DependencyKind, Dependency> sortResolvedDeps(
*
* <ol>
* <li>Apply the per-target transitions specified in {@code asDeps}. This can be used, e.g., to
* apply {@link RuleTransitionFactory}s over global top-level configurations.
* apply {@link
* com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory}s over global
* top-level configurations.
* <li>(Optionally) trim configurations to only the fragments the targets actually need. This is
* triggered by {@link BuildConfiguration.Options#trimConfigurations}.
* </ol>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@
import com.google.devtools.build.lib.analysis.config.transitions.NoTransition;
import com.google.devtools.build.lib.analysis.config.transitions.NullTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.Target;
import javax.annotation.Nullable;

Expand Down Expand Up @@ -53,7 +53,7 @@ public static ConfigurationTransition evaluateTransition(
BuildConfiguration fromConfig,
ConfigurationTransition baseTransition,
Target toTarget,
@Nullable RuleTransitionFactory trimmingTransitionFactory) {
@Nullable TransitionFactory<Rule> trimmingTransitionFactory) {

// I. The null configuration always remains the null configuration. We could fold this into
// (III), but NoTransition doesn't work if the source is the null configuration.
Expand Down Expand Up @@ -94,7 +94,7 @@ public static ConfigurationTransition evaluateTransition(
private static ConfigurationTransition applyRuleTransition(
ConfigurationTransition currentTransition, Target toTarget) {
Rule associatedRule = toTarget.getAssociatedRule();
RuleTransitionFactory transitionFactory =
TransitionFactory<Rule> transitionFactory =
associatedRule.getRuleClassObject().getTransitionFactory();
return applyTransitionFromFactory(currentTransition, toTarget, transitionFactory);
}
Expand All @@ -107,10 +107,10 @@ private static ConfigurationTransition applyRuleTransition(
private static ConfigurationTransition applyTransitionFromFactory(
ConfigurationTransition currentTransition,
Target toTarget,
@Nullable RuleTransitionFactory transitionFactory) {
@Nullable TransitionFactory<Rule> transitionFactory) {
if (transitionFactory != null) {
return ComposingTransition.of(
currentTransition, transitionFactory.buildTransitionFor(toTarget.getAssociatedRule()));
currentTransition, transitionFactory.create(toTarget.getAssociatedRule()));
}
return currentTransition;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ public class EnvironmentRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.cfg(HostTransition.INSTANCE)
.override(attr("tags", Type.STRING_LIST)
// No need to show up in ":all", etc. target patterns.
.value(ImmutableList.of("manual"))
.nonconfigurable("low-level attribute, used in TargetUtils without configurations"))
.cfg(HostTransition.createFactory())
.override(
attr("tags", Type.STRING_LIST)
// No need to show up in ":all", etc. target patterns.
.value(ImmutableList.of("manual"))
.nonconfigurable("low-level attribute, used in TargetUtils without configurations"))
/* <!-- #BLAZE_RULE(environment).ATTRIBUTE(fulfills) -->
The set of environments this one is considered a valid "standin" for.
<p>
Expand All @@ -55,10 +56,12 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
Environments may only fulfill other environments in the same environment group.
</p>
<!-- #END_BLAZE_RULE.ATTRIBUTE --> */
.add(attr(FULFILLS_ATTRIBUTE, BuildType.LABEL_LIST)
.allowedRuleClasses(EnvironmentRule.RULE_NAME)
.allowedFileTypes(FileTypeSet.NO_FILE)
.nonconfigurable("used for defining constraint models - this shouldn't be configured"))
.add(
attr(FULFILLS_ATTRIBUTE, BuildType.LABEL_LIST)
.allowedRuleClasses(EnvironmentRule.RULE_NAME)
.allowedFileTypes(FileTypeSet.NO_FILE)
.nonconfigurable(
"used for defining constraint models - this shouldn't be configured"))
.exemptFromConstraintChecking("this rule *defines* a constraint")
.setUndocumented()
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,12 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.StarlarkDefinedConfigTransition;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.Attribute;
import com.google.devtools.build.lib.packages.BuildType;
import com.google.devtools.build.lib.packages.RawAttributeMapper;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleTransitionFactory;
import com.google.devtools.build.lib.packages.StructImpl;
import com.google.devtools.build.lib.packages.StructProvider;
import com.google.devtools.build.lib.syntax.Environment;
Expand All @@ -37,7 +37,7 @@
import java.util.List;

/**
* This class implements {@link RuleTransitionFactory} to provide a starlark-defined transition that
* This class implements {@link TransitionFactory} to provide a starlark-defined transition that
* rules can apply to their own configuration. This transition has access to (1) the a map of the
* current configuration's build settings and (2) the configured* attributes of the given rule (not
* its dependencies').
Expand All @@ -48,7 +48,7 @@
*
* <p>For starlark-defined attribute transitions, see {@link StarlarkAttributeTransitionProvider}.
*/
public class StarlarkRuleTransitionProvider implements RuleTransitionFactory {
public class StarlarkRuleTransitionProvider implements TransitionFactory<Rule> {

private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition;

Expand All @@ -62,10 +62,17 @@ public StarlarkDefinedConfigTransition getStarlarkDefinedConfigTransitionForTest
}

@Override
public PatchTransition buildTransitionFor(Rule rule) {
public PatchTransition create(Rule rule) {
return new FunctionPatchTransition(starlarkDefinedConfigTransition, rule);
}

@Override
public boolean isSplit() {
// The transitions returned by this factory are guaranteed not to be splits.
return false;
}

/** The actual transition used by the rule. */
class FunctionPatchTransition extends StarlarkTransition implements PatchTransition {
private final StructImpl attrObject;

Expand Down
Loading

0 comments on commit b3b3e8b

Please sign in to comment.