Allow native rule analysis implementations to see Starlark-defined builtins
This implements the behavior of exports.bzl's `exported_to_java` dictionary: Any entry added to that dict becomes visible to all native rule implementations. This can be used to offload constants and helper logic into @_builtins, without necessarily first migrating everything that depends on it into @_builtins -- i.e. this lets us create a native-@_builtins sandwich.
A follow-up CL will make it easier to call Starlark-defined helper functions from native rules. Currently the caller would have to set up their own StarlarkThread and BazelStarlarkContext.
In principle, this feature of builtins injection should work for native aspects as well, but I haven't verified whether this CL makes it possible.
Note that there is no conceptual cyclic dependency between native logic and @_builtins. Native code can (in the future) expose internal-only symbols to @_builtins, and @_builtins can export helpers back to native code, but the former happens at rule class provider construction time, and the latter requires a RuleContext (or Skyframe environment). For instance, you can define "MyNativeProvider" in native code and instantiate it in a @_builtins .bzl, even at the top-level. But in order for the provider's constructor to access my_exported_builtins_defined_symbol, you'd have to pass in a RuleContext, which is only available during the analysis phase.
Implementation notes:
- CachingAnalysisEnvironment stores a StarlarkBuiltinsValue upon construction. The caller of the constructor is responsible for requesting this value from Skyframe and handling (or in the case of tests, ignoring) the possibility of a missing Skyframe dep, though the dep should not be missing since builtins were needed for the loading phase to succeed.
- CachingAnalysisEnvironment's access to the StarlarkSemantics now indirects through the builtins value, so the total number of Skyframe edges per configured target is not increased.
- TestRuleClassProvider#addMinimalRules is updated with definitions needed to run the analysis phase. Also created a separate list in TestConstants having just the default flags needed for this rule class provider. The notion behind addMinimalRules is that there should be some minimal set of core rules that are required by the package loading and analysis machinery, without necessarily bringing in the native rules we eventually want to migrate to Starlark.
Performance note: In theory, this CL can cause some configured targets to have an additional skyframe edge. This only occurs in cases where the configured target did not already request the StarlarkSemantics. All Starlark-defined rule types request the StarlarkSemantics, as do at least several Cc, Java, and Python rules (e.g. indirectly via helpers on RuleContext). So any increase should be pretty limited in scope.
Work toward #11437.
PiperOrigin-RevId: 348091304
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java
index 28051d4..3289785 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/AnalysisEnvironment.java
@@ -15,6 +15,7 @@
package com.google.devtools.build.lib.analysis;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
import com.google.devtools.build.lib.actions.ActionKeyContext;
@@ -43,9 +44,7 @@
/** Returns a callback to be used in this build for reporting analysis errors. */
ExtendedEventHandler getEventHandler();
- /**
- * Returns whether any errors were reported to this instance.
- */
+ /** Returns whether any errors were reported to this instance. */
boolean hasErrors();
/**
@@ -118,16 +117,14 @@
*/
Artifact getFilesetArtifact(PathFragment rootRelativePath, ArtifactRoot root);
- /**
- * Returns the middleman factory associated with the build.
- */
+ /** Returns the middleman factory associated with the build. */
// TODO(bazel-team): remove this method and replace it with delegate methods.
MiddlemanFactory getMiddlemanFactory();
/**
* Returns the generating action for the given local artifact.
*
- * If the artifact was created in another analysis environment (e.g. by a different configured
+ * <p>If the artifact was created in another analysis environment (e.g. by a different configured
* target instance) or the artifact is a source artifact, it returns null.
*/
ActionAnalysisMetadata getLocalGeneratingAction(Artifact artifact);
@@ -141,8 +138,8 @@
/**
* Returns the Skyframe SkyFunction.Environment if available. Otherwise, null.
*
- * <p>If you need to use this for something other than genquery, please think long and hard
- * about that.
+ * <p>If you need to use this for something other than genquery, please think long and hard about
+ * that.
*/
SkyFunction.Environment getSkyframeEnv();
@@ -153,6 +150,17 @@
StarlarkSemantics getStarlarkSemantics() throws InterruptedException;
/**
+ * Returns the {@code exported_to_java} map defined by the Starlark {@code @_builtins}
+ * pseudo-repo.
+ *
+ * <p>If builtins injection is disabled, this map is empty.
+ *
+ * @see StarlarkBuiltinsFunction
+ */
+ // TODO(#11437): Update docstring once injection can no longer be disabled.
+ ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException;
+
+ /**
* Returns the Artifact that is used to hold the non-volatile workspace status for the current
* build request.
*/
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BUILD b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
index d468ebd0a8..d52424e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BUILD
@@ -414,8 +414,8 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_value_creation_exception",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_value",
- "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:sane_analysis_exception",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:transitive_target_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:transitive_target_value",
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
index 1ba4623..c7827f5 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CachingAnalysisEnvironment.java
@@ -16,6 +16,7 @@
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Lists;
import com.google.devtools.build.lib.actions.ActionAnalysisMetadata;
@@ -33,7 +34,7 @@
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.packages.Target;
import com.google.devtools.build.lib.skyframe.BuildInfoCollectionValue;
-import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
import com.google.devtools.build.lib.skyframe.WorkspaceStatusValue;
import com.google.devtools.build.lib.util.Pair;
import com.google.devtools.build.lib.vfs.PathFragment;
@@ -77,6 +78,9 @@
private MiddlemanFactory middlemanFactory;
private ExtendedEventHandler errorEventListener;
private SkyFunction.Environment skyframeEnv;
+ // TODO(bazel-team): Should this be nulled out by disable()? Alternatively, does disable() even
+ // need to exist?
+ private final StarlarkBuiltinsValue starlarkBuiltinsValue;
/**
* Map of artifacts to either themselves or to {@code Pair<Artifact, String>} if
* --experimental_extended_sanity_checks is enabled. In the latter case, the string will contain
@@ -101,7 +105,8 @@
boolean extendedSanityChecks,
boolean allowAnalysisFailures,
ExtendedEventHandler errorEventListener,
- SkyFunction.Environment env) {
+ SkyFunction.Environment env,
+ StarlarkBuiltinsValue starlarkBuiltinsValue) {
this.artifactFactory = artifactFactory;
this.actionKeyContext = actionKeyContext;
this.owner = Preconditions.checkNotNull(owner);
@@ -110,6 +115,7 @@
this.allowAnalysisFailures = allowAnalysisFailures;
this.errorEventListener = errorEventListener;
this.skyframeEnv = env;
+ this.starlarkBuiltinsValue = starlarkBuiltinsValue;
middlemanFactory = new MiddlemanFactory(artifactFactory, this);
artifacts = new HashMap<>();
}
@@ -359,7 +365,12 @@
@Override
public StarlarkSemantics getStarlarkSemantics() throws InterruptedException {
- return PrecomputedValue.STARLARK_SEMANTICS.get(skyframeEnv);
+ return starlarkBuiltinsValue.starlarkSemantics;
+ }
+
+ @Override
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ return starlarkBuiltinsValue.exportedToJava;
}
@Override
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
index c0d460a..ca08292 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/AspectFunction.java
@@ -645,15 +645,20 @@
OrderedSetMultimap<DependencyKind, ConfiguredTargetAndData> directDeps,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
throws AspectFunctionException, InterruptedException {
- SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
-
- StoredEventHandler events = new StoredEventHandler();
- CachingAnalysisEnvironment analysisEnvironment = view.createAnalysisEnvironment(
- key, false, events, env, aspectConfiguration);
+ // Should be successfully evaluated and cached from the loading phase.
+ StarlarkBuiltinsValue starlarkBuiltinsValue =
+ (StarlarkBuiltinsValue) env.getValue(StarlarkBuiltinsValue.key());
if (env.valuesMissing()) {
return null;
}
+ SkyframeBuildView view = buildViewProvider.getSkyframeBuildView();
+
+ StoredEventHandler events = new StoredEventHandler();
+ CachingAnalysisEnvironment analysisEnvironment =
+ view.createAnalysisEnvironment(
+ key, false, events, env, aspectConfiguration, starlarkBuiltinsValue);
+
ConfiguredAspect configuredAspect;
if (aspect.getDefinition().applyToGeneratingRules()
&& associatedTarget.getTarget() instanceof OutputFile) {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
index 0d6a5e6..af65678 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/ConfiguredTargetFunction.java
@@ -378,7 +378,7 @@
configConditions,
toolchainContexts,
transitivePackagesForPackageRootResolution);
- if (configuredTargetProgress != null) {
+ if (ans != null && configuredTargetProgress != null) {
configuredTargetProgress.doneConfigureTarget();
}
return ans;
@@ -999,13 +999,18 @@
@Nullable ToolchainCollection<ResolvedToolchainContext> toolchainContexts,
@Nullable NestedSetBuilder<Package> transitivePackagesForPackageRootResolution)
throws ConfiguredTargetFunctionException, InterruptedException {
- StoredEventHandler events = new StoredEventHandler();
- CachingAnalysisEnvironment analysisEnvironment =
- view.createAnalysisEnvironment(configuredTargetKey, false, events, env, configuration);
- if (env.valuesMissing()) {
+ // Should be successfully evaluated and cached from the loading phase.
+ StarlarkBuiltinsValue starlarkBuiltinsValue =
+ (StarlarkBuiltinsValue) env.getValue(StarlarkBuiltinsValue.key());
+ if (starlarkBuiltinsValue == null) {
return null;
}
+ StoredEventHandler events = new StoredEventHandler();
+ CachingAnalysisEnvironment analysisEnvironment =
+ view.createAnalysisEnvironment(
+ configuredTargetKey, false, events, env, configuration, starlarkBuiltinsValue);
+
Preconditions.checkNotNull(depValueMap);
ConfiguredTarget configuredTarget;
try {
diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
index 02abf4d..fdc0057 100644
--- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeBuildView.java
@@ -899,7 +899,8 @@
boolean isSystemEnv,
ExtendedEventHandler eventHandler,
Environment env,
- BuildConfiguration config) {
+ BuildConfiguration config,
+ StarlarkBuiltinsValue starlarkBuiltinsValue) {
boolean extendedSanityChecks = config != null && config.extendedSanityChecks();
boolean allowAnalysisFailures = config != null && config.allowAnalysisFailures();
return new CachingAnalysisEnvironment(
@@ -910,7 +911,8 @@
extendedSanityChecks,
allowAnalysisFailures,
eventHandler,
- env);
+ env,
+ starlarkBuiltinsValue);
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
index 0754bba..0f262cc 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestUtil.java
@@ -187,6 +187,11 @@
}
@Override
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ return original.getStarlarkDefinedBuiltins();
+ }
+
+ @Override
public Artifact getStableWorkspaceStatusArtifact() throws InterruptedException {
return original.getStableWorkspaceStatusArtifact();
}
@@ -401,6 +406,11 @@
}
@Override
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ return null;
+ }
+
+ @Override
public Artifact getFilesetArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
return null;
}
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
index cd19ef5..53ed528 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BUILD
@@ -50,10 +50,8 @@
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
"//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/config_matching_provider",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
"//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
- "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_provider",
"//src/main/java/com/google/devtools/build/lib/analysis:config/host_transition",
"//src/main/java/com/google/devtools/build/lib/analysis:config/invalid_configuration_exception",
"//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
@@ -106,6 +104,7 @@
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:target_pattern_phase_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_context_key",
"//src/main/java/com/google/devtools/build/lib/skyframe:toolchain_exception",
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
index 8154db4..d4dc82d 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewForTesting.java
@@ -85,11 +85,13 @@
import com.google.devtools.build.lib.skyframe.SkyFunctionEnvironmentForTesting;
import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.skyframe.ToolchainContextKey;
import com.google.devtools.build.lib.skyframe.ToolchainException;
import com.google.devtools.build.lib.skyframe.UnloadedToolchainContext;
import com.google.devtools.build.lib.util.OrderedSetMultimap;
+import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.ValueOrException;
import java.util.Collection;
@@ -471,7 +473,12 @@
StarlarkTransition.TransitionException, InvalidExecGroupException {
BuildConfiguration targetConfig =
skyframeExecutor.getConfiguration(eventHandler, target.getConfigurationKey());
- CachingAnalysisEnvironment env =
+ SkyFunction.Environment skyframeEnv =
+ skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler);
+ StarlarkBuiltinsValue starlarkBuiltinsValue =
+ (StarlarkBuiltinsValue)
+ Preconditions.checkNotNull(skyframeEnv.getValue(StarlarkBuiltinsValue.key()));
+ CachingAnalysisEnvironment analysisEnv =
new CachingAnalysisEnvironment(
getArtifactFactory(),
skyframeExecutor.getActionKeyContext(),
@@ -483,8 +490,9 @@
targetConfig.extendedSanityChecks(),
targetConfig.allowAnalysisFailures(),
eventHandler,
- skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler));
- return getRuleContextForTesting(eventHandler, target, env, configurations);
+ skyframeEnv,
+ starlarkBuiltinsValue);
+ return getRuleContextForTesting(eventHandler, target, analysisEnv, configurations);
}
/**
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
index bbcf729..9393eff 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/util/BuildViewTestCase.java
@@ -149,6 +149,7 @@
import com.google.devtools.build.lib.skyframe.SequencedSkyframeExecutor;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
import com.google.devtools.build.lib.testutil.BlazeTestUtils;
import com.google.devtools.build.lib.testutil.FoundationTestCase;
@@ -626,7 +627,11 @@
view.setArtifactRoots(new PackageRootsNoSymlinkCreation(Root.fromPath(rootDirectory)));
}
- protected CachingAnalysisEnvironment getTestAnalysisEnvironment() {
+ protected CachingAnalysisEnvironment getTestAnalysisEnvironment() throws InterruptedException {
+ SkyFunction.Environment env = skyframeExecutor.getSkyFunctionEnvironmentForTesting(reporter);
+ StarlarkBuiltinsValue starlarkBuiltinsValue =
+ (StarlarkBuiltinsValue)
+ Preconditions.checkNotNull(env.getValue(StarlarkBuiltinsValue.key()));
return new CachingAnalysisEnvironment(
view.getArtifactFactory(),
actionKeyContext,
@@ -646,7 +651,8 @@
/*extendedSanityChecks=*/ false,
/*allowAnalysisFailures=*/ false,
reporter,
- skyframeExecutor.getSkyFunctionEnvironmentForTesting(reporter));
+ env,
+ starlarkBuiltinsValue);
}
/**
@@ -2119,6 +2125,11 @@
}
@Override
+ public ImmutableMap<String, Object> getStarlarkDefinedBuiltins() throws InterruptedException {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
public Artifact getFilesetArtifact(PathFragment rootRelativePath, ArtifactRoot root) {
throw new UnsupportedOperationException();
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
index c856b57..cdda8a7 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/BUILD
@@ -208,8 +208,10 @@
"//src/main/java/com/google/devtools/build/lib/collect/nestedset",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/skyframe:configured_target_key",
+ "//src/main/java/com/google/devtools/build/lib/skyframe:starlark_builtins_value",
"//src/main/java/com/google/devtools/build/lib/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
+ "//src/main/java/com/google/devtools/build/skyframe",
"//third_party:guava",
"//third_party:junit4",
"//third_party:truth",
diff --git a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
index 6577abd..2fe0e5d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/android/ResourceTestBase.java
@@ -16,6 +16,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
+import com.google.common.base.Preconditions;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
@@ -35,11 +36,13 @@
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.StoredEventHandler;
import com.google.devtools.build.lib.skyframe.ConfiguredTargetKey;
+import com.google.devtools.build.lib.skyframe.StarlarkBuiltinsValue;
import com.google.devtools.build.lib.vfs.DigestHashFunction;
import com.google.devtools.build.lib.vfs.FileSystem;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.lib.vfs.Root;
import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem;
+import com.google.devtools.build.skyframe.SkyFunction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
@@ -225,14 +228,16 @@
* AndroidConfiguration}.
*/
public RuleContext getRuleContextForActionTesting(ConfiguredTarget dummyTarget) throws Exception {
-
RuleContext dummy = getRuleContext(dummyTarget);
-
ExtendedEventHandler eventHandler = new StoredEventHandler();
- return view.getRuleContextForTesting(
- eventHandler,
- dummyTarget,
- /* env= */ new CachingAnalysisEnvironment(
+
+ SkyFunction.Environment skyframeEnv =
+ skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler);
+ StarlarkBuiltinsValue starlarkBuiltinsValue =
+ (StarlarkBuiltinsValue)
+ Preconditions.checkNotNull(skyframeEnv.getValue(StarlarkBuiltinsValue.key()));
+ CachingAnalysisEnvironment analysisEnv =
+ new CachingAnalysisEnvironment(
view.getArtifactFactory(),
skyframeExecutor.getActionKeyContext(),
ConfiguredTargetKey.builder()
@@ -243,7 +248,13 @@
targetConfig.extendedSanityChecks(),
targetConfig.allowAnalysisFailures(),
eventHandler,
- skyframeExecutor.getSkyFunctionEnvironmentForTesting(eventHandler)),
+ skyframeEnv,
+ starlarkBuiltinsValue);
+
+ return view.getRuleContextForTesting(
+ eventHandler,
+ dummyTarget,
+ analysisEnv,
new BuildConfigurationCollection(
ImmutableList.of(dummy.getConfiguration()), dummy.getHostConfiguration()));
}
diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
index 320a97a..4f6c902 100644
--- a/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
+++ b/src/test/java/com/google/devtools/build/lib/skyframe/BuiltinsInjectionTest.java
@@ -18,9 +18,14 @@
import static com.google.common.truth.Truth.assertWithMessage;
import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
+import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
+import com.google.devtools.build.lib.analysis.ConfiguredTarget;
+import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.analysis.util.MockRule;
+import com.google.devtools.build.lib.analysis.util.MockRuleDefaults;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.testutil.TestRuleClassProvider;
@@ -43,13 +48,35 @@
@RunWith(JUnit4.class)
public class BuiltinsInjectionTest extends BuildViewTestCase {
+ /** A simple dummy rule that doesn't do anything. */
private static final MockRule OVERRIDABLE_RULE = () -> MockRule.define("overridable_rule");
+ /**
+ * A dummy native rule that reads from exported_to_java the symbol "builtins_defined_symbol", and
+ * prints its value to the event handler.
+ */
+ private static final MockRule SANDWICH_RULE =
+ () -> MockRule.factory(SandwichFactory.class).define("sandwich_rule");
+
+ // Must be public due to reflective construction of rule factories.
+ /** Factory for SANDWICH_RULE. (Javadoc'd to pacify linter.) */
+ public static class SandwichFactory extends MockRuleDefaults.DefaultConfiguredTargetFactory {
+ @Override
+ public ConfiguredTarget create(RuleContext ruleContext)
+ throws InterruptedException, RuleErrorException, ActionConflictException {
+ AnalysisEnvironment env = ruleContext.getAnalysisEnvironment();
+ Object value = env.getStarlarkDefinedBuiltins().get("builtins_defined_symbol");
+ env.getEventHandler().handle(Event.info("builtins_defined_symbol :: " + value.toString()));
+ return super.create(ruleContext);
+ }
+ }
+
@Override
protected Iterable<String> getDefaultsForConfiguration() {
// Override BuildViewTestCase's behavior of setting all sorts of extra options that don't exist
// on our minimal rule class provider.
- return ImmutableList.of();
+ // We do need the host platform. Set it to something trivial.
+ return ImmutableList.of("--host_platform=//minimal_buildenv/platforms:default_host");
}
@Override
@@ -57,6 +84,10 @@
// Don't let the AnalysisMock sneak in any WORKSPACE file content, which may depend on
// repository rules that our minimal rule class provider doesn't have.
analysisMock.setupMockClient(mockToolsConfig, ImmutableList.of());
+ // Provide a trivial platform definition.
+ mockToolsConfig.create(
+ "minimal_buildenv/platforms/BUILD", //
+ "platform(name = 'default_host')");
}
@Override
@@ -69,6 +100,7 @@
// Add some mock symbols to override.
builder
.addRuleDefinition(OVERRIDABLE_RULE)
+ .addRuleDefinition(SANDWICH_RULE)
.addStarlarkAccessibleTopLevels("overridable_symbol", "original_value")
.addStarlarkAccessibleTopLevels(
"flag_guarded_symbol",
@@ -363,6 +395,22 @@
// TODO(#11437): Once we allow access to native symbols via _internal, verify that flag guarding
// works correctly within builtins.
+ @Test
+ public void nativeRulesCanUseSymbolsFromBuiltins() throws Exception {
+ writeExportsBzl(
+ "exported_toplevels = {}",
+ "exported_rules = {}",
+ "exported_to_java = {'builtins_defined_symbol': 'value_from_builtins'}");
+ scratch.file(
+ "pkg/BUILD", //
+ "sandwich_rule(name = 'sandwich')");
+
+ getConfiguredTarget("//pkg:sandwich");
+ assertContainsEvent("builtins_defined_symbol :: value_from_builtins");
+ }
+
+ // TODO(#11437): Verify whether this works for native-defined aspects as well.
+
/**
* Tests for injection, under inlining of {@link BzlLoadFunction}.
*
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD
index c48d579..11ac8ba 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD
+++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD
@@ -48,10 +48,10 @@
"//src/main/java/com/google/devtools/build/lib/actions",
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
"//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
+ "//src/main/java/com/google/devtools/build/lib/analysis:common_prerequisite_validator",
"//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
"//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
"//src/main/java/com/google/devtools/build/lib/analysis:platform_configuration",
- "//src/main/java/com/google/devtools/build/lib/analysis:platform_options",
"//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
"//src/main/java/com/google/devtools/build/lib/analysis:template_variable_info",
"//src/main/java/com/google/devtools/build/lib/clock",
@@ -60,6 +60,8 @@
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/rules:core_rules",
+ "//src/main/java/com/google/devtools/build/lib/rules/config",
+ "//src/main/java/com/google/devtools/build/lib/rules/platform",
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/util:filetype",
"//src/main/java/com/google/devtools/build/lib/util:os",
diff --git a/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java b/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
index 52df323..0b0857a 100644
--- a/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
+++ b/src/test/java/com/google/devtools/build/lib/testutil/TestRuleClassProvider.java
@@ -24,10 +24,10 @@
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
+import com.google.devtools.build.lib.analysis.CommonPrerequisiteValidator;
import com.google.devtools.build.lib.analysis.ConfiguredRuleClassProvider;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
import com.google.devtools.build.lib.analysis.PlatformConfiguration;
-import com.google.devtools.build.lib.analysis.PlatformOptions;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory;
import com.google.devtools.build.lib.analysis.RuleContext;
@@ -37,11 +37,14 @@
import com.google.devtools.build.lib.analysis.TemplateVariableInfo;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.cmdline.Label;
+import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
import com.google.devtools.build.lib.collect.nestedset.Order;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.Type;
+import com.google.devtools.build.lib.rules.config.ConfigRules;
import com.google.devtools.build.lib.rules.core.CoreRules;
+import com.google.devtools.build.lib.rules.platform.PlatformRules;
import com.google.devtools.build.lib.util.FileTypeSet;
import java.lang.reflect.Method;
import java.util.Map;
@@ -72,6 +75,8 @@
addStandardRules(builder);
// TODO(b/174773026): Eliminate TestingDummyRule/MockToolchainRule from this class, push them
// down into the tests that use them. It's better for tests to avoid spooky mocks at a distance.
+ // The same might also be said for the cleared-workspace variant of getRuleClassProvider(). If
+ // we eliminate both, TestRuleClassProvider probably doesn't need to exist anymore.
builder.addRuleDefinition(new TestingDummyRule());
builder.addRuleDefinition(new MockToolchainRule());
if (clearSuffix) {
@@ -96,6 +101,9 @@
return ruleClassProviderWithClearedSuffix;
}
+ // TODO(bazel-team): The logic for the "minimal" rule class provider is currently split between
+ // TestRuleClassProvider and BuiltinsInjectionTest's overrides of BuildViewTestCase setup helpers.
+ // Consider refactoring this together into one place as a new MinimalAnalysisMock.
/**
* Adds a few essential rules to a builder, such that it is usable but does not contain all the
* rule classes known to the production environment.
@@ -103,10 +111,41 @@
public static void addMinimalRules(ConfiguredRuleClassProvider.Builder builder) {
// TODO(bazel-team): See also TrimmableTestConfigurationFragments#installFragmentsAndNativeRules
// for alternative/additional setup. Consider factoring that one to use this method.
- builder.setToolsRepository("@").setRunfilesPrefix("test");
+ builder
+ .setToolsRepository("@")
+ .setRunfilesPrefix("test")
+ .setPrerequisiteValidator(new MinimalPrerequisiteValidator());
CoreRules.INSTANCE.init(builder);
builder.addConfigurationOptions(CoreOptions.class);
- builder.addConfigurationOptions(PlatformOptions.class);
+ PlatformRules.INSTANCE.init(builder);
+ ConfigRules.INSTANCE.init(builder);
+ }
+
+ private static class MinimalPrerequisiteValidator extends CommonPrerequisiteValidator {
+ @Override
+ public boolean isSameLogicalPackage(
+ PackageIdentifier thisPackage, PackageIdentifier prerequisitePackage) {
+ return thisPackage.equals(prerequisitePackage);
+ }
+
+ @Override
+ protected boolean packageUnderExperimental(PackageIdentifier packageIdentifier) {
+ return false;
+ }
+
+ @Override
+ protected boolean checkVisibilityForExperimental(RuleContext.Builder context) {
+ // It does not matter whether we return true or false here if packageUnderExperimental always
+ // returns false.
+ return true;
+ }
+
+ @Override
+ protected boolean allowExperimentalDeps(RuleContext.Builder context) {
+ // It does not matter whether we return true or false here if packageUnderExperimental always
+ // returns false.
+ return false;
+ }
}
/** A dummy rule with some dummy attributes. */
