From 93677c68f0bc688dbfa75484688160cdbdae7328 Mon Sep 17 00:00:00 2001 From: nharmata Date: Fri, 1 Apr 2022 13:19:21 -0700 Subject: [PATCH] Make `BundledFileSystem#getDigest` more useful. See the added comment for motivation. PiperOrigin-RevId: 438895870 --- .../analysis/ConfiguredRuleClassProvider.java | 28 ++++++++++++------- 1 file changed, 18 insertions(+), 10 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java index 005147e50a8fe2..49e9e3657c05af 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java @@ -106,26 +106,34 @@ default ImmutableList requires() { /** An InMemoryFileSystem for bundled builtins .bzl files. */ public static class BundledFileSystem extends InMemoryFileSystem { - - private static final byte[] EMPTY_DIGEST = new byte[0]; - public BundledFileSystem() { super(DigestHashFunction.SHA256); } - // Bundled files are guaranteed to not change throughout the lifetime of the Bazel server, so it - // is permissible to use a fake digest. This helps avoid peculiarities in the interaction of - // InMemoryFileSystem and Skyframe. See cl/354809138 for further discussion, including of - // possible (but unlikely) future caveats of this approach. + // Pretend the digest of a bundled file is uniquely determined by its name, not its contents. + // + // The contents bundled files are guaranteed to not change throughout the lifetime of the Bazel + // server, we do not need to detect changes to a bundled file's contents. Not needing to worry + // about get the actual digest and detect changes to that digest helps avoid peculiarities in + // the interaction of InMemoryFileSystem and Skyframe. See cl/354809138 for further discussion, + // including of possible (but unlikely) future caveats of this approach. + // + // On the other hand, we do need to want different bundled files to have different digests. That + // way the bzl environment hashes for bzl rule classes defined in two different bundled files + // are guaranteed to be different, even if their set of transitive load statements is the same. + // This is important because it's possible for bzl rule classes defined in different files to + // have the same name string, and various part of Blaze rely on the pair of + // "rule class name string" and "bzl environment hash" to uniquely identify a bzl rule class. + // See b/226379109 for details. @Override protected synchronized byte[] getFastDigest(PathFragment path) { - return EMPTY_DIGEST; + return getDigest(path); } @Override - protected synchronized byte[] getDigest(PathFragment path) throws IOException { - return EMPTY_DIGEST; + protected synchronized byte[] getDigest(PathFragment path) { + return getDigestFunction().getHashFunction().hashString(path.toString(), UTF_8).asBytes(); } }