Skip to content

Commit 88a230f

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Add --lockfile_mode=refresh
RELNOTES: The new `refresh` value for `--lockfile_mode` behaves like the `update` mode, but additionally forces a refresh of mutable registry content (yanked versions and missing module versions) when switched to or from time to time while enabled. Closes #22333. PiperOrigin-RevId: 633737372 Change-Id: I2cae552f6adfcd07a5b3263b5e4997e21653f106
1 parent df9f76a commit 88a230f

11 files changed

Lines changed: 311 additions & 17 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/bazel/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ java_library(
4141
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
4242
"//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
4343
"//src/main/java/com/google/devtools/build/lib/bazel/rules/android",
44+
"//src/main/java/com/google/devtools/build/lib/clock",
4445
"//src/main/java/com/google/devtools/build/lib/cmdline",
4546
"//src/main/java/com/google/devtools/build/lib/events",
4647
"//src/main/java/com/google/devtools/build/lib/pkgcache",

‎src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@
8080
import com.google.devtools.build.lib.bazel.rules.android.AndroidNdkRepositoryRule;
8181
import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryFunction;
8282
import com.google.devtools.build.lib.bazel.rules.android.AndroidSdkRepositoryRule;
83+
import com.google.devtools.build.lib.clock.Clock;
8384
import com.google.devtools.build.lib.cmdline.RepositoryName;
8485
import com.google.devtools.build.lib.events.Event;
8586
import com.google.devtools.build.lib.pkgcache.PackageOptions;
@@ -121,6 +122,7 @@
121122
import com.google.devtools.common.options.OptionsParsingResult;
122123
import java.io.IOException;
123124
import java.lang.reflect.InvocationTargetException;
125+
import java.time.Instant;
124126
import java.util.LinkedHashMap;
125127
import java.util.List;
126128
import java.util.Map;
@@ -159,6 +161,8 @@ public class BazelRepositoryModule extends BlazeModule {
159161
private CheckDirectDepsMode checkDirectDepsMode = CheckDirectDepsMode.WARNING;
160162
private BazelCompatibilityMode bazelCompatibilityMode = BazelCompatibilityMode.ERROR;
161163
private LockfileMode bazelLockfileMode = LockfileMode.UPDATE;
164+
private Clock clock;
165+
private Instant lastRegistryInvalidation = Instant.EPOCH;
162166

163167
private Optional<Path> vendorDirectory;
164168
private List<String> allowedYankedVersions = ImmutableList.of();
@@ -526,6 +530,21 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
526530
starlarkRepositoryFunction.setRepositoryRemoteExecutor(remoteExecutor);
527531
singleExtensionEvalFunction.setRepositoryRemoteExecutor(remoteExecutor);
528532
delegatingDownloader.setDelegate(env.getRuntime().getDownloaderSupplier().get());
533+
534+
clock = env.getClock();
535+
try {
536+
var lastRegistryInvalidationValue =
537+
(PrecomputedValue)
538+
env.getSkyframeExecutor()
539+
.getEvaluator()
540+
.getExistingValue(RegistryFunction.LAST_INVALIDATION.getKey());
541+
if (lastRegistryInvalidationValue != null) {
542+
lastRegistryInvalidation = (Instant) lastRegistryInvalidationValue.get();
543+
}
544+
} catch (InterruptedException e) {
545+
// Not thrown in Bazel.
546+
throw new IllegalStateException(e);
547+
}
529548
}
530549
}
531550

@@ -556,6 +575,10 @@ private String getAbsolutePath(String path, CommandEnvironment env) {
556575

557576
@Override
558577
public ImmutableList<Injected> getPrecomputedValues() {
578+
Instant now = clock.now();
579+
if (now.isAfter(lastRegistryInvalidation.plus(RegistryFunction.INVALIDATION_INTERVAL))) {
580+
lastRegistryInvalidation = now;
581+
}
559582
return ImmutableList.of(
560583
PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides),
561584
PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, moduleOverrides),
@@ -582,7 +605,8 @@ public ImmutableList<Injected> getPrecomputedValues() {
582605
PrecomputedValue.injected(
583606
YankedVersionsUtil.ALLOWED_YANKED_VERSIONS, allowedYankedVersions),
584607
PrecomputedValue.injected(
585-
RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, disableNativeRepoRules));
608+
RepositoryDelegatorFunction.DISABLE_NATIVE_REPO_RULES, disableNativeRepoRules),
609+
PrecomputedValue.injected(RegistryFunction.LAST_INVALIDATION, lastRegistryInvalidation));
586610
}
587611

