Make BuildViewTestCase's use of custom RuleClassProviders and bzl inlining less error prone
This refactoring aims to convert the hours I wasted into saved time for somebody else.
Problem:
BuildViewTestCase allows you to supply an alternative RuleClassProvider by overriding getRuleClassProvider(). Although named as an accessor ("get..."), most overrides actually construct a new RuleClassProvider inside the implementation. This is safe so long as the method is only called once, in initializeSkyframeExecutor().
However, this interracts poorly with another, less common paradigm: Testing of BzlLoadFunction inlining. This is done in StarlarkIntegrationTest by defining a @Before hook (initializeLookupFunctions()) that grabs the current Skyframe executor's PackageFunction and tells it to use a newly constructed BzlLoadFunction -- a BzlLoadFunction that obtains its RuleClassProvider by... you guessed it, calling getRuleClassProvider(). Yet this too is safe, so long as the default implementation of getRuleClassProvider() is used, which memoizes its result deep inside the AnalysisMock.
But combining these two features together leads to disaster. You end up with bzl loading machinery whose native rule definitions (`native.cc_library`, etc.) are cloned versions of those in the rest of the Skyframe machinery. This can lead to infuriatingly subtle failures down the line.
Solution:
BuildViewTestCase's getRuleClassProvider() is renamed to createRuleClassProvider() to make it clear that it is not freely repeatable. A true accessor, getRuleClassProvider(), is introduced to take its place and declared `final`.
initializeLookupFunctions() is factored into BuildViewTestCase. Subclasses can simply override usesInliningBzlLoadFunction() to request inlining.
Thanks to shreyax@ for the debugging assistance.
Cleanup work on way toward #11437.
RELNOTES: None
PiperOrigin-RevId: 315329662
diff --git a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
index b49af3d..47f1fcb 100644
--- a/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
+++ b/src/test/java/com/google/devtools/build/lib/analysis/CircularDependencyTest.java
@@ -290,7 +290,7 @@
}));
@Override
- protected ConfiguredRuleClassProvider getRuleClassProvider() {
+ protected ConfiguredRuleClassProvider createRuleClassProvider() {
ConfiguredRuleClassProvider.Builder builder =
new ConfiguredRuleClassProvider.Builder()
.addRuleDefinition(NORMAL_DEPENDER)
