diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java index 847592a7c02f4c..a45e846d4065cb 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleTransitionProvider.java @@ -25,7 +25,6 @@ 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.cmdline.Label; import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.packages.Attribute; @@ -71,13 +70,13 @@ public StarlarkDefinedConfigTransition getStarlarkDefinedConfigTransitionForTest */ private static class CacheKey { private final StarlarkDefinedConfigTransition starlarkDefinedConfigTransition; - private final Label ruleLabel; + private final Rule rule; private final int hashCode; CacheKey(StarlarkDefinedConfigTransition starlarkDefinedConfigTransition, Rule rule) { this.starlarkDefinedConfigTransition = starlarkDefinedConfigTransition; - this.ruleLabel = rule.getLabel(); - this.hashCode = Objects.hash(starlarkDefinedConfigTransition, ruleLabel); + this.rule = rule; + this.hashCode = Objects.hash(starlarkDefinedConfigTransition, rule); } @Override @@ -90,7 +89,7 @@ public boolean equals(Object other) { } return (this.starlarkDefinedConfigTransition.equals( ((CacheKey) other).starlarkDefinedConfigTransition) - && this.ruleLabel.equals(((CacheKey) other).ruleLabel)); + && this.rule.equals(((CacheKey) other).rule)); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java index d93d501ee1714a..0edc1d9bde6633 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/StarlarkRuleTransitionProviderTest.java @@ -30,6 +30,9 @@ import com.google.devtools.build.lib.skyframe.ConfiguredTargetAndData; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; +import com.google.devtools.build.lib.vfs.ModifiedFileSet; +import com.google.devtools.build.lib.vfs.PathFragment; +import com.google.devtools.build.lib.vfs.Root; import com.google.testing.junit.testparameterinjector.TestParameterInjector; import com.google.testing.junit.testparameterinjector.TestParameters; import java.util.List; @@ -1691,4 +1694,67 @@ public void testNoPlatformChange() throws Exception { .platforms) .containsExactly(Label.parseAbsoluteUnchecked("//platforms:my_platform")); } + + @Test + public void testTransitionsStillTriggerWhenOnlyRuleAttributesChange() throws Exception { + scratch.file( + "test/defs.bzl", + "def _transition_impl(settings, attr):", + " return {", + " '//command_line_option:foo': attr.my_attr,", + " }", + "_my_transition = transition(", + " implementation = _transition_impl,", + " inputs = [],", + " outputs = [", + " '//command_line_option:foo',", + " ]", + ")", + "def _rule_impl(ctx):", + " return []", + "my_rule = rule(", + " implementation = _rule_impl,", + " cfg = _my_transition,", + " attrs = {", + " 'my_attr': attr.string(),", + " '_allowlist_function_transition': attr.label(", + " default = '//tools/allowlists/function_transition_allowlist',", + " ),", + " },", + ")"); + writeAllowlistFile(); + + scratch.file( + "test/BUILD", + "load('//test:defs.bzl', 'my_rule')", + "my_rule(", + " name = 'buildme',", + " my_attr = 'first build',", + ")"); + assertThat( + getConfiguration(getConfiguredTarget("//test:buildme")) + .getOptions() + .get(DummyTestOptions.class) + .foo) + .isEqualTo("first build"); + + scratch.overwriteFile( + "test/BUILD", + "load('//test:defs.bzl', 'my_rule')", + "my_rule(", + " name = 'buildme',", + " my_attr = 'second build',", + ")"); + skyframeExecutor.invalidateFilesUnderPathForTesting( + reporter, + ModifiedFileSet.builder().modify(PathFragment.create("test/BUILD")).build(), + Root.fromPath(rootDirectory)); + + assertThat( + getConfiguration(getConfiguredTarget("//test:buildme")) + .getOptions() + .get(DummyTestOptions.class) + .foo) + .isEqualTo("second build"); + } }