Include actual MODULE.bazel location in stack traces

Instead of `<name>@<version>/MODULE.bazel`, the actual location of a MODULE.bazel file is now reported in stack traces, either as a file path or a URL. Closes #18572. PiperOrigin-RevId: 538597414 Change-Id: I038e070d9bd4397bba2ed491597cadd0b56d6481
bazelbuild · Jun 7, 2023 · 2659f49 · 2659f49
1 parent 10fb5f6
commit 2659f49
Show file tree

Hide file tree

Showing 10 changed files with 110 additions and 47 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
@@ -16,6 +16,7 @@ java_library(
     srcs = [
         "ArchiveRepoSpecBuilder.java",
         "AttributeValues.java",
+        "ModuleFile.java",
         "ModuleKey.java",
         "RepoSpec.java",
         "Version.java",

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
@@ -88,12 +88,12 @@ private Optional<byte[]> grabFile(String url, ExtendedEventHandler eventHandler)
   }
 
   @Override
-  public Optional<byte[]> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler)
+  public Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler)
       throws IOException, InterruptedException {
-    return grabFile(
+    String url =
         constructUrl(
-            getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel"),
-        eventHandler);
+            getUrl(), "modules", key.getName(), key.getVersion().toString(), "MODULE.bazel");
+    return grabFile(url, eventHandler).map(content -> ModuleFile.create(content, url));
   }
 
   /** Represents fields available in {@code bazel_registry.json} for the registry. */

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFile.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFile.java
@@ -0,0 +1,33 @@
+// Copyright 2023 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+package com.google.devtools.build.lib.bazel.bzlmod;
+
+import com.google.auto.value.AutoValue;
+
+/** A MODULE.bazel file's content and location. */
+@AutoValue
+public abstract class ModuleFile {
+  /** The raw content of the module file. */
+  @SuppressWarnings("mutable")
+  public abstract byte[] getContent();
+
+  /** The user-facing location of the module file, e.g. a file system path or URL. */
+  public abstract String getLocation();
+
+  public static ModuleFile create(byte[] content, String location) {
+    return new AutoValue_ModuleFile(content, location);
+  }
+}
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
@@ -110,11 +110,11 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       return null;
     }
     String moduleFileHash =
