Refactor the loading phase runner creation and remove the test duplication.

The loading phase runner is now always created by the SkyframeExecutor, and
the duplicate test is dropped in favor of subclassing and overriding one
method.

--
MOS_MIGRATED_REVID=107188756

2 files changed, 9 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
index 8fa0799937..2401940881 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/CommandEnvironment.java
@@ -37,7 +37,6 @@ import com.google.devtools.build.lib.events.Reporter;
 import com.google.devtools.build.lib.exec.OutputService;
 import com.google.devtools.build.lib.packages.NoSuchThingException;
 import com.google.devtools.build.lib.packages.Target;
-import com.google.devtools.build.lib.pkgcache.LegacyLoadingPhaseRunner;
 import com.google.devtools.build.lib.pkgcache.LoadingPhaseRunner;
 import com.google.devtools.build.lib.pkgcache.PackageCacheOptions;
 import com.google.devtools.build.lib.pkgcache.PackageManager;
@@ -114,14 +113,8 @@ public final class CommandEnvironment {
     this.eventBus = eventBus;
     this.blazeModuleEnvironment = new BlazeModuleEnvironment();
 
-    if (USE_SKYFRAME_LOADING_PHASE) {
-      this.loadingPhaseRunner = runtime.getSkyframeExecutor().getLoadingPhaseRunner(
-          runtime.getPackageFactory().getRuleClassNames());
-    } else {
-      this.loadingPhaseRunner = new LegacyLoadingPhaseRunner(
-          runtime.getSkyframeExecutor().getPackageManager(),
-          runtime.getPackageFactory().getRuleClassNames());
-    }
+    this.loadingPhaseRunner = runtime.getSkyframeExecutor().getLoadingPhaseRunner(
+        runtime.getPackageFactory().getRuleClassNames(), USE_SKYFRAME_LOADING_PHASE);
     this.view = new BuildView(runtime.getDirectories(), runtime.getRuleClassProvider(),
         runtime.getSkyframeExecutor(), runtime.getCoverageReportActionFactory());
 
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 afba144e13..a660ba59c9 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
@@ -90,6 +90,7 @@ import com.google.devtools.build.lib.packages.Preprocessor.AstAfterPreprocessing
 import com.google.devtools.build.lib.packages.RuleClassProvider;
 import com.google.devtools.build.lib.packages.RuleVisibility;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.pkgcache.LegacyLoadingPhaseRunner;
 import com.google.devtools.build.lib.pkgcache.LoadingCallback;
 import com.google.devtools.build.lib.pkgcache.LoadingFailedException;
 import com.google.devtools.build.lib.pkgcache.LoadingOptions;
@@ -1692,8 +1693,12 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory {
    */
   public abstract void deleteOldNodes(long versionWindowForDirtyGc);
 
-  public LoadingPhaseRunner getLoadingPhaseRunner(Set<String> ruleClassNames) {
-    return new SkyframeLoadingPhaseRunner(ruleClassNames);
+  public LoadingPhaseRunner getLoadingPhaseRunner(Set<String> ruleClassNames, boolean useNewImpl) {
+    if (!useNewImpl) {
+      return new LegacyLoadingPhaseRunner(packageManager, ruleClassNames);
+    } else {
+      return new SkyframeLoadingPhaseRunner(ruleClassNames);
+    }
   }
 
   /**