Skip to content

Commit

Permalink
Store remote registry file hashes in the lockfile
Browse files Browse the repository at this point in the history
`MODULE.bazel`, `bazel_registry.json` and `source.json` files obtained from remote registries are stored in the repository cache and their hashes are collected in the lockfile. This speeds up incremental module resolutions, such as after adding a new `bazel_dep`.

Yanked versions are not stored in the lockfile. Their handling will be part of a follow-up PR.

Implements part of https://docs.google.com/document/d/1TjA7-M5njkI1F38IC0pm305S9EOmxcUwaCIvaSmansg/edit
Work towards bazelbuild#20369

Closes bazelbuild#21901.

PiperOrigin-RevId: 631195852
Change-Id: I35c30af7f9c3626bdbcb04c85b8c2502eeaafd3e
  • Loading branch information
fmeum authored and Kila2 committed May 13, 2024
1 parent 29b6328 commit 0c1f955
Show file tree
Hide file tree
Showing 50 changed files with 1,269 additions and 259 deletions.
2 changes: 1 addition & 1 deletion MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,16 @@

package com.google.devtools.build.lib.bazel;

import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.CharMatcher;
import com.google.common.base.Preconditions;
import com.google.common.base.Strings;
import com.google.common.base.Supplier;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.devtools.build.lib.analysis.BlazeDirectories;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
Expand Down Expand Up @@ -131,8 +135,8 @@ public class BazelRepositoryModule extends BlazeModule {
public static final String DEFAULT_CACHE_LOCATION = "cache/repos/v1";

// Default list of registries.
public static final ImmutableList<String> DEFAULT_REGISTRIES =
ImmutableList.of("https://bcr.bazel.build/");
public static final ImmutableSet<String> DEFAULT_REGISTRIES =
ImmutableSet.of("https://bcr.bazel.build/");

// A map of repository handlers that can be looked up by rule class name.
private final ImmutableMap<String, RepositoryFunction> repositoryHandlers;
Expand All @@ -150,7 +154,7 @@ public class BazelRepositoryModule extends BlazeModule {
private ImmutableMap<String, ModuleOverride> moduleOverrides = ImmutableMap.of();
private Optional<RootedPath> resolvedFileReplacingWorkspace = Optional.empty();
private FileSystem filesystem;
private List<String> registries;
private ImmutableSet<String> registries;
private final AtomicBoolean ignoreDevDeps = new AtomicBoolean(false);
private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING;
private BazelCompatibilityMode bazelCompatibilityMode = BazelCompatibilityMode.ERROR;
Expand Down Expand Up @@ -495,7 +499,7 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
}

if (repoOptions.registries != null && !repoOptions.registries.isEmpty()) {
registries = repoOptions.registries;
registries = normalizeRegistries(repoOptions.registries);
} else {
registries = DEFAULT_REGISTRIES;
}
Expand Down Expand Up @@ -525,6 +529,14 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
}
}

private static ImmutableSet<String> normalizeRegistries(List<String> registries) {
// Ensure that registries aren't duplicated even after `/modules/...` paths are appended to
// them.
return registries.stream()
.map(url -> CharMatcher.is('/').trimTrailingFrom(url))
.collect(toImmutableSet());
}

