Skip to content

Commit

Permalink
Fail the build on repository rule errors in a module extension
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
fmeum committed Jul 3, 2022
1 parent 0152338 commit eac799d
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.util.Map;
import net.starlark.java.eval.EvalException;
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;
Expand All @@ -52,7 +54,7 @@ public static Rule createRule(
String callStackEntry,
RuleClass ruleClass,
Map<String, Object> attributes)
throws InterruptedException, InvalidRuleException, NoSuchPackageException {
throws InterruptedException, InvalidRuleException, NoSuchPackageException, EvalException {
// TODO(bazel-team): Don't use the {@link Rule} class for repository rule.
// Currently, the repository rule is represented with the {@link Rule} class that's designed
// for build rules. Therefore, we have to create a package instance for it, which doesn't make
Expand All @@ -79,6 +81,10 @@ public static Rule createRule(
// This literally cannot happen -- we just created the package!
throw new IllegalStateException(e);
}
if (rule.containsErrors()) {
throw Starlark.errorf(
"failed to instantiate '%s' from this module extension", ruleClass.getName());
}
packageBuilder.build();
return rule;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
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.StarlarkSemantics;
import net.starlark.java.syntax.Location;
Expand Down Expand Up @@ -152,6 +153,8 @@ private BzlmodRepoRuleValue createRuleFromSpec(
throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
} catch (NoSuchPackageException e) {
throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
} catch (EvalException e) {
throw new BzlmodRepoRuleFunctionException(e, Transience.PERSISTENT);
}
}

Expand Down Expand Up @@ -219,5 +222,9 @@ private static final class BzlmodRepoRuleFunctionException extends SkyFunctionEx
BzlmodRepoRuleFunctionException(IOException e, Transience transience) {
super(e, transience);
}

BzlmodRepoRuleFunctionException(EvalException e, Transience transience) {
super(e, transience);
}
}
}
30 changes: 30 additions & 0 deletions src/test/py/bazel/bzlmod/bazel_module_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,36 @@ 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()

0 comments on commit eac799d

Please sign in to comment.