Move management of .bzl builtin symbols to StarlarkBuiltinsFunction
This change centralizes knowledge of the set of predeclared symbols for .bzl files (and in a future change, perhaps BUILD/WORKSPACE files) in StarlarkBuiltinsFunction. For WORKSPACE- and @builtins-loaded .bzls, where the set of predeclareds does not change, it is computed once at Skyframe initialization time. For BUILD-loaded .bzls, the original set of predeclareds is also computed and saved at init time, but in the future we'll request the injected symbols from exports.bzl and save a modified environment in the returned StarlarkBuiltinsValue.
ConfiguredRuleClassProvider and PackageFactory now provide a way to distinguish "rule logic" symbols (e.g. `CcInfo`) from generic symbols (e.g., `rule()`). This will be used in a future CL to prohibit injecting new values for generic symbols.
Work toward #11437.
Other changes:
StarlarkBuiltinsValue:
- Store the desired environment, rather than the delta between the original environment and the desired one. This makes it so we won't have to reapply the delta (and validation) on every load.
BzlLoadFunction:
- Move Module creation out of executeModule(), and rename the latter to executeBzlFile().
- Simplify comment at call to executeBzlFile(), pass in a listener instead of whole Environment.
StarlarkBuiltinsFunctionTest:
- Use mock symbols in the test RuleClassProvider. Update tests to use mock symbols instead of non-existent names, since those will break going forward.
- Add tests of getNativeRuleLogicBindings(). (This seemed a better home for them than PackageFactoryTest, particularly because there's no generic RuleClassProviderTest.)
- Add tests of exports.bzl files that perform loads.
AbstractPackageLoader:
- Let it know about StarlarkBuiltinsFunction, apparently it's needed for tests (in a follow-up CL). Added a comment to SkyframeExecutor.
WorkspaceFactory:
- inline a helper method
RELNOTES: None
PiperOrigin-RevId: 315101101
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 f78b1b5..c1a3990 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
@@ -33,6 +33,7 @@
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.EventHandler;
+import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.ExtendedEventHandler.Postable;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.BazelModuleContext;
@@ -62,7 +63,6 @@
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import com.google.devtools.build.skyframe.ValueOrException;
-import java.util.HashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
@@ -86,10 +86,18 @@
*/
public class BzlLoadFunction implements SkyFunction {
- // Creates the BazelStarlarkContext and populates the predeclared .bzl symbols.
+ // We need the RuleClassProvider to 1) create the BazelStarlarkContext for Starlark evaluation,
+ // and 2) to pass it to ASTFileLookupFunction's inlining code path.
+ // TODO(#11437): The second use can probably go away by refactoring ASTFileLookupFunction to
+ // instead accept the set of predeclared bindings. Simplify the code path and then this comment.
private final RuleClassProvider ruleClassProvider;
- // Only used to retrieve the "native" object.
- private final PackageFactory packageFactory;
+
+ // TODO(#11437): Remove once we're getting builtins from StarlarkBuiltinsValue instead.
+ private final ImmutableMap<String, Object> predeclaredForBuildBzl;
+
+ private final ImmutableMap<String, Object> predeclaredForWorkspaceBzl;
+
+ private final ImmutableMap<String, Object> predeclaredForBuiltinsBzl;
// Handles retrieving ASTFileLookupValues, either by calling Skyframe or by inlining
// ASTFileLookupFunction; the latter is not to be confused with inlining of BzlLoadFunction. See
@@ -107,7 +115,17 @@
ASTManager astManager,
@Nullable CachedBzlLoadDataManager cachedBzlLoadDataManager) {
this.ruleClassProvider = ruleClassProvider;
- this.packageFactory = packageFactory;
+ this.predeclaredForBuildBzl =
+ StarlarkBuiltinsFunction.createPredeclaredForBuildBzlUsingInjection(
+ ruleClassProvider,
+ packageFactory,
+ /*exportedToplevels=*/ ImmutableMap.of(),
+ /*exportedRules=*/ ImmutableMap.of());
+ this.predeclaredForWorkspaceBzl =
+ StarlarkBuiltinsFunction.createPredeclaredForWorkspaceBzl(
+ ruleClassProvider, packageFactory);
+ this.predeclaredForBuiltinsBzl =
+ StarlarkBuiltinsFunction.createPredeclaredForBuiltinsBzl(ruleClassProvider);
this.astManager = astManager;
this.cachedBzlLoadDataManager = cachedBzlLoadDataManager;
}
@@ -522,18 +540,18 @@
}
byte[] transitiveDigest = fp.digestAndReset();
- // executeModule does not request values from the Environment. It may post events to the
- // Environment, but events do not matter when caching BzlLoadValues.
- Module module =
- executeModule(
- file,
- key.getLabel(),
- transitiveDigest,
- loadedModules,
- starlarkSemantics,
- env,
- /*inWorkspace=*/ key instanceof BzlLoadValue.KeyForWorkspace,
- repoMapping);
+ Module module = Module.withPredeclared(starlarkSemantics, getPredeclaredEnvironment(key));
+ module.setClientData(BazelModuleContext.create(label, transitiveDigest));
+ // executeBzlFile may post events to the Environment's handler, but events do not matter when
+ // caching BzlLoadValues. Note that executing the module mutates it.
+ executeBzlFile(
+ file,
+ key.getLabel(),
+ module,
+ loadedModules,
+ starlarkSemantics,
+ env.getListener(),
+ repoMapping);
BzlLoadValue result =
new BzlLoadValue(
module, transitiveDigest, new StarlarkFileDependency(label, fileDependencies.build()));
@@ -707,45 +725,53 @@
return valuesMissing ? null : bzlLoads;
}
+ /**
+ * Obtains the predeclared environment for a .bzl file, based on the type of file and (if
+ * applicable) the injected builtins.
+ */
+ private ImmutableMap<String, Object> getPredeclaredEnvironment(BzlLoadValue.Key key) {
+ if (key instanceof BzlLoadValue.KeyForBuild) {
+ return predeclaredForBuildBzl;
+ } else if (key instanceof BzlLoadValue.KeyForWorkspace) {
+ return predeclaredForWorkspaceBzl;
+ } else if (key instanceof BzlLoadValue.KeyForBuiltins) {
+ return predeclaredForBuiltinsBzl;
+ } else {
+ throw new AssertionError("Unknown key type: " + key.getClass());
+ }
+ }
+
/** Executes the .bzl file defining the module to be loaded. */
- private Module executeModule(
+ private void executeBzlFile(
StarlarkFile file,
Label label,
- byte[] transitiveDigest,
+ Module module,
Map<String, Module> loadedModules,
StarlarkSemantics starlarkSemantics,
- Environment env,
- boolean inWorkspace,
+ ExtendedEventHandler skyframeEventHandler,
ImmutableMap<RepositoryName, RepositoryName> repositoryMapping)
throws BzlLoadFailedException, InterruptedException {
- // set up .bzl predeclared environment
- Map<String, Object> predeclared = new HashMap<>(ruleClassProvider.getEnvironment());
- predeclared.put("native", packageFactory.getNativeModule(inWorkspace));
- Module module = Module.withPredeclared(starlarkSemantics, predeclared);
- module.setClientData(BazelModuleContext.create(label, transitiveDigest));
-
try (Mutability mu = Mutability.create("loading", label)) {
StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics);
thread.setLoader(loadedModules::get);
- StoredEventHandler eventHandler = new StoredEventHandler();
- thread.setPrintHandler(Event.makeDebugPrintHandler(eventHandler));
+ StoredEventHandler starlarkEventHandler = new StoredEventHandler();
+ thread.setPrintHandler(Event.makeDebugPrintHandler(starlarkEventHandler));
ruleClassProvider.setStarlarkThreadContext(thread, label, repositoryMapping);
- execAndExport(file, label, eventHandler, module, thread);
+ execAndExport(file, label, starlarkEventHandler, module, thread);
- Event.replayEventsOn(env.getListener(), eventHandler.getEvents());
- for (Postable post : eventHandler.getPosts()) {
- env.getListener().post(post);
+ Event.replayEventsOn(skyframeEventHandler, starlarkEventHandler.getEvents());
+ for (Postable post : starlarkEventHandler.getPosts()) {
+ skyframeEventHandler.post(post);
}
- if (eventHandler.hasErrors()) {
+ if (starlarkEventHandler.hasErrors()) {
throw BzlLoadFailedException.errors(label.toPathFragment());
}
- return module;
}
}
// Precondition: file is validated and error-free.
// Precondition: thread has a valid transitiveDigest.
- // TODO(adonovan): executeModule would make a better public API than this function.
+ // TODO(adonovan): executeBzlFile would make a better public API than this function.
public static void execAndExport(
StarlarkFile file, Label label, EventHandler handler, Module module, StarlarkThread thread)
throws InterruptedException {
