Refactor the loading phase runner creation and remove the test duplic…

…ation. 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
vanwaals · Nov 6, 2015 · a876759 · a876759
1 parent 7be6c17
commit a876759
Show file tree

Hide file tree

Showing 2 changed files with 9 additions and 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
@@ -37,7 +37,6 @@
 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 CommandEnvironment(BlazeRuntime runtime, UUID commandId, EventBus eventBu
     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
@@ -90,6 +90,7 @@
 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 void prepareExecution(boolean checkOutputFiles,
    */
   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);
+    }
   }
 
   /**