Skip to content

Commit 330dc9f

Browse files
fmeumcopybara-github
fmeum
authored and
copybara-github
committed
Allow module extensions to persist facts in the lockfile
RELNOTES[NEW]: Module extensions can store a JSON-like Starlark object in `module_ctx.extension_metadata(facts = ...)` and retrieve it back in future evaluations of the extension via `module_ctx.facts` without any invalidation taking place. See bazel-contrib/rules_go#4393 for a real-world application of this new feature. Fixes bazelbuild#24777 Closes bazelbuild#26198. PiperOrigin-RevId: 819640644 Change-Id: I281503c9d4d0e60f46b57e9a231c3d2a4658d486
1 parent d50999d commit 330dc9f

22 files changed

Lines changed: 1271 additions & 289 deletions

‎scripts/bazel-lockfile-merge.jq‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,17 @@ def shallow_merge(f):
4242
# shallowly merged factors map, then shallowly merge the results.
4343
moduleExtensions: (map(.moduleExtensions | to_entries)
4444
| flatten
45-
| group_by(.key)
46-
| shallow_merge({(.[0].key): shallow_merge(.value)}))
45+
| if length > 0 then group_by(.key) | shallow_merge({(.[0].key): shallow_merge(.value)}) else {} end),
46+
# Group facts by extension ID across all lockfiles with shallowly
47+
# merged top-level keys, then shallowly merge the results. Handle the
48+
# case where some lockfiles do not have a facts key.
49+
facts: (if any(has("facts")) then
50+
map(.facts // {} | to_entries) | flatten |
51+
if length > 0 then group_by(.key) | shallow_merge({(.[0].key): shallow_merge(.value)}) else {} end
52+
else null end),
4753
}
54+
# Filter out null values for missing top-level keys such as facts.
55+
| with_entries(select(.value != null))
4856
)? //
4957
# We get here if the lockfiles with the highest lockFileVersion could not be
5058
# processed, for example because all lockfiles have lockFileVersion < 10.

‎scripts/bazel_lockfile_merge_test.sh‎

Lines changed: 110 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,4 +270,114 @@ EOF
270270
diff -u base left || fail "output differs"
271271
}
272272

273+
function test_merge_facts() {
274+
cat > base <<'EOF'
275+
{
276+
"lockFileVersion": 22,
277+
"registryFileHashes": {
278+
"https://example.org/modules/foo/1.0/MODULE.bazel": "1234"
279+
},
280+
"selectedYankedVersions": {},
281+
"moduleExtensions": {}
282+
}
283+
EOF
284+
cat > left <<'EOF'
285+
{
286+
"lockFileVersion": 22,
287+
"registryFileHashes": {
288+
"https://example.org/modules/foo/1.0/MODULE.bazel": "1234"
289+
},
290+
"selectedYankedVersions": {},
291+
"moduleExtensions": {},
292+
"facts": {
293+
"@@rules_foo+//:foo_deps.bzl%foo_deps": {
294+
"json_foo@1.0": {
295+
"sha256": "1111",
296+
"url": "https://example.org/json_foo/1.0.zip"
297+
},
298+
"cbor_foo@2.0": {
299+
"sha256": "2222",
300+
"url": "https://example.org/cbor_foo/2.0.zip"
301+
}
302+
},
303+
"@@rules_bar+//:bar_deps.bzl%bar_deps": {
304+
"json_bar@1.0": {
305+
"sha256": "3333",
306+
"url": "https://example.org/json_bar/1.0.zip"
307+
}
308+
}
309+
}
310+
}
311+
EOF
312+
cat > right <<'EOF'
313+
{
314+
"lockFileVersion": 22,
315+
"registryFileHashes": {
316+
"https://example.org/modules/foo/1.0/MODULE.bazel": "1234"
317+
},
318+
"selectedYankedVersions": {},
319+
"moduleExtensions": {},
320+
"facts": {
321+
"@@rules_foo+//:foo_deps.bzl%foo_deps": {
322+
"json_foo@1.0": {
323+
"sha256": "1111",
324+
"url": "https://example.org/json_foo/1.0.zip"
325+
},
326+
"xml_foo@3.0": {
327+
"sha256": "4444",
328+
"url": "https://example.org/xml_foo/3.0.zip"
329+
}
330+
},
331+
"@@rules_baz+//:baz_deps.bzl%baz_deps": {
332+
"json_baz@1.0": {
333+
"sha256": "5555",
334+
"url": "https://example.org/json_baz/1.0.zip"
335+
}
336+
}
337+
}
338+
}
339+
EOF
340+
cat > expected <<'EOF'
341+
{
342+
"lockFileVersion": 22,
343+
"registryFileHashes": {
344+
"https://example.org/modules/foo/1.0/MODULE.bazel": "1234"
345+
},
346+
"selectedYankedVersions": {},
347+
"moduleExtensions": {},
348+
"facts": {
349+
"@@rules_bar+//:bar_deps.bzl%bar_deps": {
350+
"json_bar@1.0": {
351+
"sha256": "3333",
352+
"url": "https://example.org/json_bar/1.0.zip"
353+
}
354+
},
355+
"@@rules_baz+//:baz_deps.bzl%baz_deps": {
356+
"json_baz@1.0": {
357+
"sha256": "5555",
358+
"url": "https://example.org/json_baz/1.0.zip"
359+
}
360+
},
361+
"@@rules_foo+//:foo_deps.bzl%foo_deps": {
362+
"cbor_foo@2.0": {
363+
"sha256": "2222",
364+
"url": "https://example.org/cbor_foo/2.0.zip"
365+
},
366+
"json_foo@1.0": {
367+
"sha256": "1111",
368+
"url": "https://example.org/json_foo/1.0.zip"
369+
},
370+
"xml_foo@3.0": {
371+
"sha256": "4444",
372+
"url": "https://example.org/xml_foo/3.0.zip"
373+
}
374+
}
375+
}
376+
}
377+
EOF
378+
379+
do_merge base left right
380+
diff -u expected left || fail "output differs"
381+
}
382+
273383
run_suite "Tests of bash completion of 'blaze' command."

