Fail the build on repository rule errors in a module extension

Fail the build on repository rule errors in a module extension Also emit a Starlark stack trace pointing to the particular call site of the repository rule. Previously, the root cause error would be printed, but the build would continue. Fixes bazelbuild#14526 (comment) Closes bazelbuild#15796. PiperOrigin-RevId: 459065865 Change-Id: I2444378c7857b1031fabfacd00f8246583bb2146
fmeum · Jul 20, 2022 · afce42c · afce42c
1 parent 594962c
commit afce42c
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 0 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlmodRepoRuleFunction.java
@@ -51,7 +51,9 @@
 import java.io.IOException;
 import java.util.Optional;
 import javax.annotation.Nullable;
+import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Module;
+import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkSemantics;
 import net.starlark.java.eval.StarlarkThread.CallStackEntry;
 import net.starlark.java.syntax.Location;
@@ -151,6 +153,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     return BzlmodRepoRuleValue.REPO_RULE_NOT_FOUND_VALUE;
   }
 
+  @Nullable
   private BzlmodRepoRuleValue createRuleFromSpec(
       RepoSpec repoSpec, StarlarkSemantics starlarkSemantics, Environment env)
       throws BzlmodRepoRuleFunctionException, InterruptedException {
@@ -185,6 +188,10 @@ private BzlmodRepoRuleValue createNativeRepoRule(
       rule =
           RuleFactory.createAndAddRule(
               pkg, ruleClass, attributeValues, env.getListener(), semantics, callStack.build());
+      if (rule.containsErrors()) {
+        throw Starlark.errorf(
+            "failed to instantiate '%s' from this module extension", ruleClass.getName());
+      }
       return new BzlmodRepoRuleValue(pkg.build(), rule.getName());
     } catch (InvalidRuleException e) {
       throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
@@ -193,6 +200,8 @@ private BzlmodRepoRuleValue createNativeRepoRule(
     } catch (NameConflictException e) {
       // This literally cannot happen -- we just created the package!
       throw new IllegalStateException(e);
+    } catch (EvalException e) {
+      throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
     }
   }
 
@@ -310,5 +319,9 @@ private static final class BzlmodRepoRuleFunctionException extends SkyFunctionEx
     BzlmodRepoRuleFunctionException(IOException e, Transience transience) {
       super(e, transience);
     }
+
+    BzlmodRepoRuleFunctionException(EvalException e, Transience transience) {
+      super(e, transience);
+    }
   }
 }
diff --git a/src/test/py/bazel/bzlmod/bazel_module_test.py b/src/test/py/bazel/bzlmod/bazel_module_test.py
@@ -332,6 +332,35 @@ def testCheckDirectDependencies(self):
         'ERROR: For repository \'B\', the root module requires module version B@1.0, but got B@1.1 in the resolved dependency graph.',
         stderr)
 
+  def testRepositoryRuleErrorInModuleExtensionFailsTheBuild(self):
+    self.ScratchFile('MODULE.bazel', [
+        'module_ext = use_extension("//pkg:extension.bzl", "module_ext")',
+        'use_repo(module_ext, "foo")',
+    ])
+    self.ScratchFile('pkg/BUILD.bazel')
+    self.ScratchFile('pkg/rules.bzl', [
+        'def _repo_rule_impl(ctx):',
+        '    ctx.file("WORKSPACE")',
+        'repo_rule = repository_rule(implementation = _repo_rule_impl)',
+    ])
+    self.ScratchFile('pkg/extension.bzl', [
+        'load(":rules.bzl", "repo_rule")',
+        'def _module_ext_impl(ctx):',
+        '    repo_rule(name = "foo", invalid_attr = "value")',
+        'module_ext = module_extension(implementation = _module_ext_impl)',
+    ])
+    exit_code, _, stderr = self.RunBazel(['run', '@foo//...'],
+                                         allow_failure=True)
+    self.AssertExitCode(exit_code, 48, stderr)
+    self.assertIn(
+        "ERROR: <builtin>: //pkg:@.module_ext.foo: no such attribute 'invalid_attr' in 'repo_rule' rule",
+        stderr)
+    self.assertTrue(
+        any([
+            '/pkg/extension.bzl", line 3, column 14, in _module_ext_impl'
+            in line for line in stderr
+        ]))
+
 
 if __name__ == '__main__':
   unittest.main()