Skip to content

Commit

Permalink
Use a single instance for all NoTransition and NoConfigTransition fac…
Browse files Browse the repository at this point in the history
…tory uses.

Work towards composable starlark transitions: #22248.

PiperOrigin-RevId: 639026960
Change-Id: I46e0ebbd25a8708c944bee69da9a9582543da9b1
  • Loading branch information
katre authored and copybara-github committed May 31, 2024
1 parent 75f7122 commit 870e7c6
Show file tree
Hide file tree
Showing 12 changed files with 23 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
public class NoConfigTransition implements PatchTransition {

@SerializationConstant public static final NoConfigTransition INSTANCE = new NoConfigTransition();
private static final TransitionFactory<? extends TransitionFactory.Data> FACTORY_INSTANCE =
new AutoValue_NoConfigTransition_Factory<>();

private NoConfigTransition() {}

Expand All @@ -57,8 +59,10 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
}

/** Returns a {@link TransitionFactory} instance that generates the transition. */
public static <T extends TransitionFactory.Data> TransitionFactory<T> createFactory() {
return new AutoValue_NoConfigTransition_Factory<>();
public static <T extends TransitionFactory.Data> TransitionFactory<T> getFactory() {
@SuppressWarnings("unchecked")
TransitionFactory<T> castFactory = (TransitionFactory<T>) FACTORY_INSTANCE;
return castFactory;
}

/** A {@link TransitionFactory} implementation that generates the transition. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@
public final class NoTransition implements PatchTransition {

@SerializationConstant public static final NoTransition INSTANCE = new NoTransition();
private static final TransitionFactory<? extends TransitionFactory.Data> FACTORY_INSTANCE =
new AutoValue_NoTransition_Factory<>();

private NoTransition() {}

Expand All @@ -33,8 +35,10 @@ public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
}

/** Returns a {@link TransitionFactory} instance that generates the no transition. */
public static <T extends TransitionFactory.Data> TransitionFactory<T> createFactory() {
return new AutoValue_NoTransition_Factory<>();
public static <T extends TransitionFactory.Data> TransitionFactory<T> getFactory() {
@SuppressWarnings("unchecked")
TransitionFactory<T> castFactory = (TransitionFactory<T>) FACTORY_INSTANCE;
return castFactory;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ public class EnvironmentRule implements RuleDefinition {
@Override
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.cfg(NoConfigTransition.createFactory())
.cfg(NoConfigTransition.getFactory())
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
.override(
attr("tags", Types.STRING_LIST)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -321,7 +321,7 @@ private static TransitionFactory<AttributeTransitionData> convertCfg(
StarlarkThread thread, @Nullable Object trans) throws EvalException {
// The most common case is no transition.
if (trans.equals("target") || trans.equals(Starlark.NONE)) {
return NoTransition.createFactory();
return NoTransition.getFactory();
}
// TODO(b/203203933): remove after removing --incompatible_disable_starlark_host_transitions.
if (trans.equals("host")) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public ExecutionTransitionFactory exec(Object execGroupUnchecked) {

@Override
public ConfigurationTransitionApi target() {
return (ConfigurationTransitionApi) NoTransition.createFactory();
return (ConfigurationTransitionApi) NoTransition.getFactory();
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -862,7 +862,7 @@ public static StarlarkRuleFunction createRule(
private static TransitionFactory<RuleTransitionData> convertConfig(@Nullable Object cfg)
throws EvalException {
if (cfg.equals(Starlark.NONE)) {
return NoTransition.createFactory();
return NoTransition.getFactory();
}
if (cfg instanceof StarlarkDefinedConfigTransition starlarkDefinedConfigTransition) {
// defined in Starlark via, cfg = transition
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,7 @@ public static class Builder<TYPE> {
private final String name;
private final Type<TYPE> type;
private TransitionFactory<AttributeTransitionData> transitionFactory =
NoTransition.createFactory();
NoTransition.getFactory();
private RuleClassNamePredicate allowedRuleClassesForLabels = ANY_RULE;
private RuleClassNamePredicate allowedRuleClassesForLabelsWarning = NO_RULE;
private FileTypeSet allowedFileTypesForLabels;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public class ConstraintSettingRule implements RuleDefinition {
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.advertiseStarlarkProvider(ConstraintSettingInfo.PROVIDER.id())
.cfg(NoConfigTransition.createFactory())
.cfg(NoConfigTransition.getFactory())
.exemptFromConstraintChecking("this rule helps *define* a constraint")
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
.removeAttribute(":action_listener")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public class ConstraintValueRule implements RuleDefinition {
public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) {
return builder
.advertiseStarlarkProvider(ConstraintValueInfo.PROVIDER.id())
.cfg(NoConfigTransition.createFactory())
.cfg(NoConfigTransition.getFactory())
.exemptFromConstraintChecking("this rule helps *define* a constraint")
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
.removeAttribute(":action_listener")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
<!-- #END_BLAZE_RULE.NAME --> */
return builder
.advertiseStarlarkProvider(PlatformInfo.PROVIDER.id())
.cfg(NoConfigTransition.createFactory())
.cfg(NoConfigTransition.getFactory())
.exemptFromConstraintChecking("this rule helps *define* a constraint")
.useToolchainResolution(ToolchainResolutionMode.DISABLED)
.removeAttribute(":action_listener")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ public void compose_split_split() {
public void compose_noTrans_first() {
TransitionFactory<StubData> patch = new StubPatchFactory(FLAG_1, "value");
TransitionFactory<StubData> composed =
ComposingTransitionFactory.of(NoTransition.createFactory(), patch);
ComposingTransitionFactory.of(NoTransition.getFactory(), patch);

assertThat(composed).isNotNull();
assertThat(composed).isEqualTo(patch);
Expand All @@ -147,7 +147,7 @@ public void compose_noTrans_first() {
public void compose_noTrans_second() {
TransitionFactory<StubData> patch = new StubPatchFactory(FLAG_1, "value");
TransitionFactory<StubData> composed =
ComposingTransitionFactory.of(patch, NoTransition.createFactory());
ComposingTransitionFactory.of(patch, NoTransition.getFactory());

assertThat(composed).isNotNull();
assertThat(composed).isEqualTo(patch);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public class NoConfigTransitionTest extends BuildViewTestCase {
private static final MockRule NO_CONFIG_RULE =
() ->
MockRule.define(
"no_config_rule", (builder, env) -> builder.cfg(NoConfigTransition.createFactory()));
"no_config_rule", (builder, env) -> builder.cfg(NoConfigTransition.getFactory()));

@Override
protected ConfiguredRuleClassProvider createRuleClassProvider() {
Expand Down

0 comments on commit 870e7c6

Please sign in to comment.