Remove the legacy loading phase runner

PiperOrigin-RevId: 199775400
werkt · Aug 2, 2018 · dab4810 · dab4810
1 parent d5bdd39
commit dab4810
Show file tree

Hide file tree

Showing 14 changed files with 38 additions and 424 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java b/src/main/java/com/google/devtools/build/lib/buildtool/BuildTool.java
@@ -433,9 +433,9 @@ public void notifyTargets(Collection<Target> targets) throws LoadingFailedExcept
       }
     };
 
-    LoadingPhaseRunner loadingPhaseRunner = env.getSkyframeExecutor().getLoadingPhaseRunner(
-        runtime.getPackageFactory().getRuleClassNames(),
-        request.getLoadingOptions().useSkyframeTargetPatternEvaluator);
+    LoadingPhaseRunner loadingPhaseRunner =
+        env.getSkyframeExecutor().getLoadingPhaseRunner(
+            runtime.getPackageFactory().getRuleClassNames());
     LoadingResult result =
         loadingPhaseRunner.execute(
             getReporter(),

diff --git a/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java b/src/main/java/com/google/devtools/build/lib/pkgcache/LoadingOptions.java
@@ -139,11 +139,7 @@ public class LoadingOptions extends OptionsBase {
   )
   public boolean buildManualTests;
 
-  // If this option is set, the value of experimental_interleave_loading_and_analysis is completely
-  // ignored. This enables a different LoadingPhaseRunner implementation which doesn't implement
-  // the loading phase at all, and therefore can't currently support the other flag. If we roll this
-  // out soonish, then we're never going to implement the legacy code path in the new
-  // implementation, making it a moot point.
+  @Deprecated
   @Option(
     name = "experimental_skyframe_target_pattern_evaluator",
     defaultValue = "true",

diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/LegacyLoadingPhaseRunner.java b/src/main/java/com/google/devtools/build/lib/skyframe/LegacyLoadingPhaseRunner.java
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
@@ -2206,12 +2206,8 @@ public abstract void detectModifiedOutputFiles(
    */
   public abstract void deleteOldNodes(long versionWindowForDirtyGc);
 
-  public LoadingPhaseRunner getLoadingPhaseRunner(Set<String> ruleClassNames, boolean useNewImpl) {
-    if (!useNewImpl) {
-      return new LegacyLoadingPhaseRunner(packageManager, ruleClassNames);
-    } else {
-      return new SkyframeLoadingPhaseRunner(ruleClassNames);
-    }
+  public LoadingPhaseRunner getLoadingPhaseRunner(Set<String> ruleClassNames) {
+    return new SkyframeLoadingPhaseRunner(ruleClassNames);
   }
 
   /**

diff --git a/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java b/src/test/java/com/google/devtools/build/lib/analysis/BuildViewTest.java
@@ -1363,16 +1363,6 @@ public void onlyAllowedRuleClassesWithWarning() throws Exception {
         + "//foo:foo: genrule rule '//foo:genlib' is unexpected here; continuing anyway");
   }
 
-  /** Runs the same test with the reduced loading phase. */
-  @TestSpec(size = Suite.SMALL_TESTS)
-  @RunWith(JUnit4.class)
-  public static class WithSkyframeLoadingPhase extends BuildViewTest {
-    @Override
-    protected FlagBuilder defaultFlags() {
-      return super.defaultFlags().with(Flag.SKYFRAME_LOADING_PHASE);
-    }
-  }
-
   /** Runs the same test with trimmed configurations. */
   @TestSpec(size = Suite.SMALL_TESTS)
   @RunWith(JUnit4.class)