diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java index 61959585c03dff..305742817d873a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleContext.java @@ -332,6 +332,10 @@ void setLockedForSubrule(boolean isLocked) { this.lockedForSubruleEvaluation = isLocked; } + boolean getLockedForSubrule() { + return lockedForSubruleEvaluation; + } + /** * Represents `ctx.outputs`. * diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java index 8661a3ea400512..fda5f4f5f15ba1 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkSubrule.java @@ -107,6 +107,9 @@ public Object call(StarlarkThread thread, Tuple args, Dict kwarg BazelRuleAnalysisThreadContext.fromOrFail(thread, getName()) .getRuleContext() .getStarlarkRuleContext(); + if (ruleContext.getLockedForSubrule()) { + throw Starlark.errorf("subrules cannot call other subrules"); + } ImmutableSet declaredSubrules = ruleContext.getSubrules(); if (!declaredSubrules.contains(this)) { throw getUndeclaredSubruleError(ruleContext); 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 1b8e2bb0d7c2f9..f11c53405bdeab 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 @@ -150,9 +150,8 @@ public void testSubrule_aspectMustDeclareSubrule() throws Exception { + " declare '_my_subrule' in 'subrules'"); } - // this is a bug that must be fixed @Test - public void testSubruleCallingSibling_succeeds() throws Exception { + public void testSubruleCallingUndeclaredSibling_fails() throws Exception { scratch.file( "subrule_testing/myrule.bzl", "def _subrule1_impl(ctx):", @@ -175,11 +174,13 @@ public void testSubruleCallingSibling_succeeds() throws Exception { "load('myrule.bzl', 'my_rule')", "my_rule(name = 'foo')"); - StructImpl provider = - getProvider("//subrule_testing:foo", "//subrule_testing:myrule.bzl", "MyInfo"); + AssertionError error = + assertThrows(AssertionError.class, () -> getConfiguredTarget("//subrule_testing:foo")); - assertThat(provider).isNotNull(); - assertThat(provider.getValue("result")).isEqualTo("result from subrule1"); + assertThat(error).isNotNull(); + assertThat(error) + .hasMessageThat() + .contains("Error in _my_subrule1: subrules cannot call other subrules"); } @Test