From 31f36262a8c5011a402b2286076ed6dcc5f94092 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Mon, 26 Jun 2023 11:33:48 -0700 Subject: [PATCH] Fail on isolated extension usages without imports Such usages have no effect and are never evaluated. Since this may not be obvious and `use_repo` fixup messages are only emitted when the extension is evaluated, fail with an error message instructing users to add a `use_repo`. Closes #18761. PiperOrigin-RevId: 543496257 Change-Id: Ia4b4ece2c15e9e5a139d2a97dcc7baba1b128a70 --- .../lib/bazel/bzlmod/ModuleFileFunction.java | 12 +++++++++++ .../bzlmod/ModuleExtensionResolutionTest.java | 5 ++++- .../bazel/bzlmod/ModuleFileFunctionTest.java | 20 +++++++++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index f9c1e0022e2c85..c436a15da12ef6 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -162,6 +162,18 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir starlarkSemantics, env); InterimModule module = moduleFileGlobals.buildModule(); + for (ModuleExtensionUsage usage : module.getExtensionUsages()) { + if (usage.getIsolationKey().isPresent() && usage.getImports().isEmpty()) { + throw errorf( + Code.BAD_MODULE, + "the isolated usage at %s of extension %s defined in %s has no effect as no " + + "repositories are imported from it. Either import one or more repositories " + + "generated by the extension with use_repo or remove the usage.", + usage.getLocation(), + usage.getExtensionName(), + usage.getExtensionBzlFile()); + } + } ImmutableMap moduleOverrides = moduleFileGlobals.buildOverrides(); Map commandOverrides = MODULE_OVERRIDES.get(env); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index a2f78bd8b11405..31fce676ac7875 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -2222,7 +2222,10 @@ private EvaluationResult evaluateIsolatedModuleExtensi workspaceRoot.getRelative("MODULE.bazel").getPathString(), String.format( "ext = use_extension('//:defs.bzl', 'ext', dev_dependency = %s, isolate = %s)", - devDependencyStr, isolateStr)); + devDependencyStr, isolateStr), + // Isolated module extensions without repo imports result in an error in + // ModuleFileFunction. + "use_repo(ext, 'some_repo')"); scratch.file( workspaceRoot.getRelative("defs.bzl").getPathString(), "def _ext_impl(ctx):", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 129094a98bd422..1c251bf970258f 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -1032,4 +1032,24 @@ public void module_calledLate() throws Exception { assertContainsEvent("if module() is called, it must be called before any other functions"); } + + @Test + public void isolatedExtensionWithoutImports() throws Exception { + scratch.file( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + "isolated_ext = use_extension('//:extensions.bzl', 'my_ext', isolate = True)"); + FakeRegistry registry = registryFactory.newFakeRegistry("/foo"); + ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl())); + + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + assertThat(result.getError().toString()) + .contains( + "the isolated usage at /workspace/MODULE.bazel:1:29 of extension my_ext defined in " + + "@//:extensions.bzl has no effect as no repositories are imported from it. " + + "Either import one or more repositories generated by the extension with " + + "use_repo or remove the usage."); + } }