‎site/en/external/extension.md‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -245,17 +245,44 @@ later. This is because the extension's identify is based on its file, so moving
245245
the extension into another file later changes your public API and is a backwards
246246
incompatible change for your users.
247247

248-
### Specify reproducibility
248+
### Specify reproducibility and use facts
249249

250250
If your extension always defines the same repositories given the same inputs
251251
(extension tags, files it reads, etc.) and in particular doesn't rely on
252252
any [downloads](/rules/lib/builtins/module_ctx#download) that aren't guarded by
253253
a checksum, consider returning
254254
[`extension_metadata`](/rules/lib/builtins/module_ctx#extension_metadata) with
255255
`reproducible = True`. This allows Bazel to skip this extension when writing to
256-
the lockfile.
257-
258-
### Specify the operating system and architecture
256+
the `MODULE.bazel` lockfile, which helps keep the lockfile small and reduces
257+
the chance of merge conflicts. Note that Bazel still caches the results of
258+
reproducible extensions in a way that persists across server restarts, so even
259+
a long-running extension can be marked as reproducible without a performance
260+
penalty.
261+
262+
If your extension relies on effectively immutable data obtained from outside
263+
the build, most commonly from the network, but you don't have a checksum
264+
available to guard the download, consider using the `facts` parameter of
265+
[`extension_metadata`](/rules/lib/builtins/module_ctx#extension_metadata) to
266+
persistently record such data and thus allow your extension to become
267+
reproducible. `facts` is expected to be a dictionary with string keys and
268+
arbitrary JSON-like Starlark values that is always persisted in the lockfile and
269+
available to future evaluations of the extension via the
270+
[`facts`](/rules/lib/builtins/module_ctx#facts) field of `module_ctx`.
271+
272+
`facts` are not invalidated even when the code of your module extension changes,
273+
so be prepared to handle the case where the structure of `facts` changes.
274+
Bazel also assumes that two different `facts` dicts produced by two different
275+
evaluations of the same extension can be shallowly merged (i.e., as if by using
276+
the `|` operator on two dicts). This is partially enforced by `module_ctx.facts`
277+
not supporting enumeration of its entries, just lookups by key.
278+
279+
An example of using `facts` would be to record a mapping from version numbers of
280+
some SDK to the an object containing the download URL and checksum of that
281+
version. The first time the extension is evaluated, it can fetch this mapping
282+
from the network, but on later evaluations it can use the mapping from `facts`
283+
to avoid the network requests.
284+
285+
### Specify dependence on operating system and architecture
259286

260287
If your extension relies on the operating system or its architecture type,
261288
ensure to indicate this in the extension definition using the `os_dependent`

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,7 @@ java_library(
125125
srcs = ["BazelLockFileModule.java"],
126126
deps = [
127127
":module_extension",
128+
":module_extension_metadata",
128129
":resolution",
129130
":resolution_impl",
130131
":serialization",
@@ -213,18 +214,21 @@ java_library(
213214
srcs = [
214215
"AttributeValuesAdapter.java",
215216
"DelegateTypeAdapterFactory.java",
217+
"FactsAdapter.java",
216218
"GsonTypeAdapterUtil.java",
217219
],
218220
deps = [
219221
":common",
220222
":module_extension",
223+
":module_extension_metadata",
221224
":resolution",
222225
"//src/main/java/com/google/devtools/build/lib/bazel/repository/cache",
223226
"//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader",
224227
"//src/main/java/com/google/devtools/build/lib/cmdline",
225228
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
226229
"//src/main/java/com/google/devtools/build/lib/util:string_encoding",
227230
"//src/main/java/net/starlark/java/eval",
231+
"//src/main/java/net/starlark/java/lib/json",
228232
"//third_party:auto_value",
229233
"//third_party:gson",
230234
"//third_party:guava",
@@ -435,6 +439,8 @@ java_library(
435439
java_library(
436440
name = "module_extension_metadata",
437441
srcs = [
442+
"Facts.java",
443+
"LockfileModuleExtensionMetadata.java",
438444
"ModuleExtensionMetadata.java",
439445
],
440446
deps = [
@@ -443,9 +449,12 @@ java_library(
443449
"//src/main/java/com/google/devtools/build/docgen/annot",
444450
"//src/main/java/com/google/devtools/build/lib/cmdline",
445451
"//src/main/java/com/google/devtools/build/lib/events",
452+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization:visible-for-serialization",
453+
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
446454
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
447455
"//src/main/java/net/starlark/java/annot",
448456
"//src/main/java/net/starlark/java/eval",
457+
"//src/main/java/net/starlark/java/types",
449458
"//third_party:auto_value",
450459
"//third_party:gson",
451460
"//third_party:guava",

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,9 +115,10 @@ public void afterCommand() {
115115
// transitive closure of the current build. Since extensions are potentially costly to evaluate,
116116
// this is seen as an advantage. Full reproducibility can be ensured by running 'bazel shutdown'
117117
// first if needed.
118+
var numExtensions = depGraphValue.getExtensionUsagesTable().rowKeySet().size();
118119
var newExtensionInfos =
119-
new HashMap<ModuleExtensionId, LockFileModuleExtension.WithFactors>(
120-
depGraphValue.getExtensionUsagesTable().rowKeySet().size());
120+
new HashMap<ModuleExtensionId, LockFileModuleExtension.WithFactors>(numExtensions);
121+
var combinedFacts = new HashMap<ModuleExtensionId, Facts>(numExtensions);
121122
var doneValues = evaluator.getDoneValues();
122123
for (var extensionId : depGraphValue.getExtensionUsagesTable().rowKeySet()) {
123124
if (extensionId.isInnate()) {
@@ -127,6 +128,7 @@ public void afterCommand() {
127128
var value = (SingleExtensionValue) doneValues.get(SingleExtensionValue.evalKey(extensionId));
128129
if (value != null) {
129130
newExtensionInfos.put(extensionId, value.lockFileInfo().get());
131+
combinedFacts.put(extensionId, value.facts());
130132
}
131133
}
132134

@@ -159,6 +161,15 @@ public void afterCommand() {
159161
.setRegistryFileHashes(remoteRegistryFileHashes)
160162
.setSelectedYankedVersions(moduleResolutionValue.getSelectedYankedVersions())
161163
.setModuleExtensions(notReproducibleExtensionInfos)
164+
.setFacts(
165+
ImmutableSortedMap.copyOf(
166+
Maps.filterEntries(
167+
combinedFacts,
168+
entry ->
169+
depGraphValue
170+
.getExtensionUsagesTable()
171+
.containsRow(entry.getKey())
172+
&& !entry.getValue().equals(Facts.EMPTY))))
162173
.build();
163174

164175
// Write the new values to the files, but only if needed. This is not just a

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,8 @@ static Builder builder() {
111111
.setLockFileVersion(LOCK_FILE_VERSION)
112112
.setRegistryFileHashes(ImmutableMap.of())
113113
.setSelectedYankedVersions(ImmutableMap.of())
114-
.setModuleExtensions(ImmutableMap.of());
114+
.setModuleExtensions(ImmutableMap.of())
115+
.setFacts(ImmutableMap.of());
115116
}
116117

117118
/** Current version of the lock file */
@@ -131,6 +132,8 @@ static Builder builder() {
131132
ModuleExtensionId, ImmutableMap<ModuleExtensionEvalFactors, LockFileModuleExtension>>
132133
getModuleExtensions();
133134

135+
public abstract ImmutableMap<ModuleExtensionId, Facts> getFacts();
136+
134137
public abstract Builder toBuilder();
135138

136139
/** Builder type for {@link BazelLockFileValue}. */
@@ -148,6 +151,8 @@ public abstract Builder setModuleExtensions(
148151
ImmutableMap<ModuleExtensionEvalFactors, LockFileModuleExtension>>
149152
value);
150153

154+
public abstract Builder setFacts(ImmutableMap<ModuleExtensionId, Facts> value);
155+
151156
public abstract BazelLockFileValue build();
152157
}
153158
}

0 commit comments

Comments
 (0)