commit 2863252a837c9863fb78a41c74f8fb0fa2e8c7ff
author brandjon <brandjon@google.com> Mon Jun 08 13:14:07 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Jun 08 13:15:36 2020 -0700
tree cf100943572e41a6ef81c7e2c0bd7991e7ca00c3
parent 232a6b8e1aa5552e00b791fcca10407d6fb2a616

Add inlining code path to StarlarkBuiltinsFunction, and tests

StarlarkBuiltinsFunction grows a computeInline() method that accepts the calling BzlLoadFunction's InliningState and forwards it back to BzlLoadFunction#computeInline. It follows the same RecordingSkyFunctionEnvironment unwrapping convention as BzlLoadFunction uses to recursively call itself inline.

CachedBzlLoadData had some assertions on how its API could be used. These appear to be overly specific and are removed. (This allows the transitive dep on builtins to be resolved in any order relative to other deps.)

The tests are just clones of the existing tests with inlining turned on.

Work toward #11437.

RELNOTES: None
PiperOrigin-RevId: 315338126

src/main/java/com/google/devtools/build/lib/skyframe/BzlLoadFunction.java

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 243b1cc..798d3a5 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
@@ -93,6 +93,9 @@
   // instead accept the set of predeclared bindings. Simplify the code path and then this comment.
   private final RuleClassProvider ruleClassProvider;
 
+  // Used for StarlarkBuiltinsFunction's inlining code path.
+  private final PackageFactory packageFactory;
+
   // Used for BUILD .bzls if injection is disabled.
   // TODO(#11437): Remove once injection is on unconditionally.
   private final StarlarkBuiltinsValue uninjectedStarlarkBuiltins;
@@ -117,6 +120,7 @@
       ASTManager astManager,
       @Nullable CachedBzlLoadDataManager cachedBzlLoadDataManager) {
     this.ruleClassProvider = ruleClassProvider;
+    this.packageFactory = packageFactory;
     this.uninjectedStarlarkBuiltins =
         StarlarkBuiltinsFunction.createStarlarkBuiltinsValueWithoutInjection(
             ruleClassProvider, packageFactory);
@@ -348,17 +352,28 @@
    */
   @Nullable
   private StarlarkBuiltinsValue getStarlarkBuiltinsValue(
-      BzlLoadValue.Key key, Environment env, StarlarkSemantics starlarkSemantics)
-      throws BuiltinsFailedException, InterruptedException {
+      BzlLoadValue.Key key,
+      Environment env,
+      StarlarkSemantics starlarkSemantics,
+      @Nullable InliningState inliningState)
+      throws BuiltinsFailedException, InconsistentFilesystemException, InterruptedException {
     // Don't request @builtins if we're in workspace evaluation, or already in @builtins evaluation.
     if (!(key instanceof BzlLoadValue.KeyForBuild)
         // TODO(#11437): Remove ability to disable injection by setting flag to empty string.
         || starlarkSemantics.experimentalBuiltinsBzlPath().equals("")) {
       return uninjectedStarlarkBuiltins;
     }
-    // May be null.
-    return (StarlarkBuiltinsValue)
-        env.getValueOrThrow(StarlarkBuiltinsValue.key(), BuiltinsFailedException.class);
+    // Result may be null.
+    return inliningState == null
+        ? (StarlarkBuiltinsValue)
+            env.getValueOrThrow(StarlarkBuiltinsValue.key(), BuiltinsFailedException.class)
+        : StarlarkBuiltinsFunction.computeInline(
+            StarlarkBuiltinsValue.key(),
+            env,
+            inliningState,
+            /*bzlLoadFunction=*/ this,
+            ruleClassProvider,
+            packageFactory);
   }
 
   @Nullable
@@ -485,7 +500,7 @@
 
     StarlarkBuiltinsValue starlarkBuiltinsValue;
     try {
-      starlarkBuiltinsValue = getStarlarkBuiltinsValue(key, env, starlarkSemantics);
+      starlarkBuiltinsValue = getStarlarkBuiltinsValue(key, env, starlarkSemantics, inliningState);
     } catch (BuiltinsFailedException e) {
       throw BzlLoadFailedException.builtinsFailed(label, e);
     }