Skip to content

Commit

Permalink
Explain why a module hasn't been found in any registry
Browse files Browse the repository at this point in the history 
This makes it easier for users to discover that a module hasn't been found because its absence was recorded in the lockfile during a previous build.

Fixes bazelbuild#24803

Closes bazelbuild#24804.

PiperOrigin-RevId: 713142592
Change-Id: Id541f5710481bd947c09d8dba315d683a1666b1c
  • Loading branch information
@fmeum @bazel-io
fmeum authored and bazel-io committed Jan 8, 2025
1 parent c8217fd commit 440c226
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 57 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/profiler",
"//src/main/java/com/google/devtools/build/lib/util:os",
Expand Down
57 changes: 34 additions & 23 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum.MissingChecksumException;
import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.profiler.Profiler;
Expand Down Expand Up @@ -131,28 +132,32 @@ private String constructUrl(String base, String... segments) {
return url.toString();
}

/** Grabs a file from the given URL. Returns {@link Optional#empty} if the file doesn't exist. */
private Optional<byte[]> grabFile(
/** Grabs a file from the given URL. Throws {@link NotFoundException} if it doesn't exist. */
private byte[] grabFile(
String url,
ExtendedEventHandler eventHandler,
DownloadManager downloadManager,
boolean useChecksum)
throws IOException, InterruptedException {
var maybeContent = doGrabFile(downloadManager, url, eventHandler, useChecksum);
if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
|| knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
&& useChecksum) {
eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
throws IOException, InterruptedException, NotFoundException {
Optional<byte[]> maybeContent = Optional.empty();
try {
maybeContent = Optional.of(doGrabFile(downloadManager, url, eventHandler, useChecksum));
return maybeContent.get();
} finally {
if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
|| knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
&& useChecksum) {
eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
}
}
return maybeContent;
}

private Optional<byte[]> doGrabFile(
private byte[] doGrabFile(
DownloadManager downloadManager,
String rawUrl,
ExtendedEventHandler eventHandler,
boolean useChecksum)
throws IOException, InterruptedException {
throws IOException, InterruptedException, NotFoundException {
Optional<Checksum> checksum;
if (knownFileHashesMode != KnownFileHashesMode.IGNORE && useChecksum) {
Optional<Checksum> knownChecksum = knownFileHashes.get(rawUrl);
Expand All @@ -174,7 +179,11 @@ private Optional<byte[]> doGrabFile(
checksum = Optional.empty();
} else {
// Guarantee reproducibility by assuming that the file still doesn't exist.
return Optional.empty();
throw new NotFoundException(
String.format(
"%s: previously not found (as recorded in %s, refresh with"
+ " --lockfile_mode=refresh)",
rawUrl, LabelConstants.MODULE_LOCKFILE_NAME));
}
} else {
// The file is known, download with a checksum to potentially obtain a repository cache hit
Expand Down Expand Up @@ -203,7 +212,7 @@ private Optional<byte[]> doGrabFile(
&& !url.getProtocol().equals("file")
&& vendorManager.isUrlVendored(url)) {
try {
return Optional.of(vendorManager.readRegistryUrl(url, checksum.get()));
return vendorManager.readRegistryUrl(url, checksum.get());
} catch (IOException e) {
throw new IOException(
String.format(
Expand All @@ -216,10 +225,9 @@ private Optional<byte[]> doGrabFile(

try (SilentCloseable c =
Profiler.instance().profile(ProfilerTask.BZLMOD, () -> "download file: " + rawUrl)) {
return Optional.of(
downloadManager.downloadAndReadOneUrlForBzlmod(url, eventHandler, clientEnv, checksum));
return downloadManager.downloadAndReadOneUrlForBzlmod(url, eventHandler, clientEnv, checksum);
} catch (FileNotFoundException e) {
return Optional.empty();
throw new NotFoundException(String.format("%s: not found", rawUrl));
} catch (IOException e) {
// Include the URL in the exception message for easier debugging.
throw new IOException(
Expand All @@ -228,14 +236,13 @@ private Optional<byte[]> doGrabFile(
}

@Override
public Optional<ModuleFile> getModuleFile(
public ModuleFile getModuleFile(
ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
throws IOException, InterruptedException {
throws IOException, InterruptedException, NotFoundException {
String url =
constructUrl(getUrl(), "modules", key.name(), key.version().toString(), "MODULE.bazel");
Optional<byte[]> maybeContent =
grabFile(url, eventHandler, downloadManager, /* useChecksum= */ true);
return maybeContent.map(content -> ModuleFile.create(content, url));
byte[] content = grabFile(url, eventHandler, downloadManager, /* useChecksum= */ true);
return ModuleFile.create(content, url);
}

/** Represents fields available in {@code bazel_registry.json} for the registry. */
Expand Down Expand Up @@ -286,8 +293,12 @@ private Optional<String> grabJsonFile(
DownloadManager downloadManager,
boolean useChecksum)
throws IOException, InterruptedException {
return grabFile(url, eventHandler, downloadManager, useChecksum)
.map(value -> new String(value, UTF_8));
try {
return Optional.of(
new String(grabFile(url, eventHandler, downloadManager, useChecksum), UTF_8));
} catch (NotFoundException e) {
return Optional.empty();
}
}

/**
Expand Down
23 changes: 17 additions & 6 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,6 @@
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
Expand All @@ -91,6 +90,7 @@
*/
public class ModuleFileFunction implements SkyFunction {

// Never empty.
public static final Precomputed<ImmutableSet<String>> REGISTRIES =
new Precomputed<>("registries");
public static final Precomputed<Boolean> IGNORE_DEV_DEPS =
Expand Down Expand Up @@ -677,14 +677,21 @@ private GetModuleFileResult getModuleFile(
// Now go through the list of registries and use the first one that contains the requested
// module.
StoredEventHandler downloadEventHandler = new StoredEventHandler();
List<String> notFoundTrace = null;
for (Registry registry : registryObjects) {
try {
Optional<ModuleFile> maybeModuleFile =
registry.getModuleFile(key, downloadEventHandler, this.downloadManager);
if (maybeModuleFile.isEmpty()) {
ModuleFile originalModuleFile;
try {
originalModuleFile =
registry.getModuleFile(key, downloadEventHandler, this.downloadManager);
} catch (Registry.NotFoundException e) {
if (notFoundTrace == null) {
notFoundTrace = new ArrayList<>();
}
notFoundTrace.add(e.getMessage());
continue;
}
ModuleFile moduleFile = maybePatchModuleFile(maybeModuleFile.get(), override, env);
ModuleFile moduleFile = maybePatchModuleFile(originalModuleFile, override, env);
if (moduleFile == null) {
return null;
}
Expand All @@ -698,7 +705,11 @@ private GetModuleFileResult getModuleFile(
}
}

throw errorf(Code.MODULE_NOT_FOUND, "module not found in registries: %s", key);
throw errorf(
Code.MODULE_NOT_FOUND,
"module %s not found in registries:\n* %s",
key,
String.join("\n* ", notFoundTrace));
}

/**
Expand Down
15 changes: 12 additions & 3 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,22 @@ public interface Registry extends NotComparableSkyValue {
/** The URL that uniquely identifies the registry. */
String getUrl();

/** Thrown when a file is not found in the registry. */
final class NotFoundException extends Exception {
public NotFoundException(String message) {
super(message);
}
}

/**
* 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.
* registry.
*
* @throws NotFoundException if the module file is not found in the registry
*/
Optional<ModuleFile> getModuleFile(
ModuleFile getModuleFile(
ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
throws IOException, InterruptedException;
throws IOException, InterruptedException, NotFoundException;

/**
* Retrieves the {@link RepoSpec} object that indicates how the contents of the module identified
Expand Down
10 changes: 7 additions & 3 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/FakeRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,12 +66,16 @@ public String getUrl() {
}

@Override
public Optional<ModuleFile> getModuleFile(
ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager) {
public ModuleFile getModuleFile(
ModuleKey key, ExtendedEventHandler eventHandler, DownloadManager downloadManager)
throws NotFoundException {
String uri = String.format("%s/modules/%s/%s/MODULE.bazel", url, key.name(), key.version());
var maybeContent = Optional.ofNullable(modules.get(key)).map(value -> value.getBytes(UTF_8));
eventHandler.post(RegistryFileDownloadEvent.create(uri, maybeContent));
return maybeContent.map(content -> ModuleFile.create(content, uri));
if (maybeContent.isEmpty()) {
throw new NotFoundException("module not found: " + key);
}
return ModuleFile.create(maybeContent.get(), uri);
}

@Override
Expand Down
57 changes: 37 additions & 20 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,12 @@ public void testHttpUrl() throws Exception {
ImmutableMap.of(),
Optional.empty());
assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.hasValue(
.isEqualTo(
ModuleFile.create(
"lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
.isEmpty();
assertThrows(
Registry.NotFoundException.class,
() -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
}

@Test
Expand Down Expand Up @@ -137,11 +138,12 @@ public void testHttpUrlWithNetrcCreds() throws Exception {

downloadManager.setNetrcCreds(new NetrcCredentials(netrc));
assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.hasValue(
.isEqualTo(
ModuleFile.create(
"lol".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
.isEmpty();
assertThrows(
Registry.NotFoundException.class,
() -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
}

@Test
Expand All @@ -160,9 +162,10 @@ public void testFileUrl() throws Exception {
ImmutableMap.of(),
Optional.empty());
assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.hasValue(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString()));
assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
.isEmpty();
.isEqualTo(ModuleFile.create("lol".getBytes(UTF_8), file.toURI().toString()));
assertThrows(
Registry.NotFoundException.class,
() -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
}

@Test
Expand Down Expand Up @@ -444,15 +447,20 @@ public void testGetModuleFileChecksums() throws Exception {
ImmutableMap.of(),
Optional.empty());
assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.hasValue(
.isEqualTo(
ModuleFile.create(
"old".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
.hasValue(
.isEqualTo(
ModuleFile.create(
"new".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/2.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
.isEmpty();
var e =
assertThrows(
Registry.NotFoundException.class,
() -> registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
assertThat(e)
.hasMessageThat()
.isEqualTo(server.getUrl() + "/myreg/modules/bar/1.0/MODULE.bazel: not found");

var recordedChecksums = eventRecorder.getRecordedHashes();
assertThat(
Expand All @@ -467,7 +475,7 @@ public void testGetModuleFileChecksums() throws Exception {
Optional.empty())
.inOrder();

registry =
Registry registry2 =
registryFactory.createRegistry(
server.getUrl() + "/myreg",
LockfileMode.UPDATE,
Expand All @@ -479,16 +487,25 @@ public void testGetModuleFileChecksums() throws Exception {
server.unserve("/myreg/modules/foo/1.0/MODULE.bazel");
server.unserve("/myreg/modules/foo/2.0/MODULE.bazel");
server.serve("/myreg/modules/bar/1.0/MODULE.bazel", "no longer 404");
assertThat(registry.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.hasValue(
assertThat(registry2.getModuleFile(createModuleKey("foo", "1.0"), reporter, downloadManager))
.isEqualTo(
ModuleFile.create(
"old".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/1.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
.hasValue(
assertThat(registry2.getModuleFile(createModuleKey("foo", "2.0"), reporter, downloadManager))
.isEqualTo(
ModuleFile.create(
"new".getBytes(UTF_8), server.getUrl() + "/myreg/modules/foo/2.0/MODULE.bazel"));
assertThat(registry.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager))
.isEmpty();
e =
assertThrows(
Registry.NotFoundException.class,
() ->
registry2.getModuleFile(createModuleKey("bar", "1.0"), reporter, downloadManager));
assertThat(e)
.hasMessageThat()
.isEqualTo(
server.getUrl()
+ "/myreg/modules/bar/1.0/MODULE.bazel: previously not found (as recorded in"
+ " MODULE.bazel.lock, refresh with --lockfile_mode=refresh)");
}

@Test
Expand Down
4 changes: 2 additions & 2 deletions src/test/py/bazel/bzlmod/bazel_overrides_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,8 +495,8 @@ def testCmdRelativeModuleOverride(self):
allow_failure=True,
)
self.assertIn(
'ERROR: Error computing the main repository mapping: module not found'
' in registries: [email protected]',
'ERROR: Error computing the main repository mapping: module [email protected] not'
' found in registries:',
stderr,
)

Expand Down

0 comments on commit 440c226

Please sign in to comment.