588612
@Override

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ public void afterCommand() {
7171
// Check the Skyframe value instead of the option since some commands (e.g. shutdown) don't
7272
// propagate the options to Skyframe, but we can only operate on Skyframe values that were
7373
// generated in UPDATE mode.
74-
if (lockfileMode != LockfileMode.UPDATE) {
74+
if (lockfileMode != LockfileMode.UPDATE && lockfileMode != LockfileMode.REFRESH) {
7575
return;
7676
}
7777
moduleResolutionValue =

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/IndexRegistry.java‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,8 +58,26 @@ public class IndexRegistry implements Registry {
5858
* registry.
5959
*/
6060
public enum KnownFileHashesMode {
61+
/**
62+
* Neither use nor update any file hashes. All registry downloads will go out to the network.
63+
*/
6164
IGNORE,
65+
/**
66+
* Use file hashes from the lockfile if available and add hashes for new files to the lockfile.
67+
* Avoid revalidation of mutable registry information (yanked versions in metadata.json and
68+
* modules that previously 404'd) by using these hashes and recording absent files in the
69+
* lockfile.
70+
*/
6271
USE_AND_UPDATE,
72+
/**
73+
* Use file hashes from the lockfile if available and add hashes for new files to the lockfile.
74+
* Always revalidate mutable registry information.
75+
*/
76+
USE_IMMUTABLE_AND_UPDATE,
77+
/**
78+
* Require file hashes for all registry downloads. In particular, mutable registry files such as
79+
* metadata.json can't be downloaded in this mode.
80+
*/
6381
ENFORCE
6482
}
6583

@@ -115,7 +133,9 @@ private Optional<byte[]> grabFile(
115133
String url, ExtendedEventHandler eventHandler, boolean useChecksum)
116134
throws IOException, InterruptedException {
117135
var maybeContent = doGrabFile(url, eventHandler, useChecksum);
118-
if (knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE && useChecksum) {
136+
if ((knownFileHashesMode == KnownFileHashesMode.USE_AND_UPDATE
137+
|| knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE)
138+
&& useChecksum) {
119139
eventHandler.post(RegistryFileDownloadEvent.create(url, maybeContent));
120140
}
121141
return maybeContent;
@@ -139,8 +159,14 @@ private Optional<byte[]> doGrabFile(
139159
// This is a new file, download without providing a checksum.
140160
checksum = Optional.empty();
141161
} else if (knownChecksum.isEmpty()) {
142-
// The file is known to not exist, so don't attempt to download it.
143-
return Optional.empty();
162+
// The file didn't exist when the lockfile was created, but it may exist now.
163+
if (knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE) {
164+
// Attempt to download the file again.
165+
checksum = Optional.empty();
166+
} else {
167+
// Guarantee reproducibility by assuming that the file still doesn't exist.
168+
return Optional.empty();
169+
}
144170
} else {
145171
// The file is known, download with a checksum to potentially obtain a repository cache hit
146172
// and ensure that the remote file hasn't changed.
@@ -441,6 +467,10 @@ public Optional<ImmutableMap<Version, String>> getYankedVersions(
441467
@Override
442468
public Optional<YankedVersionsValue> tryGetYankedVersionsFromLockfile(
443469
ModuleKey selectedModuleKey) {
470+
if (knownFileHashesMode == KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE) {
471+
// Yanked version information is inherently mutable, so always refresh it when requested.
472+
return Optional.empty();
473+
}
444474
String yankedInfo = previouslySelectedYankedVersions.get(selectedModuleKey);
445475
if (yankedInfo != null) {
446476
// The module version was selected when the lockfile was created, but known to be yanked

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Registry.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,12 +17,12 @@
1717

1818
import com.google.common.collect.ImmutableMap;
1919
import com.google.devtools.build.lib.events.ExtendedEventHandler;
20-
import com.google.devtools.build.skyframe.SkyValue;
20+
import com.google.devtools.build.skyframe.NotComparableSkyValue;
2121
import java.io.IOException;
2222
import java.util.Optional;
2323

2424
/** A database where module metadata is stored. */
25-
public interface Registry extends SkyValue {
25+
public interface Registry extends NotComparableSkyValue {
2626

2727
/** The URL that uniquely identifies the registry. */
2828
String getUrl();

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFactoryImpl.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,9 +60,11 @@ public Registry createRegistry(
6060
var knownFileHashesMode =
6161
switch (uri.getScheme()) {
6262
case "http", "https" ->
63-
lockfileMode == LockfileMode.ERROR
64-
? KnownFileHashesMode.ENFORCE
65-
: KnownFileHashesMode.USE_AND_UPDATE;
63+
switch (lockfileMode) {
64+
case ERROR -> KnownFileHashesMode.ENFORCE;
65+
case REFRESH -> KnownFileHashesMode.USE_IMMUTABLE_AND_UPDATE;
66+
case OFF, UPDATE -> KnownFileHashesMode.USE_AND_UPDATE;
67+
};
6668
case "file" -> KnownFileHashesMode.IGNORE;
6769
default ->
6870
throw new URISyntaxException(uri.toString(), "Unrecognized registry URL protocol");

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegistryFunction.java‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,16 +17,32 @@
1717

1818
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
1919
import com.google.devtools.build.lib.server.FailureDetails;
20+
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Precomputed;
2021
import com.google.devtools.build.lib.vfs.Path;
2122
import com.google.devtools.build.skyframe.SkyFunction;
2223
import com.google.devtools.build.skyframe.SkyFunctionException;
2324
import com.google.devtools.build.skyframe.SkyKey;
2425
import com.google.devtools.build.skyframe.SkyValue;
2526
import java.net.URISyntaxException;
27+
import java.time.Duration;
28+
import java.time.Instant;
2629
import javax.annotation.Nullable;
2730

2831
/** A simple SkyFunction that creates a {@link Registry} with a given URL. */
2932
public class RegistryFunction implements SkyFunction {
33+
/**
34+
* Set to the current time in {@link com.google.devtools.build.lib.bazel.BazelRepositoryModule}
35+
* after {@link #INVALIDATION_INTERVAL} has passed. This is used to refresh the mutable registry
36+
* contents cached in memory from time to time.
37+
*/
38+
public static final Precomputed<Instant> LAST_INVALIDATION =
39+
new Precomputed<>("last_registry_invalidation");
40+
41+
/**
42+
* The interval after which the mutable registry contents cached in memory should be refreshed.
43+
*/
44+
public static final Duration INVALIDATION_INTERVAL = Duration.ofHours(1);
45+
3046
private final RegistryFactory registryFactory;
3147
private final Path workspaceRoot;
3248

@@ -41,6 +57,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
4157
throws InterruptedException, RegistryException {
4258
LockfileMode lockfileMode = BazelLockFileFunction.LOCKFILE_MODE.get(env);
4359

60+
if (lockfileMode == LockfileMode.REFRESH) {
61+
RegistryFunction.LAST_INVALIDATION.get(env);
62+
}
63+
4464
BazelLockFileValue lockfile = (BazelLockFileValue) env.getValue(BazelLockFileValue.KEY);
4565
if (lockfile == null) {
4666
return null;

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
216216
// influence the evaluation of the extension and the validation also runs when the extension
217217
// result is taken from the lockfile, we can already populate the lockfile info. This is
218218
// necessary to prevent the extension from rerunning when only the imports change.
219-
if (lockfileMode.equals(LockfileMode.UPDATE)) {
219+
if (lockfileMode == LockfileMode.UPDATE || lockfileMode == LockfileMode.REFRESH) {
220220
lockFileInfo =
221221
Optional.of(
222222
new LockFileModuleExtension.WithFactors(

‎src/main/java/com/google/devtools/build/lib/bazel/repository/RepositoryOptions.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -320,9 +320,11 @@ public Converter() {
320320
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
321321
help =
322322
"Specifies how and whether or not to use the lockfile. Valid values are `update` to"
323-
+ " use the lockfile and update it if there are changes, `error` to use the lockfile"
324-
+ " but throw an error if it's not up-to-date, or `off` to neither read from or write"
325-
+ " to the lockfile.")
323+
+ " use the lockfile and update it if there are changes, `refresh` to additionally"
324+
+ " refresh mutable information (yanked versions and previously missing modules)"
325+
+ " from remote registries from time to time, `error` to use the lockfile but throw"
326+
+ " an error if it's not up-to-date, or `off` to neither read from or write to the"
327+
+ " lockfile.")
326328
public LockfileMode lockfileMode;
327329

328330
@Option(
@@ -369,10 +371,11 @@ public Converter() {
369371
/** An enum for specifying how to use the lockfile. */
370372
public enum LockfileMode {
371373
OFF, // Don't use the lockfile at all.
372-
UPDATE, // Update the lockfile when it mismatches the module.
373-
ERROR; // Throw an error when it mismatches the module.
374+
UPDATE, // Update the lockfile wh
375+
REFRESH,
376+
ERROR; // Throw an error when it mismatc
374377

375-
/** Converts to {@link BazelLockfileMode}. */
378+
/** Converts to {@link LockfileMode}. */
376379
public static class Converter extends EnumConverter<LockfileMode> {
377380
public Converter() {
378381
super(LockfileMode.class, "Lockfile mode");

‎src/test/py/bazel/bzlmod/bazel_lockfile_test.py‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -171,6 +171,41 @@ def testChangeModuleInRegistryWithLockfile(self):
171171
# Even adding a new dependency should not fail due to the registry change
172172
self.RunBazel(['build', '--nobuild', '//:all'])
173173

174+
def testChangeModuleInRegistryWithLockfileInRefreshMode(self):
175+
# Add module 'sss' to the registry with dep on 'aaa'
176+
self.main_registry.createCcModule('sss', '1.3', {'aaa': '1.1'})
177+
# Create a project with deps on 'sss'
178+
self.ScratchFile(
179+
'MODULE.bazel',
180+
[
181+
'bazel_dep(name = "sss", version = "1.3")',
182+
],
183+
)
184+
self.ScratchFile('BUILD', ['filegroup(name = "hello")'])
185+
self.RunBazel(['build', '--nobuild', '//:all'])
186+
187+
# Change registry -> update 'sss' module file (corrupt it)
188+
module_dir = self.main_registry.root.joinpath('modules', 'sss', '1.3')
189+
scratchFile(module_dir.joinpath('MODULE.bazel'), ['whatever!'])
190+
191+
# Shutdown bazel to empty any cache of the deps tree
192+
self.RunBazel(['shutdown'])
193+
# Running with the lockfile, should not recognize the registry changes
194+
# hence find no errors
195+
self.RunBazel(['build', '--nobuild', '--lockfile_mode=refresh', '//:all'])
196+
197+
self.ScratchFile(
198+
'MODULE.bazel',
199+
[
200+
'bazel_dep(name = "sss", version = "1.3")',
201+
'bazel_dep(name = "bbb", version = "1.1")',
202+
],
203+
)
204+
# Shutdown bazel to empty any cache of the deps tree
205+
self.RunBazel(['shutdown'])
206+
# Even adding a new dependency should not fail due to the registry change
207+
self.RunBazel(['build', '--nobuild', '--lockfile_mode=refresh', '//:all'])
208+
174209
def testAddModuleToRegistryWithLockfile(self):
175210
# Create a project with deps on the BCR's 'platforms' module
176211
self.ScratchFile(
@@ -206,6 +241,46 @@ def testAddModuleToRegistryWithLockfile(self):
206241
# Even adding a new dependency should not fail due to the registry change
207242
self.RunBazel(['build', '--nobuild', '//:all'])
208243

244+
def testAddModuleToRegistryWithLockfileInRefreshMode(self):
245+
# Create a project with deps on the BCR's 'platforms' module
246+
self.ScratchFile(
247+
'MODULE.bazel',
248+
[
249+
'bazel_dep(name = "platforms", version = "0.0.9")',
250+
],
251+
)
252+
self.ScratchFile('BUILD', ['filegroup(name = "hello")'])
253+
self.RunBazel(['build', '--nobuild', '//:all'])
254+
255+
# Add a broken 'platforms' module to the first registry
256+
module_dir = self.main_registry.root.joinpath(
257+
'modules', 'platforms', '0.0.9'
258+
)
259+
scratchFile(module_dir.joinpath('MODULE.bazel'), ['whatever!'])
260+
261+
# Shutdown bazel to empty any cache of the deps tree
262+
self.RunBazel(['shutdown'])
263+
# Running with the lockfile in refresh mode should recognize the addition
264+
# to the first registry
265+
exit_code, _, stderr = self.RunBazel(
266+
[
267+
'build',
268+
'--nobuild',
269+
'--lockfile_mode=refresh',
270+
'//:all',
271+
],
272+
allow_failure=True,
273+
)
274+
self.AssertExitCode(exit_code, 48, stderr)
275+
self.assertIn(
276+
(
277+
'ERROR: Error computing the main repository mapping: in module '
278+
'dependency chain <root> -> platforms@0.0.9: error parsing '
279+
'MODULE.bazel file for platforms@0.0.9'
280+
),
281+
stderr,
282+
)
283+
209284
def testChangeFlagWithLockfile(self):
210285
# Create a project with an outdated direct dep on aaa
211286
self.ScratchFile(

0 commit comments

Comments
 (0)