diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionFactories.java b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionFactories.java index 6774117ef58284..a646913e117432 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionFactories.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/TransitionFactories.java @@ -58,6 +58,11 @@ abstract static class IdentityFactory public ConfigurationTransition create(T data) { return transition(); } + + @Override + public TransitionType transitionType() { + return TransitionType.ANY; + } } /** A {@link TransitionFactory} implementation that wraps a split transition. */ @@ -66,6 +71,12 @@ abstract static class SplitTransitionFactory implements TransitionFactory { abstract SplitTransition splitTransition(); + @Override + public TransitionType transitionType() { + // Splits can only be used on attributes. + return TransitionType.ATTRIBUTE; + } + @Override public SplitTransition create(T data) { return splitTransition(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactory.java index 055ea54c5b1b21..8fa8ce7f1b7803 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/ComposingTransitionFactory.java @@ -87,6 +87,12 @@ public ConfigurationTransition create(T data) { return new ComposingTransition(transition1, transition2); } + @Override + public TransitionType transitionType() { + // Both types must match so this is correct. + return transitionFactory1().transitionType(); + } + abstract TransitionFactory transitionFactory1(); abstract TransitionFactory transitionFactory2(); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransition.java index dc5ca8b625ba4e..8459170ca9f237 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransition.java @@ -69,6 +69,11 @@ public PatchTransition create(T unused) { return INSTANCE; } + @Override + public TransitionType transitionType() { + return TransitionType.ANY; + } + @Override public boolean isTool() { return true; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java index 8980f1b57e21dc..baa6e175a00256 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NoTransition.java @@ -52,5 +52,10 @@ abstract static class Factory implements Trans public ConfigurationTransition create(T unused) { return INSTANCE; } + + @Override + public TransitionType transitionType() { + return TransitionType.ANY; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java index f08252af0f508c..75b7bbce4ea945 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/NullTransition.java @@ -57,5 +57,10 @@ abstract static class Factory implements Trans public ConfigurationTransition create(T unused) { return INSTANCE; } + + @Override + public TransitionType transitionType() { + return TransitionType.ANY; + } } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionFactory.java index 0d15a9a09c0fd1..73d01fbb12192e 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionFactory.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/config/transitions/TransitionFactory.java @@ -57,9 +57,11 @@ interface Data {} /** Returns a new {@link ConfigurationTransition}, based on the given data. */ ConfigurationTransition create(T data); - default TransitionType transitionType() { - return TransitionType.ANY; - } + /** + * Returns a {@link TransitionType} to clarify what data (if any) the factory requires to create a + * transation. + */ + TransitionType transitionType(); // TODO(https://github.com/bazelbuild/bazel/issues/7814): Once everything uses TransitionFactory, // remove these methods. 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 e3ad389724c32a..c2ad346bda7c06 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 @@ -36,6 +36,7 @@ import com.google.devtools.build.lib.analysis.config.Fragment; import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement; import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; +import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory.TransitionType; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -1142,6 +1143,8 @@ public Builder cfg(TransitionFactory transitionFactory) { name); Preconditions.checkState(this.transitionFactory == null, "Property cfg has already been set"); Preconditions.checkNotNull(transitionFactory); + Preconditions.checkArgument( + transitionFactory.transitionType().isCompatibleWith(TransitionType.RULE)); Preconditions.checkArgument(!transitionFactory.isSplit()); this.transitionFactory = transitionFactory; return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidPlatformsTransition.java b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidPlatformsTransition.java index 63cd625a543235..5996bc3aa69f7d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/android/AndroidPlatformsTransition.java +++ b/src/main/java/com/google/devtools/build/lib/rules/android/AndroidPlatformsTransition.java @@ -43,6 +43,11 @@ public static final class AndroidPlatformsTransitionFactory public PatchTransition create(RuleTransitionData unused) { return INSTANCE; } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; + } } public static TransitionFactory create() { diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/BUILD b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/BUILD index 6633467269e7fb..7253d16d136270 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/BUILD +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/BUILD @@ -18,6 +18,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/starlark_exposed_rule_transition_factory", + "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory", "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/packages", diff --git a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigFeatureFlagTransitionFactory.java b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigFeatureFlagTransitionFactory.java index 26d4598cead8db..274a595c16fdc4 100644 --- a/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigFeatureFlagTransitionFactory.java +++ b/src/main/java/com/google/devtools/build/skydoc/fakebuildapi/config/FakeConfigFeatureFlagTransitionFactory.java @@ -31,4 +31,9 @@ public void addToStarlarkRule(RuleDefinitionEnvironment ctx, RuleClass.Builder b public PatchTransition create(RuleTransitionData ruleData) { return null; } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; + } } diff --git a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java index 977854c92bb32f..d72417d5b45c65 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/AnalysisCachingTest.java @@ -27,11 +27,14 @@ import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.config.RequiresOptions; +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.PatchTransition; +import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; import com.google.devtools.build.lib.analysis.util.AnalysisCachingTestBase; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; +import com.google.devtools.build.lib.packages.RuleTransitionData; import com.google.devtools.build.lib.rules.java.JavaInfo; import com.google.devtools.build.lib.rules.java.JavaSourceJarsProvider; import com.google.devtools.build.lib.testutil.TestConstants; @@ -1073,11 +1076,19 @@ private void setupDiffResetTesting() throws Exception { return !optionsThatCanChange.contains(changedOption); }); builder.overrideTrimmingTransitionFactoryForTesting( - (ruleData) -> { - if (ruleData.rule().getRuleClassObject().getName().equals("uses_irrelevant")) { - return NoTransition.INSTANCE; + new TransitionFactory<>() { + @Override + public ConfigurationTransition create(RuleTransitionData ruleData) { + if (ruleData.rule().getRuleClassObject().getName().equals("uses_irrelevant")) { + return NoTransition.INSTANCE; + } + return DiffResetOptions.CLEAR_IRRELEVANT; + } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; } - return DiffResetOptions.CLEAR_IRRELEVANT; }); useRuleClassProvider(builder.build()); scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/BUILD index 4fb2fcae2b125c..a5c437ee0d9720 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/BUILD +++ b/src/test/java/com/google/devtools/build/lib/analysis/BUILD @@ -441,6 +441,7 @@ java_test( "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", + "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory", "//src/main/java/com/google/devtools/build/lib/analysis:configured_target", "//src/main/java/com/google/devtools/build/lib/analysis/platform", diff --git a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java index 7866b61eeb13fd..1fe61d80249ef3 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java @@ -306,7 +306,7 @@ def _impl(ctx): .mandatory() .allowedFileTypes() .cfg( - new TransitionFactory() { + new TransitionFactory<>() { @Override public SplitTransition create(AttributeTransitionData data) { return new SplitTransition() { @@ -332,6 +332,11 @@ public Map split( }; } + @Override + public TransitionType transitionType() { + return TransitionType.ATTRIBUTE; + } + @Override public boolean isSplit() { return true; diff --git a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransitionTest.java b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransitionTest.java index 93eafe0cff6edb..b0adfe9ba49d52 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransitionTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/config/transitions/NoConfigTransitionTest.java @@ -32,8 +32,7 @@ public class NoConfigTransitionTest extends BuildViewTestCase { private static final MockRule NO_CONFIG_RULE = () -> MockRule.define( - "no_config_rule", - (builder, env) -> builder.cfg(unused -> NoConfigTransition.INSTANCE)); + "no_config_rule", (builder, env) -> builder.cfg(NoConfigTransition.createFactory())); @Override protected ConfiguredRuleClassProvider createRuleClassProvider() { diff --git a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java index c7d6273597a78b..749fcc176d8f23 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubruleTest.java @@ -28,13 +28,14 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; +import com.google.devtools.build.lib.analysis.config.transitions.ConfigurationTransition; import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory; -import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory.Data; import com.google.devtools.build.lib.analysis.platform.ToolchainInfo; import com.google.devtools.build.lib.analysis.util.BuildViewTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.packages.Attribute; +import com.google.devtools.build.lib.packages.AttributeTransitionData; import com.google.devtools.build.lib.packages.AttributeValueSource; import com.google.devtools.build.lib.packages.Provider; import com.google.devtools.build.lib.packages.StarlarkInfo; @@ -701,7 +702,19 @@ public void testSubruleAttrs_cannotHaveStarlarkTransitions() throws Exception { @Test public void testSubruleAttrs_cannotHaveNativeTransitions() throws Exception { - ev.update("native_transition", (TransitionFactory) data -> null); + ev.update( + "native_transition", + new TransitionFactory() { + @Override + public ConfigurationTransition create(AttributeTransitionData data) { + return null; + } + + @Override + public TransitionType transitionType() { + return TransitionType.ATTRIBUTE; + } + }); ev.checkEvalErrorContains( "bad cfg for attribute '_foo': subrules may only have target/exec attributes.", "_my_subrule = subrule(", diff --git a/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java b/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java index 6a07722c1c4e96..61179d7ce1b735 100644 --- a/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java +++ b/src/test/java/com/google/devtools/build/lib/buildtool/ConvenienceSymlinkTest.java @@ -123,6 +123,11 @@ public PatchTransition create(RuleTransitionData ruleData) { return new PathTransition( NonconfigurableAttributeMapper.of(ruleData.rule()).get("path", STRING)); } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; + } } private static final class UselessOptionTransition implements PatchTransition { @@ -152,6 +157,11 @@ public PatchTransition create(RuleTransitionData ruleData) { return new UselessOptionTransition( NonconfigurableAttributeMapper.of(ruleData.rule()).get("value", STRING)); } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; + } } private static final class PathTestRulesModule extends BlazeModule { diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD index 8fd9a3f0592629..22e724d9236387 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/BUILD @@ -162,6 +162,7 @@ java_test( ":configured_target_query_test", "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster", "//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories", + "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory", "//src/main/java/com/google/devtools/build/lib/events", diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java index 663b8f6e108c53..050660073ce2a9 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/ConfiguredTargetQuerySemanticsTest.java @@ -287,7 +287,9 @@ public void testTopLevelTransition() throws Exception { MockRule.define( "rule_class_transition", (builder, env) -> - builder.cfg(unused -> new FooPatchTransition("SET BY PATCH")).build()); + builder + .cfg(TransitionFactories.of(new FooPatchTransition("SET BY PATCH"))) + .build()); helper.useRuleClassProvider(setRuleClassProviders(ruleClassTransition).build()); helper.setUniverseScope("//test:rule_class"); diff --git a/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java b/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java index 56f7b368f78c63..a462778acc1821 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/cquery/TransitionsOutputFormatterTest.java @@ -21,6 +21,7 @@ import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider; import com.google.devtools.build.lib.analysis.config.TransitionFactories; +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.TransitionFactory; import com.google.devtools.build.lib.analysis.util.MockRule; @@ -219,12 +220,20 @@ public void nonAttributeDependencySkipped() throws Exception { private void setUpRules() throws Exception { TransitionFactory infixTrimmingTransitionFactory = - (ruleData) -> { - if (!ruleData.rule().getName().contains("trimmed")) { - return NoTransition.INSTANCE; + new TransitionFactory<>() { + @Override + public ConfigurationTransition create(RuleTransitionData ruleData) { + if (!ruleData.rule().getName().contains("trimmed")) { + return NoTransition.INSTANCE; + } + // rename the transition so it's distinguishable from the others in tests + return new FooPatchTransition("SET BY TRIM", "FooPatchTransition(trim)"); + } + + @Override + public TransitionType transitionType() { + return TransitionType.RULE; } - // rename the transition so it's distinguishable from the others in tests - return new FooPatchTransition("SET BY TRIM", "FooPatchTransition(trim)"); }; FooPatchTransition ruleClassTransition = new FooPatchTransition("SET BY RULE CLASS PATCH"); FooPatchTransition attributePatchTransition = new FooPatchTransition("SET BY PATCH"); @@ -237,7 +246,7 @@ private void setUpRules() throws Exception { "my_rule", (builder, env) -> builder - .cfg(unused -> ruleClassTransition) + .cfg(TransitionFactories.of(ruleClassTransition)) .add( attr("patched", LABEL_LIST) .allowedFileTypes(FileTypeSet.ANY_FILE) diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/query2/testutil/BUILD index ec85754b670aa7..b4b98030786a8c 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/BUILD @@ -25,6 +25,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options", "//src/main/java/com/google/devtools/build/lib/analysis:config/toolchain_type_requirement", + "//src/main/java/com/google/devtools/build/lib/analysis:config/transition_factories", "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition", "//src/main/java/com/google/devtools/build/lib/analysis:server_directories", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", diff --git a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java index a11923f1644edc..dad914811c9538 100644 --- a/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java +++ b/src/test/java/com/google/devtools/build/lib/query2/testutil/PostAnalysisQueryTest.java @@ -27,6 +27,7 @@ import com.google.devtools.build.lib.analysis.config.BuildOptionsView; import com.google.devtools.build.lib.analysis.config.FragmentOptions; import com.google.devtools.build.lib.analysis.config.ToolchainTypeRequirement; +import com.google.devtools.build.lib.analysis.config.TransitionFactories; import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition; import com.google.devtools.build.lib.analysis.util.DummyTestFragment.DummyTestOptions; import com.google.devtools.build.lib.analysis.util.MockRule; @@ -618,7 +619,9 @@ public void testMultipleTopLevelConfigurations() throws Exception { MockRule.define( "transitioned_rule", (builder, env) -> - builder.cfg(unused -> new FooPatchTransition("SET BY PATCH")).build()); + builder + .cfg(TransitionFactories.of(new FooPatchTransition("SET BY PATCH"))) + .build()); MockRule untransitionedRule = () -> MockRule.define("untransitioned_rule"); @@ -653,7 +656,7 @@ public void testMultipleTopLevelConfigurations_multipleConfigsPrefersTopLevel() "rule_with_transition_and_dep", (builder, env) -> builder - .cfg(unused -> new FooPatchTransition("SET BY PATCH")) + .cfg(TransitionFactories.of(new FooPatchTransition("SET BY PATCH"))) .addAttribute( attr("dep", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE).build()) .build());