Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explain why a module hasn't been found in any registry #24804

Closed
wants to merge 3 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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",
Original file line number Diff line number Diff line change
@@ -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;
@@ -132,27 +133,31 @@ private String constructUrl(String base, String... segments) {
}

/** Grabs a file from the given URL. Returns {@link Optional#empty} if the file doesn't exist. */
private Optional<byte[]> grabFile(
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);
@@ -174,7 +179,10 @@ 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
@@ -203,7 +211,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(
@@ -216,10 +224,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(
@@ -228,14 +235,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. */
@@ -286,8 +292,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();
}
}

/**
Original file line number Diff line number Diff line change
@@ -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;
@@ -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 =
@@ -691,14 +691,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;
}
@@ -712,7 +719,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));
}

/**
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
@@ -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
@@ -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
@@ -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(
@@ -467,7 +475,7 @@ public void testGetModuleFileChecksums() throws Exception {
Optional.empty())
.inOrder();

registry =
Registry registry2 =
registryFactory.createRegistry(
server.getUrl() + "/myreg",
LockfileMode.UPDATE,
@@ -479,16 +487,24 @@ 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
3 changes: 1 addition & 2 deletions src/test/py/bazel/bzlmod/bazel_overrides_test.py
Original file line number Diff line number Diff line change
@@ -495,8 +495,7 @@ def testCmdRelativeModuleOverride(self):
allow_failure=True,
)
self.assertIn(
'ERROR: Error computing the main repository mapping: module not found'
' in registries: ss@1.0',
'ERROR: Error computing the main repository mapping: module ss@1.0 not found in registries:',
stderr,
)

Loading