Fix bug in subrules that permitted a subrule to call another without …

…declaring a dependency on it PiperOrigin-RevId: 580162487 Change-Id: Id44e0b7e957113efcc7e72a96e762fb035411225
bazelbuild · Nov 7, 2023 · 804c503 · 804c503
1 parent b1184e7
commit 804c503
Show file tree

Hide file tree

Showing 3 changed files with 14 additions and 6 deletions.
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
@@ -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
@@ -107,6 +107,9 @@ public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwarg
         BazelRuleAnalysisThreadContext.fromOrFail(thread, getName())
             .getRuleContext()
             .getStarlarkRuleContext();
+    if (ruleContext.getLockedForSubrule()) {
+      throw Starlark.errorf("subrules cannot call other subrules");
+    }
     ImmutableSet<? extends StarlarkSubruleApi> 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
@@ -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