/**
* If the given path is absolute path, leave it as it is. If the given path is a relative path, it
* is relative to the current working directory. If the given path starts with '%workspace%, it is
Expand Down
13 changes: 6 additions & 7 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ java_library(
],
deps = [
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
"//third_party:gson",
Expand Down Expand Up @@ -77,11 +75,12 @@ java_library(
"Registry.java",
"RegistryFactory.java",
"RegistryFactoryImpl.java",
"RegistryFileDownloadEvent.java",
],
deps = [
":common",
"//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 All @@ -97,17 +96,15 @@ java_library(
name = "bazel_lockfile_module",
srcs = ["BazelLockFileModule.java"],
deps = [
":exception",
":resolution",
":resolution_impl",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
"//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
"//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/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/util:abrupt_exit_exception",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/skyframe",
"//third_party:flogger",
"//third_party:guava",
"//third_party:jsr305",
Expand Down Expand Up @@ -144,6 +141,7 @@ java_library(
"RegistryKey.java",
"RegistryOverride.java",
"RepoSpecKey.java",
"RepoSpecValue.java",
"SingleExtensionUsagesValue.java",
"SingleExtensionValue.java",
"SingleVersionOverride.java",
Expand All @@ -160,6 +158,7 @@ java_library(
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//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/packages",
Expand Down Expand Up @@ -226,6 +225,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_version_info",
"//src/main/java/com/google/devtools/build/lib/bazel:bazel_version",
"//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/bazel/repository/starlark",
"//src/main/java/com/google/devtools/build/lib/cmdline",
Expand All @@ -248,7 +248,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util:os",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/annot",
"//src/main/java/net/starlark/java/eval",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
import static com.google.common.collect.ImmutableBiMap.toImmutableBiMap;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static java.util.stream.Collectors.counting;
import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toSet;

import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Preconditions;
Expand All @@ -30,6 +30,7 @@
import com.google.common.collect.ImmutableBiMap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableTable;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
Expand All @@ -49,7 +50,6 @@
import com.google.devtools.build.skyframe.SkyValue;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Set;
import javax.annotation.Nullable;

/**
Expand Down Expand Up @@ -186,7 +186,6 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
return null;
}

ImmutableList<String> registries = ImmutableList.copyOf(ModuleFileFunction.REGISTRIES.get(env));
ImmutableMap<String, String> moduleOverrides =
ModuleFileFunction.MODULE_OVERRIDES.get(env).entrySet().stream()
.collect(
Expand All @@ -202,7 +201,7 @@ static BzlmodFlagsAndEnvVars getFlagsAndEnvVars(Environment env) throws Interrup
String envYanked = allowedYankedVersionsFromEnv.getValue();

return BzlmodFlagsAndEnvVars.create(
registries,
ModuleFileFunction.REGISTRIES.get(env),
moduleOverrides,
yankedVersions,
nullToEmpty(envYanked),
Expand Down Expand Up @@ -257,14 +256,14 @@ private static ImmutableBiMap<RepositoryName, ModuleKey> computeCanonicalRepoNam
ImmutableMap<ModuleKey, Module> depGraph) {
// Find modules with multiple versions in the dep graph. Currently, the only source of such
// modules is multiple_version_override.
Set<String> multipleVersionsModules =
ImmutableSet<String> multipleVersionsModules =
depGraph.keySet().stream()
.collect(groupingBy(ModuleKey::getName, counting()))
.entrySet()
.stream()
.filter(entry -> entry.getValue() > 1)
.map(Entry::getKey)
.collect(toSet());
.collect(toImmutableSet());

// If there is a unique version of this module in the entire dep graph, we elide the version
// from the canonical repository name. This has a number of benefits:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.FileValue;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.cmdline.LabelConstants;
Expand Down Expand Up @@ -55,7 +56,7 @@ public class BazelLockFileFunction implements SkyFunction {

private static final BzlmodFlagsAndEnvVars EMPTY_FLAGS =
BzlmodFlagsAndEnvVars.create(
ImmutableList.of(), ImmutableMap.of(), ImmutableList.of(), "", false, "", "");
ImmutableSet.of(), ImmutableMap.of(), ImmutableList.of(), "", false, "", "");

private static final BazelLockFileValue EMPTY_LOCKFILE =
BazelLockFileValue.builder()
Expand All @@ -65,6 +66,7 @@ public class BazelLockFileFunction implements SkyFunction {
.setLocalOverrideHashes(ImmutableMap.of())
.setModuleDepGraph(ImmutableMap.of())
.setModuleExtensions(ImmutableMap.of())
.setRegistryFileHashes(ImmutableMap.of())
.build();

public BazelLockFileFunction(Path rootDirectory) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,19 @@
import com.google.common.flogger.GoogleLogger;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.bazel.repository.downloader.Checksum;
import com.google.devtools.build.lib.cmdline.LabelConstants;
import com.google.devtools.build.lib.runtime.BlazeModule;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
import com.google.devtools.build.lib.util.AbruptExitException;
import com.google.devtools.build.lib.vfs.FileSystemUtils;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.RootedPath;
import java.io.IOException;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;

Expand All @@ -45,6 +46,7 @@ public class BazelLockFileModule extends BlazeModule {

private SkyframeExecutor executor;
private Path workspaceRoot;
private boolean enabled;
@Nullable private BazelModuleResolutionEvent moduleResolutionEvent;

private static final GoogleLogger logger = GoogleLogger.forEnclosingClass();
Expand All @@ -53,20 +55,44 @@ public class BazelLockFileModule extends BlazeModule {
public void beforeCommand(CommandEnvironment env) {
executor = env.getSkyframeExecutor();
workspaceRoot = env.getWorkspace();
RepositoryOptions options = env.getOptions().getOptions(RepositoryOptions.class);
if (options.lockfileMode.equals(LockfileMode.UPDATE)) {
env.getEventBus().register(this);
}

enabled =
env.getOptions().getOptions(RepositoryOptions.class).lockfileMode == LockfileMode.UPDATE;
moduleResolutionEvent = null;
env.getEventBus().register(this);
}

@Override
public void afterCommand() throws AbruptExitException {
if (moduleResolutionEvent == null) {
public void afterCommand() {
if (!enabled || moduleResolutionEvent == null) {
// Command does not use Bazel modules or the lockfile mode is not update.
// Since Skyframe caches events, they are replayed even when nothing has changed.
return;
}

BazelDepGraphValue depGraphValue;
BazelModuleResolutionValue moduleResolutionValue;
try {
depGraphValue =
(BazelDepGraphValue) executor.getEvaluator().getExistingValue(BazelDepGraphValue.KEY);
moduleResolutionValue =
(BazelModuleResolutionValue)
executor.getEvaluator().getExistingValue(BazelModuleResolutionValue.KEY);
} catch (InterruptedException e) {
// Not thrown in Bazel.
throw new IllegalStateException(e);
}

BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue();
ImmutableMap<String, Optional<Checksum>> fileHashes;
if (moduleResolutionValue == null) {
// BazelDepGraphFunction took the dep graph from the lockfile and didn't cause evaluation of
// BazelModuleResolutionFunction. The file hashes in the lockfile are still up-to-date.
fileHashes = oldLockfile.getRegistryFileHashes();
} else {
fileHashes = ImmutableSortedMap.copyOf(moduleResolutionValue.getRegistryFileHashes());
}

// All nodes corresponding to module extensions that have been evaluated in the current build
// are done at this point. Look up entries by eval keys to record results even if validation
// later fails due to invalid imports.
Expand All @@ -88,24 +114,16 @@ public void afterCommand() throws AbruptExitException {
newExtensionInfos.put(key.argument(), value.getLockFileInfo().get());
}
});
var combinedExtensionInfos =
combineModuleExtensions(
oldLockfile.getModuleExtensions(), newExtensionInfos, depGraphValue);

BazelDepGraphValue depGraphValue;
try {
depGraphValue =
(BazelDepGraphValue) executor.getEvaluator().getExistingValue(BazelDepGraphValue.KEY);
} catch (InterruptedException e) {
// Not thrown in Bazel.
throw new IllegalStateException(e);
}

BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue();
// Create an updated version of the lockfile, keeping only the extension results from the old
// lockfile that are still up-to-date and adding the newly resolved extension results.
BazelLockFileValue newLockfile =
moduleResolutionEvent.getResolutionOnlyLockfileValue().toBuilder()
.setModuleExtensions(
combineModuleExtensions(
oldLockfile.getModuleExtensions(), newExtensionInfos, depGraphValue))
.setRegistryFileHashes(fileHashes)
.setModuleExtensions(combinedExtensionInfos)
.build();

// Write the new value to the file, but only if needed. This is not just a performance
Expand All @@ -115,7 +133,6 @@ public void afterCommand() throws AbruptExitException {
if (!newLockfile.equals(oldLockfile)) {
updateLockfile(workspaceRoot, newLockfile);
}
this.moduleResolutionEvent = null;
}

/**
Expand All @@ -140,8 +157,6 @@ public void afterCommand() throws AbruptExitException {
var factorToLockedExtension = entry.getValue();
ModuleExtensionEvalFactors firstEntryFactors =
factorToLockedExtension.keySet().iterator().next();
LockFileModuleExtension firstEntryExtension =
factorToLockedExtension.values().iterator().next();
// All entries for a single extension share the same usages digest, so it suffices to check
// the first entry.
if (shouldKeepExtension(
Expand Down
Loading

0 comments on commit 0c1f955

Please sign in to comment.