-        new Fingerprint().addBytes(getModuleFileResult.moduleFileContents).hexDigestAndReset();
+        new Fingerprint().addBytes(getModuleFileResult.moduleFile.getContent()).hexDigestAndReset();
 
     ModuleFileGlobals moduleFileGlobals =
         execModuleFile(
-            getModuleFileResult.moduleFileContents,
+            getModuleFileResult.moduleFile,
             getModuleFileResult.registry,
             moduleKey,
             // Dev dependencies should always be ignored if the current module isn't the root module
@@ -151,8 +151,8 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir
     if (env.getValue(FileValue.key(moduleFilePath)) == null) {
       return null;
     }
-    byte[] moduleFile = readFile(moduleFilePath.asPath());
-    String moduleFileHash = new Fingerprint().addBytes(moduleFile).hexDigestAndReset();
+    ModuleFile moduleFile = readModuleFile(moduleFilePath.asPath());
+    String moduleFileHash = new Fingerprint().addBytes(moduleFile.getContent()).hexDigestAndReset();
     ModuleFileGlobals moduleFileGlobals =
         execModuleFile(
             moduleFile,
@@ -189,15 +189,15 @@ private SkyValue computeForRootModule(StarlarkSemantics starlarkSemantics, Envir
   }
 
   private ModuleFileGlobals execModuleFile(
-      byte[] moduleFile,
+      ModuleFile moduleFile,
       @Nullable Registry registry,
       ModuleKey moduleKey,
       boolean ignoreDevDeps,
       StarlarkSemantics starlarkSemantics,
       Environment env)
       throws ModuleFileFunctionException, InterruptedException {
     StarlarkFile starlarkFile =
-        StarlarkFile.parse(ParserInput.fromUTF8(moduleFile, moduleKey + "/MODULE.bazel"));
+        StarlarkFile.parse(ParserInput.fromUTF8(moduleFile.getContent(), moduleFile.getLocation()));
     if (!starlarkFile.ok()) {
       Event.replayEventsOn(env.getListener(), starlarkFile.errors());
       throw errorf(Code.BAD_MODULE, "error parsing MODULE.bazel file for %s", moduleKey);
@@ -224,7 +224,7 @@ private ModuleFileGlobals execModuleFile(
   }
 
   private static class GetModuleFileResult {
-    byte[] moduleFileContents;
+    ModuleFile moduleFile;
     // `registry` can be null if this module has a non-registry override.
     @Nullable Registry registry;
   }
@@ -249,7 +249,7 @@ private GetModuleFileResult getModuleFile(
         return null;
       }
       GetModuleFileResult result = new GetModuleFileResult();
-      result.moduleFileContents = readFile(moduleFilePath.asPath());
+      result.moduleFile = readModuleFile(moduleFilePath.asPath());
       return result;
     }
 
@@ -285,11 +285,11 @@ private GetModuleFileResult getModuleFile(
     GetModuleFileResult result = new GetModuleFileResult();
     for (Registry registry : registryObjects) {
       try {
-        Optional<byte[]> moduleFile = registry.getModuleFile(key, env.getListener());
-        if (!moduleFile.isPresent()) {
+        Optional<ModuleFile> moduleFile = registry.getModuleFile(key, env.getListener());
+        if (moduleFile.isEmpty()) {
           continue;
         }
-        result.moduleFileContents = moduleFile.get();
+        result.moduleFile = moduleFile.get();
         result.registry = registry;
         return result;
       } catch (IOException e) {
@@ -301,9 +301,10 @@ private GetModuleFileResult getModuleFile(
     throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key);
   }
 
-  private static byte[] readFile(Path path) throws ModuleFileFunctionException {
+  private static ModuleFile readModuleFile(Path path) throws ModuleFileFunctionException {
     try {
-      return FileSystemUtils.readWithKnownFileSize(path, path.getFileSize());
+      return ModuleFile.create(
+          FileSystemUtils.readWithKnownFileSize(path, path.getFileSize()), path.getPathString());
     } catch (IOException e) {
       throw errorf(Code.MODULE_NOT_FOUND, "MODULE.bazel expected but not found at %s", path);
     }

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java
@@ -31,7 +31,7 @@ public interface Registry {
    * Retrieves the contents of the module file of the module identified by {@code key} from the
    * registry. Returns {@code Optional.empty()} when the module is not found in this registry.
    */
-  Optional<byte[]> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler)
+  Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler)
       throws IOException, InterruptedException;
 
   /**

diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java
@@ -63,8 +63,16 @@ public String getUrl() {
   }
 
   @Override
-  public Optional<byte[]> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler) {
-    return Optional.ofNullable(modules.get(key)).map(value -> value.getBytes(UTF_8));
+  public Optional<ModuleFile> getModuleFile(ModuleKey key, ExtendedEventHandler eventHandler) {
+    return Optional.ofNullable(modules.get(key))
+        .map(value -> value.getBytes(UTF_8))
+        .map(
+            content ->
+                ModuleFile.create(
+                    content,
+                    String.format(
+                        "%s/modules/%s/%s/MODULE.bazel",
+                        url, key.getName(), key.getVersion().toString())));
   }
 
   @Override

diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java
@@ -72,7 +72,9 @@ public void testHttpUrl() throws Exception {
 
     Registry registry = registryFactory.getRegistryWithUrl(server.getUrl() + "/myreg");
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter))
-        .hasValue("lol".getBytes(UTF_8));
+        .hasValue(
+            ModuleFile.create(
+                "lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
     assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter)).isEmpty();
   }
 
@@ -94,7 +96,9 @@ public void testHttpUrlWithNetrcCreds() throws Exception {
 
     downloadManager.setNetrcCreds(new NetrcCredentials(netrc));
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter))
-        .hasValue("lol".getBytes(UTF_8));
+        .hasValue(
+            ModuleFile.create(
+                "lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
     assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter)).isEmpty();
   }
 
@@ -110,7 +114,7 @@ public void testFileUrl() throws Exception {
         registryFactory.getRegistryWithUrl(
             new File(tempFolder.getRoot(), "fakereg").toURI().toString());
     assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter))
-        .hasValue("lol".getBytes(UTF_8));
+        .hasValue(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString()));
     assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter)).isEmpty();
   }
 

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
@@ -1191,7 +1191,7 @@ public void importNonExistentRepo() throws Exception {
         .contains(
             "module extension \"ext\" from \"//:defs.bzl\" does not generate repository"
                 + " \"missing_repo\", yet it is imported as \"my_repo\" in the usage at"
-                + " <root>/MODULE.bazel:1:20");
+                + " /ws/MODULE.bazel:1:20");
   }
 
   @Test
@@ -1658,7 +1658,7 @@ public void extensionMetadata() throws Exception {
 
     assertEventCount(1, eventCollector);
     assertContainsEvent(
-        "WARNING <root>/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
+        "WARNING /ws/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
             + " reported incorrect imports of repositories via use_repo():\n"
             + "\n"
             + "Imported, but not created by the extension (will cause the build to fail):\n"
@@ -1736,11 +1736,11 @@ public void extensionMetadata_all() throws Exception {
         .isEqualTo(
             "module extension \"ext\" from \"@ext~1.0//:defs.bzl\" does not generate repository "
                 + "\"invalid_dep\", yet it is imported as \"invalid_dep\" in the usage at "
-                + "<root>/MODULE.bazel:3:20");
+                + "/ws/MODULE.bazel:3:20");
 
     assertEventCount(1, eventCollector);
     assertContainsEvent(
-        "WARNING <root>/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
+        "WARNING /ws/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
             + " reported incorrect imports of repositories via use_repo():\n"
             + "\n"
             + "Imported, but not created by the extension (will cause the build to fail):\n"
@@ -1815,11 +1815,11 @@ public void extensionMetadata_allDev() throws Exception {
         .isEqualTo(
             "module extension \"ext\" from \"@ext~1.0//:defs.bzl\" does not generate repository "
                 + "\"invalid_dep\", yet it is imported as \"invalid_dep\" in the usage at "
-                + "<root>/MODULE.bazel:3:20");
+                + "/ws/MODULE.bazel:3:20");
 
     assertEventCount(1, eventCollector);
     assertContainsEvent(
-        "WARNING <root>/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
+        "WARNING /ws/MODULE.bazel:3:20: The module extension ext defined in @ext//:defs.bzl"
             + " reported incorrect imports of repositories via use_repo():\n"
             + "\n"
             + "Imported, but not created by the extension (will cause the build to fail):\n"