Fail on isolated extension usages without imports (#18793)

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 Co-authored-by: Fabian Meumertzheim <[email protected]>
bazelbuild · Jun 27, 2023 · 1f4fcb0 · 1f4fcb0
1 parent ad7957a
commit 1f4fcb0
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 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
@@ -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<String, ModuleOverride> moduleOverrides = moduleFileGlobals.buildOverrides();
     Map<String, ModuleOverride> 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
@@ -2222,7 +2222,10 @@ private EvaluationResult<SingleExtensionEvalValue> 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
@@ -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<RootModuleFileValue> 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.");
+  }
 }