Remove LoadingResult

Instead, refactor the code to use TargetPatternPhaseValue exclusively. This
removes the need to convert from TargetPatternPhaseValue to LoadingResult, and
prepares for interleaving.

It also reduces the number of Skyframe calls which may speed up null builds a
bit, as a followup for https://github.com/bazelbuild/bazel/commit/1067310e18cb9ac203110726de0be53bdc403cea.

PiperOrigin-RevId: 205989338

1 files changed, 12 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
index caca6a7aef..ec049cd5fd 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BuildView.java
@@ -56,7 +56,6 @@ import com.google.devtools.build.lib.packages.NoSuchTargetException;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
-import com.google.devtools.build.lib.pkgcache.LoadingResult;
 import com.google.devtools.build.lib.pkgcache.PackageManager.PackageManagerStatistics;
 import com.google.devtools.build.lib.skyframe.AspectValue;
 import com.google.devtools.build.lib.skyframe.AspectValue.AspectKey;
@@ -66,6 +65,7 @@ import com.google.devtools.build.lib.skyframe.CoverageReportValue;
 import com.google.devtools.build.lib.skyframe.SkyframeAnalysisResult;
 import com.google.devtools.build.lib.skyframe.SkyframeBuildView;
 import com.google.devtools.build.lib.skyframe.SkyframeExecutor;
+import com.google.devtools.build.lib.skyframe.TargetPatternPhaseValue;
 import com.google.devtools.build.lib.syntax.SkylarkImport;
 import com.google.devtools.build.lib.syntax.SkylarkImports;
 import com.google.devtools.build.lib.syntax.SkylarkImports.SkylarkImportSyntaxException;
@@ -174,9 +174,7 @@ public class BuildView {
   /** Returns the collection of configured targets corresponding to any of the provided targets. */
   @VisibleForTesting
   static LinkedHashSet<ConfiguredTarget> filterTestsByTargets(
-      Collection<ConfiguredTarget> targets, Collection<Target> allowedTargets) {
-    Set<Label> allowedTargetLabels =
-        allowedTargets.stream().map(Target::getLabel).collect(Collectors.toSet());
+      Collection<ConfiguredTarget> targets, Set<Label> allowedTargetLabels) {
     return targets
         .stream()
         .filter(ct -> allowedTargetLabels.contains(ct.getLabel()))
@@ -185,7 +183,7 @@ public class BuildView {
 
   @ThreadCompatible
   public AnalysisResult update(
-      LoadingResult loadingResult,
+      TargetPatternPhaseValue loadingResult,
       BuildConfigurationCollection configurations,
       List<String> aspects,
       AnalysisOptions viewOptions,
@@ -201,7 +199,9 @@ public class BuildView {
     skyframeBuildView.resetEvaluatedConfiguredTargetKeysSet();
     skyframeBuildView.resetEvaluationActionCount();
 
-    Collection<Target> targets = loadingResult.getTargets();
+    // TODO(ulfjack): Expensive. Maybe we don't actually need the targets, only the labels?
+    Collection<Target> targets =
+        loadingResult.getTargets(eventHandler, skyframeExecutor.getPackageManager());
     eventBus.post(new AnalysisPhaseStartedEvent(targets));
 
     skyframeBuildView.setConfigurations(eventHandler, configurations);
@@ -358,7 +358,7 @@ public class BuildView {
 
   private AnalysisResult createResult(
       ExtendedEventHandler eventHandler,
-      LoadingResult loadingResult,
+      TargetPatternPhaseValue loadingResult,
       BuildConfigurationCollection configurations,
       TopLevelArtifactContext topLevelOptions,
       AnalysisOptions viewOptions,
@@ -366,7 +366,7 @@ public class BuildView {
       Set<ConfiguredTarget> targetsToSkip,
       List<TargetAndConfiguration> topLevelTargetsWithConfigs)
       throws InterruptedException {
-    Collection<Target> testsToRun = loadingResult.getTestsToRun();
+    Set<Label> testsToRun = loadingResult.getTestsToRunLabels();
     Set<ConfiguredTarget> configuredTargets =
         Sets.newLinkedHashSet(skyframeAnalysisResult.getConfiguredTargets());
     ImmutableSet<AspectValue> aspects = ImmutableSet.copyOf(skyframeAnalysisResult.getAspects());
@@ -469,10 +469,11 @@ public class BuildView {
 
   @Nullable
   public static String createErrorMessage(
-      LoadingResult loadingResult, @Nullable SkyframeAnalysisResult skyframeAnalysisResult) {
-    return loadingResult.hasTargetPatternError()
+      TargetPatternPhaseValue loadingResult,
+      @Nullable SkyframeAnalysisResult skyframeAnalysisResult) {
+    return loadingResult.hasError()
         ? "command succeeded, but there were errors parsing the target pattern"
-        : loadingResult.hasLoadingError()
+        : loadingResult.hasPostExpansionError()
                 || (skyframeAnalysisResult != null && skyframeAnalysisResult.hasLoadingError())
             ? "command succeeded, but there were loading phase errors"
             : (skyframeAnalysisResult != null && skyframeAnalysisResult.hasAnalysisError())