From 7897203286e2d1f4a2e3e2b920308f243b1ff3a6 Mon Sep 17 00:00:00 2001 From: Googler Date: Thu, 3 Aug 2023 12:17:55 -0700 Subject: [PATCH] Don't include builtins in the transitive digest for WORKSPACE/MODULE-loaded .bzl files See code comment for why. PiperOrigin-RevId: 553553630 Change-Id: I88806eb57f83c212d348f5a29d45dab98286a962 --- .../devtools/build/lib/skyframe/BzlLoadFunction.java | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java index 999d50e8d6fe8f..e78abec9b7e3aa 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java +++ b/src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java @@ -1325,7 +1325,15 @@ private ImmutableMap getAndDigestPredeclaredEnvironment( if (injectionDisabled || isFileSafeForUninjectedEvaluation(key)) { return starlarkEnv.getUninjectedWorkspaceBzlEnv(); } - fp.addBytes(builtins.transitiveDigest); + // Note that we don't actually fingerprint the injected builtins here. The actual builtins + // values should not be used in WORKSPACE-loaded or MODULE-loaded .bzl files; they're only + // injected to avoid certain type errors at loading time (e.g. #17713). If we included their + // digest, we'd be causing widespread repo refetches when _any_ builtin bzl file changes + // (when Bazel upgrades, for example), and potentially even thrashing if the user is using + // Bazelisk. Thus we make the explicit choice to not fingerprint the injected builtins, and + // thereby prohibit any meaningful use of injected builtins in WORKSPACE/MODULE-loaded .bzl + // files. This additionally means that native repo rules should not be migrated to + // @_builtins; they should just live in @bazel_tools instead. return builtins.predeclaredForWorkspaceBzl; } else if (key instanceof BzlLoadValue.KeyForBuiltins) { return starlarkEnv.getBuiltinsBzlEnv();