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/analysis/ConfiguredRuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/analysis/ConfiguredRuleClassProvider.java
index efe2d4d..86675b6 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
@@ -519,6 +519,8 @@
private final PrerequisiteValidator prerequisiteValidator;
+ private final ImmutableMap<String, Object> nativeRuleSpecificBindings;
+
private final ImmutableMap<String, Object> environment;
private final ImmutableList<SymlinkDefinition> symlinkDefinitions;
@@ -574,7 +576,9 @@
this.toolchainTaggedTrimmingTransition = toolchainTaggedTrimmingTransition;
this.shouldInvalidateCacheForOptionDiff = shouldInvalidateCacheForOptionDiff;
this.prerequisiteValidator = prerequisiteValidator;
- this.environment = createEnvironment(starlarkAccessibleJavaClasses, starlarkBootstraps);
+ this.nativeRuleSpecificBindings =
+ createNativeRuleSpecificBindings(starlarkAccessibleJavaClasses, starlarkBootstraps);
+ this.environment = createEnvironment(nativeRuleSpecificBindings);
this.symlinkDefinitions = symlinkDefinitions;
this.reservedActionMnemonics = reservedActionMnemonics;
this.actionEnvironmentProvider = actionEnvironmentProvider;
@@ -725,19 +729,24 @@
return BuildOptions.of(configurationOptions, optionsProvider);
}
- private static ImmutableMap<String, Object> createEnvironment(
+ private static ImmutableMap<String, Object> createNativeRuleSpecificBindings(
ImmutableMap<String, Object> starlarkAccessibleTopLevels,
ImmutableList<Bootstrap> bootstraps) {
- ImmutableMap.Builder<String, Object> envBuilder = ImmutableMap.builder();
+ ImmutableMap.Builder<String, Object> bindings = ImmutableMap.builder();
+ bindings.putAll(starlarkAccessibleTopLevels);
+ for (Bootstrap bootstrap : bootstraps) {
+ bootstrap.addBindingsToBuilder(bindings);
+ }
+ return bindings.build();
+ }
+ private static ImmutableMap<String, Object> createEnvironment(
+ ImmutableMap<String, Object> nativeRuleSpecificBindings) {
+ ImmutableMap.Builder<String, Object> envBuilder = ImmutableMap.builder();
// Add predeclared symbols of the Bazel build language.
StarlarkModules.addStarlarkGlobalsToBuilder(envBuilder);
-
- envBuilder.putAll(starlarkAccessibleTopLevels.entrySet());
- for (Bootstrap bootstrap : bootstraps) {
- bootstrap.addBindingsToBuilder(envBuilder);
- }
-
+ // Add all the extensions registered with the rule class provider.
+ envBuilder.putAll(nativeRuleSpecificBindings);
return envBuilder.build();
}
@@ -755,6 +764,15 @@
}
@Override
+ public ImmutableMap<String, Object> getNativeRuleSpecificBindings() {
+ // Include rule-related stuff like CcInfo, but not core stuff like rule(). Essentially, this
+ // is intended to include things that could in principle be migrated to Starlark (and hence
+ // should be overridable by @builtins); in practice it means anything specifically registered
+ // with the RuleClassProvider.
+ return nativeRuleSpecificBindings;
+ }
+
+ @Override
public ImmutableMap<String, Object> getEnvironment() {
return environment;
}
diff --git a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
index 857b72d..2ecb069 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/PackageFactory.java
@@ -39,7 +39,6 @@
import com.google.devtools.build.lib.profiler.SilentCloseable;
import com.google.devtools.build.lib.syntax.Argument;
import com.google.devtools.build.lib.syntax.CallExpression;
-import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.DefStatement;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
@@ -84,11 +83,11 @@
import javax.annotation.Nullable;
/**
- * The package factory is responsible for constructing Package instances
- * from a BUILD file's abstract syntax tree (AST).
+ * The package factory is responsible for constructing Package instances from a BUILD file's
+ * abstract syntax tree (AST).
*
- * <p>A PackageFactory is a heavy-weight object; create them sparingly.
- * Typically only one is needed per client application.
+ * <p>A PackageFactory is a heavy-weight object; create them sparingly. Typically only one is needed
+ * per client application.
*/
public final class PackageFactory {
@@ -105,9 +104,7 @@
/** Update the environment of the native module. */
void updateNative(ImmutableMap.Builder<String, Object> env);
- /**
- * Returns the extra arguments to the {@code package()} statement.
- */
+ /** Returns the extra arguments to the {@code package()} statement. */
Iterable<PackageArgument<?>> getPackageArguments();
}
@@ -124,6 +121,9 @@
private final ImmutableList<EnvironmentExtension> environmentExtensions;
private final ImmutableMap<String, PackageArgument<?>> packageArguments;
+ private final ImmutableMap<String, Object> nativeModuleBindingsForBuild;
+ private final ImmutableMap<String, Object> nativeModuleBindingsForWorkspace;
+
private final Package.Builder.Helper packageBuilderHelper;
private final PackageValidator packageValidator;
private final PackageLoadingListener packageLoadingListener;
@@ -169,6 +169,9 @@
* <p>Do not call this constructor directly in tests; please use
* TestConstants#PACKAGE_FACTORY_BUILDER_FACTORY_FOR_TESTING instead.
*/
+ // TODO(bazel-team): Maybe store `version` in the RuleClassProvider rather than passing it in
+ // here? It's an extra constructor parameter that all the tests have to give, and it's only needed
+ // so WorkspaceFactory can add an extra top-level builtin.
public PackageFactory(
RuleClassProvider ruleClassProvider,
Iterable<EnvironmentExtension> environmentExtensions,
@@ -182,23 +185,22 @@
setGlobbingThreads(100);
this.environmentExtensions = ImmutableList.copyOf(environmentExtensions);
this.packageArguments = createPackageArguments();
- this.nativeModule = newNativeModule();
- this.workspaceNativeModule = WorkspaceFactory.newNativeModule(ruleClassProvider, version);
+ this.nativeModuleBindingsForBuild =
+ createNativeModuleBindingsForBuild(
+ ruleFunctions, packageArguments, this.environmentExtensions);
+ this.nativeModuleBindingsForWorkspace =
+ createNativeModuleBindingsForWorkspace(ruleClassProvider, version);
this.packageBuilderHelper = packageBuilderHelper;
this.packageValidator = packageValidator;
this.packageLoadingListener = packageLoadingListener;
}
- /**
- * Sets the syscalls cache used in globbing.
- */
+ /** Sets the syscalls cache used in globbing. */
public void setSyscalls(AtomicReference<? extends UnixGlob.FilesystemCalls> syscalls) {
this.syscalls = Preconditions.checkNotNull(syscalls);
}
- /**
- * Sets the max number of threads to use for globbing.
- */
+ /** Sets the max number of threads to use for globbing. */
public void setGlobbingThreads(int globbingThreads) {
if (executor == null || executor.getParallelism() != globbingThreads) {
executor = NamedForkJoinPool.newNamedPool("globbing pool", globbingThreads);
@@ -217,10 +219,7 @@
this.maxDirectoriesToEagerlyVisitInGlobbing = maxDirectoriesToEagerlyVisitInGlobbing;
}
- /**
- * Returns the immutable, unordered set of names of all the known rule
- * classes.
- */
+ /** Returns the immutable, unordered set of names of all the known rule classes. */
public Set<String> getRuleClassNames() {
return ruleFactory.getRuleClassNames();
}
@@ -233,9 +232,7 @@
return ruleFactory.getRuleClass(ruleClassName);
}
- /**
- * Returns the {@link RuleClassProvider} of this {@link PackageFactory}.
- */
+ /** Returns the {@link RuleClassProvider} of this {@link PackageFactory}. */
public RuleClassProvider getRuleClassProvider() {
return ruleClassProvider;
}
@@ -244,9 +241,26 @@
return environmentExtensions;
}
+ /** Returns the bindings to add to the "native" module, for BUILD-loaded .bzl files. */
+ public ImmutableMap<String, Object> getNativeModuleBindingsForBuild() {
+ return nativeModuleBindingsForBuild;
+ }
+
+ /** Returns the bindings to add to the "native" module, for WORKSPACE-loaded .bzl files. */
+ public ImmutableMap<String, Object> getNativeModuleBindingsForWorkspace() {
+ return nativeModuleBindingsForWorkspace;
+ }
+
/**
- * Creates the list of arguments for the 'package' function.
+ * Returns the subset of bindings of the "native" module (for BUILD-loaded .bzls) that are rules.
+ *
+ * <p>Excludes non-rule functions such as {@code glob()}.
*/
+ public ImmutableMap<String, ?> getNativeRules() {
+ return ruleFunctions;
+ }
+
+ /** Creates the map of arguments for the 'package' function. */
private ImmutableMap<String, PackageArgument<?>> createPackageArguments() {
ImmutableList.Builder<PackageArgument<?>> arguments =
ImmutableList.<PackageArgument<?>>builder().addAll(DefaultPackageArguments.get());
@@ -593,41 +607,26 @@
this.globber = globber;
}
- /**
- * Returns the Label of this Package.
- */
+ /** Returns the Label of this Package's BUILD file. */
public Label getLabel() {
return pkgBuilder.getBuildFileLabel();
}
- /**
- * Sets a Make variable.
- */
+ /** Sets a Make variable. */
public void setMakeVariable(String name, String value) {
pkgBuilder.setMakeVariable(name, value);
}
- /**
- * Returns the builder of this Package.
- */
+ /** Returns the builder of this Package. */
public Package.Builder getBuilder() {
return pkgBuilder;
}
}
- private final ClassObject nativeModule;
- private final ClassObject workspaceNativeModule;
-
- /** @return the Starlark struct to bind to "native" */
- public ClassObject getNativeModule(boolean workspace) {
- return workspace ? workspaceNativeModule : nativeModule;
- }
-
- /**
- * Returns a native module with the functions created using the {@link RuleClassProvider}
- * of this {@link PackageFactory}.
- */
- private ClassObject newNativeModule() {
+ private static ImmutableMap<String, Object> createNativeModuleBindingsForBuild(
+ ImmutableMap<String, BuiltinRuleFunction> ruleFunctions,
+ ImmutableMap<String, PackageArgument<?>> packageArguments,
+ ImmutableList<EnvironmentExtension> environmentExtensions) {
ImmutableMap.Builder<String, Object> builder = new ImmutableMap.Builder<>();
builder.putAll(StarlarkNativeModule.BINDINGS_FOR_BUILD_FILES);
builder.putAll(ruleFunctions);
@@ -635,7 +634,12 @@
for (EnvironmentExtension ext : environmentExtensions) {
ext.updateNative(builder);
}
- return StructProvider.STRUCT.create(builder.build(), "no native function or rule '%s'");
+ return builder.build();
+ }
+
+ private static ImmutableMap<String, Object> createNativeModuleBindingsForWorkspace(
+ RuleClassProvider ruleClassProvider, String version) {
+ return WorkspaceFactory.createNativeModuleBindings(ruleClassProvider, version);
}
private void populateEnvironment(ImmutableMap.Builder<String, Object> env) {
diff --git a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
index 9afa306..7676916 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/RuleClassProvider.java
@@ -60,10 +60,20 @@
ImmutableMap<RepositoryName, RepositoryName> repoMapping);
/**
+ * Returns all the predeclared top-level symbols (for .bzl files) that belong to native rule sets,
+ * and hence are allowed to be overridden by builtins-injection.
+ *
+ * <p>For example, {@code CcInfo} is included, but {@code rule()} is not.
+ *
+ * @see StarlarkBuiltinsFunction
+ */
+ ImmutableMap<String, Object> getNativeRuleSpecificBindings();
+
+ /**
* Returns the predeclared environment for a loading-phase thread. Includes "native", though its
* value may be inappropriate for a WORKSPACE file. Excludes universal bindings (e.g. True, len).
*/
- // TODO(adonovan): update doc comment. And does it really include native?
+ // TODO(adonovan, brandjon): update doc comment. And does it really include native?
ImmutableMap<String, Object> getEnvironment();
/**
diff --git a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
index df31189..98e522c 100644
--- a/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/packages/WorkspaceFactory.java
@@ -27,7 +27,6 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.PackageFactory.EnvironmentExtension;
-import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.EvalUtils;
@@ -105,7 +104,7 @@
this.workspaceGlobals = new WorkspaceGlobals(allowOverride, ruleFactory);
this.starlarkSemantics = starlarkSemantics;
this.workspaceFunctions =
- WorkspaceFactory.createWorkspaceFunctions(
+ createWorkspaceFunctions(
allowOverride, ruleFactory, this.workspaceGlobals, starlarkSemantics);
}
@@ -377,10 +376,24 @@
return defaultSystemJavabaseDir != null ? defaultSystemJavabaseDir.toString() : "";
}
- private static ClassObject newNativeModule(
- ImmutableMap<String, Object> workspaceFunctions, String version) {
- ImmutableMap.Builder<String, Object> env = new ImmutableMap.Builder<>();
- Starlark.addMethods(env, new StarlarkNativeModule());
+ /** Returns the entries to populate the "native" module with, for WORKSPACE-loaded .bzl files. */
+ static ImmutableMap<String, Object> createNativeModuleBindings(
+ RuleClassProvider ruleClassProvider, String version) {
+ // Machinery to build the collection of workspace functions.
+ RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
+ WorkspaceGlobals workspaceGlobals = new WorkspaceGlobals(/*allowOverride=*/ false, ruleFactory);
+ // TODO(bazel-team): StarlarkSemantics should be a parameter here, as native module can be
+ // configured by flags. [brandjon: This should be possible now that we create the native module
+ // in StarlarkBuiltinsFunction. We could defer creation until the StarlarkSemantics are known.
+ // But mind that some code may depend on being able to enumerate all possible entries regardless
+ // of the particular semantics.]
+ ImmutableMap<String, Object> workspaceFunctions =
+ createWorkspaceFunctions(
+ /*allowOverride=*/ false, ruleFactory, workspaceGlobals, StarlarkSemantics.DEFAULT);
+
+ // Determine the contents for native.
+ ImmutableMap.Builder<String, Object> bindings = new ImmutableMap.Builder<>();
+ Starlark.addMethods(bindings, new StarlarkNativeModule());
for (Map.Entry<String, Object> entry : workspaceFunctions.entrySet()) {
String name = entry.getKey();
if (name.startsWith("$")) {
@@ -393,22 +406,11 @@
if (name.equals("workspace")) {
continue;
}
- env.put(entry);
+ bindings.put(entry);
}
+ bindings.put("bazel_version", version);
- env.put("bazel_version", version);
- return StructProvider.STRUCT.create(env.build(), "no native function or rule '%s'");
- }
-
- static ClassObject newNativeModule(RuleClassProvider ruleClassProvider, String version) {
- RuleFactory ruleFactory = new RuleFactory(ruleClassProvider);
- WorkspaceGlobals workspaceGlobals = new WorkspaceGlobals(false, ruleFactory);
- // TODO(ichern): StarlarkSemantics should be a parameter here, as native module can be
- // configured by flags.
- return WorkspaceFactory.newNativeModule(
- WorkspaceFactory.createWorkspaceFunctions(
- false, ruleFactory, workspaceGlobals, StarlarkSemantics.DEFAULT),
- version);
+ return bindings.build();
}
public Map<String, Module> getLoadedModules() {
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 {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
index 9b59e0d..3554a28 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java
@@ -479,6 +479,8 @@
// We don't check for duplicates in order to allow extraSkyfunctions to override existing
// entries.
Map<SkyFunctionName, SkyFunction> map = new HashMap<>();
+ // IF YOU ADD A NEW SKYFUNCTION: If your Skyfunction can be used transitively by package
+ // loading, make sure to register it in AbstractPackageLoader as well.
map.put(SkyFunctions.PRECOMPUTED, new PrecomputedFunction());
map.put(SkyFunctions.CLIENT_ENVIRONMENT_VARIABLE, new ClientEnvironmentFunction(clientEnv));
map.put(SkyFunctions.ACTION_ENVIRONMENT_VARIABLE, new ActionEnvironmentFunction());
@@ -501,7 +503,9 @@
map.put(
SkyFunctions.AST_FILE_LOOKUP,
new ASTFileLookupFunction(ruleClassProvider, DigestHashFunction.getDefaultUnchecked()));
- map.put(SkyFunctions.STARLARK_BUILTINS, new StarlarkBuiltinsFunction());
+ map.put(
+ SkyFunctions.STARLARK_BUILTINS,
+ new StarlarkBuiltinsFunction(ruleClassProvider, pkgFactory));
map.put(SkyFunctions.BZL_LOAD, newBzlLoadFunction(ruleClassProvider, pkgFactory));
map.put(SkyFunctions.GLOB, newGlobFunction());
map.put(SkyFunctions.TARGET_PATTERN, new TargetPatternFunction());
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
index 97f9ba3..a610a56 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunction.java
@@ -16,6 +16,9 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.packages.PackageFactory;
+import com.google.devtools.build.lib.packages.RuleClassProvider;
+import com.google.devtools.build.lib.packages.StructProvider;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Location;
@@ -24,23 +27,48 @@
import com.google.devtools.build.skyframe.SkyFunctionException;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
+import java.util.LinkedHashMap;
+import java.util.Map;
import javax.annotation.Nullable;
+// TODO(#11437): Teach ASTFileLookup about builtins keys, then have them modify their env the same
+// way as BzlLoadFunction. This will allow us to define the _internal symbol for @builtins.
+
// TODO(#11437): Determine places where we need to teach Skyframe about this Skyfunction. Look for
// special treatment of BzlLoadFunction or ASTFileLookupFunction in existing code.
// TODO(#11437): Add support to StarlarkModuleCycleReporter to pretty-print cycles involving
// @builtins. Blocked on us actually loading files from @builtins.
+// TODO(#11437): Add tombstone feature: If a native symbol is a tombstone object, this signals to
+// StarlarkBuiltinsFunction that the corresponding symbol *must* be defined by @builtins.
+// Furthermore, if exports.bzl also says the symbol is a tombstone, any attempt to use it results
+// in failure, as if the symbol doesn't exist at all (or with a user-friendly error message saying
+// to migrate by adding a load()). Combine tombstones with reading the current incompatible flags
+// within @builtins for awesomeness.
+
+// TODO(#11437): Currently, BUILD-loaded .bzls and WORKSPACE-loaded .bzls have the same initial
+// static environment. Therefore, we should apply injection of top-level symbols to both
+// environments, not just BUILD .bzls. Likewise for builtins that are shared by BUILD and WORKSPACE
+// files, and for when the dynamic `native` values of the two .bzl dialects are unified. This can be
+// facilitated by 1) making PackageFactory better understand the similarities between BUILD and
+// WORKSPACE, e.g. by refactoring things like WorkspaceFactory#createWorkspaceFunctions into
+// PackageFactory; and 2) making PackageFactory responsible for performing the actual injection
+// (given the override mappings from exports.bzl) and returning the modified environments. Then any
+// refactoring to unify BUILD with WORKPSACE and BUILD-bzl with WORKSPACE-bzl can proceed in
+// PackageFactory without regard to this file.
+
/**
* A Skyframe function that evaluates the {@code @builtins} pseudo-repository and reports the values
* exported by {@code @builtins//:exports.bzl}.
*
+ * <p>The process of "builtins injection" refers to evaluating this Skyfunction and applying its
+ * result to {@link BzlLoadFunction}'s computation. See also the <a
+ * href="https://docs.google.com/document/d/1GW7UVo1s9X0cti9OMgT3ga5ozKYUWLPk9k8c4-34rC4">design
+ * doc</a>:
+ *
* <p>This function has a trivial key, so there can only be one value in the build at a time. It has
* a single dependency, on the result of evaluating the exports.bzl file to a {@link BzlLoadValue}.
- *
- * <p>See also the design doc:
- * https://docs.google.com/document/d/1GW7UVo1s9X0cti9OMgT3ga5ozKYUWLPk9k8c4-34rC4/edit
*/
public class StarlarkBuiltinsFunction implements SkyFunction {
@@ -67,7 +95,16 @@
// @builtins namespace.
Label.parseAbsoluteUnchecked("//tools/builtins_staging:exports.bzl"));
- StarlarkBuiltinsFunction() {}
+ // Used to obtain the default .bzl top-level environment, sans "native".
+ private final RuleClassProvider ruleClassProvider;
+ // Used to obtain the default contents of the "native" object.
+ private final PackageFactory packageFactory;
+
+ public StarlarkBuiltinsFunction(
+ RuleClassProvider ruleClassProvider, PackageFactory packageFactory) {
+ this.ruleClassProvider = ruleClassProvider;
+ this.packageFactory = packageFactory;
+ }
@Override
public SkyValue compute(SkyKey skyKey, Environment env)
@@ -85,8 +122,10 @@
ImmutableMap<String, Object> exportedToplevels = getDict(module, "exported_toplevels");
ImmutableMap<String, Object> exportedRules = getDict(module, "exported_rules");
ImmutableMap<String, Object> exportedToJava = getDict(module, "exported_to_java");
- return new StarlarkBuiltinsValue(
- exportedToplevels, exportedRules, exportedToJava, transitiveDigest);
+ ImmutableMap<String, Object> predeclared =
+ createPredeclaredForBuildBzlUsingInjection(
+ ruleClassProvider, packageFactory, exportedToplevels, exportedRules);
+ return new StarlarkBuiltinsValue(predeclared, exportedToJava, transitiveDigest);
} catch (EvalException ex) {
ex.ensureLocation(EXPORTS_ENTRYPOINT_LOC);
throw new StarlarkBuiltinsFunctionException(ex);
@@ -94,6 +133,98 @@
}
/**
+ * Returns the set of predeclared symbols to initialize a Starlark {@link Module} with, for
+ * evaluating .bzls loaded from a BUILD file.
+ */
+ // TODO(#11437): Make private in follow-up CL to retrieve predeclared environment from
+ // StarlarkBuiltinsValue instead of this static helper.
+ static ImmutableMap<String, Object> createPredeclaredForBuildBzlUsingInjection(
+ RuleClassProvider ruleClassProvider,
+ PackageFactory packageFactory,
+ ImmutableMap<String, Object> exportedToplevels,
+ ImmutableMap<String, Object> exportedRules) {
+ // TODO(#11437): Validate that all symbols supplied to exportedToplevels and exportedRules are
+ // existing rule-logic symbols.
+
+ // It's probably not necessary to preserve order, but let's do it just in case.
+ Map<String, Object> predeclared = new LinkedHashMap<>();
+
+ // Determine the top-level bindings.
+ predeclared.putAll(ruleClassProvider.getEnvironment());
+ predeclared.putAll(exportedToplevels);
+ // TODO(#11437): We *should* be able to uncomment the following line, but the native module is
+ // added prematurely (without its rule-logic fields) and overridden unconditionally. Fix this
+ // once ASTFileLookupFunction takes in the set of predeclared bindings (currently it directly
+ // // checks getEnvironment()).
+ // Preconditions.checkState(!predeclared.containsKey("native"));
+
+ // Determine the "native" module.
+ // TODO(bazel-team): Use the same "native" object for both BUILD- and WORKSPACE-loaded .bzls,
+ // and just have it be a dynamic error to call the wrong thing at the wrong time. This is a
+ // breaking change.
+ Map<String, Object> nativeEntries = new LinkedHashMap<>();
+ for (Map.Entry<String, Object> entry :
+ packageFactory.getNativeModuleBindingsForBuild().entrySet()) {
+ String symbolName = entry.getKey();
+ Object replacementSymbol = exportedRules.get(symbolName);
+ if (replacementSymbol != null) {
+ nativeEntries.put(symbolName, replacementSymbol);
+ } else {
+ nativeEntries.put(symbolName, entry.getValue());
+ }
+ }
+ predeclared.put("native", createNativeModule(nativeEntries));
+
+ return ImmutableMap.copyOf(predeclared);
+ }
+
+ /** Returns a {@link StarlarkBuiltinsValue} that completely ignores injected builtins. */
+ // TODO(#11437): Delete once injection cannot be disabled.
+ static StarlarkBuiltinsValue createStarlarkBuiltinsValueWithoutInjection(
+ RuleClassProvider ruleClassProvider, PackageFactory packageFactory) {
+ ImmutableMap<String, Object> predeclared =
+ createPredeclaredForBuildBzlUsingInjection(
+ ruleClassProvider,
+ packageFactory,
+ /*exportedToplevels=*/ ImmutableMap.of(),
+ /*exportedRules=*/ ImmutableMap.of());
+ return new StarlarkBuiltinsValue(
+ /*predeclaredForBuildBzl=*/ predeclared,
+ /*exportedToJava=*/ ImmutableMap.of(),
+ /*transitiveDigest=*/ new byte[] {});
+ }
+
+ /**
+ * Returns the set of predeclared symbols to initialize a Starlark {@link Module} with, for
+ * evaluating .bzls loaded from a WORKSPACE file.
+ */
+ static ImmutableMap<String, Object> createPredeclaredForWorkspaceBzl(
+ RuleClassProvider ruleClassProvider, PackageFactory packageFactory) {
+ // Preserve order, just in case.
+ Map<String, Object> predeclared = new LinkedHashMap<>();
+ predeclared.putAll(ruleClassProvider.getEnvironment());
+ Object nativeModule = createNativeModule(packageFactory.getNativeModuleBindingsForWorkspace());
+ // TODO(#11437): Assert not already present; see createPreclaredsForBuildBzl.
+ predeclared.put("native", nativeModule);
+ return ImmutableMap.copyOf(predeclared);
+ }
+
+ /**
+ * Returns the set of predeclared symbols to initialize a Starlark {@link Module} with, for
+ * evaluating .bzls loaded from the {@code @builtins} pseudo-repository.
+ */
+ // TODO(#11437): create the _internal name, prohibit other rule logic names. Take in a
+ // PackageFactory.
+ static ImmutableMap<String, Object> createPredeclaredForBuiltinsBzl(
+ RuleClassProvider ruleClassProvider) {
+ return ruleClassProvider.getEnvironment();
+ }
+
+ private static Object createNativeModule(Map<String, Object> bindings) {
+ return StructProvider.STRUCT.create(bindings, "no native function or rule '%s'");
+ }
+
+ /**
* Attempts to retrieve the string-keyed dict named {@code dictName} from the given {@code
* module}.
*
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java
index 2cf1e44..3e04747 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsValue.java
@@ -23,19 +23,16 @@
* A Skyframe value representing the Starlark symbols defined by the {@code @builtins}
* pseudo-repository.
*
- * <p>These are parsed from {@code @builtins//:exports.bzl}, but not validated until they're used by
- * {@link PackageFunction} and {@link BzlLoadFunction}.
+ * <p>These are parsed from {@code @builtins//:exports.bzl}.
*/
public final class StarlarkBuiltinsValue implements SkyValue {
// These are all deeply immutable (the Starlark values are already frozen), so let's skip the
// accessors and mutators.
- /** Contents of the {@code exported_toplevels} dict. */
- public final ImmutableMap<String, Object> exportedToplevels;
-
- /** Contents of the {@code exported_rules} dict. */
- public final ImmutableMap<String, Object> exportedRules;
+ /** Top-level predeclared symbols for a .bzl file (loaded on behalf of a BUILD file). */
+ // TODO(#11437): Corresponding predeclaredForBuild for BUILD files
+ public final ImmutableMap<String, Object> predeclaredForBuildBzl;
/** Contents of the {@code exported_to_java} dict. */
public final ImmutableMap<String, Object> exportedToJava;
@@ -44,12 +41,10 @@
public final byte[] transitiveDigest;
public StarlarkBuiltinsValue(
- ImmutableMap<String, Object> exportedToplevels,
- ImmutableMap<String, Object> exportedRules,
+ ImmutableMap<String, Object> predeclaredForBuildBzl,
ImmutableMap<String, Object> exportedToJava,
byte[] transitiveDigest) {
- this.exportedToplevels = exportedToplevels;
- this.exportedRules = exportedRules;
+ this.predeclaredForBuildBzl = predeclaredForBuildBzl;
this.exportedToJava = exportedToJava;
this.transitiveDigest = transitiveDigest;
}
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
index 25e2735..8f588f6 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/packages/AbstractPackageLoader.java
@@ -67,6 +67,7 @@
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
+import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsFunction;
import com.google.devtools.build.lib.skyframe.WorkspaceASTFunction;
import com.google.devtools.build.lib.skyframe.WorkspaceFileFunction;
import com.google.devtools.build.lib.skyframe.WorkspaceNameFunction;
@@ -444,6 +445,9 @@
SkyFunctions.AST_FILE_LOOKUP,
new ASTFileLookupFunction(ruleClassProvider, digestHashFunction))
.put(
+ SkyFunctions.STARLARK_BUILTINS,
+ new StarlarkBuiltinsFunction(ruleClassProvider, pkgFactory))
+ .put(
SkyFunctions.BZL_LOAD,
BzlLoadFunction.create(
ruleClassProvider,
diff --git a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
index a4104f1..48247c5 100644
--- a/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/packages/PackageFactoryTest.java
@@ -150,9 +150,8 @@
assertThat(pkg.getRule("has_dupe")).isNotNull();
assertThat(pkg.getRule("dep")).isNotNull();
assertThat(pkg.getRule("has_dupe").containsErrors()).isTrue();
- assertThat(pkg.getRule("dep").containsErrors()).isTrue(); // because all rules in an
- // errant package are
- // themselves errant.
+ // All rules in an errant package are themselves errant.
+ assertThat(pkg.getRule("dep").containsErrors()).isTrue();
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunctionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunctionTest.java
index 6847bc5..74689b9 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunctionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/StarlarkBuiltinsFunctionTest.java
@@ -18,23 +18,54 @@
import static com.google.devtools.build.skyframe.ErrorInfoSubjectFactory.assertThatErrorInfo;
import static com.google.devtools.build.skyframe.EvaluationResultSubjectFactory.assertThatEvaluationResult;
+import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
+import com.google.devtools.build.lib.analysis.util.MockRule;
import com.google.devtools.build.lib.skyframe.util.SkyframeExecutorTestUtils;
+import com.google.devtools.build.lib.syntax.ClassObject;
import com.google.devtools.build.lib.syntax.EvalException;
-import com.google.devtools.build.lib.syntax.StarlarkList;
+import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
import com.google.devtools.build.skyframe.EvaluationResult;
import com.google.devtools.build.skyframe.SkyKey;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
-/**
- * Tests for {@link StarlarkBuiltinsFunction}, and {@code @builtins} resolution behavior in {@link
- * {@link BzlLoadFunction}.
- */
+/** Tests for {@link StarlarkBuiltinsFunction}. */
@RunWith(JUnit4.class)
public class StarlarkBuiltinsFunctionTest extends BuildViewTestCase {
+ private static final MockRule OVERRIDABLE_RULE = () -> MockRule.define("overridable_rule");
+
+ @Override
+ protected ConfiguredRuleClassProvider getRuleClassProvider() {
+ // Add a fake rule and top-level symbol to override.
+ ConfiguredRuleClassProvider.Builder builder =
+ new ConfiguredRuleClassProvider.Builder()
+ .addRuleDefinition(OVERRIDABLE_RULE)
+ .addStarlarkAccessibleTopLevels("overridable_symbol", "original_value");
+ TestRuleClassProvider.addStandardRules(builder);
+ return builder.build();
+ }
+
+ @Test
+ public void getNativeRuleLogicBindings_inPackageFactory() throws Exception {
+ assertThat(getPackageFactory().getNativeRules()).containsKey("cc_library");
+ assertThat(getPackageFactory().getNativeRules()).doesNotContainKey("glob");
+ assertThat(getPackageFactory().getNativeRules()).containsKey("overridable_rule");
+ }
+
+ @Test
+ public void getNativeRuleLogicBindings_inRuleClassProvider() throws Exception {
+ assertThat(getRuleClassProvider().getNativeRuleSpecificBindings()).containsKey("CcInfo");
+ assertThat(getRuleClassProvider().getNativeRuleSpecificBindings()).doesNotContainKey("rule");
+ assertThat(getRuleClassProvider().getNativeRuleSpecificBindings())
+ .containsKey("overridable_symbol");
+ }
+
+ // TODO(#11437): Add tests for predeclared env of BUILD (and WORKSPACE?) files, once
+ // StarlarkBuiltinsFunction manages that functionality.
+
/** Sets up exports.bzl with the given contents and evaluates the {@code @builtins}. */
private EvaluationResult<StarlarkBuiltinsValue> evalBuiltins(String... lines) throws Exception {
scratch.file("tools/builtins_staging/BUILD");
@@ -59,31 +90,62 @@
}
@Test
- public void success() throws Exception {
+ public void evalExportsSuccess() throws Exception {
EvaluationResult<StarlarkBuiltinsValue> result =
evalBuiltins(
- "exported_toplevels = {'a': 1, 'b': 2}",
- "exported_rules = {'b': True}",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}");
+ "exported_toplevels = {'overridable_symbol': 'new_value'}",
+ "exported_rules = {'overridable_rule': 'new_rule'}",
+ "exported_to_java = {'for_native_code': 'secret_sauce'}");
SkyKey key = StarlarkBuiltinsValue.key();
assertThatEvaluationResult(result).hasNoError();
StarlarkBuiltinsValue value = result.get(key);
- assertThat(value.exportedToplevels).containsExactly("a", 1, "b", 2).inOrder();
- assertThat(value.exportedRules).containsExactly("b", true);
- assertThat(value.exportedToJava)
- .containsExactly("c", StarlarkList.of(/*mutability=*/ null, 1, 2, 3), "d", "foo")
- .inOrder();
+
+ // Universe symbols are omitted (they're added by the interpreter).
+ assertThat(value.predeclaredForBuildBzl).doesNotContainKey("print");
+ // Generic Bazel symbols are present.
+ assertThat(value.predeclaredForBuildBzl).containsKey("rule");
+ // Non-overridden symbols are present.
+ assertThat(value.predeclaredForBuildBzl).containsKey("CcInfo");
+ // Overridden symbol.
+ assertThat(value.predeclaredForBuildBzl).containsEntry("overridable_symbol", "new_value");
+ // Overridden native field.
+ Object nativeField =
+ ((ClassObject) value.predeclaredForBuildBzl.get("native")).getValue("overridable_rule");
+ assertThat(nativeField).isEqualTo("new_rule");
+ // Stuff for native rules.
+ assertThat(value.exportedToJava).containsExactly("for_native_code", "secret_sauce").inOrder();
// No test of the digest.
}
@Test
- public void missingDictSymbol() throws Exception {
+ public void evalExportsSuccess_withLoad() throws Exception {
+ // TODO(#11437): Use @builtins//:... syntax, once supported. Don't create a real package.
+ scratch.file("builtins_helper/BUILD");
+ scratch.file(
+ "builtins_helper/dummy.bzl", //
+ "toplevels = {'overridable_symbol': 'new_value'}");
+
+ EvaluationResult<StarlarkBuiltinsValue> result =
+ evalBuiltins(
+ "load('//builtins_helper:dummy.bzl', 'toplevels')",
+ "exported_toplevels = toplevels",
+ "exported_rules = {}",
+ "exported_to_java = {}");
+
+ SkyKey key = StarlarkBuiltinsValue.key();
+ assertThatEvaluationResult(result).hasNoError();
+ StarlarkBuiltinsValue value = result.get(key);
+ assertThat(value.predeclaredForBuildBzl).containsEntry("overridable_symbol", "new_value");
+ }
+
+ @Test
+ public void evalExportsFails_missingDictSymbol() throws Exception {
Exception ex =
evalBuiltinsToException(
- "exported_toplevels = {'a': 1, 'b': 2}",
+ "exported_toplevels = {}", //
"# exported_rules missing",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}");
+ "exported_to_java = {}");
assertThat(ex).isInstanceOf(EvalException.class);
assertThat(ex)
.hasMessageThat()
@@ -91,23 +153,23 @@
}
@Test
- public void badSymbolType() throws Exception {
+ public void evalExportsFails_badSymbolType() throws Exception {
Exception ex =
evalBuiltinsToException(
- "exported_toplevels = {'a': 1, 'b': 2}",
+ "exported_toplevels = {}", //
"exported_rules = None",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}");
+ "exported_to_java = {}");
assertThat(ex).isInstanceOf(EvalException.class);
assertThat(ex).hasMessageThat().contains("got NoneType for 'exported_rules dict', want dict");
}
@Test
- public void badDictKey() throws Exception {
+ public void evalExportsFails_badDictKey() throws Exception {
Exception ex =
evalBuiltinsToException(
- "exported_toplevels = {'a': 1, 'b': 2}",
+ "exported_toplevels = {}", //
"exported_rules = {1: 'a'}",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}");
+ "exported_to_java = {}");
assertThat(ex).isInstanceOf(EvalException.class);
assertThat(ex)
.hasMessageThat()
@@ -115,13 +177,13 @@
}
@Test
- public void parseError() throws Exception {
+ public void evalExportsFails_parseError() throws Exception {
reporter.removeHandler(failFastHandler);
Exception ex =
evalBuiltinsToException(
- "exported_toplevels = {'a': 1, 'b': 2}",
- "exported_rules = {'b': True}",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}",
+ "exported_toplevels = {}",
+ "exported_rules = {}",
+ "exported_to_java = {}",
"asdf asdf # <-- parse error");
assertThat(ex)
.hasMessageThat()
@@ -129,16 +191,35 @@
}
@Test
- public void evalError() throws Exception {
+ public void evalExportsFails_evalError() throws Exception {
reporter.removeHandler(failFastHandler);
Exception ex =
evalBuiltinsToException(
- "exported_toplevels = {'a': 1, 'b': 2}",
- "exported_rules = {'b': True}",
- "exported_to_java = {'c': [1, 2, 3], 'd': 'foo'}",
+ "exported_toplevels = {}",
+ "exported_rules = {}",
+ "exported_to_java = {}",
"1 // 0 # <-- dynamic error");
assertThat(ex)
.hasMessageThat()
.contains("Extension file 'tools/builtins_staging/exports.bzl' has errors");
}
+
+ @Test
+ public void evalExportsFails_errorInDependency() throws Exception {
+ reporter.removeHandler(failFastHandler);
+ // TODO(#11437): Use @builtins//:... syntax, once supported. Don't create a real package.
+ scratch.file("builtins_helper/BUILD");
+ scratch.file(
+ "builtins_helper/dummy.bzl", //
+ "1 // 0 # <-- dynamic error");
+ Exception ex =
+ evalBuiltinsToException(
+ "load('//builtins_helper:dummy.bzl', 'dummy')",
+ "exported_toplevels = {}",
+ "exported_rules = {}",
+ "exported_to_java = {}");
+ assertThat(ex)
+ .hasMessageThat()
+ .contains("Extension file 'builtins_helper/dummy.bzl' has errors");
+ }
}
